From c999d750660562d1651a2705531a0d6b2359de10 Mon Sep 17 00:00:00 2001
From: Devlin Cronin <rdevlin.cronin@chromium.org>
Date: Fri, 31 Jul 2026 18:07:33 -0700
Subject: [PATCH] [Extensions] Allow for potential window destruction during API calls

windows.create() and tabs.create() allow extensions to create a window
with certain properties, including maximized, minimized, fullscreen,
etc. On very certain platforms in certain condiditions, performing these
operations can theoretically cause the window to destruct.

These occurrences should be exceptionally rare, but can still
theoretically happen. Guard against this by using a WeakPtr to track
browser destruction.

Bug: 537109028
Change-Id: I552eb73ff8f037f9944f8a1f71c122d5c007fd4e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8169721
Reviewed-by: Eva Su <evasu@chromium.org>
Commit-Queue: Devlin Cronin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1672205}
---

diff --git a/chrome/browser/extensions/api/tabs/tabs_api.cc b/chrome/browser/extensions/api/tabs/tabs_api.cc
index 0041a0c..51689ba 100644
--- a/chrome/browser/extensions/api/tabs/tabs_api.cc
+++ b/chrome/browser/extensions/api/tabs/tabs_api.cc
@@ -1167,6 +1167,7 @@
   if (!new_window) {
     return Error(ExtensionTabUtil::kBrowserWindowNotAllowed);
   }
+
   // NOTE: Even though `new_window` was returned, it may not be fully
   // initialized on non-desktop platforms. See documentation on
   // CreateBrowserWindow().
@@ -1330,8 +1331,14 @@
     focused = *create_data_->focused;
   }
 
+  // Some of the Show() operations below may feasibly cause the window to
+  // destruct. Guard appropriately.
+  base::WeakPtr<BrowserWindowInterface> weak_window = new_window->GetWeakPtr();
+  // Reset `new_window` to prevent it from being used.
+  new_window = nullptr;
+
   if (focused) {
-    new_window->GetWindow()->Show();
+    weak_window->GetWindow()->Show();
   } else {
     // TODO(https://crbug.com/431004500): Port to desktop android.
 #if !BUILDFLAG(IS_ANDROID)
@@ -1344,24 +1351,28 @@
     // the old active browser.
     if (last_active_bwi && last_active_bwi->IsActive()) {
       ScopedPinBrowserAtFront scoper(last_active_bwi);
-      new_window->GetWindow()->ShowInactive();
+      weak_window->GetWindow()->ShowInactive();
     } else {
-      new_window->GetWindow()->ShowInactive();
+      weak_window->GetWindow()->ShowInactive();
     }
 #else
-    new_window->GetWindow()->ShowInactive();
+    weak_window->GetWindow()->ShowInactive();
 #endif  // BUILDFLAG(IS_ANDROID)
   }
 
+  if (!weak_window || weak_window->IsDeleteScheduled()) {
+    return Error(ExtensionTabUtil::kBrowserWindowNotAllowed);
+  }
+
 // Despite creating the window with initial_show_state() ==
 // ui::mojom::WindowShowState::kMinimized above, on Linux the window is not
 // created as minimized.
 // TODO(crbug.com/40254339): Remove this workaround when linux is fixed.
 // TODO(crbug.com/40254339): Find a fix for wayland as well.
 #if BUILDFLAG(IS_LINUX) && BUILDFLAG(SUPPORTS_OZONE_X11)
