From 5b6bee7705d4838bea2388ad2a831fa5e2597e61 Mon Sep 17 00:00:00 2001
From: Justin Cohen <justincohen@google.com>
Date: Mon, 18 May 2026 19:08:19 -0700
Subject: [PATCH] cocoa: Fix TouchBarNotificationBridge heap-use-after-free

Reorder checks in OnBrowserClosed to avoid use-after-free when
resetting owner_.browser. Also clean up observer registrations
on destruction.

Bug: 496012368
Change-Id: I4eb69e69eda25d54b4d83fa92274178cd2daa77c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7856402
Commit-Queue: Justin Cohen <justincohen@google.com>
Reviewed-by: Thomas Lukaszewicz <tluk@chromium.org>
Auto-Submit: Justin Cohen <justincohen@google.com>
Cr-Commit-Position: refs/heads/main@{#1632569}
---

diff --git a/chrome/browser/ui/cocoa/touchbar/browser_window_default_touch_bar.mm b/chrome/browser/ui/cocoa/touchbar/browser_window_default_touch_bar.mm
index adf87b09..58a86d9 100644
--- a/chrome/browser/ui/cocoa/touchbar/browser_window_default_touch_bar.mm
+++ b/chrome/browser/ui/cocoa/touchbar/browser_window_default_touch_bar.mm
@@ -163,6 +163,9 @@
 
   ~TouchBarNotificationBridge() override {
     UpdateWebContents(nullptr);
+    if (browser_) {
+      browser_->command_controller()->RemoveCommandObserver(this);
+    }
   }
 
   void UpdateTouchBar() { [[owner_ controller] invalidateTouchBar]; }
@@ -194,12 +197,13 @@
 
   // BrowserCollectionObserver:
   void OnBrowserClosed(BrowserWindowInterface* browser) override {
+    if (browser == browser_) {
+      browser_->command_controller()->RemoveCommandObserver(this);
+      browser_ = nullptr;
+    }
     if (browser == owner_.browser) {
       owner_.browser = nullptr;
     }
-    if (browser == browser_) {
-      browser_ = nullptr;
-    }
   }
 
   // BookmarkTabHelperObserver:
diff --git a/chrome/browser/ui/cocoa/touchbar/browser_window_default_touch_bar_unittest.mm b/chrome/browser/ui/cocoa/touchbar/browser_window_default_touch_bar_unittest.mm
index 3b33e31a..93feca3 100644
--- a/chrome/browser/ui/cocoa/touchbar/browser_window_default_touch_bar_unittest.mm
+++ b/chrome/browser/ui/cocoa/touchbar/browser_window_default_touch_bar_unittest.mm
@@ -15,7 +15,10 @@
 #include "chrome/browser/search_engines/template_url_service_factory_test_util.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_command_controller.h"
+#include "chrome/browser/ui/browser_window/public/browser_collection_observer.h"
 #include "chrome/browser/ui/browser_window/public/browser_window_features.h"
+#include "chrome/browser/ui/browser_window/public/global_browser_collection.h"
+#include "chrome/browser/ui/browser_window/public/global_browser_collection_platform_delegate.h"
 #include "chrome/browser/ui/cocoa/test/cocoa_test_helper.h"
 #include "chrome/browser/ui/exclusive_access/exclusive_access_manager.h"
 #include "chrome/browser/ui/exclusive_access/fullscreen_controller.h"
@@ -266,3 +269,18 @@
   // Restore the original state.
   SetShowHomeButton(home_button_showing);
 }
+
+// Tests that closing a browser doesn't cause a use-after-free when resetting
+// the browser property of the Touch Bar.
+TEST_F(BrowserWindowDefaultTouchBarUnitTest, OnBrowserClosedNoCrash) {
+  EXPECT_NE(nil, touch_bar_);
+  EXPECT_EQ(browser(), touch_bar_.browser);
+
+  // Simulate OnBrowserClosed from GlobalBrowserCollection.
+  BrowserCollectionObserver* platform_delegate =
+      GlobalBrowserCollection::GetInstance()->GetPlatformDelegate();
+  platform_delegate->OnBrowserClosed(browser());
+
+  // The Touch Bar's browser property should be reset, and the bridge destroyed.
+  EXPECT_EQ(nullptr, touch_bar_.browser);
+}

diff --git a/chrome/browser/ui/cocoa/touchbar/browser_window_default_touch_bar_unittest.mm b/chrome/browser/ui/cocoa/touchbar/browser_window_default_touch_bar_unittest.mm
index 3b33e31a..93feca3 100644
--- a/chrome/browser/ui/cocoa/touchbar/browser_window_default_touch_bar_unittest.mm
+++ b/chrome/browser/ui/cocoa/touchbar/browser_window_default_touch_bar_unittest.mm
@@ -15,7 +15,10 @@
 #include "chrome/browser/search_engines/template_url_service_factory_test_util.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_command_controller.h"
+#include "chrome/browser/ui/browser_window/public/browser_collection_observer.h"
 #include "chrome/browser/ui/browser_window/public/browser_window_features.h"
+#include "chrome/browser/ui/browser_window/public/global_browser_collection.h"
+#include "chrome/browser/ui/browser_window/public/global_browser_collection_platform_delegate.h"
 #include "chrome/browser/ui/cocoa/test/cocoa_test_helper.h"
 #include "chrome/browser/ui/exclusive_access/exclusive_access_manager.h"
 #include "chrome/browser/ui/exclusive_access/fullscreen_controller.h"
@@ -266,3 +269,18 @@
   // Restore the original state.
   SetShowHomeButton(home_button_showing);
 }
+
+// Tests that closing a browser doesn't cause a use-after-free when resetting
+// the browser property of the Touch Bar.
+TEST_F(BrowserWindowDefaultTouchBarUnitTest, OnBrowserClosedNoCrash) {
+  EXPECT_NE(nil, touch_bar_);
+  EXPECT_EQ(browser(), touch_bar_.browser);
+
+  // Simulate OnBrowserClosed from GlobalBrowserCollection.
+  BrowserCollectionObserver* platform_delegate =
+      GlobalBrowserCollection::GetInstance()->GetPlatformDelegate();
+  platform_delegate->OnBrowserClosed(browser());
+
+  // The Touch Bar's browser property should be reset, and the bridge destroyed.
+  EXPECT_EQ(nullptr, touch_bar_.browser);
+}