Extensions: Navigate to default chrome page on extension unload

Currently, when an extension is unloaded, Chrome closes all tabs
associated with the extensions content. If the extension overrode a
Chrome page (namely NTP, bookmarks, history), Chrome would not close
that tab and instead refresh/navigate to the default Chrome page.

However, when there is only one tab in the browser; the tab contains
extension related content; and the tab is not a Chrome page override,
Chrome would close the tab, which thereby closes the browser window.

In such case, Chrome should not close the tab, but instead navigate to
the default NTP chrome page.

Bug: 794472
Change-Id: I5a3438817ad7f442fdd534efe481e35dd7b36a4a
Reviewed-on: https://chromium-review.googlesource.com/862183
Commit-Queue: catmullings <catmullings@chromium.org>
Reviewed-by: catmullings <catmullings@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#530015}

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

@@ -134,14 +134,16 @@ class BrowserActionInteractiveTest : public ExtensionApiTest {
   }
 
   // Open an extension popup via the chrome.browserAction.openPopup API.
-  void OpenPopupViaAPI() {
+  void OpenPopupViaAPI(bool will_reply) {
     // Setup the notification observer to wait for the popup to finish loading.
     content::WindowedNotificationObserver frame_observer(
         content::NOTIFICATION_LOAD_COMPLETED_MAIN_FRAME,
         content::NotificationService::AllSources());
+    ExtensionTestMessageListener listener("ready", will_reply);
     // Show first popup in first window and expect it to have loaded.
     ASSERT_TRUE(RunExtensionSubtest("browser_action/open_popup",
                                     "open_popup_succeeds.html")) << message_;
+    EXPECT_TRUE(listener.WaitUntilSatisfied());
     frame_observer.Wait();
     EnsurePopupActive();
   }
@@ -205,7 +207,7 @@ IN_PROC_BROWSER_TEST_F(BrowserActionInteractiveTest, TestOpenPopup) {
   // Setup extension message listener to wait for javascript to finish running.
   ExtensionTestMessageListener listener("ready", true);
   {
-    OpenPopupViaAPI();
+    OpenPopupViaAPI(true);
     EXPECT_TRUE(browserActionBar.HasPopup());
     browserActionBar.HidePopup();
   }
@@ -331,7 +333,7 @@ IN_PROC_BROWSER_TEST_F(BrowserActionInteractiveTest,
   if (!ShouldRunPopupTest())
     return;
 
-  OpenPopupViaAPI();
+  OpenPopupViaAPI(false);
   ExtensionService* service = extensions::ExtensionSystem::Get(
       browser()->profile())->extension_service();
   ASSERT_FALSE(
@@ -348,7 +350,7 @@ IN_PROC_BROWSER_TEST_F(BrowserActionInteractiveTest,
 IN_PROC_BROWSER_TEST_F(BrowserActionInteractiveTest, FocusLossClosesPopup1) {
   if (!ShouldRunPopupTest())
     return;
-  OpenPopupViaAPI();
+  OpenPopupViaAPI(false);
   ClosePopupViaFocusLoss();
 }
 
@@ -376,7 +378,7 @@ IN_PROC_BROWSER_TEST_F(BrowserActionInteractiveTest, TabSwitchClosesPopup) {
   ASSERT_EQ(2, browser()->tab_strip_model()->count());
   EXPECT_EQ(browser()->tab_strip_model()->GetWebContentsAt(1),
             browser()->tab_strip_model()->GetActiveWebContents());
-  OpenPopupViaAPI();
+  OpenPopupViaAPI(false);
 
   content::WindowedNotificationObserver observer(
       extensions::NOTIFICATION_EXTENSION_HOST_DESTROYED,
@@ -394,7 +396,7 @@ IN_PROC_BROWSER_TEST_F(BrowserActionInteractiveTest,
     return;
 
   // First, we open a popup.
-  OpenPopupViaAPI();
+  OpenPopupViaAPI(false);
   BrowserActionTestUtil browser_action_test_util(browser());
   EXPECT_TRUE(browser_action_test_util.HasPopup());
 
@@ -451,7 +453,7 @@ IN_PROC_BROWSER_TEST_F(BrowserActionInteractiveTest,
   if (!ShouldRunPopupTest())
     return;
 
-  OpenPopupViaAPI();
+  OpenPopupViaAPI(false);
   BrowserActionTestUtil test_util(browser());
   const gfx::NativeView view = test_util.GetPopupNativeView();
   EXPECT_NE(static_cast<gfx::NativeView>(NULL), view);

chrome/browser/extensions/extension_web_ui_browsertest.cc

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/extensions/extension_browsertest.h"
+#include "chrome/browser/extensions/extension_service.h"
+#include "chrome/browser/ui/browser.h"
+#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/web_contents.h"
+#include "content/public/test/browser_test_utils.h"
+#include "extensions/browser/extension_registry.h"
+#include "extensions/browser/test_extension_registry_observer.h"
+#include "extensions/common/extension.h"
+#include "net/test/embedded_test_server/embedded_test_server.h"
+
+using ExtensionWebUIBrowserTest = ExtensionBrowserTest;
+
+namespace extensions {
+
+IN_PROC_BROWSER_TEST_F(ExtensionWebUIBrowserTest,
+                       NavigateToDefaultChromePageOnExtensionUnload) {
+  TabStripModel* tab_strip_model = browser()->tab_strip_model();
+
+  const Extension* extension =
+      LoadExtension(test_data_dir_.AppendASCII("api_test")
+                        .AppendASCII("override")
+                        .AppendASCII("history"));
+  ASSERT_TRUE(extension);
+  std::string extension_id = extension->id();
+
+  // Open the chrome://history/ page in the current tab.
+  GURL extension_url(chrome::kChromeUIHistoryURL);
+  ui_test_utils::NavigateToURL(browser(), extension_url);
+  content::WaitForLoadStop(tab_strip_model->GetActiveWebContents());
+
+  // Check that the chrome://history/ page contains the extension's content.
+  std::string location;
+  EXPECT_TRUE(content::ExecuteScriptAndExtractString(
+      browser()->tab_strip_model()->GetActiveWebContents(),
+      "domAutomationController.send(location.href);\n", &location));
+  EXPECT_EQ(extension_id, GURL(location).host());  // Extension has control.
+
+  ASSERT_EQ(1, tab_strip_model->count());
+
+  // Uninstall the extension.
+  ExtensionService* service =
+      ExtensionSystem::Get(browser()->profile())->extension_service();
+  ExtensionRegistry* registry = ExtensionRegistry::Get(browser()->profile());
+  TestExtensionRegistryObserver registry_observer(registry);
+  service->UnloadExtension(extension->id(), UnloadedExtensionReason::UNINSTALL);
+  registry_observer.WaitForExtensionUnloaded();
+
+  // Check that the opened chrome://history/ page contains the default chrome
+  // history page content.
+  ASSERT_EQ(1, tab_strip_model->count());
+  content::WaitForLoadStop(tab_strip_model->GetActiveWebContents());
+  EXPECT_EQ(
+      chrome::kChromeUIHistoryURL,
+      tab_strip_model->GetActiveWebContents()->GetLastCommittedURL().spec());
+
+  EXPECT_TRUE(content::ExecuteScriptAndExtractString(
+      tab_strip_model->GetActiveWebContents(),
+      "domAutomationController.send(location.href);\n", &location));
+  // Default history page has control.
+  EXPECT_EQ(chrome::kChromeUIHistoryURL, GURL(location).spec());
+}
+
+}  // namespace extensions

chrome/browser/ui/browser.cc

@@ -2103,7 +2103,21 @@ void Browser::OnExtensionUnloaded(content::BrowserContext* browser_context,
            web_contents->GetURL().host_piece() == extension->id()) ||
           (extensions::TabHelper::FromWebContents(web_contents)
                ->extension_app() == extension)) {
-        tab_strip_model_->CloseWebContentsAt(i, TabStripModel::CLOSE_NONE);
+        if (tab_strip_model_->count() > 1) {
+          tab_strip_model_->CloseWebContentsAt(i, TabStripModel::CLOSE_NONE);
+        } else {
+          // If there is only 1 tab remaining, do not close it and instead
+          // navigate to the default NTP. Note that if there is an installed
+          // extension that overrides the NTP page, that extension's content
+          // will override the NTP contents.
+          GURL url(chrome::kChromeUINewTabURL);
+          web_contents->GetController().LoadURL(
+              url,
+              content::Referrer::SanitizeForRequest(
+                  url,
+                  content::Referrer(url, blink::kWebReferrerPolicyDefault)),
+              ui::PAGE_TRANSITION_RELOAD, std::string());
+        }
       } else {
         chrome::UnmuteIfMutedByExtension(web_contents, extension->id());
       }

chrome/browser/ui/browser_browsertest.cc

@@ -34,6 +34,7 @@
 #include "chrome/browser/devtools/devtools_window_testing.h"
 #include "chrome/browser/extensions/extension_browsertest.h"
 #include "chrome/browser/extensions/extension_service.h"
+#include "chrome/browser/extensions/extension_tab_util.h"
 #include "chrome/browser/extensions/extension_util.h"
 #include "chrome/browser/extensions/tab_helper.h"
 #include "chrome/browser/first_run/first_run.h"
@@ -106,6 +107,7 @@
 #include "content/public/test/test_navigation_observer.h"
 #include "extensions/browser/extension_registry.h"
 #include "extensions/browser/extension_system.h"
+#include "extensions/browser/test_extension_registry_observer.h"
 #include "extensions/browser/uninstall_reason.h"
 #include "extensions/common/constants.h"
 #include "extensions/common/extension.h"
@@ -1118,6 +1120,45 @@ IN_PROC_BROWSER_TEST_F(BrowserTest, MAYBE_TabClosingWhenRemovingExtension) {
   ASSERT_EQ(1, browser()->tab_strip_model()->count());
 }
 
+// Tests that when an extension is unloaded, if only one tab is opened
+// containing extenions-related content, then the tab is kept open and is
+// directed to the default NTP.
+IN_PROC_BROWSER_TEST_F(BrowserTest, NavigateToDefaultNTPPageOnExtensionUnload) {
+  ASSERT_TRUE(embedded_test_server()->Start());
+  TabStripModel* tab_strip_model = browser()->tab_strip_model();
+
+  const Extension* extension =
+      LoadExtension(test_data_dir_.AppendASCII("options_page/"));
+  ASSERT_TRUE(extension);
+
+  GURL extension_url = extension->GetResourceURL("options.html");
+  ui_test_utils::NavigateToURL(browser(), extension_url);
+  content::WaitForLoadStop(tab_strip_model->GetActiveWebContents());
+
+  ASSERT_EQ(1, browser()->tab_strip_model()->count());
+  EXPECT_EQ(
+      extension_url.spec(),
+      tab_strip_model->GetActiveWebContents()->GetLastCommittedURL().spec());
+
+  // Uninstall the extension.
+  ExtensionService* service =
+      extensions::ExtensionSystem::Get(browser()->profile())
+          ->extension_service();
+  extensions::ExtensionRegistry* registry =
+      extensions::ExtensionRegistry::Get(browser()->profile());
+  extensions::TestExtensionRegistryObserver registry_observer(registry);
+  service->UnloadExtension(extension->id(),
+                           extensions::UnloadedExtensionReason::UNINSTALL);
+  registry_observer.WaitForExtensionUnloaded();
+  content::WaitForLoadStop(tab_strip_model->GetActiveWebContents());
+
+  // There should only be one tab now, with the NTP loaded.
+  ASSERT_EQ(1, tab_strip_model->count());
+  EXPECT_EQ(
+      chrome::kChromeUINewTabURL,
+      tab_strip_model->GetActiveWebContents()->GetLastCommittedURL().spec());
+}
+
 // Open with --app-id=<id>, and see that an application tab opens by default.
 IN_PROC_BROWSER_TEST_F(BrowserTest, AppIdSwitch) {
   ASSERT_TRUE(embedded_test_server()->Start());
@@ -2372,7 +2413,7 @@ IN_PROC_BROWSER_TEST_F(BrowserTest, GetSizeForNewRenderView) {
       web_contents->GetContainerBounds().size();
   RenderViewSizeObserver observer(web_contents, browser()->window());
 
-  // Navigate to a non-NTP page, without resizing WebContentsView.
+  // Navigate to a non-NTP, without resizing WebContentsView.
   ui_test_utils::NavigateToURL(browser(),
                                embedded_test_server()->GetURL("/title1.html"));
   ASSERT_EQ(BookmarkBar::HIDDEN, browser()->bookmark_bar_state());
@@ -2410,7 +2451,7 @@ IN_PROC_BROWSER_TEST_F(BrowserTest, GetSizeForNewRenderView) {
   EXPECT_EQ(wcv_commit_size0, web_contents->GetContainerBounds().size());
 #endif
 
-  // Navigate to another non-NTP page, without resizing WebContentsView.
+  // Navigate to another non-NTP, without resizing WebContentsView.
   ui_test_utils::NavigateToURL(browser(),
                                https_test_server.GetURL("/title2.html"));
   ASSERT_EQ(BookmarkBar::HIDDEN, browser()->bookmark_bar_state());
@@ -2426,7 +2467,7 @@ IN_PROC_BROWSER_TEST_F(BrowserTest, GetSizeForNewRenderView) {
             web_contents->GetRenderWidgetHostView()->GetViewBounds().size());
   EXPECT_EQ(wcv_commit_size1, web_contents->GetContainerBounds().size());
 
-  // Navigate from NTP to a non-NTP page, resizing WebContentsView while
+  // Navigate from NTP to a non-NTP, resizing WebContentsView while
   // navigation entry is pending.
   ui_test_utils::NavigateToURL(browser(), GURL("chrome://newtab"));
   gfx::Size wcv_resize_insets(1, 1);

chrome/test/BUILD.gn

@@ -1252,6 +1252,7 @@ test("browser_tests") {
         "../browser/extensions/extension_unload_browsertest.cc",
         "../browser/extensions/extension_url_rewrite_browsertest.cc",
         "../browser/extensions/extension_view_host_factory_browsertest.cc",
+        "../browser/extensions/extension_web_ui_browsertest.cc",
         "../browser/extensions/extension_websocket_apitest.cc",
         "../browser/extensions/extension_webui_apitest.cc",
         "../browser/extensions/extension_with_management_policy_apitest.cc",