From 9bb3f8bce630aceaf883ad66729eb2c7444893e4 Mon Sep 17 00:00:00 2001
From: Matt Reynolds <[email protected]>
Date: Wed, 29 Apr 2026 18:52:35 -0700
Subject: [PATCH] usb: Hop UsbServiceAndroid JNI callbacks to the service sequence

DeviceAttached, DeviceDetached, and DevicePermissionRequestComplete are
invoked by ChromeUsbService.java from BroadcastReceiver callbacks on the
Android main looper, but mutate sequence-bound state (devices_by_id_,
the UsbService observer list via NotifyDeviceAdded/Removed) without
hopping to the service sequence. This is the same data race pattern
fixed in SerialDeviceEnumeratorAndroid by commit eaffd0c6d1
(crbug.com/497000161).

Add task_runner_, capture SequencedTaskRunner::GetCurrentDefault() in
the constructor, split each JNI entry into a trampoline that
PostTasks to the service sequence, and add DCHECK_CALLED_ON_VALID_SEQUENCE
on the UsbService sequence_checker_ for sequence-bound methods.

This patch is based on an initial proposal by Muhammad Aadil.

Bug: 502959826
Change-Id: Ief6297751fe718dc9bf7f16c582c7dbfe895205c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7788050
Commit-Queue: Matt Reynolds <[email protected]>
Reviewed-by: Alvin Ji <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1622887}
---

diff --git a/services/device/usb/android/java/src/org/chromium/device/usb/ChromeUsbService.java b/services/device/usb/android/java/src/org/chromium/device/usb/ChromeUsbService.java
index 218e177..2b69d641 100644
--- a/services/device/usb/android/java/src/org/chromium/device/usb/ChromeUsbService.java
+++ b/services/device/usb/android/java/src/org/chromium/device/usb/ChromeUsbService.java
@@ -17,6 +17,7 @@
 
 import org.jni_zero.CalledByNative;
 import org.jni_zero.JNINamespace;
+import org.jni_zero.NativeClassQualifiedName;
 import org.jni_zero.NativeMethods;
 
 import org.chromium.base.ContextUtils;
@@ -38,13 +39,13 @@
     private static final String TAG = "Usb";
     private static final String ACTION_USB_PERMISSION = "org.chromium.device.ACTION_USB_PERMISSION";
 
-    long mUsbServiceAndroid;
+    long mUsbServiceJniDelegate;
     UsbManager mUsbManager;
     @Nullable BroadcastReceiver mUsbPermissionReceiver;
     @Nullable BroadcastReceiver mUsbDeviceChangeReceiver;
 
-    private ChromeUsbService(long usbServiceAndroid) {
-        mUsbServiceAndroid = usbServiceAndroid;
+    private ChromeUsbService(long usbServiceJniDelegate) {
+        mUsbServiceJniDelegate = usbServiceJniDelegate;
         mUsbManager =
                 (UsbManager)
                         ContextUtils.getApplicationContext().getSystemService(Context.USB_SERVICE);
@@ -53,8 +54,8 @@
     }
 
     @CalledByNative
-    private static ChromeUsbService create(long usbServiceAndroid) {
-        return new ChromeUsbService(usbServiceAndroid);
+    private static ChromeUsbService create(long usbServiceJniDelegate) {
+        return new ChromeUsbService(usbServiceJniDelegate);
     }
 
     @CalledByNative
@@ -81,7 +82,7 @@
         if (mUsbManager.hasPermission(device)) {
             ChromeUsbServiceJni.get()
                     .devicePermissionRequestComplete(
-                            mUsbServiceAndroid, device.getDeviceId(), true);
+                            mUsbServiceJniDelegate, device.getDeviceId(), true);
         } else {
             Context context = ContextUtils.getApplicationContext();
             Intent intent = new Intent(ACTION_USB_PERMISSION);
@@ -110,7 +111,7 @@
                                 assumeNonNull(intent.getParcelableExtra(UsbManager.EXTRA_DEVICE));
                         ChromeUsbServiceJni.get()
                                 .devicePermissionRequestComplete(
-                                        mUsbServiceAndroid,
+                                        mUsbServiceJniDelegate,
                                         device.getDeviceId(),
                                         intent.getBooleanExtra(
                                                 UsbManager.EXTRA_PERMISSION_GRANTED, false));
@@ -123,11 +124,12 @@
                         UsbDevice device =
                                 assumeNonNull(intent.getParcelableExtra(UsbManager.EXTRA_DEVICE));
                         if (UsbManager.ACTION_USB_DEVICE_ATTACHED.equals(intent.getAction())) {
-                            ChromeUsbServiceJni.get().deviceAttached(mUsbServiceAndroid, device);
+                            ChromeUsbServiceJni.get()
+                                    .deviceAttached(mUsbServiceJniDelegate, device);
                         } else if (UsbManager.ACTION_USB_DEVICE_DETACHED.equals(
                                 intent.getAction())) {
                             ChromeUsbServiceJni.get()
-                                    .deviceDetached(mUsbServiceAndroid, device.getDeviceId());
+                                    .deviceDetached(mUsbServiceJniDelegate, device.getDeviceId());
                         }
                     }
                 };