-  if (BrowserInitState::From(new_window)->initial_show_state() ==
+  if (BrowserInitState::From(weak_window.get())->initial_show_state() ==
       ui::mojom::WindowShowState::kMinimized) {
-    new_window->GetWindow()->Minimize();
+    weak_window->GetWindow()->Minimize();
   }
 #endif  // BUILDFLAG(IS_LINUX) && BUILDFLAG(SUPPORTS_OZONE_X11)
 
@@ -1372,7 +1383,7 @@
   if (create_data_ &&
       create_data_->state == windows::WindowState::kLockedFullscreen) {
 #if BUILDFLAG(IS_CHROMEOS)
-    Browser* const target_browser = new_window->GetBrowserForMigrationOnly();
+    Browser* const target_browser = weak_window->GetBrowserForMigrationOnly();
     if (target_browser) {
       auto* delegate =
           ash::BrowserController::GetInstance()->GetDelegate(target_browser);
@@ -1383,7 +1394,7 @@
 #endif  // BUILDFLAG(IS_CHROMEOS)
   }
 
-  if (new_window->GetProfile()->IsOffTheRecord() &&
+  if (weak_window->GetProfile()->IsOffTheRecord() &&
       !browser_context()->IsOffTheRecord() &&
       !include_incognito_information()) {
     // Don't expose incognito windows if extension itself works in non-incognito
@@ -1392,7 +1403,7 @@
   }
 
   return WithArguments(ExtensionTabUtil::CreateWindowValueForExtension(
-      *new_window, extension(), WindowController::kPopulateTabs,
+      *weak_window, extension(), WindowController::kPopulateTabs,
       source_context_type()));
 }
 
@@ -2151,9 +2162,10 @@
   // browser *and* it's attempting to close? Should that be *or*? This goes
   // back to the dawn of time, AKA the initial implementation in 2014:
   // https://codereview.chromium.org/245933002.
-  if (browser && browser->GetType() != BrowserWindowInterface::TYPE_NORMAL &&
-      UnloadController::From(browser->GetBrowserForMigrationOnly())
-          ->is_attempting_to_close_browser()) {
+  if (browser && (browser->IsDeleteScheduled() ||
+                  (browser->GetType() != BrowserWindowInterface::TYPE_NORMAL &&
+                   UnloadController::From(browser->GetBrowserForMigrationOnly())
+                       ->is_attempting_to_close_browser()))) {
     browser = nullptr;
     fallback_to_tabbed_browser = true;
   }
@@ -2244,7 +2256,18 @@
     return;
   }
 
-  browser->GetWindow()->Show();
+  // The Show() call below could feasibly cause the window to close on some
+  // platforms.
+  base::WeakPtr<BrowserWindowInterface> weak_browser = browser->GetWeakPtr();
+  // Reset `browser` to prevent it from being used.
+  browser = nullptr;
+
+  weak_browser->GetWindow()->Show();
+
+  if (!weak_browser || weak_browser->IsDeleteScheduled()) {
+    Respond(Error(ExtensionTabUtil::kBrowserWindowNotAllowed));
+    return;
+  }
 
   // Re-fetch the opener, if one was specified. This call might fail if the
   // opener tab was destroyed while the window was being created. In that case,
@@ -2257,7 +2280,7 @@
                                  &opener, nullptr);
   }
 
-  OpenTabInBrowser(*browser, opener);
+  OpenTabInBrowser(*weak_browser, opener);
 }
 
 void TabsCreateFunction::OpenTabInBrowser(BrowserWindowInterface& browser,
diff --git a/chrome/browser/extensions/api/tabs/tabs_test.cc b/chrome/browser/extensions/api/tabs/tabs_test.cc
index e4c9c9c7..b817967 100644
--- a/chrome/browser/extensions/api/tabs/tabs_test.cc
+++ b/chrome/browser/extensions/api/tabs/tabs_test.cc
@@ -745,6 +745,84 @@
             extension_url);
 }
 
+namespace {
+
+// Simulates the browser immediately closing upon activation. This can
+// potentially happen synchronously as part of showing a window on some
+// platforms, but that's difficult to reproduce in a test directly.
+class CloseOnBrowserCreatedObserver : public BrowserCollectionObserver {
+ public:
+  explicit CloseOnBrowserCreatedObserver(BrowserWindowInterface* ignore_browser)
+      : ignore_browser_(ignore_browser) {
+    observation_.Observe(GlobalBrowserCollection::GetInstance());
+  }
+
+  void OnBrowserActivated(BrowserWindowInterface* browser) override {
+    if (browser != ignore_browser_ && !closed_new_window_) {
+      closed_new_window_ = true;
+      browser->GetWindow()->Close();
+    }
+  }
+
+ private:
+  bool closed_new_window_ = false;
+  raw_ptr<BrowserWindowInterface> ignore_browser_;
+  base::ScopedObservation<GlobalBrowserCollection, BrowserCollectionObserver>
+      observation_{this};
+};
+
+}  // namespace
+
+// Tests that a browser window that's closed as soon as it's shown is
+// gracefully handled (in windows.create). Regression test for
+// https://crbug.com/537109028.
+IN_PROC_BROWSER_TEST_F(ExtensionTabsTest,
+                       WindowsCreateFunctionWindowClosedOnShow) {
+  CloseOnBrowserCreatedObserver observer(browser_window_interface());
+
+  auto function = base::MakeRefCounted<WindowsCreateFunction>();
+  function->SetRenderFrameHost(GetTabListInterface()
+                                   ->GetActiveTab()

diff --git a/chrome/browser/extensions/api/tabs/tabs_test.cc b/chrome/browser/extensions/api/tabs/tabs_test.cc
index e4c9c9c7..b817967 100644
--- a/chrome/browser/extensions/api/tabs/tabs_test.cc
+++ b/chrome/browser/extensions/api/tabs/tabs_test.cc
@@ -745,6 +745,84 @@
             extension_url);
 }
 
