From 7aafca3e8c40a59540cd2bbc4dc7af446024d0a8 Mon Sep 17 00:00:00 2001
From: Andrey Kosyakov <caseq@chromium.org>
Date: Tue, 04 Aug 2026 14:36:33 -0700
Subject: [PATCH] Rework browser context getters of DevToolsManagerDelegate

- introduce GetBrowserContext(contextId), use it in most places we were
    iterating list of browser contexts to find a context;
- make sure GetBrowserContexts() returns WeakPtrs, not raw ptrs, just to
    safeguard possible future usages;
- add a test for the previously present UaF when iterating browser contexts while destroying them.

Fixed: 516965176
Change-Id: I346c0f74b8024218151827d966f140e9f5ee92d6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8145402
Auto-Submit: Andrey Kosyakov <caseq@chromium.org>
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Commit-Queue: Andrey Kosyakov <caseq@chromium.org>
Reviewed-by: Sergey Ulanov <sergeyu@chromium.org>
Reviewed-by: Alex Rudenko <alexrudenko@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1673682}
---

diff --git a/chrome/browser/devtools/chrome_devtools_manager_delegate.cc b/chrome/browser/devtools/chrome_devtools_manager_delegate.cc
index c9f49cd..3e7c8a2 100644
--- a/chrome/browser/devtools/chrome_devtools_manager_delegate.cc
+++ b/chrome/browser/devtools/chrome_devtools_manager_delegate.cc
@@ -446,7 +446,7 @@
                    params.navigated_or_inserted_contents);
 }
 
-std::vector<content::BrowserContext*>
+std::vector<base::WeakPtr<content::BrowserContext>>
 ChromeDevToolsManagerDelegate::GetBrowserContexts() {
   return DevToolsBrowserContextManager::GetInstance().GetBrowserContexts();
 }
@@ -457,6 +457,12 @@
       .GetDefaultBrowserContext();
 }
 
+content::BrowserContext* ChromeDevToolsManagerDelegate::GetBrowserContext(
+    const std::string& context_id) {
+  return DevToolsBrowserContextManager::GetInstance().GetProfileById(
+      context_id);
+}
+
 content::BrowserContext* ChromeDevToolsManagerDelegate::CreateBrowserContext() {
   return DevToolsBrowserContextManager::GetInstance().CreateBrowserContext();
 }
diff --git a/chrome/browser/devtools/chrome_devtools_manager_delegate.h b/chrome/browser/devtools/chrome_devtools_manager_delegate.h
index a48b3a9eb..5a1fb50 100644
--- a/chrome/browser/devtools/chrome_devtools_manager_delegate.h
+++ b/chrome/browser/devtools/chrome_devtools_manager_delegate.h
@@ -46,8 +46,11 @@
   // Resets |device_manager_|.
   void ResetAndroidDeviceManagerForTesting();
 
-  std::vector<content::BrowserContext*> GetBrowserContexts() override;
+  std::vector<base::WeakPtr<content::BrowserContext>> GetBrowserContexts()
+      override;
   content::BrowserContext* GetDefaultBrowserContext() override;
+  content::BrowserContext* GetBrowserContext(
+      const std::string& context_id) override;
 
   // Closes browser soon, not in the current task.
   static void CloseBrowserSoon();
diff --git a/chrome/browser/devtools/devtools_browser_context_manager.cc b/chrome/browser/devtools/devtools_browser_context_manager.cc
index fd8128c..7b3a276 100644
--- a/chrome/browser/devtools/devtools_browser_context_manager.cc
+++ b/chrome/browser/devtools/devtools_browser_context_manager.cc
@@ -68,11 +68,11 @@
   return otr_profile;
 }
 
-std::vector<content::BrowserContext*>
+std::vector<base::WeakPtr<content::BrowserContext>>
 DevToolsBrowserContextManager::GetBrowserContexts() {
-  std::vector<content::BrowserContext*> result;
+  std::vector<base::WeakPtr<content::BrowserContext>> result;
   for (const auto& profile_pair : otr_profiles_)
-    result.push_back(profile_pair.second);
+    result.push_back(profile_pair.second->GetWeakPtr());
   return result;
 }
 