@@ -153,11 +155,13 @@
 
     @NativeMethods
     interface Natives {
-        void deviceAttached(long nativeUsbServiceAndroid, @Nullable UsbDevice device);
+        @NativeClassQualifiedName("UsbServiceAndroid::JniDelegate")
+        void deviceAttached(long nativePointer, @Nullable UsbDevice device);
 
-        void deviceDetached(long nativeUsbServiceAndroid, int deviceId);
+        @NativeClassQualifiedName("UsbServiceAndroid::JniDelegate")
+        void deviceDetached(long nativePointer, int deviceId);
 
-        void devicePermissionRequestComplete(
-                long nativeUsbServiceAndroid, int deviceId, boolean granted);
+        @NativeClassQualifiedName("UsbServiceAndroid::JniDelegate")
+        void devicePermissionRequestComplete(long nativePointer, int deviceId, boolean granted);
     }
 }
diff --git a/services/device/usb/usb_service_android.cc b/services/device/usb/usb_service_android.cc
index 69f2645..96ebd936 100644
--- a/services/device/usb/usb_service_android.cc
+++ b/services/device/usb/usb_service_android.cc
@@ -22,10 +22,71 @@
 
 namespace device {
 
-UsbServiceAndroid::UsbServiceAndroid() : UsbService() {
+// Bounces JNI callbacks to `task_runner_` (the service sequence). Holds a weak
+// reference to the service since it may be destroyed. The weak pointer must be
+// checked on the service sequence.
+class UsbServiceAndroid::JniDelegate
+    : public base::RefCountedThreadSafe<JniDelegate> {
+ public:
+  explicit JniDelegate(base::WeakPtr<UsbServiceAndroid> service)
+      : service_(std::move(service)),
+        task_runner_(base::SequencedTaskRunner::GetCurrentDefault()) {}
+
+  void DeviceAttached(JNIEnv* env,
+                      const base::android::JavaRef<jobject>& usb_device) {
+    task_runner_->PostTask(
+        FROM_HERE, base::BindOnce(&JniDelegate::DeviceAttachedInternal, this,
+                                  base::android::ScopedJavaGlobalRef<jobject>(
+                                      env, usb_device)));
+  }
+
+  void DeviceDetached(int32_t device_id) {
+    task_runner_->PostTask(
+        FROM_HERE,
+        base::BindOnce(&JniDelegate::DeviceDetachedInternal, this, device_id));
+  }
+
+  void DevicePermissionRequestComplete(int32_t device_id, bool granted) {
+    task_runner_->PostTask(
+        FROM_HERE,
+        base::BindOnce(&JniDelegate::DevicePermissionRequestCompleteInternal,
+                       this, device_id, granted));
+  }
+
+ private:
+  friend class base::RefCountedThreadSafe<JniDelegate>;
+  ~JniDelegate() = default;
+
+  void DeviceAttachedInternal(
+      base::android::ScopedJavaGlobalRef<jobject> usb_device) {
+    if (service_) {
+      service_->DeviceAttachedInternal(usb_device);
+    }
+  }
+
+  void DeviceDetachedInternal(int32_t device_id) {
+    if (service_) {
+      service_->DeviceDetachedInternal(device_id);
+    }
+  }
+
+  void DevicePermissionRequestCompleteInternal(int32_t device_id,
+                                               bool granted) {
+    if (service_) {
+      service_->DevicePermissionRequestCompleteInternal(device_id, granted);
+    }
+  }
+
+  base::WeakPtr<UsbServiceAndroid> service_;
+  scoped_refptr<base::SequencedTaskRunner> task_runner_;
+};
+
+UsbServiceAndroid::UsbServiceAndroid()
+    : task_runner_(base::SequencedTaskRunner::GetCurrentDefault()) {
+  jni_delegate_ = base::MakeRefCounted<JniDelegate>(weak_factory_.GetWeakPtr());
   JNIEnv* env = AttachCurrentThread();
-  j_object_.Reset(
-      Java_ChromeUsbService_create(env, reinterpret_cast<int64_t>(this)));
+  j_object_.Reset(Java_ChromeUsbService_create(
+      env, reinterpret_cast<int64_t>(jni_delegate_.get())));
   ScopedJavaLocalRef<jobjectArray> devices =
       Java_ChromeUsbService_getDevices(env, j_object_);
   for (auto usb_device : devices.CreateView(env)) {
@@ -36,20 +97,24 @@
 }