+namespace {
+
+// Simulates the browser immediately closing upon activation. This can
+// potentially happen synchronously as part of showing a window on some
+// platforms, but that's difficult to reproduce in a test directly.
+class CloseOnBrowserCreatedObserver : public BrowserCollectionObserver {
+ public:
+  explicit CloseOnBrowserCreatedObserver(BrowserWindowInterface* ignore_browser)
+      : ignore_browser_(ignore_browser) {
+    observation_.Observe(GlobalBrowserCollection::GetInstance());
+  }
+
+  void OnBrowserActivated(BrowserWindowInterface* browser) override {
+    if (browser != ignore_browser_ && !closed_new_window_) {
+      closed_new_window_ = true;
+      browser->GetWindow()->Close();
+    }
+  }
+
+ private:
+  bool closed_new_window_ = false;
+  raw_ptr<BrowserWindowInterface> ignore_browser_;
+  base::ScopedObservation<GlobalBrowserCollection, BrowserCollectionObserver>
+      observation_{this};
+};
+
+}  // namespace
+
+// Tests that a browser window that's closed as soon as it's shown is
+// gracefully handled (in windows.create). Regression test for
+// https://crbug.com/537109028.
+IN_PROC_BROWSER_TEST_F(ExtensionTabsTest,
+                       WindowsCreateFunctionWindowClosedOnShow) {
+  CloseOnBrowserCreatedObserver observer(browser_window_interface());
+
+  auto function = base::MakeRefCounted<WindowsCreateFunction>();
+  function->SetRenderFrameHost(GetTabListInterface()
+                                   ->GetActiveTab()
+                                   ->GetContents()
+                                   ->GetPrimaryMainFrame());
+  scoped_refptr<const Extension> extension(ExtensionBuilder("Test").Build());
+  function->set_extension(extension.get());
+
+  static const char kArgs[] = R"([{"url": "about:blank"}])";
+  std::string error = api_test_utils::RunFunctionAndReturnError(
+      function.get(), kArgs, profile());
+  EXPECT_EQ(ExtensionTabUtil::kBrowserWindowNotAllowed, error);
+}
+
+// Tests that a browser window that's closed as soon as it's shown is
+// gracefully handled (in tabs.create). Regression test for
+// https://crbug.com/537109028.
+IN_PROC_BROWSER_TEST_F(ExtensionTabsTest,
+                       TabsCreateFunctionWindowClosedOnShow) {
+  Browser* incognito_browser = CreateIncognitoBrowser();
+  CloseBrowserSynchronously(browser());
+
+  auto observer =
+      std::make_unique<CloseOnBrowserCreatedObserver>(incognito_browser);
+
+  scoped_refptr<const Extension> extension(ExtensionBuilder("Test").Build());
+  auto function = base::MakeRefCounted<TabsCreateFunction>();
+  function->SetRenderFrameHost(TabListInterface::From(incognito_browser)
+                                   ->GetActiveTab()
+                                   ->GetContents()
+                                   ->GetPrimaryMainFrame());
+  function->set_extension(extension.get());
+
+  const std::string args = base::StringPrintf(
+      R"([{"url": "%s"}])", extension->GetResourceURL("page.html").spec());
+  std::string error = api_test_utils::RunFunctionAndReturnError(
+      function.get(), args, incognito_browser->GetProfile());
+  EXPECT_EQ(ExtensionTabUtil::kBrowserWindowNotAllowed, error);
+  observer.reset();  // Reset the observer to avoid any dangling pointer issues.
+  // Close the incognito browser to clean up.
+  CloseBrowserSynchronously(incognito_browser);
+}
+
 IN_PROC_BROWSER_TEST_F(ExtensionTabsTest,
                        DefaultToIncognitoWhenItIsForcedAndNoArgs) {
   static const char kEmptyArgs[] = "[]";