Commit 50bec60b authored by Scott Violet's avatar Scott Violet Committed by Commit Bot

weblayer: fix possible NPE when switching tabs

ModalDialogManager.dismissDialog() may call onLastDialogDismissed()
even if nothing was hidden. BrowserViewController was assuming this
never happened. This matters on
BrowserViewController.onLastDialogDismissed() assumed mTab was non-null.
The fix is to add a null check to BrowserViewController.

BUG=1121388
TEST=covered by test

Change-Id: I2c806018a635d9434bf10c653b2615ea645b8e04
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2382880
Commit-Queue: Scott Violet <sky@chromium.org>
Reviewed-by: default avatarEvan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#803284}
parent 58ea45f9
......@@ -29,6 +29,7 @@ import org.chromium.content_public.browser.test.util.Criteria;
import org.chromium.content_public.browser.test.util.CriteriaHelper;
import org.chromium.content_public.browser.test.util.TestThreadUtils;
import org.chromium.net.test.util.TestWebServer;
import org.chromium.weblayer.Browser;
import org.chromium.weblayer.LoadError;
import org.chromium.weblayer.NavigateParams;
import org.chromium.weblayer.Navigation;
......@@ -37,6 +38,7 @@ import org.chromium.weblayer.NavigationController;
import org.chromium.weblayer.NavigationState;
import org.chromium.weblayer.Tab;
import org.chromium.weblayer.TabCallback;
import org.chromium.weblayer.TabListCallback;
import org.chromium.weblayer.WebLayer;
import org.chromium.weblayer.shell.InstrumentationActivity;
......@@ -828,6 +830,62 @@ public class NavigationTest {
assertFalse(mCallback.onStartedCallback.isPageInitiated());
}
// Verifies the following sequence doesn't crash:
// 1. create a new background tab.
// 2. show modal dialog.
// 3. destroy tab with modal dialog.
// 4. switch to background tab created in step 1.
// This is a regression test for https://crbug.com/1121388.
@Test
@SmallTest
@MinWebLayerVersion(87)
public void testDestroyTabWithModalDialog() throws Exception {
// Load a page with a form.
InstrumentationActivity activity =
mActivityTestRule.launchShellWithUrl(mActivityTestRule.getTestDataURL("form.html"));
assertNotNull(activity);
setNavigationCallback(activity);
// Touch the page; this should submit the form.
int currentCallCount = mCallback.onCompletedCallback.getCallCount();
EventUtils.simulateTouchCenterOfView(activity.getWindow().getDecorView());
String targetUrl = mActivityTestRule.getTestDataURL("simple_page.html");
mCallback.onCompletedCallback.assertCalledWith(currentCallCount, targetUrl);
Tab secondTab = runOnUiThreadBlocking(() -> activity.getTab().getBrowser().createTab());
// Make sure a tab modal shows after we attempt a reload.
Boolean isTabModalShowingResult[] = new Boolean[1];
CallbackHelper callbackHelper = new CallbackHelper();
runOnUiThreadBlocking(() -> {
Tab tab = activity.getTab();
Browser browser = tab.getBrowser();
TabCallback callback = new TabCallback() {
@Override
public void onTabModalStateChanged(boolean isTabModalShowing) {
tab.unregisterTabCallback(this);
isTabModalShowingResult[0] = isTabModalShowing;
callbackHelper.notifyCalled();
}
};
tab.registerTabCallback(callback);
browser.registerTabListCallback(new TabListCallback() {
@Override
public void onTabRemoved(Tab tab) {
browser.unregisterTabListCallback(this);
browser.setActiveTab(secondTab);
}
});
tab.getNavigationController().reload();
});
callbackHelper.waitForFirst();
runOnUiThreadBlocking(() -> {
Tab tab = activity.getTab();
tab.getBrowser().destroyTab(tab);
});
}
/**
* This test verifies calling destroyTab() from within onNavigationFailed doesn't crash.
*/
......
......@@ -299,7 +299,12 @@ public final class BrowserViewController
private void onDialogVisibilityChanged(boolean showing) {
if (WebLayerFactoryImpl.getClientMajorVersion() < 82) return;
if (mModalDialogManager.getCurrentType() == ModalDialogType.TAB) {
// ModalDialogManager.onLastDialogDismissed() may be called if |mTab| is currently null.
// This is because in some situations ModalDialogManager calls onLastDialogDismissed() even
// if there were no dialogs present and dismissDialog() is called. This matters as
// dismissDialog() may be called when |mTab| is null.
// TODO(sky): fix ModalDialogManager and remove mTab conditional.
if (mModalDialogManager.getCurrentType() == ModalDialogType.TAB && mTab != null) {
try {
mTab.getClient().onTabModalStateChanged(showing);
} catch (RemoteException e) {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment