weblayer: ensures TabImpl::OpenURLFromTab handles WebContents deletion

Specifically if the NewTabCallback deletes then tab, then we shouldn't
try to load a url.

BUG=1142090
TEST=testOpenPopupFromInfoBarAndNewTabCallbackDestroysTab

Change-Id: Ib31e7cb52d7dec6d1724c1e3471b7c5ace72c858
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2496160Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Commit-Queue: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#820910}

weblayer/browser/android/javatests/src/org/chromium/weblayer/test/PopupTest.java

@@ -15,9 +15,12 @@ import org.junit.Rule;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 
+import org.chromium.base.test.util.CallbackHelper;
 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.weblayer.NewTabCallback;
+import org.chromium.weblayer.Tab;
 import org.chromium.weblayer.TestWebLayer;
 import org.chromium.weblayer.shell.InstrumentationActivity;
 
@@ -65,4 +68,40 @@ public final class PopupTest {
         EventUtils.simulateTouchCenterOfView(mActivity.findViewById(buttonId));
         callback.waitForNewTab();
     }
+
+    @Test
+    @SmallTest
+    // This is a regression test for https://crbug.com/1142090 and verifies no crash when
+    // NewTabCallback.onNewTab() destroys the supplied tab.
+    public void testOpenPopupFromInfoBarAndNewTabCallbackDestroysTab() throws Exception {
+        CallbackHelper helper = new CallbackHelper();
+        NewTabCallback callback = new NewTabCallback() {
+            @Override
+            public void onNewTab(Tab tab, int mode) {
+                tab.getBrowser().destroyTab(tab);
+                helper.notifyCalled();
+            }
+        };
+        TestThreadUtils.runOnUiThreadBlocking(
+                () -> { mActivity.getBrowser().getActiveTab().setNewTabCallback(callback); });
+
+        // Try to open a popup.
+        mActivityTestRule.executeScriptSync("window.open('about:blank')", true);
+
+        // Make sure the infobar shows up and the popup has not been opened.
+        String packageName =
+                TestWebLayer.getWebLayerContext(mActivity.getApplicationContext()).getPackageName();
+        int buttonId = ResourceUtil.getIdentifier(mRemoteContext, "id/button_primary", packageName);
+        CriteriaHelper.pollInstrumentationThread(() -> {
+            Criteria.checkThat(mActivity.findViewById(buttonId), Matchers.notNullValue());
+        });
+        TestThreadUtils.runOnUiThreadBlocking(
+                () -> { Assert.assertEquals(mActivity.getBrowser().getTabs().size(), 1); });
+
+        // Click the button on the infobar to open the popup.
+        EventUtils.simulateTouchCenterOfView(mActivity.findViewById(buttonId));
+
+        // Wait for tab to be destroyed.
+        helper.waitForFirst();
+    }
 }

weblayer/browser/tab_impl.cc

@@ -45,6 +45,7 @@
 #include "content/public/browser/render_widget_host_view.h"
 #include "content/public/browser/renderer_preferences_util.h"
 #include "content/public/browser/web_contents.h"
+#include "content/public/browser/web_contents_observer.h"
 #include "third_party/blink/public/common/renderer_preferences/renderer_preferences.h"
 #include "third_party/blink/public/common/web_preferences/web_preferences.h"
 #include "third_party/blink/public/mojom/window_features/window_features.mojom.h"
@@ -232,6 +233,15 @@ std::set<TabImpl*>& GetTabs() {
   return *s_all_tab_impl;
 }
 
+// Simulates a WeakPtr for WebContents. Specifically if the WebContents
+// supplied to the constructor is destroyed then web_contents() returns
+// null.
+class WebContentsTracker : public content::WebContentsObserver {
+ public:
+  explicit WebContentsTracker(content::WebContents* web_contents)
+      : content::WebContentsObserver(web_contents) {}
+};
+
 }  // namespace
 
 #if defined(OS_ANDROID)
@@ -854,15 +864,15 @@ content::WebContents* TabImpl::OpenURLFromTab(
   std::unique_ptr<content::WebContents> new_tab_contents =
       content::WebContents::Create(content::WebContents::CreateParams(
           web_contents()->GetBrowserContext()));
-  content::WebContents* new_tab_contents_raw = new_tab_contents.get();
+  WebContentsTracker tracker(new_tab_contents.get());
   bool was_blocked = false;
   AddNewContents(web_contents(), std::move(new_tab_contents), params.url,
                  params.disposition, {}, params.user_gesture, &was_blocked);
-  if (was_blocked)
+  if (was_blocked || !tracker.web_contents())
     return nullptr;
-  new_tab_contents_raw->GetController().LoadURLWithParams(
+  tracker.web_contents()->GetController().LoadURLWithParams(
       content::NavigationController::LoadURLParams(params));
-  return new_tab_contents_raw;
+  return tracker.web_contents();
 }
 
 void TabImpl::ShowRepostFormWarningDialog(content::WebContents* source) {