diff --git a/chrome/browser/devtools/devtools_browser_context_manager.h b/chrome/browser/devtools/devtools_browser_context_manager.h
index cd2383a..1898b7ee 100644
--- a/chrome/browser/devtools/devtools_browser_context_manager.h
+++ b/chrome/browser/devtools/devtools_browser_context_manager.h
@@ -29,7 +29,7 @@
       const DevToolsBrowserContextManager&) = delete;
 
   Profile* GetProfileById(const std::string& browser_context_id);
-  std::vector<content::BrowserContext*> GetBrowserContexts();
+  std::vector<base::WeakPtr<content::BrowserContext>> GetBrowserContexts();
   content::BrowserContext* GetDefaultBrowserContext();
   content::BrowserContext* CreateBrowserContext();
   void DisposeBrowserContext(
diff --git a/chrome/browser/headless/test/headless_mode_protocol_browsertest.cc b/chrome/browser/headless/test/headless_mode_protocol_browsertest.cc
index 5b85610..48249af 100644
--- a/chrome/browser/headless/test/headless_mode_protocol_browsertest.cc
+++ b/chrome/browser/headless/test/headless_mode_protocol_browsertest.cc
@@ -523,4 +523,7 @@
 HEADLESS_MODE_PROTOCOL_TEST(NormalWindowHasOpener,
                             "shared/normal-window-has-opener.js")
 
+HEADLESS_MODE_PROTOCOL_TEST(GetBrowserContexts,
+                            "shared/get-browser-contexts.js")
+
 }  // namespace headless
diff --git a/components/headless/test/data/protocol/shared/get-browser-contexts-expected.txt b/components/headless/test/data/protocol/shared/get-browser-contexts-expected.txt
new file mode 100644
index 0000000..2784d0c
--- /dev/null
+++ b/components/headless/test/data/protocol/shared/get-browser-contexts-expected.txt
@@ -0,0 +1,11 @@
+Tests Target.getBrowserContexts
+Initial browserContextIds count: 0
+Has defaultBrowserContextId: true
+browserContextIds count after create: 1
+Contains created context: true
+browserContextIds count after dispose: 0
+Attempting to dispose default browser context...
+Error disposing default context: {
+    code : -32000
+    message : Cannot dispose default browser context.
+}
\ No newline at end of file
diff --git a/components/headless/test/data/protocol/shared/get-browser-contexts.js b/components/headless/test/data/protocol/shared/get-browser-contexts.js
new file mode 100644
index 0000000..d6e337a4
--- /dev/null
+++ b/components/headless/test/data/protocol/shared/get-browser-contexts.js
@@ -0,0 +1,44 @@
+// Copyright 2026 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+(async function(/** @type {import('test_runner').TestRunner} */ testRunner) {
+  await testRunner.startBlank('Tests Target.getBrowserContexts');
+
+  const browserSession = await testRunner.attachFullBrowserSession();
+  const bp = browserSession.protocol;
+
+  const {result: res1} = await bp.Target.getBrowserContexts();
+  testRunner.log(
+      `Initial browserContextIds count: ${res1.browserContextIds.length}`);
+  testRunner.log(`Has defaultBrowserContextId: ${
+      typeof res1.defaultBrowserContextId === 'string' &&
+      res1.defaultBrowserContextId.length > 0}`);
+
+  const {result: {browserContextId}} = await bp.Target.createBrowserContext();
+
+  const {result: res2} = await bp.Target.getBrowserContexts();
+  testRunner.log(
+      `browserContextIds count after create: ${res2.browserContextIds.length}`);
+  testRunner.log(`Contains created context: ${
+      res2.browserContextIds.includes(browserContextId)}`);
+
+  await bp.Target.disposeBrowserContext({browserContextId});
+
+  const {result: res3} = await bp.Target.getBrowserContexts();
+  testRunner.log(`browserContextIds count after dispose: ${
+      res3.browserContextIds.length}`);
+
+  testRunner.log('Attempting to dispose default browser context...');
+  const disposeDefaultRes = await bp.Target.disposeBrowserContext({
+    browserContextId: res1.defaultBrowserContextId,
+  });
+  const error = disposeDefaultRes.error;
+  if (error && error.message) {
+    error.message = error.message.replace(
+        res1.defaultBrowserContextId, '<defaultBrowserContextId>');
+  }
+  testRunner.log(error, 'Error disposing default context:');
+
+  testRunner.completeTest();
+});
diff --git a/content/browser/devtools/protocol/browser_handler.cc b/content/browser/devtools/protocol/browser_handler.cc
index 41e483e..d027adc1 100644
--- a/content/browser/devtools/protocol/browser_handler.cc
+++ b/content/browser/devtools/protocol/browser_handler.cc
@@ -370,21 +370,18 @@
         "Browser context management is not supported.");
   if (!browser_context_id.has_value()) {
     *browser_context = delegate->GetDefaultBrowserContext();
-    if (*browser_context == nullptr)
+    if (!*browser_context) {
       return Response::ServerError(
           "Browser context management is not supported.");
+    }
     return Response::Success();
   }
