fix: make DeviceMonitorMessageWindow a NoDestructor (#54809)

* fix: mark leaked serial device observation as dangling on Windows

With dangling raw_ptr checks enabled, the Windows browser process fails
the dangling-pointer check at exit after a page uses navigator.serial.
DeviceService is destroyed after the ThreadPool has shut down, so the
serial enumerator's UI-thread helper is leaked with its observation of
the COM-port DeviceMonitorWin still active when an AtExit callback frees
the monitor. Mark that observation LeakedDanglingUntriaged, as was done
for ElectronUsbDelegate.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Xn1qtQBJvK9weVCmD9tLkv

* chore: reference upstream CL in serial dangling patch

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Xn1qtQBJvK9weVCmD9tLkv

* fix: notify DeviceMonitorWin observers before destruction

Replace the LeakedDanglingUntriaged annotation with the approach from
upstream CL 8561777: ~DeviceMonitorWin notifies observers, and the
serial enumerator's UI thread helper resets its observation there, so
no raw_ptr dangles when the monitor is freed at exit.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Xn1qtQBJvK9weVCmD9tLkv

* fix: update DeviceMonitorWin patch to upstream CL 8561777 PS7

Upstream review moved the fix to making DeviceMonitorMessageWindow a
base::NoDestructor instead of notifying observers before destruction.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Xn1qtQBJvK9weVCmD9tLkv

* fix: update DeviceMonitorWin patch to upstream CL 8561777 PS8

Adds the DeviceMonitorWin SEQUENCE_CHECKER from patchset 8.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DWqmuJ7QaqrgrGccdfquWX

---------

Co-authored-by: Claude <noreply@anthropic.com>

patches/chromium/.patches

@@ -137,3 +137,4 @@ fix_gate_webstoreprivate_on_a_delegated_availability_check.patch
 build_drop_the_unused_chrome_browser_win_dep_from_color_mixers.patch
 fix_plumb_the_requesting_frame_through_pointer_lock_requests.patch
 feat_let_the_host_of_a_remote_nswindow_set_more_of_its_behavior.patch
+device_make_devicemonitormessagewindow_a_nodestructor.patch

patches/chromium/device_make_devicemonitormessagewindow_a_nodestructor.patch

@@ -0,0 +1,291 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: John Kleinschmidt <kleinschmidtorama@gmail.com>
+Date: Thu, 22 Oct 2026 10:32:26 -0400
+Subject: [device] Make DeviceMonitorMessageWindow a NoDestructor
+
+DeviceMonitorMessageWindow, which owns the DeviceMonitorWin instances,
+was deleted by an AtExitManager callback. Any observer still alive at
+that point is left holding a dangling pointer to its monitor.
+
+This happens to SerialDeviceEnumeratorWin::UiThreadHelper at browser
+shutdown. DeviceService lives in the UI thread's SequenceLocalStorage,
+so it is destroyed by BrowserTaskExecutor::Shutdown(), which runs after
+BrowserMainLoop has shut down the ThreadPool. DeviceService holds the
+SerialPortManagerImpl in a SequenceBound on a ThreadPool sequence, so
+the task that would delete it (and the enumerator and helper it owns) is
+dropped. The helper's ScopedObservation is still live when AtExitManager
+deletes the monitor, which fails CheckDanglingRawPtrBufferEmpty() at
+exit when dangling pointer checks are enabled.
+
+The AtExit registration dates from when DeviceMonitorMessageWindow owned
+its own message window and needed to be destroyed in order with an
+AtExit-managed HidService. Neither is true anymore: it listens on
+gfx::SingletonHwnd, which is itself a NoDestructor, and HidService is
+owned by DeviceService. Make it a base::NoDestructor so the monitors
+outlive all of their observers.
+
+Bug: 572741653
+Change-Id: I85aa5586edab522b6fde5c77a5e4515b848a886b
+
+Upstreamed at https://chromium-review.googlesource.com/c/chromium/src/+/8561777.
+This patch can be removed once that CL lands.
+
+diff --git a/device/BUILD.gn b/device/BUILD.gn
+index 2cdc91e243d010941a6878b26cb164125fc25dcb..741410776eb26868aef2b05d9a3aa042c02b5b0f 100644
+--- a/device/BUILD.gn
++++ b/device/BUILD.gn
+@@ -104,6 +104,7 @@ test("device_unittests") {
+ 
+   if (is_win) {
+     sources += [
++      "base/device_monitor_win_unittest.cc",
+       "bluetooth/bluetooth_adapter_win_unittest.cc",
+       "bluetooth/bluetooth_device_win_unittest.cc",
+       "bluetooth/bluetooth_service_record_win_unittest.cc",
+diff --git a/device/base/device_monitor_win.cc b/device/base/device_monitor_win.cc
+index 72073904d7805ffe60f6077736a548ff83cde78e..ff1858cb5476f116f0595d8cbb850431e2c418ce 100644
+--- a/device/base/device_monitor_win.cc
++++ b/device/base/device_monitor_win.cc
+@@ -13,13 +13,13 @@
+ #include <map>
+ #include <memory>
+ 
+-#include "base/at_exit.h"
+ #include "base/callback_list.h"
+ #include "base/compiler_specific.h"
+ #include "base/containers/span.h"
+ #include "base/functional/bind.h"
+-#include "base/functional/callback_helpers.h"
+ #include "base/logging.h"
++#include "base/no_destructor.h"
++#include "base/sequence_checker.h"
+ #include "base/strings/string_util.h"
+ #include "base/strings/sys_string_conversions.h"
+ #include "base/task/current_thread.h"
+@@ -27,12 +27,8 @@
+ 
+ namespace device {
+ 
+-class DeviceMonitorMessageWindow;
+-
+ namespace {
+ 
+-DeviceMonitorMessageWindow* g_message_window;
+-
+ // Provides basic comparability for GUIDs so that they can be used as keys to an
+ // STL map.
+ struct CompareGUID {
+@@ -49,19 +45,19 @@ struct CompareGUID {
+ // for each unique GUID it sees.
+ class DeviceMonitorMessageWindow {
+  public:
++  // Returns nullptr if initialization fails. Initialization is retried on the
++  // next call, since it fails when called from a thread without a UI message
++  // pump.
+   static DeviceMonitorMessageWindow* GetInstance() {
+-    if (!g_message_window) {
+-      g_message_window = new DeviceMonitorMessageWindow();
+-      if (g_message_window->Init()) {
+-        base::AtExitManager::RegisterTask(
+-            base::BindOnce(&base::DeletePointer<DeviceMonitorMessageWindow>,
+-                           base::Unretained(g_message_window)));
+-      } else {
+-        delete g_message_window;
+-        g_message_window = nullptr;
+-      }
++    // Never destroyed, like the gfx::SingletonHwnd it listens on, so that
++    // observers that outlive the AtExitManager are not left with dangling
++    // pointers to the DeviceMonitorWin instances it owns.
++    static base::NoDestructor<DeviceMonitorMessageWindow> instance;
++    DCHECK_CALLED_ON_VALID_SEQUENCE(instance->sequence_checker_);
++    if (!instance->initialized_) {
++      instance->initialized_ = instance->Init();
+     }
+-    return g_message_window;
++    return instance->initialized_ ? instance.get() : nullptr;
+   }
+ 
+   DeviceMonitorMessageWindow(const DeviceMonitorMessageWindow&) = delete;
+@@ -69,6 +65,7 @@ class DeviceMonitorMessageWindow {
+       delete;
+ 
+   DeviceMonitorWin* GetForDeviceInterface(const GUID& device_interface) {
++    DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+     std::unique_ptr<DeviceMonitorWin>& device_monitor =
+         device_monitors_[device_interface];
+     if (!device_monitor) {
+@@ -77,21 +74,20 @@ class DeviceMonitorMessageWindow {
+     return device_monitor.get();
+   }
+ 
+-  DeviceMonitorWin* GetForAllInterfaces() { return &all_device_monitor_; }
++  DeviceMonitorWin* GetForAllInterfaces() {
++    DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
++    return &all_device_monitor_;
++  }
+ 
+  private:
+-  friend void base::DeletePointer<DeviceMonitorMessageWindow>(
+-      DeviceMonitorMessageWindow* message_window);
++  friend class base::NoDestructor<DeviceMonitorMessageWindow>;
+ 
+-  DeviceMonitorMessageWindow() {}
+-
+-  ~DeviceMonitorMessageWindow() {
+-    if (notify_handle_) {
+-      UnregisterDeviceNotification(notify_handle_);
+-    }
+-  }
++  DeviceMonitorMessageWindow() = default;
++  ~DeviceMonitorMessageWindow() = delete;
+ 
+   bool Init() {
++    DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
++
+     // gfx::SingletonHwnd is a process-wide singleton which only creates its
+     // window if it is first instantiated on a thread running a UI message
+     // pump. Bail out before touching it so that a caller on the wrong thread
+@@ -107,8 +103,7 @@ class DeviceMonitorMessageWindow {
+       return false;
+     }
+ 
+-    // base::Unretained() is safe because |subscription_| is owned by this
+-    // object and unregisters the callback when it is destroyed.
++    // base::Unretained() is safe because this object is never destroyed.
+     subscription_ =
+         gfx::SingletonHwnd::GetInstance()->RegisterCallback(base::BindRepeating(
+             &DeviceMonitorMessageWindow::OnWndProc, base::Unretained(this)));
+@@ -120,6 +115,7 @@ class DeviceMonitorMessageWindow {
+         DEVICE_NOTIFY_WINDOW_HANDLE | DEVICE_NOTIFY_ALL_INTERFACE_CLASSES);
+     if (!notify_handle_) {
+       PLOG(ERROR) << "Failed to register for device notifications";
++      subscription_ = {};
+       return false;
+     }
+ 
+@@ -127,6 +123,7 @@ class DeviceMonitorMessageWindow {
+   }
+ 
+   void OnWndProc(HWND hwnd, UINT message, WPARAM wparam, LPARAM lparam) {
++    DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+     if (message != WM_DEVICECHANGE ||
+         (wparam != DBT_DEVICEARRIVAL && wparam != DBT_DEVICEREMOVECOMPLETE)) {
+       return;
+@@ -168,6 +165,9 @@ class DeviceMonitorMessageWindow {
+   DeviceMonitorWin all_device_monitor_;
+   HDEVNOTIFY notify_handle_ = NULL;
+   base::CallbackListSubscription subscription_;
++  bool initialized_ = false;
++
++  SEQUENCE_CHECKER(sequence_checker_);
+ };
+ 
+ void DeviceMonitorWin::Observer::OnDeviceAdded(
+@@ -202,10 +202,12 @@ DeviceMonitorWin* DeviceMonitorWin::GetForAllInterfaces() {
+ DeviceMonitorWin::~DeviceMonitorWin() {}
+ 
+ void DeviceMonitorWin::AddObserver(Observer* observer) {
++  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+   observer_list_.AddObserver(observer);
+ }
+ 
+ void DeviceMonitorWin::RemoveObserver(Observer* observer) {
++  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+   observer_list_.RemoveObserver(observer);
+ }
+ 
+@@ -213,6 +215,7 @@ DeviceMonitorWin::DeviceMonitorWin() {}
+ 
+ void DeviceMonitorWin::NotifyDeviceAdded(const GUID& class_guid,
+                                          const std::wstring& device_path) {
++  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+   for (auto& observer : observer_list_) {
+     observer.OnDeviceAdded(class_guid, device_path);
+   }
+@@ -220,6 +223,7 @@ void DeviceMonitorWin::NotifyDeviceAdded(const GUID& class_guid,
+ 
+ void DeviceMonitorWin::NotifyDeviceRemoved(const GUID& class_guid,
+                                            const std::wstring& device_path) {
++  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+   for (auto& observer : observer_list_) {
+     observer.OnDeviceRemoved(class_guid, device_path);
+   }
+diff --git a/device/base/device_monitor_win.h b/device/base/device_monitor_win.h
+index 0b1c2a4edd9cccc22cdd1b947b35f8e86adccb65..1ada9645dd940d49b5a19fc2699c79079fb5db82 100644
+--- a/device/base/device_monitor_win.h
++++ b/device/base/device_monitor_win.h
+@@ -8,6 +8,7 @@
+ #include <string>
+ 
+ #include "base/observer_list.h"
++#include "base/sequence_checker.h"
+ #include "base/win/windows_types.h"
+ #include "device/base/device_base_export.h"
+ 
+@@ -48,6 +49,10 @@ class DEVICE_BASE_EXPORT DeviceMonitorWin {
+   void NotifyDeviceRemoved(const GUID& class_guid,
+                            const std::wstring& device_path);
+ 
++  // `observer_list_` is not thread-safe, so observers must be added and
++  // removed on the sequence that delivers notifications.
++  SEQUENCE_CHECKER(sequence_checker_);
++
+   base::ObserverList<Observer>::Unchecked observer_list_;
+ };
+ 
+diff --git a/device/base/device_monitor_win_unittest.cc b/device/base/device_monitor_win_unittest.cc
+new file mode 100644
+index 0000000000000000000000000000000000000000..fea61c4b0a98d31bcd1174425746076addd91ae5
+--- /dev/null
++++ b/device/base/device_monitor_win_unittest.cc
+@@ -0,0 +1,50 @@
++// 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.
++
++#include "device/base/device_monitor_win.h"
++
++#include "base/at_exit.h"
++#include "base/scoped_observation.h"
++#include "base/test/task_environment.h"
++#include "testing/gtest/include/gtest/gtest.h"
++
++namespace device {
++
++namespace {
++
++class TestObserver : public DeviceMonitorWin::Observer {
++ public:
++  void Observe(DeviceMonitorWin* monitor) { observation_.Observe(monitor); }
++  void Reset() { observation_.Reset(); }
++
++ private:
++  base::ScopedObservation<DeviceMonitorWin, DeviceMonitorWin::Observer>
++      observation_{this};
++};
++
++}  // namespace
++
++// Observers can outlive the AtExitManager (e.g. the serial enumerator's UI
++// thread helper, which is leaked at browser shutdown), so the monitors must
++// not be destroyed by it. If they were, `observer` would be left with a
++// dangling pointer to the monitor and would call RemoveObserver() on it after
++// it was freed.
++TEST(DeviceMonitorWinTest, MonitorOutlivesAtExitManager) {
++  base::test::SingleThreadTaskEnvironment task_environment(
++      base::test::SingleThreadTaskEnvironment::MainThreadType::UI);
++
++  TestObserver observer;
++  DeviceMonitorWin* monitor = nullptr;
++  {
++    base::ShadowingAtExitManager at_exit_manager;
++    monitor = DeviceMonitorWin::GetForAllInterfaces();
++    ASSERT_TRUE(monitor);
++    observer.Observe(monitor);
++  }
++
++  EXPECT_EQ(monitor, DeviceMonitorWin::GetForAllInterfaces());
++  observer.Reset();
++}
++
++}  // namespace device