Don't change the window disposition for new windows without a user gesture

The popup blocker will block them anyways, and if it's disabled, we
shouldn't change the disposition chosen by the web page

BUG=158274

Review URL: https://codereview.chromium.org/527913002

Cr-Commit-Position: refs/heads/master@{#292931}

chrome/browser/extensions/window_open_apitest.cc

@@ -147,8 +147,8 @@ IN_PROC_BROWSER_TEST_F(ExtensionApiTest, WindowOpenPopupIframe) {
   ASSERT_TRUE(LoadExtension(
       test_data_dir_.AppendASCII("window_open").AppendASCII("popup_iframe")));
 
-  const int num_tabs = 0;
-  const int num_popups = 1;
+  const int num_tabs = 1;
+  const int num_popups = 0;
   EXPECT_TRUE(WaitForTabsAndPopups(browser(), num_tabs, num_popups, 0));
 }
 

chrome/browser/ui/blocked_content/popup_blocker_browsertest.cc

@@ -113,19 +113,6 @@ class PopupBlockerBrowserTest : public InProcessBrowserTest {
     return popup_blocker_helper->GetBlockedPopupsCount();
   }
 
-  void NavigateAndCheckPopupShown(const GURL& url) {
-    content::WindowedNotificationObserver observer(
-        chrome::NOTIFICATION_TAB_ADDED,
-        content::NotificationService::AllSources());
-    ui_test_utils::NavigateToURL(browser(), url);
-    observer.Wait();
-
-    ASSERT_EQ(2u, chrome::GetBrowserCount(browser()->profile(),
-                                          browser()->host_desktop_type()));
-
-    ASSERT_EQ(0, GetBlockedContentsCount());
-  }
-
   enum WhatToExpect {
     ExpectPopup,
     ExpectTab
@@ -136,6 +123,28 @@ class PopupBlockerBrowserTest : public InProcessBrowserTest {
     DontCheckTitle
   };
 
+  void NavigateAndCheckPopupShown(const GURL& url,
+                                  WhatToExpect what_to_expect) {
+    content::WindowedNotificationObserver observer(
+        chrome::NOTIFICATION_TAB_ADDED,
+        content::NotificationService::AllSources());
+    ui_test_utils::NavigateToURL(browser(), url);
+    observer.Wait();
+
+    if (what_to_expect == ExpectPopup) {
+      ASSERT_EQ(2u,
+                chrome::GetBrowserCount(browser()->profile(),
+                                        browser()->host_desktop_type()));
+    } else {
+      ASSERT_EQ(1u,
+                chrome::GetBrowserCount(browser()->profile(),
+                                        browser()->host_desktop_type()));
+      ASSERT_EQ(2, browser()->tab_strip_model()->count());
+    }
+
+    ASSERT_EQ(0, GetBlockedContentsCount());
+  }
+
   // Navigates to the test indicated by |test_name| using |browser| which is
   // expected to try to open a popup. Verifies that the popup was blocked and
   // then opens the blocked popup. Once the popup stopped loading, verifies
@@ -290,7 +299,7 @@ IN_PROC_BROWSER_TEST_F(PopupBlockerBrowserTest,
                           std::string(),
                           CONTENT_SETTING_ALLOW);
 
-  NavigateAndCheckPopupShown(url);
+  NavigateAndCheckPopupShown(url, ExpectTab);
 }
 
 // Verify that content settings are applied based on the top-level frame URL.
@@ -306,7 +315,7 @@ IN_PROC_BROWSER_TEST_F(PopupBlockerBrowserTest,
 
   // Popup from the iframe should be allowed since the top-level URL is
   // whitelisted.
-  NavigateAndCheckPopupShown(url);
+  NavigateAndCheckPopupShown(url, ExpectTab);
 
   // Whitelist iframe URL instead.
   GURL::Replacements replace_host;
@@ -337,7 +346,8 @@ IN_PROC_BROWSER_TEST_F(PopupBlockerBrowserTest,
       embedded_test_server()->GetURL("/popup_blocker/popup-on-unload.html"));
   ui_test_utils::NavigateToURL(browser(), url);
 
-  NavigateAndCheckPopupShown(embedded_test_server()->GetURL("/popup_blocker/"));
+  NavigateAndCheckPopupShown(embedded_test_server()->GetURL("/popup_blocker/"),
+                             ExpectPopup);
 }
 
 // Verify that when you unblock popup, the popup shows in history and omnibox.
@@ -347,7 +357,7 @@ IN_PROC_BROWSER_TEST_F(PopupBlockerBrowserTest,
       switches::kDisablePopupBlocking);
   GURL url(embedded_test_server()->GetURL(
       "/popup_blocker/popup-blocked-to-post-blank.html"));
-  NavigateAndCheckPopupShown(url);
+  NavigateAndCheckPopupShown(url, ExpectTab);
 
   std::string search_string =
       "data:text/html,<title>Popup Success!</title>you should not see this "

content/renderer/render_view_impl.cc

@@ -2027,15 +2027,6 @@ void RenderViewImpl::show(WebNavigationPolicy policy) {
 
   DCHECK(opener_id_ != MSG_ROUTING_NONE);
 
-  // Force new windows to a popup if they were not opened with a user gesture.
-  if (!opened_by_user_gesture_) {
-    // We exempt background tabs for compat with older versions of Chrome.
-    // TODO(darin): This seems bogus.  These should have a user gesture, so
-    // we probably don't need this check.
-    if (policy != blink::WebNavigationPolicyNewBackgroundTab)
-      policy = blink::WebNavigationPolicyNewPopup;
-  }
-
   // NOTE: initial_pos_ may still have its default values at this point, but
   // that's okay.  It'll be ignored if disposition is not NEW_POPUP, or the
   // browser process will impose a default position otherwise.