Revert of Allow top-level navigation in extension pop-ups if it only triggers a download.

chrome/browser/extensions/api/extension_action/browser_action_apitest.cc

Index: chrome/browser/extensions/api/extension_action/browser_action_apitest.cc
diff --git a/chrome/browser/extensions/api/extension_action/browser_action_apitest.cc b/chrome/browser/extensions/api/extension_action/browser_action_apitest.cc
index f64a27981a92d3f5f24d5d93e7194da4e693b423..a8f61c9e8e30fa7545fcc3c368392fdcb7fdc905 100644
--- a/chrome/browser/extensions/api/extension_action/browser_action_apitest.cc
+++ b/chrome/browser/extensions/api/extension_action/browser_action_apitest.cc
@@ -24,14 +24,10 @@
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/common/url_constants.h"
 #include "chrome/test/base/ui_test_utils.h"
-#include "content/public/browser/browser_context.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/test/browser_test_utils.h"
-#include "content/public/test/content_browser_test_utils.h"
-#include "content/public/test/download_test_observer.h"
-#include "content/public/test/test_utils.h"
 #include "extensions/browser/extension_registry.h"
 #include "extensions/browser/extension_system.h"
 #include "extensions/browser/notification_types.h"
@@ -42,7 +38,6 @@
 #include "extensions/test/result_catcher.h"
 #include "net/dns/mock_host_resolver.h"
 #include "net/test/embedded_test_server/embedded_test_server.h"
-#include "testing/gmock/include/gmock/gmock.h"
 #include "ui/base/resource/resource_bundle.h"
 #include "ui/gfx/geometry/rect.h"
 #include "ui/gfx/geometry/size.h"
@@ -110,7 +105,7 @@
     return browser_action_test_util_.get();
   }
 
