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

Commit

Permalink
Speculative hardening of fullscreen infobar code.
Browse files Browse the repository at this point in the history
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
TBR=acleung

Review URL: https://codereview.chromium.org/1322063003

Cr-Commit-Position: refs/heads/master@{#347017}
(cherry picked from commit 4d97826)

Review URL: https://codereview.chromium.org/1340693002 .

Cr-Commit-Position: refs/branch-heads/2490@{#235}
Cr-Branched-From: 7790a35-refs/heads/master@{#344925}
  • Loading branch information
yfriedman committed Sep 11, 2015
1 parent b773f25 commit 44470c6
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
*/
public class FullscreenInfoBarDelegate {
private final FullscreenHtmlApiHandler mHandler;
private final Tab mTab;
private final boolean mIsForIncognitoTab;
private long mNativeFullscreenInfoBarDelegate = 0;

/**
Expand All @@ -31,7 +31,7 @@ private FullscreenInfoBarDelegate(
FullscreenHtmlApiHandler handler, Tab tab) {
assert tab != null;
mHandler = handler;
mTab = tab;
mIsForIncognitoTab = tab.isIncognito();
mNativeFullscreenInfoBarDelegate = nativeLaunchFullscreenInfoBar(tab);
}

Expand All @@ -40,7 +40,7 @@ private FullscreenInfoBarDelegate(
*/
protected void closeFullscreenInfoBar() {
if (mNativeFullscreenInfoBarDelegate != 0) {
nativeCloseFullscreenInfoBar(mNativeFullscreenInfoBarDelegate, mTab);
nativeCloseFullscreenInfoBar(mNativeFullscreenInfoBarDelegate);
}
}

Expand All @@ -51,7 +51,7 @@ protected void closeFullscreenInfoBar() {
*/
@CalledByNative
private void onFullscreenAllowed(String origin) {
FullscreenInfo fullscreenInfo = new FullscreenInfo(origin, null, mTab.isIncognito());
FullscreenInfo fullscreenInfo = new FullscreenInfo(origin, null, mIsForIncognitoTab);
fullscreenInfo.setContentSetting(ContentSetting.ALLOW);
}

Expand All @@ -72,5 +72,5 @@ private void onInfoBarDismissed() {
}

private native long nativeLaunchFullscreenInfoBar(Tab tab);
private native void nativeCloseFullscreenInfoBar(long nativeFullscreenInfoBarDelegate, Tab tab);
private native void nativeCloseFullscreenInfoBar(long nativeFullscreenInfoBarDelegate);
}
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,10 @@ FullscreenInfoBarDelegate::~FullscreenInfoBarDelegate() {
}

void FullscreenInfoBarDelegate::CloseFullscreenInfoBar(
JNIEnv* env, jobject obj, jobject tab) {
JNIEnv* env, jobject obj) {
j_delegate_.Reset();
TabAndroid* tab_android = TabAndroid::GetNativeTab(env, tab);
InfoBarService::FromWebContents(tab_android->web_contents())->RemoveInfoBar(
infobar());
if (infobar() && infobar()->owner())
infobar()->owner()->RemoveInfoBar(infobar());
}

int FullscreenInfoBarDelegate::GetIconID() const {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#ifndef CHROME_BROWSER_ANDROID_FULLSCREEN_INFOBAR_DELEGATE_H_
#define CHROME_BROWSER_ANDROID_FULLSCREEN_INFOBAR_DELEGATE_H_
#ifndef CHROME_BROWSER_ANDROID_FULLSCREEN_FULLSCREEN_INFOBAR_DELEGATE_H_
#define CHROME_BROWSER_ANDROID_FULLSCREEN_FULLSCREEN_INFOBAR_DELEGATE_H_

#include <string>

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

// Called to close the infobar.
void CloseFullscreenInfoBar(JNIEnv* env, jobject obj, jobject tab);
void CloseFullscreenInfoBar(JNIEnv* env, jobject obj);

// ConfirmInfoBarDelegate:
int GetIconID() const override;
Expand All @@ -40,4 +40,4 @@ class FullscreenInfoBarDelegate : public ConfirmInfoBarDelegate {
DISALLOW_COPY_AND_ASSIGN(FullscreenInfoBarDelegate);
};

#endif // CHROME_BROWSER_ANDROID_FULLSCREEN_INFOBAR_DELEGATE_H_
#endif // CHROME_BROWSER_ANDROID_FULLSCREEN_FULLSCREEN_INFOBAR_DELEGATE_H_

0 comments on commit 44470c6

Please sign in to comment.