-
-  std::string context_id = browser_context_id.value();
-  for (auto* context : delegate->GetBrowserContexts()) {
-    if (context->UniqueId() == context_id) {
-      *browser_context = context;
-      return Response::Success();
-    }
+  *browser_context = delegate->GetBrowserContext(browser_context_id.value());

diff --git a/chrome/browser/headless/test/headless_mode_protocol_browsertest.cc b/chrome/browser/headless/test/headless_mode_protocol_browsertest.cc
index 5b85610..48249af 100644
--- a/chrome/browser/headless/test/headless_mode_protocol_browsertest.cc
+++ b/chrome/browser/headless/test/headless_mode_protocol_browsertest.cc
@@ -523,4 +523,7 @@
 HEADLESS_MODE_PROTOCOL_TEST(NormalWindowHasOpener,
                             "shared/normal-window-has-opener.js")
 
+HEADLESS_MODE_PROTOCOL_TEST(GetBrowserContexts,
+                            "shared/get-browser-contexts.js")
+
 }  // namespace headless
diff --git a/components/headless/test/data/protocol/shared/get-browser-contexts-expected.txt b/components/headless/test/data/protocol/shared/get-browser-contexts-expected.txt
new file mode 100644
index 0000000..2784d0c
--- /dev/null
+++ b/components/headless/test/data/protocol/shared/get-browser-contexts-expected.txt
@@ -0,0 +1,11 @@
+Tests Target.getBrowserContexts
+Initial browserContextIds count: 0
+Has defaultBrowserContextId: true
+browserContextIds count after create: 1
+Contains created context: true
+browserContextIds count after dispose: 0
+Attempting to dispose default browser context...
+Error disposing default context: {
+    code : -32000
+    message : Cannot dispose default browser context.
+}
\ No newline at end of file
diff --git a/components/headless/test/data/protocol/shared/get-browser-contexts.js b/components/headless/test/data/protocol/shared/get-browser-contexts.js
new file mode 100644
index 0000000..d6e337a4
--- /dev/null
+++ b/components/headless/test/data/protocol/shared/get-browser-contexts.js
@@ -0,0 +1,44 @@
+// Copyright 2026 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+(async function(/** @type {import('test_runner').TestRunner} */ testRunner) {
+  await testRunner.startBlank('Tests Target.getBrowserContexts');
+
+  const browserSession = await testRunner.attachFullBrowserSession();
+  const bp = browserSession.protocol;
+
+  const {result: res1} = await bp.Target.getBrowserContexts();
+  testRunner.log(
+      `Initial browserContextIds count: ${res1.browserContextIds.length}`);
+  testRunner.log(`Has defaultBrowserContextId: ${
+      typeof res1.defaultBrowserContextId === 'string' &&
+      res1.defaultBrowserContextId.length > 0}`);
+
+  const {result: {browserContextId}} = await bp.Target.createBrowserContext();
+
+  const {result: res2} = await bp.Target.getBrowserContexts();
+  testRunner.log(
+      `browserContextIds count after create: ${res2.browserContextIds.length}`);
+  testRunner.log(`Contains created context: ${
+      res2.browserContextIds.includes(browserContextId)}`);
+
+  await bp.Target.disposeBrowserContext({browserContextId});
+
+  const {result: res3} = await bp.Target.getBrowserContexts();
+  testRunner.log(`browserContextIds count after dispose: ${
+      res3.browserContextIds.length}`);
+
+  testRunner.log('Attempting to dispose default browser context...');
+  const disposeDefaultRes = await bp.Target.disposeBrowserContext({
+    browserContextId: res1.defaultBrowserContextId,
+  });
+  const error = disposeDefaultRes.error;
+  if (error && error.message) {
+    error.message = error.message.replace(
+        res1.defaultBrowserContextId, '<defaultBrowserContextId>');
+  }
+  testRunner.log(error, 'Error disposing default context:');
+
+  testRunner.completeTest();
+});
diff --git a/headless/test/data/protocol/sanity/context-disposal-expected.txt b/headless/test/data/protocol/sanity/context-disposal-expected.txt
new file mode 100644
index 0000000..11bbc95b
--- /dev/null
+++ b/headless/test/data/protocol/sanity/context-disposal-expected.txt
@@ -0,0 +1,4 @@
+Tests context disposal on detach from page exposing DevTools protocol.
+Browser contexts count before detach: 65
+Detaching browser session...
+Browser contexts count after detach: 0
diff --git a/headless/test/data/protocol/sanity/context-disposal.js b/headless/test/data/protocol/sanity/context-disposal.js
new file mode 100644
index 0000000..b924333
--- /dev/null
+++ b/headless/test/data/protocol/sanity/context-disposal.js
@@ -0,0 +1,62 @@
+// Copyright 2026 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+(async function(/** @type {import('test_runner').TestRunner} */ testRunner) {
+  testRunner.log(
+      'Tests context disposal on detach from page exposing DevTools protocol.');
+
+  // Create extra browser session.
+  const {result: {sessionId: browserSessionId}} =
+      await testRunner.browserP().Target.attachToBrowserTarget({});
+  const browserSession = testRunner.createSessionFor(browserSessionId);
+  const bp = browserSession.protocol;
+
+  const {result: {browserContextId}} =
+      await bp.Target.createBrowserContext({disposeOnDetach: true});
+
+  const {result: {targetId}} = await bp.Target.createTarget({
+    browserContextId,
+    url: 'about:blank',
+  });
+  const {result: {sessionId}} =
+      await bp.Target.attachToTarget({targetId, flatten: true});
+  const session = testRunner.createSessionFor(sessionId);
+
+  await bp.Target.exposeDevToolsProtocol({targetId, bindingName: 'cdp'});
+
+  const testFrameworkURL =
+      new URL(
+          '/resources/inspector-protocol-test-subtarget.html', location.href)
+          .href;
+  await session.navigate(testFrameworkURL);
+
+  await session.evaluateAsync(async () => {
+    const testRunner = new TestRunner(
+        '', '', DevToolsAPI._log, () => {}, DevToolsAPI._fetch, {});
+    const bp = testRunner.browserP();
+    const promises = [];
+    for (let i = 0; i < 64; i++) {
+      promises.push(bp.Target.createBrowserContext({disposeOnDetach: true}));
+    }
+    await Promise.all(promises);
+  });
+
+  const contextsBefore =
+      (await testRunner.browserP().Target.getBrowserContexts())
+          .result.browserContextIds;
+  testRunner.log(
+      `Browser contexts count before detach: ${contextsBefore.length}`);
+
+  testRunner.log('Detaching browser session...');
+  await testRunner.browserP().Target.detachFromTarget(
+      {sessionId: browserSessionId});
+
+  const contextsAfter =
+      (await testRunner.browserP().Target.getBrowserContexts())
+          .result.browserContextIds;
+  testRunner.log(
+      `Browser contexts count after detach: ${contextsAfter.length}\n`);
+
+  testRunner.completeTest();
+});
diff --git a/headless/test/headless_protocol_browsertest.cc b/headless/test/headless_protocol_browsertest.cc
index d99612a..e4b74e3 100644
--- a/headless/test/headless_protocol_browsertest.cc
+++ b/headless/test/headless_protocol_browsertest.cc
@@ -728,6 +728,8 @@
 
 HEADLESS_PROTOCOL_TEST(BlockNewWebContents, "sanity/block-new-web-contents.js")
 
+HEADLESS_PROTOCOL_TEST(ContextDisposal, "sanity/context-disposal.js")
+
 HEADLESS_PROTOCOL_TEST(ScreenRotationSecondaryScreen,
                        "sanity/screen-rotation-secondary-screen.js")
 
@@ -848,4 +850,6 @@
 HEADLESS_PROTOCOL_TEST(GetCanvasContextWebGL,
                        "shared/get-canvas-context-webgl.js")
 
+HEADLESS_PROTOCOL_TEST(GetBrowserContexts, "shared/get-browser-contexts.js")
+
 }  // namespace headless