Fix crash when activity is destroyed while loading a page

BrowserControllerImpl::DidFinishNavigation can be called while
destroying WebContents:

https://cs.chromium.org/chromium/src/content/browser/web_contents/web_contents_impl.cc?rcl=81163fe37074bf1ea2bb5a76fe88b2f49e86f23a&l=694
https://cs.chromium.org/chromium/src/content/browser/frame_host/navigation_request.cc?rcl=81163fe37074bf1ea2bb5a76fe88b2f49e86f23a&l=1062

This led to a nullptr dereference.

Bug: 1021041
Change-Id: I1894912ed77e377dd0e1ffa2b746921f13f392b2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1897833
Auto-Submit: Pavel Shmakov <pshmakov@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#712649}

weblayer/browser/browser_controller_impl.cc

@@ -119,8 +119,10 @@ BrowserControllerImpl::BrowserControllerImpl(
 }
 
 BrowserControllerImpl::~BrowserControllerImpl() {
-  // Destruct this now to avoid it calling back when this object is partially
-  // destructed.
+  // Destruct WebContents now to avoid it calling back when this object is
+  // partially destructed. DidFinishNavigation can be called while destroying
+  // WebContents, so stop observing first.
+  Observe(nullptr);
   web_contents_.reset();
 }
 

weblayer/shell/android/BUILD.gn

@@ -217,7 +217,7 @@ instrumentation_test_apk("weblayer_instrumentation_test_apk") {
     "javatests/src/org/chromium/weblayer/test/EventUtils.java",
     "javatests/src/org/chromium/weblayer/test/EventUtils.java",
     "javatests/src/org/chromium/weblayer/test/ExecuteScriptTest.java",
-    "javatests/src/org/chromium/weblayer/test/FragmentRestoreTest.java",
+    "javatests/src/org/chromium/weblayer/test/BrowserFragmentLifecycleTest.java",
     "javatests/src/org/chromium/weblayer/test/FullscreenCallbackTest.java",
     "javatests/src/org/chromium/weblayer/test/InputTypesTest.java",
     "javatests/src/org/chromium/weblayer/test/InstrumentationActivityTestRule.java",

weblayer/shell/android/javatests/src/org/chromium/weblayer/test/FragmentRestoreTest.java → weblayer/shell/android/javatests/src/org/chromium/weblayer/test/BrowserFragmentLifecycleTest.java

@@ -4,7 +4,11 @@
 
 package org.chromium.weblayer.test;
 
+import android.net.Uri;
 import android.support.test.filters.SmallTest;
+import android.support.v4.app.FragmentManager;
+
+import androidx.annotation.NonNull;
 
 import org.junit.Rule;
 import org.junit.Test;
@@ -13,17 +17,22 @@ import org.junit.runner.RunWith;
 import org.chromium.base.test.BaseJUnit4ClassRunner;
 import org.chromium.content_public.browser.test.util.TestThreadUtils;
 import org.chromium.weblayer.BrowserController;
+import org.chromium.weblayer.Navigation;
+import org.chromium.weblayer.NavigationCallback;
+import org.chromium.weblayer.NavigationController;
 import org.chromium.weblayer.shell.InstrumentationActivity;
 
+import java.util.concurrent.CountDownLatch;
+
 @RunWith(BaseJUnit4ClassRunner.class)
-public class FragmentRestoreTest {
+public class BrowserFragmentLifecycleTest {
     @Rule
     public InstrumentationActivityTestRule mActivityTestRule =
             new InstrumentationActivityTestRule();
 
     @Test
     @SmallTest
-    public void successfullyLoadsUrlAfterRotation() {
+    public void successfullyLoadsUrlAfterRecreation() {
         InstrumentationActivity activity = mActivityTestRule.launchShellWithUrl("about:blank");
         BrowserController controller = TestThreadUtils.runOnUiThreadBlockingNoException(
                 () -> activity.getBrowserController());
@@ -39,4 +48,28 @@ public class FragmentRestoreTest {
         url = "data:text,bar";
         mActivityTestRule.navigateAndWait(controller, url, false);
     }
+
+    // https://crbug.com/1021041
+    @Test
+    @SmallTest
+    public void handlesFragmentDestroyWhileNavigating() throws InterruptedException {
+        CountDownLatch latch = new CountDownLatch(1);
+        InstrumentationActivity activity = mActivityTestRule.launchShellWithUrl("about:blank");
+        TestThreadUtils.runOnUiThreadBlocking(() -> {
+            NavigationController navigationController =
+                    activity.getBrowserController().getNavigationController();
+            navigationController.registerNavigationCallback(new NavigationCallback() {
+                @Override
+                public void readyToCommitNavigation(@NonNull Navigation navigation) {
+                    FragmentManager fm = activity.getSupportFragmentManager();
+                    fm.beginTransaction()
+                            .remove(fm.getFragments().get(0))
+                            .runOnCommit(latch::countDown)
+                            .commit();
+                }
+            });
+            navigationController.navigate(Uri.parse("data:text,foo"));
+        });
+        latch.await();
+    }
 }