Store devices that only need to be enumerated once

device/usb/usb_service_impl.cc

Index: device/usb/usb_service_impl.cc
diff --git a/device/usb/usb_service_impl.cc b/device/usb/usb_service_impl.cc
index 2624991c9401dd051eb0e20ca4eee5fb0890db67..0cec2a817f6b4406a83fbdb1429d6b22275057aa 100644
--- a/device/usb/usb_service_impl.cc
+++ b/device/usb/usb_service_impl.cc
@@ -417,10 +417,18 @@ void UsbServiceImpl::OnDeviceList(libusb_device** platform_devices,
                            base::Bind(&UsbServiceImpl::RefreshDevicesComplete,
                                       weak_factory_.GetWeakPtr()));
   std::list<PlatformUsbDevice> new_devices;
+  std::set<PlatformUsbDevice> existing_ignored_devices;
 
   // Look for new and existing devices.
   for (size_t i = 0; i < device_count; ++i) {
     PlatformUsbDevice platform_device = platform_devices[i];
+    // Ignore some devices.
+    if (ContainsValue(ignored_devices_, platform_device)) {
+      existing_ignored_devices.insert(platform_device);
+      refresh_complete.Run();
+      continue;
+    }
+
     auto it = platform_devices_.find(platform_device);
 
     if (it == platform_devices_.end()) {
@@ -445,6 +453,14 @@ void UsbServiceImpl::OnDeviceList(libusb_device** platform_devices,
     }
   }
 
+  // Remove devices not seen in this enumeration from |ignored_devices_|.
+  for (auto it = ignored_devices_.begin(); it != ignored_devices_.end();
+       /* incremented internally */) {
+    auto current = it++;
+    if (!ContainsValue(existing_ignored_devices, *current))
+      ignored_devices_.erase(current);

[Reilly Grant (use Gerrit), 2016/04/22 22:21:24]

We should libusb_unref_device here and in the UsbServiceImpl destructor. Can you
do a sanity test (by adding an OutputDebugString in libusb) that we are indeed
not leaking libusb_device objects when they are unplugged?

[juncai, 2016/04/26 19:26:44]

Done.

Also did some sanity check and found a potential device leak at function
UsbServiceImpl::OnDeviceList() and fixed it. I think after the fix, when
unplugging either hub or non-hub device, Chrome doesn't leak any device.

+  }
+
   for (PlatformUsbDevice platform_device : new_devices) {
     EnumerateDevice(platform_device, refresh_complete);
   }
@@ -488,6 +504,7 @@ void UsbServiceImpl::EnumerateDevice(PlatformUsbDevice platform_device,
   if (rv == LIBUSB_SUCCESS) {
     if (descriptor.bDeviceClass == LIBUSB_CLASS_HUB) {
       // Don't try to enumerate hubs. We never want to connect to a hub.
+      ignored_devices_.insert(platform_device);
       refresh_complete.Run();
       return;
     }
@@ -497,13 +514,16 @@ void UsbServiceImpl::EnumerateDevice(PlatformUsbDevice platform_device,
     base::Closure add_device =
         base::Bind(&UsbServiceImpl::AddDevice, weak_factory_.GetWeakPtr(),
                    refresh_complete, device);
+    base::Closure enumeration_failed = base::Bind(
+        &UsbServiceImpl::EnumerationFailed, weak_factory_.GetWeakPtr(),
+        platform_device, refresh_complete);
     bool read_bos_descriptors = descriptor.bcdUSB >= kUsbVersion2_1;
 
 #if defined(USE_UDEV)
     blocking_task_runner_->PostTask(
         FROM_HERE,
         base::Bind(&EnumerateUdevDevice, device, read_bos_descriptors,
-                   task_runner_, add_device, refresh_complete));
+                   task_runner_, add_device, enumeration_failed));
 #else
     if (descriptor.iManufacturer == 0 && descriptor.iProduct == 0 &&
         descriptor.iSerialNumber == 0 && !read_bos_descriptors) {
@@ -513,7 +533,7 @@ void UsbServiceImpl::EnumerateDevice(PlatformUsbDevice platform_device,
       device->Open(base::Bind(&OnDeviceOpenedReadDescriptors,
                               descriptor.iManufacturer, descriptor.iProduct,
                               descriptor.iSerialNumber, read_bos_descriptors,
-                              add_device, refresh_complete));
+                              add_device, enumeration_failed));
     }
 #endif
   } else {
@@ -616,4 +636,11 @@ void UsbServiceImpl::OnPlatformDeviceRemoved(
   libusb_unref_device(platform_device);
 }
 
+void UsbServiceImpl::EnumerationFailed(PlatformUsbDevice platform_device,
+                                       const base::Closure& refresh_complete) {
+  libusb_ref_device(platform_device);
+  ignored_devices_.insert(platform_device);
+  refresh_complete.Run();
+}
+
 }  // namespace device