Don't consume user gestures when we don't support multiple windows

There's no point in protecting against multiple popups in that case.. BUG=762482 R=torne@chromium.org Change-Id: Ic8c953ff5235c5cb32b1db318c3828b86d722a95 Reviewed-on: https://chromium-review.googlesource.com/695543 Commit-Queue: Jochen Eisinger <jochen@chromium.org> Reviewed-by: Richard Coles <torne@chromium.org> Cr-Commit-Position: refs/heads/master@{#506363}
Denger-Network · Oct 4, 2017 · 8cb2849 · 8cb2849
1 parent e2925a1
commit 8cb2849
Show file tree

Hide file tree

Showing 2 changed files with 28 additions and 12 deletions.
diff --git a/android_webview/javatests/src/org/chromium/android_webview/test/AwSettingsTest.java b/android_webview/javatests/src/org/chromium/android_webview/test/AwSettingsTest.java
@@ -1140,10 +1140,10 @@ class AwSettingsJavaScriptPopupsTestHelper extends AwSettingsTestHelper<Boolean>
         private static final String POPUP_ENABLED = "Popup enabled";
         private static final String POPUP_BLOCKED = "Popup blocked";
 
-        AwSettingsJavaScriptPopupsTestHelper(
-                AwTestContainerView containerView,
-                TestAwContentsClient contentViewClient) throws Throwable {
+        AwSettingsJavaScriptPopupsTestHelper(AwTestContainerView containerView,
+                TestAwContentsClient contentViewClient, boolean openTwice) throws Throwable {
             super(containerView, contentViewClient, true);
+            mOpenTwice = openTwice;
         }
 
         @Override
@@ -1183,6 +1183,7 @@ private String getData() {
                     + "<script>"
                     + "    function tryOpenWindow() {"
                     + "        var newWindow = window.open('about:blank');"
+                    + (mOpenTwice ? "newWindow = window.open('about:blank');" : "")
                     + "        if (newWindow) {"
                     + "            if (newWindow === window) {"
                     + "                newWindow.document.write("
@@ -1198,6 +1199,8 @@ private String getData() {
                     + "</script></head>"
                     + "<body onload='tryOpenWindow()'></body></html>";
         }
+
+        private boolean mOpenTwice;
     }
 
     class AwSettingsCacheModeTestHelper extends AwSettingsTestHelper<Integer> {
@@ -2263,10 +2266,10 @@ public void testTextZoomAutosizingWithTwoViews() throws Throwable {
     @Feature({"AndroidWebView", "Preferences"})
     public void testJavaScriptPopupsWithTwoViews() throws Throwable {
         ViewPair views = createViews();
-        runPerViewSettingsTest(
-                new AwSettingsJavaScriptPopupsTestHelper(views.getContainer0(), views.getClient0()),
+        runPerViewSettingsTest(new AwSettingsJavaScriptPopupsTestHelper(
+                                       views.getContainer0(), views.getClient0(), false),
                 new AwSettingsJavaScriptPopupsTestHelper(
-                        views.getContainer1(), views.getClient1()));
+                        views.getContainer1(), views.getClient1(), false));
     }
 
     @Test
@@ -2282,10 +2285,21 @@ public void testJavaScriptPopupsAndMultiWindowsWithTwoViews() throws Throwable {
         });
         views.getClient0().getOnCreateWindowHelper().setReturnValue(true);
         views.getClient1().getOnCreateWindowHelper().setReturnValue(true);
-        runPerViewSettingsTest(
-                new AwSettingsJavaScriptPopupsTestHelper(views.getContainer0(), views.getClient0()),
+        runPerViewSettingsTest(new AwSettingsJavaScriptPopupsTestHelper(
+                                       views.getContainer0(), views.getClient0(), false),
                 new AwSettingsJavaScriptPopupsTestHelper(
-                        views.getContainer1(), views.getClient1()));
+                        views.getContainer1(), views.getClient1(), false));
+    }
+
+    @Test
+    @SmallTest
+    @Feature({"AndroidWebView", "Preferences"})
+    public void testJavaScriptPopupsOpenTwice() throws Throwable {
+        final ViewPair views = createViews();
+        runPerViewSettingsTest(new AwSettingsJavaScriptPopupsTestHelper(
+                                       views.getContainer0(), views.getClient0(), true),
+                new AwSettingsJavaScriptPopupsTestHelper(
+                        views.getContainer1(), views.getClient1(), true));
     }
 
     @Test

diff --git a/content/renderer/render_view_impl.cc b/content/renderer/render_view_impl.cc
@@ -1359,15 +1359,17 @@ WebView* RenderViewImpl::CreateView(WebLocalFrame* creator,
   if (err || reply->route_id == MSG_ROUTING_NONE)
     return nullptr;
 
-  WebUserGestureIndicator::ConsumeUserGesture();
-
   // For Android WebView, we support a pop-up like behavior for window.open()
   // even if the embedding app doesn't support multiple windows. In this case,
   // window.open() will return "window" and navigate it to whatever URL was
-  // passed.
+  // passed. We also don't need to consume user gestures to protect against
+  // multiple windows being opened, because, well, the app doesn't support
+  // multiple windows.
   if (reply->route_id == GetRoutingID())
     return webview();
 
+  WebUserGestureIndicator::ConsumeUserGesture();
+
   // While this view may be a background extension page, it can spawn a visible
   // render view. So we just assume that the new one is not another background
   // page instead of passing on our own value.