From 948ae77b975645df6c6e7f164ca8d0c3b9542979 Mon Sep 17 00:00:00 2001
From: Devlin Cronin <[email protected]>
Date: Mon, 17 Aug 2026 14:37:54 -0700
Subject: [PATCH] Reland "[Extensions] Close devtools when devtools extensions are unloaded"

Reland Notes:
This was reverted because the associated test was flaky on Android. This
reland:
* Adds a sepculative fix for the test (there was a variable scoping
  issue that could lead to a race), and
* Proactively, temporarily disables the test on Android. The test passes
  locally and the flakes did not indicate a failure in the production
  code, so we can have reasonable confidence in its correctness. A
  followup will re-enable the test on android, so it can be easily
  re-disabled if necessary.

Original change's description:
> [Extensions] Close devtools when devtools extensions are unloaded
>
> Extensions can hook into devtools windows. When these extensions are
> unloaded, we should close the associated devtools windows. This is
> important, since an unloaded + reloaded extension can have different
> APIs and permissions, which aren't captured in the devtools scripts.
>
> This is a bit disruptive if you have a devtools window open, but
> a) is consistent with how we handle many other extension contexts (e.g.,
>    closing open tabs, popups, unloading iframes, etc)
> b) should be relatively rare -- extension unloads typically happen
>    either when the extension is idle or from a direct signal (e.g.,
>    a user action like disabling the extension).
>
> This ensures extension contexts -- including devtools contexts -- are
> kept in sync with the latest version of the extension.
>
> Bug: 483435192
> Change-Id: I856a58b00f1ca291bd910539e0d7e7a515f54505
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8253197
> Commit-Queue: Devlin Cronin <[email protected]>
> Reviewed-by: Danil Somsikov <[email protected]>
> Cr-Commit-Position: refs/heads/main@{#1679051}

Bug: 483435192, 546216109
Change-Id: Ib5d6faee0c807ec46c2406e7821cf3cbd505c2a3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8259849
Commit-Queue: Devlin Cronin <[email protected]>
Reviewed-by: Brandon Wylie <[email protected]>
Reviewed-by: Xi Han <[email protected]>
Reviewed-by: Danil Somsikov <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1680824}
---

diff --git a/chrome/browser/devtools/devtools_ui_bindings.cc b/chrome/browser/devtools/devtools_ui_bindings.cc
index d860040..fd6e29b 100644
--- a/chrome/browser/devtools/devtools_ui_bindings.cc
+++ b/chrome/browser/devtools/devtools_ui_bindings.cc
@@ -868,6 +868,11 @@
   ThemeServiceFactory::GetForProfile(profile_->GetOriginalProfile())
       ->AddObserver(this);
 #endif
+#if BUILDFLAG(ENABLE_EXTENSIONS_CORE)
+  if (auto* registry = extensions::ExtensionRegistry::Get(profile_)) {
+    extension_registry_observation_.Observe(registry);
+  }
+#endif
   can_access_aida_ = IsAnyAidaPoweredFeatureEnabled();
   is_local_frontend_ =
       IsLocalDevToolsFrontendURL(web_contents_->GetLastCommittedURL());
@@ -2866,6 +2871,7 @@
             .Set("runtimeAllowedHosts", std::move(runtime_allowed_hosts))
             .Set("runtimeBlockedHosts", std::move(runtime_blocked_hosts)));
     results.Append(std::move(extension_info));
+    devtools_extension_ids_.insert(extension->id());
   }
 
   CallClientMethod("DevToolsAPI", "setOriginsForbiddenForExtensions",
@@ -2875,6 +2881,30 @@
 #endif  // BUILDFLAG(ENABLE_EXTENSIONS_CORE)
 }
 
