Skip to content
This repository was archived by the owner on Apr 3, 2020. It is now read-only.

Commit 4d97826

Browse files
yfriedmanCommit bot
authored and
Commit bot
committed
Speculative hardening of fullscreen infobar code.
While this probably isn't the principle infobar crash, it does have a few bad smells: 1) InfoBarDelegate's already have an infobar() accessor. Use that instead of a cached tab instance. 2) Many places in infobar code that handle user events check the context to ensure they're still valid before handling them but this didn't. BUG=481758 Review URL: https://codereview.chromium.org/1322063003 Cr-Commit-Position: refs/heads/master@{#347017}
1 parent 9589e4d commit 4d97826

File tree

3 files changed

+12
-13
lines changed

3 files changed

+12
-13
lines changed

chrome/android/java/src/org/chromium/chrome/browser/fullscreen/FullscreenInfoBarDelegate.java

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
*/
1515
public class FullscreenInfoBarDelegate {
1616
private final FullscreenHtmlApiHandler mHandler;
17-
private final Tab mTab;
17+
private final boolean mIsForIncognitoTab;
1818
private long mNativeFullscreenInfoBarDelegate = 0;
1919

2020
/**
@@ -31,7 +31,7 @@ private FullscreenInfoBarDelegate(
3131
FullscreenHtmlApiHandler handler, Tab tab) {
3232
assert tab != null;
3333
mHandler = handler;
34-
mTab = tab;
34+
mIsForIncognitoTab = tab.isIncognito();
3535
mNativeFullscreenInfoBarDelegate = nativeLaunchFullscreenInfoBar(tab);
3636
}
3737

@@ -40,7 +40,7 @@ private FullscreenInfoBarDelegate(
4040
*/
4141
protected void closeFullscreenInfoBar() {
4242
if (mNativeFullscreenInfoBarDelegate != 0) {
43-
nativeCloseFullscreenInfoBar(mNativeFullscreenInfoBarDelegate, mTab);
43+
nativeCloseFullscreenInfoBar(mNativeFullscreenInfoBarDelegate);
4444
}
4545
}
4646

@@ -51,7 +51,7 @@ protected void closeFullscreenInfoBar() {
5151
*/
5252
@CalledByNative
5353
private void onFullscreenAllowed(String origin) {
54-
FullscreenInfo fullscreenInfo = new FullscreenInfo(origin, null, mTab.isIncognito());
54+
FullscreenInfo fullscreenInfo = new FullscreenInfo(origin, null, mIsForIncognitoTab);
5555
fullscreenInfo.setContentSetting(ContentSetting.ALLOW);
5656
}
5757

@@ -72,5 +72,5 @@ private void onInfoBarDismissed() {
7272
}
7373

7474
private native long nativeLaunchFullscreenInfoBar(Tab tab);
75-
private native void nativeCloseFullscreenInfoBar(long nativeFullscreenInfoBarDelegate, Tab tab);
75+
private native void nativeCloseFullscreenInfoBar(long nativeFullscreenInfoBarDelegate);
7676
}

chrome/browser/android/fullscreen/fullscreen_infobar_delegate.cc

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -52,11 +52,10 @@ FullscreenInfoBarDelegate::~FullscreenInfoBarDelegate() {
5252
}
5353

5454
void FullscreenInfoBarDelegate::CloseFullscreenInfoBar(
55-
JNIEnv* env, jobject obj, jobject tab) {
55+
JNIEnv* env, jobject obj) {
5656
j_delegate_.Reset();
57-
TabAndroid* tab_android = TabAndroid::GetNativeTab(env, tab);
58-
InfoBarService::FromWebContents(tab_android->web_contents())->RemoveInfoBar(
59-
infobar());
57+
if (infobar() && infobar()->owner())
58+
infobar()->owner()->RemoveInfoBar(infobar());
6059
}
6160

6261
int FullscreenInfoBarDelegate::GetIconID() const {

chrome/browser/android/fullscreen/fullscreen_infobar_delegate.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,8 @@
22
// Use of this source code is governed by a BSD-style license that can be
33
// found in the LICENSE file.
44

5-
#ifndef CHROME_BROWSER_ANDROID_FULLSCREEN_INFOBAR_DELEGATE_H_
6-
#define CHROME_BROWSER_ANDROID_FULLSCREEN_INFOBAR_DELEGATE_H_
5+
#ifndef CHROME_BROWSER_ANDROID_FULLSCREEN_FULLSCREEN_INFOBAR_DELEGATE_H_
6+
#define CHROME_BROWSER_ANDROID_FULLSCREEN_FULLSCREEN_INFOBAR_DELEGATE_H_
77

88
#include <string>
99

@@ -24,7 +24,7 @@ class FullscreenInfoBarDelegate : public ConfirmInfoBarDelegate {
2424
~FullscreenInfoBarDelegate() override;
2525

2626
// Called to close the infobar.
27-
void CloseFullscreenInfoBar(JNIEnv* env, jobject obj, jobject tab);
27+
void CloseFullscreenInfoBar(JNIEnv* env, jobject obj);
2828

2929
// ConfirmInfoBarDelegate:
3030
int GetIconID() const override;
@@ -40,4 +40,4 @@ class FullscreenInfoBarDelegate : public ConfirmInfoBarDelegate {
4040
DISALLOW_COPY_AND_ASSIGN(FullscreenInfoBarDelegate);
4141
};
4242

43-
#endif // CHROME_BROWSER_ANDROID_FULLSCREEN_INFOBAR_DELEGATE_H_
43+
#endif // CHROME_BROWSER_ANDROID_FULLSCREEN_FULLSCREEN_INFOBAR_DELEGATE_H_

0 commit comments

Comments
 (0)