From 5c2010f1a7ea14ead6d4a9848cce9f783ddd9fc1 Mon Sep 17 00:00:00 2001
From: Mitsuru Oshima <[email protected]>
Date: Thu, 30 Jul 2026 13:21:50 -0700
Subject: [PATCH] views/aura/wm: Use weak pointer for capture window

This CL changes capture_window_ in DesktopCaptureClient,
DefaultCaptureClient, and CaptureController to use base::WeakPtr
instead of raw_ptr to avoid dangling pointers when the capture window is
destroyed.

It also updates WindowEventDispatcher::UpdateCapture to block
the deletion of new_capture_window, because deleting it and
recursively call UpdateCapture can cause unexpected result.

If this ever happens in the field, we'll work on a fix for that
scenario.

Bug: 515428251, 502041642

TAG=agy

CONV=c8f9d59f-22c6-456c-9f6d-52b1cd075207

Change-Id: I9740e11f2d8f93a94f4a1aa0de6ce8795c34a09e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8127178
Reviewed-by: Thomas Lukaszewicz <[email protected]>
Commit-Queue: Mitsuru Oshima <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1671366}
---

diff --git a/ui/aura/client/default_capture_client.cc b/ui/aura/client/default_capture_client.cc
index ee11652..413b328 100644
--- a/ui/aura/client/default_capture_client.cc
+++ b/ui/aura/client/default_capture_client.cc
@@ -4,6 +4,7 @@
 
 #include "ui/aura/client/default_capture_client.h"
 
+#include "base/no_destructor.h"
 #include "base/observer_list.h"
 #include "ui/aura/client/capture_client_observer.h"
 #include "ui/aura/env.h"
@@ -16,7 +17,10 @@
 namespace {
 
 // Track the active capture window across root windows.
-Window* global_capture_window_ = nullptr;
+base::WeakPtr<Window>& GetGlobalCaptureWindowRef() {
+  static base::NoDestructor<base::WeakPtr<Window>> global_capture_window;
+  return *global_capture_window;
+}
 
 }  // namespace
 
@@ -27,20 +31,25 @@
 }
 
 DefaultCaptureClient::~DefaultCaptureClient() {
-  if (global_capture_window_ == capture_window_)
-    global_capture_window_ = nullptr;
+  if (GetGlobalCaptureWindowRef().get() == capture_window_.get()) {
+    GetGlobalCaptureWindowRef() = nullptr;
+  }
   SetCaptureClient(root_window_, nullptr);
 }
 
 void DefaultCaptureClient::SetCapture(Window* window) {
-  if (capture_window_ == window)
+  if (capture_window_.get() == window) {
     return;
+  }
+  auto new_capture_window_weak =
+      window ? window->GetWeakPtrAsWindow() : nullptr;
   if (window)
     Env::GetInstance()->gesture_recognizer()->CancelActiveTouchesExcept(window);
 
-  Window* old_capture_window = capture_window_;
-  capture_window_ = window;
-  global_capture_window_ = window;
+  auto old_capture_window = capture_window_;
+
+  capture_window_ = new_capture_window_weak;
+  GetGlobalCaptureWindowRef() = new_capture_window_weak;
 
   CaptureDelegate* capture_delegate = nullptr;
   if (capture_window_) {
@@ -54,24 +63,26 @@
     capture_delegate->ReleaseNativeCapture();
   }
 
-  capture_delegate->UpdateCapture(old_capture_window, capture_window_);
+  capture_delegate->UpdateCapture(old_capture_window.get(),
+                                  capture_window_.get());
 
   observers_.Notify(&CaptureClientObserver::OnCaptureChanged,
-                    old_capture_window, capture_window_);
+                    old_capture_window.get(), capture_window_.get());
 }
 
 void DefaultCaptureClient::ReleaseCapture(Window* window) {
-  if (capture_window_ != window)
+  if (capture_window_.get() != window) {
     return;
-  SetCapture(NULL);
+  }
+  SetCapture(nullptr);
 }
 
 Window* DefaultCaptureClient::GetCaptureWindow() {
-  return capture_window_;
+  return capture_window_.get();
 }
 
 Window* DefaultCaptureClient::GetGlobalCaptureWindow() {
-  return global_capture_window_;
+  return GetGlobalCaptureWindowRef().get();
 }
 
 void DefaultCaptureClient::AddObserver(CaptureClientObserver* observer) {
diff --git a/ui/aura/client/default_capture_client.h b/ui/aura/client/default_capture_client.h
index 8a0c3cd..b283438 100644
--- a/ui/aura/client/default_capture_client.h
+++ b/ui/aura/client/default_capture_client.h
@@ -6,6 +6,7 @@
 #define UI_AURA_CLIENT_DEFAULT_CAPTURE_CLIENT_H_
 
 #include "base/memory/raw_ptr.h"
+#include "base/memory/weak_ptr.h"
 #include "base/observer_list.h"
 #include "ui/aura/aura_export.h"
 #include "ui/aura/client/capture_client.h"
@@ -33,7 +34,7 @@
 
  private:
   raw_ptr<Window> root_window_;  // May be null.
-  raw_ptr<Window> capture_window_;
+  base::WeakPtr<Window> capture_window_;
   base::ObserverList<CaptureClientObserver>::Unchecked observers_;
 };
 
