From de842303f0ce7951dc1253492a8acd258773a34e Mon Sep 17 00:00:00 2001
From: Alvin Ji <alvinji@chromium.org>
Date: Mon, 14 Sep 2026 13:02:05 -0700
Subject: [PATCH] usb: Notify all service worker instances on permission revocation

ServiceWorkerUsbDelegateObserver previously stored only the latest
WebUsbServiceImpl instance per service worker registration, overwriting
older instances. If multiple service worker versions (e.g. active and
updating) ran simultaneously for a registration, revoking device
permissions only notified the newest instance and left open device
connections active in older instances.

Update ServiceWorkerUsbDelegateObserver to track all live
WebUsbServiceImpl instances per registration and notify all of them on
permission revocation, device manager connection error, and device
events. Also ensure device removal events are forwarded to running
service workers even when no background event listeners are registered.

BUG=503625361
TAG=agy
CONV=92a9c2cd-ac9f-413a-8008-ad618c94a60d

Change-Id: Idc172c5d80e175def08e49f63fbaebf76d986379
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8383036
Reviewed-by: Matt Reynolds <mattreynolds@chromium.org>
Commit-Queue: Alvin Ji <alvinji@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1697461}
---
diff --git a/content/browser/service_worker/service_worker_usb_delegate_observer.cc b/content/browser/service_worker/service_worker_usb_delegate_observer.cc
index 074a975b..e3fe23a 100644
--- a/content/browser/service_worker/service_worker_usb_delegate_observer.cc
+++ b/content/browser/service_worker/service_worker_usb_delegate_observer.cc
@@ -60,9 +60,10 @@
 
 void ServiceWorkerUsbDelegateObserver::OnDeviceManagerConnectionError() {
   for (auto const& [id, info] : registration_id_map()) {
-    auto* usb_service = GetUsbService(id);
-    if (usb_service) {
-      usb_service->OnDeviceManagerConnectionError();
+    for (auto const& usb_service : GetUsbServices(id)) {
+      if (usb_service) {
+        usb_service->OnDeviceManagerConnectionError();
+      }
     }
   }
 }
@@ -70,9 +71,10 @@
 void ServiceWorkerUsbDelegateObserver::OnPermissionRevoked(
     const url::Origin& origin) {
   for (auto const& [id, info] : registration_id_map()) {
-    auto* usb_service = GetUsbService(id);
-    if (usb_service) {
-      usb_service->OnPermissionRevoked(origin);
+    for (auto const& usb_service : GetUsbServices(id)) {
+      if (usb_service) {
+        usb_service->OnPermissionRevoked(origin);
+      }
     }
   }
 }
@@ -81,13 +83,14 @@
     int64_t registration_id,
     base::WeakPtr<WebUsbServiceImpl> usb_service) {
   Register(registration_id);
-  // `usb_services_` may already have an entry for `registration_id` in a case
-  // where the service worker went to sleep and now is worken up. In that
-  // case, the WebUsbServiceImpl from `usb_services_[registration_id]` is the
-  // weak ptr of previous WebUsbServiceImpl before the service worker went to
-  // sleep. We don't care about the previous WebUsbServiceImpl, so here just
-  // overwrite it with `usb_service`, which is the latest one.
-  usb_services_[registration_id] = usb_service;
+  // Multiple WebUsbServiceImpl instances can exist for the same
+  // `registration_id` (e.g., an installing version evaluating top-level
+  // `navigator.usb` listeners creates a new instance while the active version
+  // is still running). Prune invalidated weak pointers and append `usb_service`
+  // so all live instances are notified on permission revocation.
+  auto& services = usb_services_[registration_id];
+  std::erase_if(services, [](const auto& service) { return !service; });
+  services.push_back(std::move(usb_service));
 }
 
 void ServiceWorkerUsbDelegateObserver::RegistrationAdded(
@@ -102,6 +105,12 @@
 
 void ServiceWorkerUsbDelegateObserver::RegistrationRemoved(
     int64_t registration_id) {
+  for (auto const& usb_service : GetUsbServices(registration_id)) {
+    if (usb_service) {
+      usb_service->OnDeviceManagerConnectionError();
+    }
+  }
+  usb_services_.erase(registration_id);
   if (registration_id_map().empty()) {
     usb_delegate_observation.Reset();
   }
@@ -111,21 +120,46 @@
     const device::mojom::UsbDeviceInfo& device_info,
     UsbServiceDeviceEventCallback callback) {
   for (auto const& [id, info] : registration_id_map()) {
-    // No need to proceed if the registration doesn't have any event listeners.
-    if (!info.has_event_handlers) {
-      continue;
-    }
-    // Forward it to UsbService if the service worker is running, UsbService is
-    // available, and it has clients registered.
-    auto* usb_service = GetUsbService(id);
-    if (usb_service) {
+    // Forward the event to all currently running (kRunning) WebUsbServiceImpl
+    // instances so that open device connections are cleaned up immediately
+    // (e.g. on device removal), even if no JS event listeners were registered.
+    // Track whether the event was delivered to an already-running ACTIVATED or
+    // ACTIVATING version whose client is ready so we know if we need to wake it
+    // up below.
+    bool delivered_to_active_version = false;
+    for (auto const& usb_service : GetUsbServices(id)) {
+      if (!usb_service) {
+        continue;
+      }
       auto version = usb_service->service_worker_version();
       if (version &&
           version->running_status() == blink::EmbeddedWorkerStatus::kRunning) {
-        callback.Run(device_info, usb_service);
-        continue;
+        auto status = version->status();
+        callback.Run(device_info, usb_service.get());
+        // Mark as delivered only if the active version (ACTIVATING or
+        // ACTIVATED) already has a registered client. If `clients()` is empty
+        // (SetClient Mojo call still in flight), leave this false so
+        // DispatchEventToWorker queues a pending callback rather than losing
+        // the event. Checking ACTIVATING prevents double-notifying when it
+        // transitions to ACTIVATED in DispatchEventToWorker.
+        if ((status == ServiceWorkerVersion::ACTIVATED ||
+             status == ServiceWorkerVersion::ACTIVATING) &&
+            !usb_service->clients().empty()) {
+          delivered_to_active_version = true;
+        }
       }
     }
+    if (delivered_to_active_version) {
+      continue;
+    }
+
+    // Check `has_event_handlers` after notifying running services above, since
+    // running workers must clean up open device connections (e.g. on device
+    // removal) even without JS listeners, whereas a stopped/sleeping ACTIVATED
+    // worker should only be woken up if it registered JS event listeners.
+    if (!info.has_event_handlers) {
+      continue;
+    }
 
     // Avoid waking up the worker if eventually the device event won't be
     // delivered for the WebUsbService.
@@ -153,33 +187,46 @@
   }
 
   auto registration_id = version->registration_id();
-  auto* usb_service = GetUsbService(registration_id);
-  // Even when the service worker is in the running state, the WebUsbService may
-  // not be available or the render-side DeviceManagerClient may not have yet
-  // registered. This is because the service worker is set to running state
-  // after the script is evaluated, but the inter-process request that creates
-  // the WebUsbService or gets the DeviceManagerClient registered may still be
-  // in progress. In order to handle this case, the callback is stored and will
-  // be processed when the WebUsbService is ready and the DeviceManagerClient is
-  // registered with the WebUsbService.
-  if (!usb_service || usb_service->clients().empty()) {
+  // Deliver the event only to WebUsbServiceImpl instances belonging to the
+  // woken `version` (ignoring services from other versions of the same
+  // registration) that already have a registered render-side
+  // DeviceManagerClient.
+  bool has_ready_service = false;
+  for (auto const& usb_service : GetUsbServices(registration_id)) {
+    if (usb_service &&
+        usb_service->service_worker_version().get() == version.get() &&
+        !usb_service->clients().empty()) {
+      callback.Run(*device_info, usb_service.get());
+      has_ready_service = true;
+    }
+  }
+  // Even when `version` is in the running state, its WebUsbServiceImpl may not
+  // yet be created or its render-side DeviceManagerClient may not yet be
+  // registered (since the worker enters kRunning after script evaluation while
+  // the Mojo requests to create WebUsbServiceImpl / register
+  // DeviceManagerClient are still in flight). Store the callback to be run once
+  // `version`'s WebUsbServiceImpl client is registered.
+  if (!has_ready_service) {
     AddPendingCallback(
         version.get(),
         base::BindOnce(&ServiceWorkerUsbDelegateObserver::WorkerStarted,
                        base::Unretained(this), std::move(device_info),
                        std::move(callback), version, service_worker_status));
-    return;
   }
-  callback.Run(*device_info, usb_service);
 }
 
-WebUsbServiceImpl* ServiceWorkerUsbDelegateObserver::GetUsbService(
-    int64_t registration_id) {
+std::vector<base::WeakPtr<WebUsbServiceImpl>>
+ServiceWorkerUsbDelegateObserver::GetUsbServices(int64_t registration_id) {
   auto it = usb_services_.find(registration_id);
   if (it == usb_services_.end()) {
-    return nullptr;
+    return {};
   }
-  return it->second.get();
+  std::erase_if(it->second, [](const auto& service) { return !service; });
+  if (it->second.empty()) {
+    usb_services_.erase(it);
+    return {};
+  }
+  return it->second;
 }
 
 }  // namespace content
diff --git a/content/browser/service_worker/service_worker_usb_delegate_observer.h b/content/browser/service_worker/service_worker_usb_delegate_observer.h
index d108c048..0ed0aa9 100644
--- a/content/browser/service_worker/service_worker_usb_delegate_observer.h
+++ b/content/browser/service_worker/service_worker_usb_delegate_observer.h
@@ -5,6 +5,8 @@
 #ifndef CONTENT_BROWSER_SERVICE_WORKER_SERVICE_WORKER_USB_DELEGATE_OBSERVER_H_
 #define CONTENT_BROWSER_SERVICE_WORKER_SERVICE_WORKER_USB_DELEGATE_OBSERVER_H_
 
+#include <vector>
+
 #include "base/memory/weak_ptr.h"
 #include "base/scoped_observation_traits.h"
 #include "content/browser/service_worker/service_worker_device_delegate_observer.h"
@@ -43,13 +45,15 @@
   void OnDeviceManagerConnectionError() override;
   void OnPermissionRevoked(const url::Origin& origin) override;
 
-  // Register the `usb_service` to be the latest WebUsbService for
-  // `registraiton_id`.
+  // Register the `usb_service` for `registration_id`. There can be multiple
+  // live WebUsbServiceImpl instances for a registration (e.g., when a waiting
+  // service worker version starts while the active version is still running).
   void RegisterUsbService(int64_t registration_id,
                           base::WeakPtr<WebUsbServiceImpl> usb_service);
 
-  WebUsbServiceImpl* GetUsbServiceForTesting(int64_t registration_id) {
-    return GetUsbService(registration_id);
+  std::vector<base::WeakPtr<WebUsbServiceImpl>> GetUsbServicesForTesting(
+      int64_t registration_id) {
+    return GetUsbServices(registration_id);
   }
 
  private:
@@ -73,12 +77,13 @@
                      scoped_refptr<ServiceWorkerVersion> version,
                      blink::ServiceWorkerStatusCode service_worker_status);
 
-  // Get UsbService for the `registration_id`. It can be null if no live
-  // UsbService for the `registration_id`.
-  WebUsbServiceImpl* GetUsbService(int64_t registration_id);
+  // Get all live UsbServices for the `registration_id`.
+  std::vector<base::WeakPtr<WebUsbServiceImpl>> GetUsbServices(
+      int64_t registration_id);
 
-  // The map for registration id to the latest registered UsbService.
-  base::flat_map<int64_t, base::WeakPtr<WebUsbServiceImpl>> usb_services_;
+  // The map for registration id to the list of live registered UsbServices.
+  base::flat_map<int64_t, std::vector<base::WeakPtr<WebUsbServiceImpl>>>
+      usb_services_;
 
   base::ScopedObservation<UsbDelegate, ServiceWorkerUsbDelegateObserver>
       usb_delegate_observation{this};
diff --git a/content/browser/service_worker/service_worker_usb_delegate_observer_unittest.cc b/content/browser/service_worker/service_worker_usb_delegate_observer_unittest.cc
index 5508a9b4..9666d6c 100644
--- a/content/browser/service_worker/service_worker_usb_delegate_observer_unittest.cc
+++ b/content/browser/service_worker/service_worker_usb_delegate_observer_unittest.cc
@@ -2,6 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "content/browser/service_worker/service_worker_usb_delegate_observer.h"
+
 #include <cstddef>
 #include <memory>
 #include <vector>
@@ -10,12 +12,12 @@
 #include "base/run_loop.h"
 #include "base/strings/stringprintf.h"
 #include "base/test/gmock_callback_support.h"
+#include "base/test/run_until.h"
 #include "base/test/test_future.h"
 #include "content/browser/service_worker/embedded_worker_test_helper.h"
 #include "content/browser/service_worker/service_worker_device_delegate_observer_unittest.h"
 #include "content/browser/service_worker/service_worker_registration.h"
 #include "content/browser/service_worker/service_worker_test_utils.h"
-#include "content/browser/service_worker/service_worker_usb_delegate_observer.h"
 #include "content/browser/service_worker/service_worker_version.h"
 #include "content/browser/usb/usb_test_utils.h"
 #include "content/browser/usb/web_usb_service_impl.h"
@@ -425,20 +427,19 @@
     auto* version = context()->GetLiveVersion(version_ids[idx]);
     ASSERT_NE(version, nullptr);
     EXPECT_EQ(version->running_status(), blink::EmbeddedWorkerStatus::kRunning);
-    EXPECT_EQ(context()
-                  ->usb_delegate_observer()
-                  ->GetUsbServiceForTesting(registrations[idx]->id())
-                  ->clients()
-                  .size(),
-              1u);
+    auto services =
+        context()->usb_delegate_observer()->GetUsbServicesForTesting(
+            registrations[idx]->id());
+    ASSERT_EQ(services.size(), 1u);
+    EXPECT_EQ(services[0]->clients().size(), 1u);
   }
   usb_delegate().OnDeviceManagerConnectionError();
   for (size_t idx = 0; idx < num_workers; ++idx) {
-    EXPECT_TRUE(context()
-                    ->usb_delegate_observer()
-                    ->GetUsbServiceForTesting(registrations[idx]->id())
-                    ->clients()
-                    .empty());
+    auto services =
+        context()->usb_delegate_observer()->GetUsbServicesForTesting(
+            registrations[idx]->id());
+    ASSERT_EQ(services.size(), 1u);
+    EXPECT_TRUE(services[0]->clients().empty());
   }
 }
 
@@ -498,6 +499,85 @@
 }
 
 TEST_F(ServiceWorkerUsbDelegateObserverTest,
+       OnPermissionRevokedMultipleServicesForSameRegistration) {
+  device::MockUsbMojoDevice mock_device1;
+  device::MockUsbMojoDevice mock_device2;
+  auto fake_device_info1 = CreateFakeDevice();
+  auto fake_device_info2 = CreateFakeDevice();
+  auto device_info1 = ConnectDevice(fake_device_info1, &mock_device1);
+  auto device_info2 = ConnectDevice(fake_device_info2, &mock_device2);
+
+  const GURL origin_url(kTestUrl);
+  auto registration = InstallServiceWorker(origin_url);
+  auto* version1 = registration->active_version();
+  ASSERT_NE(version1, nullptr);
+  StartServiceWorker(version1);
+  auto usb_service1 = CreateUsbService(version1);
+
+  // Create an installing version for the same registration to simulate a
+  // service worker update where both versions are simultaneously running.
+  auto version2 =
+      CreateNewServiceWorkerVersion(context()->registry(), registration,
+                                    GURL("https://www.google.com/worker2.js"),
+                                    blink::mojom::ScriptType::kClassic);
+  version2->set_fetch_handler_type(
+      ServiceWorkerVersion::FetchHandlerType::kNotSkippable);
+  version2->SetStatus(ServiceWorkerVersion::Status::INSTALLING);
+  registration->SetInstallingVersion(version2);
+  StartServiceWorker(version2.get());
+  auto usb_service2 = CreateUsbService(version2.get());
+
+  EXPECT_EQ(context()
+                ->usb_delegate_observer()
+                ->GetUsbServicesForTesting(registration->id())
+                .size(),
+            2u);
+
+  // Both versions open a USB device connection.
+  mojo::Remote<device::mojom::UsbDevice> device1;
+  usb_service1->GetDevice(device_info1->guid,
+                          device1.BindNewPipeAndPassReceiver());
+  EXPECT_CALL(mock_device1, Open)
+      .WillOnce(base::test::RunOnceCallback<0>(NewUsbOpenDeviceSuccess()));
+  TestFuture<device::mojom::UsbOpenDeviceResultPtr> open_future1;
+  device1->Open(open_future1.GetCallback());
+  EXPECT_TRUE(open_future1.Get()->is_success());
+
+  mojo::Remote<device::mojom::UsbDevice> device2;
+  usb_service2->GetDevice(device_info2->guid,
+                          device2.BindNewPipeAndPassReceiver());
+  EXPECT_CALL(mock_device2, Open)
+      .WillOnce(base::test::RunOnceCallback<0>(NewUsbOpenDeviceSuccess()));
+  TestFuture<device::mojom::UsbOpenDeviceResultPtr> open_future2;
+  device2->Open(open_future2.GetCallback());
+  EXPECT_TRUE(open_future2.Get()->is_success());
+
+  // When permission is revoked for the origin, BOTH WebUsbServiceImpl
+  // instances must be notified and close their open device connections.
+  auto origin = url::Origin::Create(origin_url);
+  TestFuture<void> close_future1;
+  TestFuture<void> close_future2;
+  EXPECT_CALL(usb_delegate(), GetDeviceInfo(_, device_info1->guid))
+      .WillOnce(Return(device_info1.get()));
+  EXPECT_CALL(usb_delegate(), GetDeviceInfo(_, device_info2->guid))
+      .WillOnce(Return(device_info2.get()));
+  EXPECT_CALL(usb_delegate(),
+              HasDevicePermission(_, nullptr, origin, Ref(*device_info1)))
+      .WillOnce(Return(false));
+  EXPECT_CALL(usb_delegate(),
+              HasDevicePermission(_, nullptr, origin, Ref(*device_info2)))
+      .WillOnce(Return(false));
+  EXPECT_CALL(mock_device1, Close)
+      .WillOnce(RunClosure(close_future1.GetRepeatingCallback()));
+  EXPECT_CALL(mock_device2, Close)
... (file diff truncated)

diff --git a/content/browser/service_worker/service_worker_usb_delegate_observer_unittest.cc b/content/browser/service_worker/service_worker_usb_delegate_observer_unittest.cc
index 5508a9b4..9666d6c 100644
--- a/content/browser/service_worker/service_worker_usb_delegate_observer_unittest.cc
+++ b/content/browser/service_worker/service_worker_usb_delegate_observer_unittest.cc
@@ -2,6 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "content/browser/service_worker/service_worker_usb_delegate_observer.h"
+
 #include <cstddef>
 #include <memory>
 #include <vector>
@@ -10,12 +12,12 @@
 #include "base/run_loop.h"
 #include "base/strings/stringprintf.h"
 #include "base/test/gmock_callback_support.h"
+#include "base/test/run_until.h"
 #include "base/test/test_future.h"
 #include "content/browser/service_worker/embedded_worker_test_helper.h"
 #include "content/browser/service_worker/service_worker_device_delegate_observer_unittest.h"
 #include "content/browser/service_worker/service_worker_registration.h"
 #include "content/browser/service_worker/service_worker_test_utils.h"
-#include "content/browser/service_worker/service_worker_usb_delegate_observer.h"
 #include "content/browser/service_worker/service_worker_version.h"
 #include "content/browser/usb/usb_test_utils.h"
 #include "content/browser/usb/web_usb_service_impl.h"
@@ -425,20 +427,19 @@
     auto* version = context()->GetLiveVersion(version_ids[idx]);
     ASSERT_NE(version, nullptr);
     EXPECT_EQ(version->running_status(), blink::EmbeddedWorkerStatus::kRunning);
-    EXPECT_EQ(context()
-                  ->usb_delegate_observer()
-                  ->GetUsbServiceForTesting(registrations[idx]->id())
-                  ->clients()
-                  .size(),
-              1u);
+    auto services =
+        context()->usb_delegate_observer()->GetUsbServicesForTesting(
+            registrations[idx]->id());
+    ASSERT_EQ(services.size(), 1u);
+    EXPECT_EQ(services[0]->clients().size(), 1u);
   }
   usb_delegate().OnDeviceManagerConnectionError();
   for (size_t idx = 0; idx < num_workers; ++idx) {
-    EXPECT_TRUE(context()
-                    ->usb_delegate_observer()
-                    ->GetUsbServiceForTesting(registrations[idx]->id())
-                    ->clients()
-                    .empty());
+    auto services =
+        context()->usb_delegate_observer()->GetUsbServicesForTesting(
+            registrations[idx]->id());
+    ASSERT_EQ(services.size(), 1u);
+    EXPECT_TRUE(services[0]->clients().empty());
   }
 }
 