-  WebContents* OpenPopup(int index) {
+  bool OpenPopup(int index) {
     ResultCatcher catcher;
     content::WindowedNotificationObserver popup_observer(
         content::NOTIFICATION_LOAD_COMPLETED_MAIN_FRAME,
@@ -118,13 +113,7 @@
     GetBrowserActionsBar()->Press(index);
     popup_observer.Wait();
     EXPECT_TRUE(catcher.GetNextResult()) << catcher.message();
-
-    if (!GetBrowserActionsBar()->HasPopup())
-      return nullptr;
-
-    const auto& source = static_cast<const content::Source<WebContents>&>(
-        popup_observer.source());
-    return source.ptr();
+    return GetBrowserActionsBar()->HasPopup();
   }
 
   ExtensionAction* GetBrowserAction(const Extension& extension) {
@@ -805,192 +794,5 @@
   EXPECT_TRUE(catcher.GetNextResult()) << message_;
 }
 
-class NavigatingExtensionPopupBrowserTest : public BrowserActionApiTest {
- public:
-  const Extension& popup_extension() { return *popup_extension_; }
-  const Extension& other_extension() { return *other_extension_; }
-
-  void SetUpOnMainThread() override {
-    BrowserActionApiTest::SetUpOnMainThread();
-
-    host_resolver()->AddRule("*", "127.0.0.1");
-    ASSERT_TRUE(embedded_test_server()->Start());
-
-    // Load an extension with a pop-up.
-    ASSERT_TRUE(popup_extension_ = LoadExtension(test_data_dir_.AppendASCII(
-                    "browser_action/popup_with_form")));
-
-    // Load another extension (that we can try navigating to).
-    ASSERT_TRUE(other_extension_ = LoadExtension(test_data_dir_.AppendASCII(
-                    "browser_action/popup_with_iframe")));
-  }
-
-  enum ExpectedNavigationStatus {
-    EXPECTING_NAVIGATION_SUCCESS,
-    EXPECTING_NAVIGATION_FAILURE,
-  };
-
-  void TestPopupNavigationViaGet(
-      const GURL& target_url,
-      ExpectedNavigationStatus expected_navigation_status) {
-    std::string navigation_starting_script =
-        "window.location = '" + target_url.spec() + "';\n";
-    TestPopupNavigation(target_url, expected_navigation_status,
-                        navigation_starting_script);
-  }
-
-  void TestPopupNavigationViaPost(
-      const GURL& target_url,
-      ExpectedNavigationStatus expected_navigation_status) {
-    std::string navigation_starting_script =
-        "var form = document.getElementById('form');\n"
-        "form.action = '" + target_url.spec() + "';\n"
-        "form.submit();\n";
-    TestPopupNavigation(target_url, expected_navigation_status,
-                        navigation_starting_script);
-  }
-
- private:
-  void TestPopupNavigation(const GURL& target_url,
-                           ExpectedNavigationStatus expected_navigation_status,
-                           std::string navigation_starting_script) {
-    // Were there any failures so far (e.g. in SetUpOnMainThread)?
-    ASSERT_FALSE(HasFailure());
-
-    // Simulate a click on the browser action to open the popup.
-    WebContents* popup = OpenPopup(0);
-    ASSERT_TRUE(popup);
-    GURL popup_url = popup_extension().GetResourceURL("popup.html");
-    EXPECT_EQ(popup_url, popup->GetLastCommittedURL());
-
-    // Note that the |setTimeout| call below is needed to make sure
-    // ExecuteScriptAndExtractBool returns *after* a scheduled navigation has
-    // already started.
-    std::string script_to_execute =
-        navigation_starting_script +
-        "setTimeout(\n"
-        "    function() { window.domAutomationController.send(true); },\n"
-        "    0);\n";
-
-    // Try to navigate the pop-up.
-    bool ignored_script_result = false;
-    content::WebContentsDestroyedWatcher popup_destruction_watcher(popup);
-    EXPECT_TRUE(ExecuteScriptAndExtractBool(popup, script_to_execute,
-                                            &ignored_script_result));
-    popup = popup_destruction_watcher.web_contents();
-
-    // Verify if the popup navigation succeeded or failed as expected.
-    if (!popup) {
-      // If navigation ends up in a tab, then the tab will be focused and
-      // therefore the popup will be closed, destroying associated WebContents -
-      // don't do any verification in this case.
-      ADD_FAILURE() << "Navigation should not close extension pop-up";
-    } else {
-      // If the extension popup is still opened, then wait until there is no
-      // load in progress, and verify whether the navigation succeeded or not.
-      WaitForLoadStop(popup);
-      if (expected_navigation_status == EXPECTING_NAVIGATION_SUCCESS) {
-        EXPECT_EQ(target_url, popup->GetLastCommittedURL())
-            << "Navigation to " << target_url
-            << " should succeed in an extension pop-up";
-      } else {
-        EXPECT_NE(target_url, popup->GetLastCommittedURL())
-            << "Navigation to " << target_url
-            << " should fail in an extension pop-up";
-        EXPECT_THAT(
-            popup->GetLastCommittedURL(),
-            ::testing::AnyOf(::testing::Eq(popup_url),
-                             ::testing::Eq(GURL("chrome-extension://invalid")),
-                             ::testing::Eq(GURL("about:blank"))));
-      }
-    }
-
-    // Make sure that the web navigation did not succeed somewhere outside of
-    // the extension popup (as it might if ExtensionViewHost::OpenURLFromTab
-    // forwards the navigation to Browser::OpenURL [which doesn't specify a
-    // source WebContents]).
-    TabStripModel* tabs = browser()->tab_strip_model();
-    for (int i = 0; i < tabs->count(); i++) {
-      WebContents* tab_contents = tabs->GetWebContentsAt(i);
-      WaitForLoadStop(tab_contents);
-      EXPECT_NE(target_url, tab_contents->GetLastCommittedURL())
-          << "Navigating an extension pop-up should not affect tabs.";
-    }
-
-    // Close the pop-up.
-    EXPECT_TRUE(GetBrowserActionsBar()->HidePopup());
-  }
-
-  const Extension* popup_extension_;
-  const Extension* other_extension_;
-};
-
-// Tests that an extension pop-up cannot be navigated to a web page.
-IN_PROC_BROWSER_TEST_F(NavigatingExtensionPopupBrowserTest, Webpage) {
-  GURL web_url(embedded_test_server()->GetURL("foo.com", "/title1.html"));
-  TestPopupNavigationViaGet(web_url, EXPECTING_NAVIGATION_FAILURE);
-  TestPopupNavigationViaPost(web_url, EXPECTING_NAVIGATION_FAILURE);
-}
-
-// Tests that an extension pop-up can be navigated to another page
-// in the same extension.
-IN_PROC_BROWSER_TEST_F(NavigatingExtensionPopupBrowserTest,
-                       PageInSameExtension) {
-  GURL other_page_in_same_extension =
-      popup_extension().GetResourceURL("other_page.html");
-  TestPopupNavigationViaGet(other_page_in_same_extension,
-                            EXPECTING_NAVIGATION_SUCCESS);
-  TestPopupNavigationViaPost(other_page_in_same_extension,
-                             EXPECTING_NAVIGATION_SUCCESS);
-}
-
-// Tests that an extension pop-up cannot be navigated to a page
-// in another extension.
-IN_PROC_BROWSER_TEST_F(NavigatingExtensionPopupBrowserTest,
-                       PageInOtherExtension) {
-  GURL other_extension_url = other_extension().GetResourceURL("other.html");
-  TestPopupNavigationViaGet(other_extension_url, EXPECTING_NAVIGATION_FAILURE);
-  TestPopupNavigationViaPost(other_extension_url, EXPECTING_NAVIGATION_FAILURE);
-}
-
-// Tests that navigating an extension pop-up to a http URI that returns
-// Content-Disposition: attachment; filename=...
-// works: No navigation, but download shelf visible + download goes through.
-//
-// Note - there is no "...ViaGet" flavour of this test, because we don't care
-// (yet) if GET succeeds with the download or not (it probably should succeed
-// for consistency with POST, but it always failed in M54 and before).  After
-// abandoing ShouldFork/OpenURL for all methods (not just for POST) [see comment
-// about https://crbug.com/646261 in ChromeContentRendererClient::ShouldFork]
-// GET should automagically start working for downloads.
-// TODO(lukasza): https://crbug.com/650694: Add a "Get" flavour of the test once
-// the download works both for GET and POST requests.
-IN_PROC_BROWSER_TEST_F(NavigatingExtensionPopupBrowserTest, DownloadViaPost) {
-  content::DownloadTestObserverTerminal downloads_observer(
-      content::BrowserContext::GetDownloadManager(browser()->profile()),
-      1,  // == wait_count (only waiting for "download-test3.gif").
-      content::DownloadTestObserver::ON_DANGEROUS_DOWNLOAD_FAIL);
-
-  // Navigate to a URL that replies with
-  // Content-Disposition: attachment; filename=...
-  // header.
-  GURL download_url(
-      embedded_test_server()->GetURL("foo.com", "/download-test3.gif"));
-  TestPopupNavigationViaPost(download_url, EXPECTING_NAVIGATION_FAILURE);
-
-  // Verify that "download-test3.gif got downloaded.
-  downloads_observer.WaitForFinished();
-  EXPECT_EQ(0u, downloads_observer.NumDangerousDownloadsSeen());
-  EXPECT_EQ(1u, downloads_observer.NumDownloadsSeenInState(
-                    content::DownloadItem::COMPLETE));
-
-  // The test verification below is applicable only to scenarios where the
-  // download shelf is supported - on ChromeOS, instead of the download shelf,
-  // there is a download notification in the right-bottom corner of the screen.
-#if !defined(OS_CHROMEOS)
-  EXPECT_TRUE(browser()->window()->IsDownloadShelfVisible());
-#endif
-}
-
 }  // namespace
 }  // namespace extensions