diff --git a/ui/aura/window_event_dispatcher.cc b/ui/aura/window_event_dispatcher.cc
index 35fd163f..99178114 100644
--- a/ui/aura/window_event_dispatcher.cc
+++ b/ui/aura/window_event_dispatcher.cc
@@ -430,6 +430,12 @@
   if (mouse_moved_handler_ && !window()->Contains(mouse_moved_handler_))
     mouse_moved_handler_ = nullptr;
 
+  std::unique_ptr<Window::ScopedDeleteBlocker> new_capture_blocker;
+  if (new_capture) {
+    new_capture_blocker =
+        std::make_unique<Window::ScopedDeleteBlocker>(new_capture);
+  }
+
   if (old_capture && old_capture->GetRootWindow() == window() &&
       old_capture->delegate()) {
     // Send a capture changed event with the most recent mouse screen location.
diff --git a/ui/aura/window_event_dispatcher_unittest.cc b/ui/aura/window_event_dispatcher_unittest.cc
index 2f0ea0c5..33e1c5e 100644
--- a/ui/aura/window_event_dispatcher_unittest.cc
+++ b/ui/aura/window_event_dispatcher_unittest.cc
@@ -2594,6 +2594,25 @@
   std::unique_ptr<aura::Window> capture_window_;
 };
 
+class DeleteOtherWindowOnCaptureLostDelegate : public test::TestWindowDelegate {
+ public:
+  DeleteOtherWindowOnCaptureLostDelegate() = default;
+  ~DeleteOtherWindowOnCaptureLostDelegate() override = default;
+
+  void set_other_window(Window* other) { other_ = other; }
+
+  // test::TestWindowDelegate:
+  void OnCaptureLost() override {
+    if (other_) {
+      Window* raw_other = std::exchange(other_, nullptr);
+      delete raw_other;
+    }
+  }
+
+ private:
+  raw_ptr<Window> other_ = nullptr;
+};
+
 }  // namespace
 
 // Verifies handling loss of capture by the capture window being hidden.
@@ -2612,6 +2631,29 @@
   EXPECT_EQ(NULL, capture_window_tracker.capture_window());
 }
 
+using WindowEventDispatcherDeathTest = WindowEventDispatcherTest;
+
+// Verifies that deleting the new capture window during OnCaptureLost of the
+// old capture window causes a crash (due to ScopedDeleteBlocker).
+TEST_F(WindowEventDispatcherDeathTest, DeleteNewCaptureDuringOnCaptureLost) {
+  DeleteOtherWindowOnCaptureLostDelegate d1;
+  test::TestWindowDelegate d2;
+  std::unique_ptr<Window> w1(CreateNormalWindow(1, root_window(), &d1));
+

diff --git a/ui/aura/window_event_dispatcher_unittest.cc b/ui/aura/window_event_dispatcher_unittest.cc
index 2f0ea0c5..33e1c5e 100644
--- a/ui/aura/window_event_dispatcher_unittest.cc
+++ b/ui/aura/window_event_dispatcher_unittest.cc
@@ -2594,6 +2594,25 @@
   std::unique_ptr<aura::Window> capture_window_;
 };
 
+class DeleteOtherWindowOnCaptureLostDelegate : public test::TestWindowDelegate {
+ public:
+  DeleteOtherWindowOnCaptureLostDelegate() = default;
+  ~DeleteOtherWindowOnCaptureLostDelegate() override = default;
+
+  void set_other_window(Window* other) { other_ = other; }
+
+  // test::TestWindowDelegate:
+  void OnCaptureLost() override {
+    if (other_) {
+      Window* raw_other = std::exchange(other_, nullptr);
+      delete raw_other;
+    }
+  }
+
+ private:
+  raw_ptr<Window> other_ = nullptr;
+};
+
 }  // namespace
 
 // Verifies handling loss of capture by the capture window being hidden.
@@ -2612,6 +2631,29 @@
   EXPECT_EQ(NULL, capture_window_tracker.capture_window());
 }
 
+using WindowEventDispatcherDeathTest = WindowEventDispatcherTest;
+
+// Verifies that deleting the new capture window during OnCaptureLost of the
+// old capture window causes a crash (due to ScopedDeleteBlocker).
+TEST_F(WindowEventDispatcherDeathTest, DeleteNewCaptureDuringOnCaptureLost) {
+  DeleteOtherWindowOnCaptureLostDelegate d1;
+  test::TestWindowDelegate d2;
+  std::unique_ptr<Window> w1(CreateNormalWindow(1, root_window(), &d1));
+
+  Window* w2 = CreateNormalWindow(2, root_window(), &d2);
+
+  d1.set_other_window(w2);
+
+  client::GetCaptureClient(root_window())->SetCapture(w1.get());
+  EXPECT_EQ(w1.get(),
+            client::GetCaptureClient(root_window())->GetCaptureWindow());
+
+  // This will trigger UpdateCapture(w1, w2).
+  // w1's OnCaptureLost will delete w2, which should crash due to
+  // ScopedDeleteBlocker.
+  EXPECT_DEATH(client::GetCaptureClient(root_window())->SetCapture(w2), "");
+}
+
 namespace {
 
 class RunLoopHandler : public ui::EventHandler {