Commit 98cc07c2 authored by Scott Violet's avatar Scott Violet Committed by Commit Bot

modal-dialogs: avoid possible NPE

The NPE occurs with the following sequence:
1. show tab-modal dialog.
2. some portion of the containing view hierarchy is removed from a window.
3. dialog dismissed.
4. animation started in 1 finishes, triggering trying to use an object
   that was set to null.

Step 3 needs to cancel the start animation.

BUG=1127254
TEST=covered by test

Change-Id: I58c80c0a1e8330c0b87b2a56e228d67bc61b2cba
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2406933
Commit-Queue: Scott Violet <sky@chromium.org>
Reviewed-by: default avatarTheresa  <twellington@chromium.org>
Cr-Commit-Position: refs/heads/master@{#806696}
parent 1e6642e9
......@@ -542,6 +542,23 @@ public class ChromeTabModalPresenterTest {
Assert.assertEquals(BrowserControlsState.BOTH, getBrowserControlsConstraints());
}
@Test
@SmallTest
@Feature({"ModalDialog"})
// Ensures an exception isn't thrown when a dialog is dismissed and the View is no longer
// attached to a Window. See https://crbug.com/1127254 for the specifics.
public void testDismissAfterRemovingView() throws Throwable {
PropertyModel dialog1 = createDialog(mActivity, mManager, "1", null);
TestThreadUtils.runOnUiThreadBlocking(() -> {
mManager.showDialog(dialog1, ModalDialogType.TAB);
ViewGroup containerParent = (ViewGroup) mTabModalPresenter.getContainerParentForTest();
// This is a bit hacky and intended to correspond to a case where the hosting
// ViewGroup is no longer attached to a Window.
containerParent.removeAllViews();
mManager.dismissAllDialogs(DialogDismissalCause.UNKNOWN);
});
}
@BrowserControlsState
private int getBrowserControlsConstraints() {
return TestThreadUtils.runOnUiThreadBlockingNoException(
......
......@@ -131,6 +131,10 @@ public abstract class TabModalPresenter extends ModalDialogManager.Presenter {
// has not yet started.
if (ViewCompat.isAttachedToWindow(mDialogView)) {
runExitAnimation();
} else {
// Cancel any existing animations as when the animation completes it may try to make use
// of objects that have been set to null.
mDialogContainer.animate().cancel();
}
if (mModelChangeProcessor != null) {
......
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