[Extensions] Close opaque-origin windows opened by extensions on extension unload
When an extension is unloaded, we close any tabs that were on the
extension origin to prevent it from running any further. However, an
extension can also open a window with an opaque origin (e.g.,
about:blank) and modify it (e.g. to include a script tag). We should
close these windows on extension unload as well to prevent it from
continuing to run.
Note that it's pretty much fundamentally impossible to truly prevent
continued execution by the extension, such as in the case of content
scripts (there's no such thing as un-injecting a content script), but
it's good to fix this specific case.
See bug for more details.
Bug: 894477
Change-Id: Ie6cdd9b6c05279ca9178e291a4d785ae53f69906
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1589246
Reviewed-by: Scott Violet <[email protected]>
Reviewed-by: Karan Bhatia <[email protected]>
Reviewed-by: Nasko Oskov <[email protected]>
Commit-Queue: Devlin <[email protected]>
Cr-Commit-Position: refs/heads/master@{#672971}
diff --git a/chrome/browser/extensions/extension_unload_browsertest.cc b/chrome/browser/extensions/extension_unload_browsertest.cc
index 687a51d..5306e1d 100644
--- a/chrome/browser/extensions/extension_unload_browsertest.cc
+++ b/chrome/browser/extensions/extension_unload_browsertest.cc
@@ -3,19 +3,71 @@
// found in the LICENSE file.
#include "base/feature_list.h"
+#include "base/run_loop.h"
+#include "base/scoped_observer.h"
#include "chrome/browser/extensions/extension_browsertest.h"
#include "chrome/browser/extensions/extension_service.h"
#include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "chrome/browser/ui/tabs/tab_strip_model_observer.h"
#include "chrome/test/base/ui_test_utils.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/no_renderer_crashes_assertion.h"
+#include "content/public/test/test_navigation_observer.h"
+#include "content/public/test/test_utils.h"
+#include "extensions/browser/disable_reason.h"
+#include "extensions/common/constants.h"
+#include "extensions/common/extension.h"
+#include "extensions/test/test_extension_dir.h"
#include "net/dns/mock_host_resolver.h"
#include "net/test/embedded_test_server/embedded_test_server.h"
#include "services/network/public/cpp/features.h"
#include "ui/base/window_open_disposition.h"
+#include "url/origin.h"
+#include "url/url_constants.h"
namespace extensions {
+namespace {
+
+// A helper class to wait for a particular tab count. Requires the tab strip
+// to outlive this object.
+class TestTabStripModelObserver : public TabStripModelObserver {
+ public:
+ explicit TestTabStripModelObserver(TabStripModel* model)
+ : model_(model), desired_count_(0), scoped_observer_(this) {
+ scoped_observer_.Add(model);
+ }
+ ~TestTabStripModelObserver() override = default;
+
+ void WaitForTabCount(int count) {
+ if (model_->count() == count)
+ return;
+ desired_count_ = count;
+ run_loop_.Run();
+ }
+
+ private:
+ // TabStripModelObserver:
+ void OnTabStripModelChanged(
+ TabStripModel* tab_strip_model,
+ const TabStripModelChange& change,
+ const TabStripSelectionChange& selection) override {
+ if (model_->count() == desired_count_)
+ run_loop_.Quit();
+ }
+
+ TabStripModel* model_;
+ int desired_count_;
+ base::RunLoop run_loop_;
+ ScopedObserver<TabStripModel, TabStripModelObserver> scoped_observer_;
+
+ DISALLOW_COPY_AND_ASSIGN(TestTabStripModelObserver);
+};
+
+} // namespace
+
class ExtensionUnloadBrowserTest : public ExtensionBrowserTest {
public:
void SetUpOnMainThread() override {
@@ -92,6 +144,106 @@
->IsRenderFrameLive());
}
+// Tests that windows with opaque origins opened by the extension are closed
+// when the extension is unloaded. Regression test for https://crbug.com/894477.
+IN_PROC_BROWSER_TEST_F(ExtensionUnloadBrowserTest, OpenedOpaqueWindows) {
+ TestExtensionDir test_dir;
+ constexpr char kManifest[] =
+ R"({
+ "name": "Test",
+ "manifest_version": 2,
+ "version": "0.1",
+ "background": {
+ "scripts": ["background.js"]
+ }
+ })";
+ test_dir.WriteManifest(kManifest);
+ test_dir.WriteFile(FILE_PATH_LITERAL("background.js"),
+ "window.open('about:blank');");
+
+ const GURL about_blank(url::kAboutBlankURL);
+ content::TestNavigationObserver about_blank_observer(about_blank);
+ about_blank_observer.StartWatchingNewWebContents();
+ const Extension* extension = LoadExtension(test_dir.UnpackedPath());
+ ASSERT_TRUE(extension);
+ about_blank_observer.WaitForNavigationFinished();
+
+ EXPECT_EQ(2, browser()->tab_strip_model()->count());
+ content::WebContents* web_contents =
+ browser()->tab_strip_model()->GetActiveWebContents();
+ EXPECT_EQ(about_blank, web_contents->GetLastCommittedURL());
+ url::Origin frame_origin =
+ web_contents->GetMainFrame()->GetLastCommittedOrigin();
+ url::SchemeHostPort precursor_tuple =
+ frame_origin.GetTupleOrPrecursorTupleIfOpaque();
+ EXPECT_EQ(kExtensionScheme, precursor_tuple.scheme());
+ EXPECT_EQ(extension->id(), precursor_tuple.host());
+
+ TestTabStripModelObserver test_tab_strip_model_observer(
+ browser()->tab_strip_model());
+ extension_service()->DisableExtension(extension->id(),
+ disable_reason::DISABLE_USER_ACTION);
+ test_tab_strip_model_observer.WaitForTabCount(1);
+
+ EXPECT_EQ(1, browser()->tab_strip_model()->count());
+}
+
+IN_PROC_BROWSER_TEST_F(ExtensionUnloadBrowserTest, CrashedTabs) {
+ TestExtensionDir test_dir;
+ test_dir.WriteManifest(
+ R"({
+ "name": "test extension",
+ "manifest_version": 2,
+ "version": "0.1"
+ })");
+ test_dir.WriteFile(FILE_PATH_LITERAL("page.html"),
+ "<!doctype html><html><body>Hello world</body></html>");
+ scoped_refptr<const Extension> extension(
+ LoadExtension(test_dir.UnpackedPath()));
+ ASSERT_TRUE(extension);
+ const GURL page_url = extension->GetResourceURL("page.html");
+ ui_test_utils::NavigateToURLWithDisposition(
+ browser(), page_url, WindowOpenDisposition::NEW_FOREGROUND_TAB,
+ ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION);
+
+ EXPECT_EQ(2, browser()->tab_strip_model()->count());
+
+ content::WebContents* active_tab =
+ browser()->tab_strip_model()->GetActiveWebContents();
+ EXPECT_EQ(page_url, active_tab->GetLastCommittedURL());
+
+ {
+ content::ScopedAllowRendererCrashes allow_renderer_crashes(
+ active_tab->GetMainFrame()->GetProcess());
+ ui_test_utils::NavigateToURLWithDisposition(
+ browser(), GURL("chrome://crash"), WindowOpenDisposition::CURRENT_TAB,
+ ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION);
+ }
+
+ // There should still be two open tabs, but the active one is crashed.
+ EXPECT_EQ(2, browser()->tab_strip_model()->count());
+ EXPECT_TRUE(active_tab->IsCrashed());
+
+ // Even though the tab is crashed, it should still have the last committed
+ // URL of the extension page.
+ EXPECT_EQ(page_url, active_tab->GetLastCommittedURL());
+
+ // Unloading the extension should close the crashed tab, since its origin was
+ // still the extension's origin.
+ TestTabStripModelObserver test_tab_strip_model_observer(
+ browser()->tab_strip_model());
+ extension_service()->DisableExtension(extension->id(),
+ disable_reason::DISABLE_USER_ACTION);
+ test_tab_strip_model_observer.WaitForTabCount(1);
+
+ EXPECT_EQ(1, browser()->tab_strip_model()->count());
+ EXPECT_NE(extension->url().GetOrigin(), browser()
+ ->tab_strip_model()
+ ->GetActiveWebContents()
+ ->GetLastCommittedURL()
+ .GetOrigin());
+}
+
// TODO(devlin): Investigate what to do for embedded iframes.
} // namespace extensions