@@ -498,6 +499,85 @@
 }
 
 TEST_F(ServiceWorkerUsbDelegateObserverTest,
+       OnPermissionRevokedMultipleServicesForSameRegistration) {
+  device::MockUsbMojoDevice mock_device1;
+  device::MockUsbMojoDevice mock_device2;
+  auto fake_device_info1 = CreateFakeDevice();
+  auto fake_device_info2 = CreateFakeDevice();
+  auto device_info1 = ConnectDevice(fake_device_info1, &mock_device1);
+  auto device_info2 = ConnectDevice(fake_device_info2, &mock_device2);
+
+  const GURL origin_url(kTestUrl);
+  auto registration = InstallServiceWorker(origin_url);
+  auto* version1 = registration->active_version();
+  ASSERT_NE(version1, nullptr);
+  StartServiceWorker(version1);
+  auto usb_service1 = CreateUsbService(version1);
+
+  // Create an installing version for the same registration to simulate a
+  // service worker update where both versions are simultaneously running.
+  auto version2 =
+      CreateNewServiceWorkerVersion(context()->registry(), registration,
+                                    GURL("https://www.google.com/worker2.js"),
+                                    blink::mojom::ScriptType::kClassic);
+  version2->set_fetch_handler_type(
+      ServiceWorkerVersion::FetchHandlerType::kNotSkippable);
+  version2->SetStatus(ServiceWorkerVersion::Status::INSTALLING);
+  registration->SetInstallingVersion(version2);
+  StartServiceWorker(version2.get());
+  auto usb_service2 = CreateUsbService(version2.get());
+
+  EXPECT_EQ(context()
+                ->usb_delegate_observer()
+                ->GetUsbServicesForTesting(registration->id())
+                .size(),
+            2u);
+
+  // Both versions open a USB device connection.
+  mojo::Remote<device::mojom::UsbDevice> device1;
+  usb_service1->GetDevice(device_info1->guid,
+                          device1.BindNewPipeAndPassReceiver());
+  EXPECT_CALL(mock_device1, Open)
+      .WillOnce(base::test::RunOnceCallback<0>(NewUsbOpenDeviceSuccess()));
+  TestFuture<device::mojom::UsbOpenDeviceResultPtr> open_future1;
+  device1->Open(open_future1.GetCallback());
+  EXPECT_TRUE(open_future1.Get()->is_success());
+
+  mojo::Remote<device::mojom::UsbDevice> device2;
+  usb_service2->GetDevice(device_info2->guid,
+                          device2.BindNewPipeAndPassReceiver());
+  EXPECT_CALL(mock_device2, Open)
+      .WillOnce(base::test::RunOnceCallback<0>(NewUsbOpenDeviceSuccess()));
+  TestFuture<device::mojom::UsbOpenDeviceResultPtr> open_future2;
+  device2->Open(open_future2.GetCallback());
+  EXPECT_TRUE(open_future2.Get()->is_success());
+
+  // When permission is revoked for the origin, BOTH WebUsbServiceImpl
+  // instances must be notified and close their open device connections.
+  auto origin = url::Origin::Create(origin_url);
+  TestFuture<void> close_future1;
+  TestFuture<void> close_future2;
+  EXPECT_CALL(usb_delegate(), GetDeviceInfo(_, device_info1->guid))
+      .WillOnce(Return(device_info1.get()));
+  EXPECT_CALL(usb_delegate(), GetDeviceInfo(_, device_info2->guid))
+      .WillOnce(Return(device_info2.get()));
+  EXPECT_CALL(usb_delegate(),
+              HasDevicePermission(_, nullptr, origin, Ref(*device_info1)))
+      .WillOnce(Return(false));
+  EXPECT_CALL(usb_delegate(),
+              HasDevicePermission(_, nullptr, origin, Ref(*device_info2)))
+      .WillOnce(Return(false));
+  EXPECT_CALL(mock_device1, Close)
+      .WillOnce(RunClosure(close_future1.GetRepeatingCallback()));
+  EXPECT_CALL(mock_device2, Close)
+      .WillOnce(RunClosure(close_future2.GetRepeatingCallback()));
+
+  usb_delegate().OnPermissionRevoked(origin);
+  EXPECT_TRUE(close_future1.Wait());
+  EXPECT_TRUE(close_future2.Wait());
+}
+
+TEST_F(ServiceWorkerUsbDelegateObserverTest,
        RemovedFromUsbDelegateObserverWhenNoRegistration) {
   const GURL origin(kTestUrl);
   EXPECT_TRUE(usb_delegate().observer_list().empty());
@@ -506,22 +586,103 @@
   ASSERT_NE(version, nullptr);
   StartServiceWorker(version);
   auto usb_service = CreateUsbService(version);
+  MockDeviceManagerClient device_manager_client;
+  RegisterUsbManagerClient(usb_service, device_manager_client);
+  auto services = context()->usb_delegate_observer()->GetUsbServicesForTesting(
+      registration->id());
+  ASSERT_EQ(services.size(), 1u);
+  base::WeakPtr<WebUsbServiceImpl> service_impl = services[0];
+  ASSERT_TRUE(service_impl);
+  EXPECT_EQ(service_impl->clients().size(), 1u);
   EXPECT_FALSE(usb_delegate().observer_list().empty());
 
+  TestFuture<void> connection_error_future;
+  EXPECT_CALL(device_manager_client, ConnectionError())
+      .WillOnce(RunClosure(connection_error_future.GetRepeatingCallback()));
+
   TestFuture<blink::ServiceWorkerStatusCode> unregister_future;
   context()->UnregisterServiceWorker(
       registration->scope(), registration->key(),
       /*is_immediate=*/true, ServiceWorkerRegistration::DeleteInitiator::kTest,
       unregister_future.GetCallback());
   EXPECT_EQ(unregister_future.Get<0>(), blink::ServiceWorkerStatusCode::kOk);
+  EXPECT_TRUE(connection_error_future.Wait());
   // Wait until all of the
   // ServiceWorkerDeviceDelegateObserver::OnRegistrationDeleted are called.
-  base::RunLoop().RunUntilIdle();
-  EXPECT_TRUE(usb_delegate().observer_list().empty());
+  ASSERT_TRUE(base::test::RunUntil(
+      [&]() { return usb_delegate().observer_list().empty(); }));
+  ASSERT_TRUE(service_impl);
+  EXPECT_TRUE(service_impl->clients().empty());
 }
 
 TEST_F(ServiceWorkerUsbDelegateObserverTest,
-       HasLatestUsbServiceAfterServiceWorkerStopThenStart) {
+       ActivatingWorkerNotDoubleNotified) {
+  const GURL origin(kTestUrl);
+  auto registration = InstallServiceWorker(origin);
+  auto* version = registration->active_version();
+  ASSERT_NE(version, nullptr);
+  StartServiceWorker(version);
+  version->SetStatus(ServiceWorkerVersion::Status::ACTIVATING);
+
+  auto usb_service = CreateUsbService(version);
+  MockDeviceManagerClient device_manager_client;
+  RegisterUsbManagerClient(usb_service, device_manager_client);
+
+  device::MockUsbMojoDevice mock_device;
+  auto fake_device_info = CreateFakeDevice();
+  TestFuture<device::mojom::UsbDeviceInfoPtr> device_added_future;
+  EXPECT_CALL(device_manager_client, OnDeviceAdded)
+      .Times(1)
+      .WillOnce([&](auto d) { device_added_future.SetValue(std::move(d)); });
+
+  ConnectDevice(fake_device_info, &mock_device);
+  EXPECT_EQ(device_added_future.Get()->guid, fake_device_info->guid());
+
+  // Transitioning from ACTIVATING to ACTIVATED should not notify a second time.
+  version->SetStatus(ServiceWorkerVersion::Status::ACTIVATED);
+  FlushUsbServicePipe(usb_service);
+}
+
+TEST_F(ServiceWorkerUsbDelegateObserverTest,
+       RunningWorkerWithoutClientQueuesPendingCallback) {
+  const GURL origin(kTestUrl);
+  auto registration = InstallServiceWorker(origin);
+  auto* version = registration->active_version();
+  ASSERT_NE(version, nullptr);
+  StartServiceWorker(version);
+  EXPECT_EQ(version->running_status(), blink::EmbeddedWorkerStatus::kRunning);
+  EXPECT_EQ(version->status(), ServiceWorkerVersion::Status::ACTIVATED);
+
+  // Create WebUsbServiceImpl while the worker is kRunning, but do not register
+  // the client yet (simulating SetClient Mojo call still in flight).
+  auto usb_service = CreateUsbService(version);
+
+  device::MockUsbMojoDevice mock_device;
+  auto fake_device_info = CreateFakeDevice();
+  ConnectDevice(fake_device_info, &mock_device);
+  EXPECT_TRUE(base::test::RunUntil([&]() {
+    return context()
+        ->usb_delegate_observer()
+        ->GetPendingCallbacksForTesting()
+        .contains(version->version_id());
+  }));
+
+  // When SetClient completes, the queued event should be delivered.
+  MockDeviceManagerClient device_manager_client;
+  TestFuture<device::mojom::UsbDeviceInfoPtr> device_added_future;
+  EXPECT_CALL(device_manager_client, OnDeviceAdded).WillOnce([&](auto d) {
+    device_added_future.SetValue(std::move(d));
+  });
+  RegisterUsbManagerClient(usb_service, device_manager_client);
+  EXPECT_EQ(device_added_future.Get()->guid, fake_device_info->guid());
+  EXPECT_FALSE(context()
+                   ->usb_delegate_observer()
+                   ->GetPendingCallbacksForTesting()
+                   .contains(version->version_id()));
+}
+
+TEST_F(ServiceWorkerUsbDelegateObserverTest,
+       PrunesStoppedUsbServiceAfterServiceWorkerStopThenStart) {
   device::MockUsbMojoDevice mock_device;
   auto fake_device_info = CreateFakeDevice();
   auto device_info = ConnectDevice(fake_device_info, &mock_device);
@@ -532,8 +693,10 @@
   ASSERT_NE(version, nullptr);
   StartServiceWorker(version);
   auto usb_service = CreateUsbService(version);
-  EXPECT_TRUE(context()->usb_delegate_observer()->GetUsbServiceForTesting(
-      registration->id()));
+  EXPECT_FALSE(context()
+                   ->usb_delegate_observer()
+                   ->GetUsbServicesForTesting(registration->id())
+                   .empty());
 
   // Create a connection so that we can get to the point when the UsbService is
   // destroyed by expecting DecrementConnectionCount being called.
@@ -557,14 +720,19 @@
   StopServiceWorker(version);
   usb_service.reset();
   run_loop.Run();
-  EXPECT_FALSE(context()->usb_delegate_observer()->GetUsbServiceForTesting(
-      registration->id()));
+  EXPECT_TRUE(context()
+                  ->usb_delegate_observer()
+                  ->GetUsbServicesForTesting(registration->id())
+                  .empty());
 
   // Then start the worker and create a new UsbService.
   StartServiceWorker(version);
   usb_service = CreateUsbService(version);
-  EXPECT_TRUE(context()->usb_delegate_observer()->GetUsbServiceForTesting(
-      registration->id()));
+  EXPECT_EQ(context()
+                ->usb_delegate_observer()
+                ->GetUsbServicesForTesting(registration->id())
+                .size(),
+            1u);
 }
 
 TEST_F(ServiceWorkerUsbDelegateObserverTest,
@@ -801,6 +969,35 @@
       context()->usb_delegate_observer()->registration_id_map().empty());
 }
 
+TEST_F(ServiceWorkerUsbDelegateObserverNoEventHandlersTest,
+       DeviceRemovedClosesOpenDeviceConnection) {
+  device::MockUsbMojoDevice mock_device;
+  auto fake_device_info = CreateFakeDevice();
+  auto device_info = ConnectDevice(fake_device_info, &mock_device);
+
+  const GURL origin(kTestUrl);
+  auto registration = InstallServiceWorker(origin);
... (truncated)