+#if BUILDFLAG(ENABLE_EXTENSIONS_CORE)
+void DevToolsUIBindings::OnExtensionUnloaded(
+    content::BrowserContext* browser_context,
+    const extensions::Extension* extension,
+    extensions::UnloadedExtensionReason reason) {
+  // If an extension that had devtools bindings was unloaded, we just close the
+  // devtools window.
+  // This is important, because extensions might be reloaded with different
+  // privileges, and we need to ensure we clear out any old state or bindings.
+  // This is also inline with our behavior for other extension pages, like
+  // tabs, popups, etc.
+  // Extensions aren't unloaded that often (and should only be so when they're
+  // idle or via a direct signal, e.g. from the user), so this shouldn't be too
+  // disruptive.
+  if (devtools_extension_ids_.contains(extension->id())) {
+    CloseWindow();
+  }
+}
+
+void DevToolsUIBindings::OnShutdown(extensions::ExtensionRegistry* registry) {
+  extension_registry_observation_.Reset();
+}
+#endif  // BUILDFLAG(ENABLE_EXTENSIONS_CORE)
+
 void DevToolsUIBindings::RegisterExtensionsAPI(const std::string& origin,
                                                const std::string& script) {
   extensions_api_[origin + "/"] = script;
diff --git a/chrome/browser/devtools/devtools_ui_bindings.h b/chrome/browser/devtools/devtools_ui_bindings.h
index f652f24..0d3b9e8 100644
--- a/chrome/browser/devtools/devtools_ui_bindings.h
+++ b/chrome/browser/devtools/devtools_ui_bindings.h
@@ -29,12 +29,20 @@
 #include "components/prefs/pref_change_registrar.h"
 #include "content/public/browser/devtools_agent_host.h"
 #include "content/public/browser/devtools_frontend_host.h"
+#include "extensions/buildflags/buildflags.h"
 #include "ui/gfx/geometry/size.h"
 
 #if !BUILDFLAG(IS_ANDROID)
 #include "chrome/browser/themes/theme_service_observer.h"
 #endif
 
+#if BUILDFLAG(ENABLE_EXTENSIONS_CORE)
+#include "base/scoped_observation.h"
+#include "extensions/browser/extension_registry.h"
+#include "extensions/browser/extension_registry_observer.h"
+#include "extensions/common/extension_id.h"
+#endif
+
 namespace content {
 class NavigationHandle;
 class WebContents;
@@ -66,6 +74,9 @@
 #if !BUILDFLAG(IS_ANDROID)
                            public ThemeServiceObserver,
 #endif
+#if BUILDFLAG(ENABLE_EXTENSIONS_CORE)
+                           public extensions::ExtensionRegistryObserver,
+#endif
                            public DevToolsFileHelper::Delegate {
   friend class DevToolsUIBindingsDispatchHttpRequestTest;
   friend class DevToolsUIBindingsDispatchHttpRequestStreamingTest;
@@ -430,6 +441,14 @@
   // Extensions support.
   void AddDevToolsExtensionsToClient();
 
+#if BUILDFLAG(ENABLE_EXTENSIONS_CORE)
+  // extensions::ExtensionRegistryObserver:
+  void OnExtensionUnloaded(content::BrowserContext* browser_context,
+                           const extensions::Extension* extension,
+                           extensions::UnloadedExtensionReason reason) override;
+  void OnShutdown(extensions::ExtensionRegistry* registry) override;
+#endif
+
   static bool GetFeatureStateForDevTools(const base::Feature& feature,
                                          std::string enabled_by_flags,
                                          std::string disabled_by_flags);
@@ -469,6 +488,12 @@
   using ExtensionsAPIs = std::map<std::string, std::string>;
   ExtensionsAPIs extensions_api_;
   std::string initial_target_id_;
+#if BUILDFLAG(ENABLE_EXTENSIONS_CORE)
+  base::ScopedObservation<extensions::ExtensionRegistry,
+                          extensions::ExtensionRegistryObserver>
+      extension_registry_observation_{this};
+  std::set<extensions::ExtensionId> devtools_extension_ids_;
+#endif
 
   DevToolsSettings settings_;
   base::TimeTicks session_start_time_;
diff --git a/chrome/browser/devtools/devtools_window.cc b/chrome/browser/devtools/devtools_window.cc
index 06a4056f..4bf7a56 100644
--- a/chrome/browser/devtools/devtools_window.cc
+++ b/chrome/browser/devtools/devtools_window.cc
@@ -1716,7 +1716,17 @@
 }
 
 void DevToolsWindow::CloseWindow() {
-  Close(DevToolsClosedByAction::kCloseButton);
+  if (is_docked_) {
+    Close(DevToolsClosedByAction::kCloseButton);
+  } else {
+#if BUILDFLAG(IS_ANDROID)
+    main_web_contents_->Close();
+#else
+    if (browser_) {
+      browser_->GetWindow()->Close();
+    }
+#endif  // BUILDFLAG(IS_ANDROID)
+  }
 }
 
 void DevToolsWindow::Close(DevToolsClosedByAction closed_by) {
diff --git a/chrome/browser/extensions/api/devtools/devtools_apitest.cc b/chrome/browser/extensions/api/devtools/devtools_apitest.cc
index 18906bba..94e3f04 100644
--- a/chrome/browser/extensions/api/devtools/devtools_apitest.cc
+++ b/chrome/browser/extensions/api/devtools/devtools_apitest.cc
@@ -2,14 +2,26 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 

diff --git a/chrome/browser/extensions/api/devtools/devtools_apitest.cc b/chrome/browser/extensions/api/devtools/devtools_apitest.cc
index 18906bba..94e3f04 100644
--- a/chrome/browser/extensions/api/devtools/devtools_apitest.cc
+++ b/chrome/browser/extensions/api/devtools/devtools_apitest.cc
@@ -2,14 +2,26 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "base/base_paths.h"
+#include "base/path_service.h"
+#include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/values.h"
+#include "chrome/browser/devtools/devtools_window_testing.h"
 #include "chrome/browser/extensions/extension_apitest.h"
+#include "chrome/browser/extensions/extension_util.h"
 #include "chrome/browser/profiles/profile.h"
 #include "content/public/test/browser_test.h"
+#include "content/public/test/browser_test_utils.h"
 #include "extensions/browser/background_script_executor.h"
+#include "extensions/browser/extension_registry.h"
+#include "extensions/browser/test_extension_registry_observer.h"
 #include "extensions/buildflags/buildflags.h"
+#include "extensions/common/extension_id.h"
+#include "extensions/test/extension_test_message_listener.h"
+#include "extensions/test/result_catcher.h"
 #include "extensions/test/test_extension_dir.h"
+#include "net/base/filename_util.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 static_assert(BUILDFLAG(ENABLE_EXTENSIONS_CORE));
@@ -78,4 +90,124 @@
   EXPECT_THAT(result.GetString(), testing::HasSubstr("Failed to fetch"));
 }
 
+// Tests that revoking file access for an extension closes the open DevTools
+// window and prevents the extension from accessing local file resources when
+// DevTools is reopened.
+// Regression test for https://crbug.com/483435192.
+// TODO(https://crbug.com/546216109): Enable on desktop android.
+#if BUILDFLAG(IS_ANDROID)
+#define MAYBE_CantGetFileResourceWhenFileAccessRevoked \
+  DISABLED_CantGetFileResourceWhenFileAccessRevoked
+#else
+#define MAYBE_CantGetFileResourceWhenFileAccessRevoked \
+  CantGetFileResourceWhenFileAccessRevoked
+#endif
+IN_PROC_BROWSER_TEST_F(DevtoolsApiTest,
+                       MAYBE_CantGetFileResourceWhenFileAccessRevoked) {
+  ASSERT_TRUE(StartEmbeddedTestServer());
+
+  TestExtensionDir devtools_extension_dir;
+  devtools_extension_dir.WriteManifest(R"({
+    "name": "Devtools Extension",
+    "version": "1.0",
+    "manifest_version": 3,
+    "devtools_page": "devtools.html"
+  })");
+  devtools_extension_dir.WriteFile(
+      FILE_PATH_LITERAL("devtools.html"),
+      "<html><head><script src='devtools.js'></script></head></html>");
+  devtools_extension_dir.WriteFile(FILE_PATH_LITERAL("devtools.js"), R"(
+      function onResourceAdded(resource) {
+        if (resource.url.includes('sentinel.js')) {
+          chrome.devtools.inspectedWindow.getResources(resources => {
+            const hasFile = resources.some(r => r.url.startsWith('file:'));
+            chrome.test.sendMessage(
+                hasFile ? 'has_file_access' : 'no_file_access');
+          });
+        }
+      }
+
+      chrome.devtools.inspectedWindow.onResourceAdded.addListener(
+          onResourceAdded);
+
+      chrome.test.sendMessage('ready');
+  )");
+
+  // Load the extension with file access enabled.
+  const Extension* devtools_extension = LoadExtension(
+      devtools_extension_dir.UnpackedPath(), {.allow_file_access = true});
+  ASSERT_TRUE(devtools_extension);
+  const ExtensionId extension_id = devtools_extension->id();
+
+  // Helper script that adds a source map referencing a local file: URL followed
+  // by a non-file sentinel resource.
+  base::FilePath test_file =
+      base::PathService::CheckedGet(base::DIR_SRC_TEST_DATA_ROOT)
+          .AppendASCII("content/test/data/devtools/navigation.html");
+  GURL file_url = net::FilePathToFileURL(test_file);
+
+  std::string inject_script = base::StringPrintf(
+      R"(
+        const script = document.createElement('script');
+        script.textContent = 'console.log("loaded");' +
+            '\n//# sourceMappingURL=data:application/json,{"version":3,"sources":["%s","%s"]}';
+        document.body.appendChild(script);
+      )",
+      file_url.spec().c_str(),
+      embedded_test_server()->GetURL("/sentinel.js").spec().c_str());
+
+  // Step 1: Open an initial page and open DevTools.
+  GURL initial_url = embedded_test_server()->GetURL("/simple.html");
+  content::WebContents* web_contents = GetActiveWebContents();
+  ASSERT_TRUE(content::NavigateToURL(web_contents, initial_url));
+
+  ExtensionTestMessageListener ready_listener_1("ready");
+  DevToolsWindow* devtools_window =
+      DevToolsWindowTesting::OpenDevToolsWindowSync(web_contents,
+                                                    /*is_docked=*/false);
+  ASSERT_TRUE(devtools_window);
+  ASSERT_TRUE(ready_listener_1.WaitUntilSatisfied());
+
+  // Inject the script and verify the extension has file access on the first
+  // run.
+  {
+    ExtensionTestMessageListener file_listener;
+    ASSERT_TRUE(content::ExecJs(web_contents, inject_script));
+    EXPECT_TRUE(file_listener.WaitUntilSatisfied());
+    EXPECT_EQ("has_file_access", file_listener.message());
+  }
+
+  // Step 2: Revoke file access for the extension while DevTools is open.
+  // This triggers an extension reload (unload + load). Verify that the DevTools
+  // window automatically closes upon unload.
+  base::RunLoop close_run_loop;
+  DevToolsWindowTesting::Get(devtools_window)
+      ->SetCloseCallback(close_run_loop.QuitClosure());
+
+  TestExtensionRegistryObserver observer(ExtensionRegistry::Get(profile()),
+                                         extension_id);
+  util::SetAllowFileAccess(extension_id, profile(), false);
+  observer.WaitForExtensionLoaded();
+  close_run_loop.Run();
+
+  // Step 3: Reopen DevTools on a fresh page and verify that the extension now
+  // reports no file access.
+  ASSERT_TRUE(content::NavigateToURL(web_contents, initial_url));
+
+  ExtensionTestMessageListener ready_listener_2("ready");
+  devtools_window = DevToolsWindowTesting::OpenDevToolsWindowSync(
+      web_contents, /*is_docked=*/false);
+  ASSERT_TRUE(devtools_window);
+  ASSERT_TRUE(ready_listener_2.WaitUntilSatisfied());
+
+  {
+    ExtensionTestMessageListener file_listener;
+    ASSERT_TRUE(content::ExecJs(web_contents, inject_script));
+    EXPECT_TRUE(file_listener.WaitUntilSatisfied());
+    EXPECT_EQ("no_file_access", file_listener.message());
+  }
+
+  DevToolsWindowTesting::CloseDevToolsWindowSync(devtools_window);
+}
+
 }  // namespace extensions