Make UsbDeviceHandle::ReleaseInterface asynchronous.

This patch adds a callback to UsbDeviceHandle::ReleaseInterface that is
called when the interface has been released. Previously this result was
returned synchronously while the actual release operation happened in
the background.

In the process I've fixed an issue where libusb_release_interface could
have been called from the UI thread if there were no pending transfers
to delay it.

BUG=None

Review URL: https://codereview.chromium.org/1664023002

Cr-Commit-Position: refs/heads/master@{#373641}

5 files changed, 63 insertions, 23 deletions

diff --git a/device/usb/mock_usb_device_handle.h b/device/usb/mock_usb_device_handle.h
index 26e6190..2cacafb 100644
--- a/device/usb/mock_usb_device_handle.h
+++ b/device/usb/mock_usb_device_handle.h
@@ -26,7 +26,8 @@ class MockUsbDeviceHandle : public UsbDeviceHandle {
                void(int configuration_value, const ResultCallback& callback));
   MOCK_METHOD2(ClaimInterface,
                void(int interface_number, const ResultCallback& callback));
-  MOCK_METHOD1(ReleaseInterface, bool(int interface_number));
+  MOCK_METHOD2(ReleaseInterface,
+               void(int interface_number, const ResultCallback& callback));
   MOCK_METHOD3(SetInterfaceAlternateSetting,
                void(int interface_number,
                     int alternate_setting,
diff --git a/device/usb/usb_device_handle.h b/device/usb/usb_device_handle.h
index b206cf1..31f0ec5 100644
--- a/device/usb/usb_device_handle.h
+++ b/device/usb/usb_device_handle.h
@@ -70,7 +70,8 @@ class UsbDeviceHandle : public base::RefCountedThreadSafe<UsbDeviceHandle> {
                                 const ResultCallback& callback) = 0;
   virtual void ClaimInterface(int interface_number,
                               const ResultCallback& callback) = 0;
-  virtual bool ReleaseInterface(int interface_number) = 0;
+  virtual void ReleaseInterface(int interface_number,
+                                const ResultCallback& callback) = 0;
   virtual void SetInterfaceAlternateSetting(int interface_number,
                                             int alternate_setting,
                                             const ResultCallback& callback) = 0;
diff --git a/device/usb/usb_device_handle_impl.cc b/device/usb/usb_device_handle_impl.cc
index fa4c085..16f343e 100644
--- a/device/usb/usb_device_handle_impl.cc
+++ b/device/usb/usb_device_handle_impl.cc
@@ -147,13 +147,19 @@ class UsbDeviceHandleImpl::InterfaceClaimer
     : public base::RefCountedThreadSafe<UsbDeviceHandleImpl::InterfaceClaimer> {
  public:
   InterfaceClaimer(scoped_refptr<UsbDeviceHandleImpl> handle,
-                   int interface_number);
+                   int interface_number,
+                   scoped_refptr<base::SingleThreadTaskRunner> task_runner);
 
+  int interface_number() const { return interface_number_; }
   int alternate_setting() const { return alternate_setting_; }
   void set_alternate_setting(const int alternate_setting) {
     alternate_setting_ = alternate_setting;
   }
 
+  void set_release_callback(const ResultCallback& callback) {
+    release_callback_ = callback;
+  }
+
  private:
   friend class base::RefCountedThreadSafe<InterfaceClaimer>;
   ~InterfaceClaimer();
@@ -161,19 +167,33 @@ class UsbDeviceHandleImpl::InterfaceClaimer
   const scoped_refptr<UsbDeviceHandleImpl> handle_;
   const int interface_number_;
   int alternate_setting_;
+  const scoped_refptr<base::SingleThreadTaskRunner> task_runner_;
+  ResultCallback release_callback_;
+  base::ThreadChecker thread_checker_;
 
   DISALLOW_COPY_AND_ASSIGN(InterfaceClaimer);
 };
 
 UsbDeviceHandleImpl::InterfaceClaimer::InterfaceClaimer(
     scoped_refptr<UsbDeviceHandleImpl> handle,
-    int interface_number)
+    int interface_number,
+    scoped_refptr<base::SingleThreadTaskRunner> task_runner)
     : handle_(handle),
       interface_number_(interface_number),
-      alternate_setting_(0) {}
+      alternate_setting_(0),
+      task_runner_(task_runner) {}
 
 UsbDeviceHandleImpl::InterfaceClaimer::~InterfaceClaimer() {
-  libusb_release_interface(handle_->handle(), interface_number_);
+  DCHECK(thread_checker_.CalledOnValidThread());
+  int rc = libusb_release_interface(handle_->handle(), interface_number_);
+  if (rc != LIBUSB_SUCCESS) {
+    USB_LOG(DEBUG) << "Failed to release interface: "
+                   << ConvertPlatformUsbErrorToString(rc);
+  }
+  if (!release_callback_.is_null()) {
+    task_runner_->PostTask(FROM_HERE,
+                           base::Bind(release_callback_, rc == LIBUSB_SUCCESS));
+  }
 }
 
 // This inner class owns the underlying libusb_transfer and may outlast
@@ -581,12 +601,13 @@ void UsbDeviceHandleImpl::ClaimInterface(int interface_number,
                  interface_number, callback));
 }
 
-bool UsbDeviceHandleImpl::ReleaseInterface(int interface_number) {
+void UsbDeviceHandleImpl::ReleaseInterface(int interface_number,
+                                           const ResultCallback& callback) {
   DCHECK(thread_checker_.CalledOnValidThread());
-  if (!device_)
-    return false;
-  if (!ContainsKey(claimed_interfaces_, interface_number))
-    return false;
+  if (!device_ || !ContainsKey(claimed_interfaces_, interface_number)) {
+    task_runner_->PostTask(FROM_HERE, base::Bind(callback, false));
+    return;
+  }
 
   // Cancel all the transfers on that interface.
   InterfaceClaimer* interface_claimer =
@@ -596,10 +617,12 @@ bool UsbDeviceHandleImpl::ReleaseInterface(int interface_number) {
       transfer->Cancel();
     }
   }
+  interface_claimer->AddRef();
+  interface_claimer->set_release_callback(callback);
+  blocking_task_runner_->ReleaseSoon(FROM_HERE, interface_claimer);
   claimed_interfaces_.erase(interface_number);
 
   RefreshEndpointMap();
-  return true;
 }
 
 void UsbDeviceHandleImpl::SetInterfaceAlternateSetting(
@@ -790,25 +813,28 @@ void UsbDeviceHandleImpl::ClaimInterfaceOnBlockingThread(
     int interface_number,
     const ResultCallback& callback) {
   int rv = libusb_claim_interface(handle_, interface_number);
-  if (rv != LIBUSB_SUCCESS) {
+  scoped_refptr<InterfaceClaimer> interface_claimer;
+  if (rv == LIBUSB_SUCCESS) {
+    interface_claimer =
+        new InterfaceClaimer(this, interface_number, task_runner_);
+  } else {
     USB_LOG(EVENT) << "Failed to claim interface: "
                    << ConvertPlatformUsbErrorToString(rv);
   }
   task_runner_->PostTask(
       FROM_HERE, base::Bind(&UsbDeviceHandleImpl::ClaimInterfaceComplete, this,
-                            interface_number, rv == LIBUSB_SUCCESS, callback));
+                            interface_claimer, callback));
 }
 
 void UsbDeviceHandleImpl::ClaimInterfaceComplete(
-    int interface_number,
-    bool success,
+    scoped_refptr<InterfaceClaimer> interface_claimer,
     const ResultCallback& callback) {
-  if (success) {
-    claimed_interfaces_[interface_number] =
-        new InterfaceClaimer(this, interface_number);
+  if (interface_claimer) {
+    claimed_interfaces_[interface_claimer->interface_number()] =
+        interface_claimer;
     RefreshEndpointMap();
   }
-  callback.Run(success);
+  callback.Run(interface_claimer);
 }
 
 void UsbDeviceHandleImpl::SetInterfaceAlternateSettingOnBlockingThread(
diff --git a/device/usb/usb_device_handle_impl.h b/device/usb/usb_device_handle_impl.h
index ce2daff..0cb2311 100644
--- a/device/usb/usb_device_handle_impl.h
+++ b/device/usb/usb_device_handle_impl.h
@@ -53,7 +53,8 @@ class UsbDeviceHandleImpl : public UsbDeviceHandle {
                         const ResultCallback& callback) override;
   void ClaimInterface(int interface_number,
                       const ResultCallback& callback) override;
-  bool ReleaseInterface(int interface_number) override;
+  void ReleaseInterface(int interface_number,
+                        const ResultCallback& callback) override;
   void SetInterfaceAlternateSetting(int interface_number,
                                     int alternate_setting,
                                     const ResultCallback& callback) override;
@@ -116,8 +117,7 @@ class UsbDeviceHandleImpl : public UsbDeviceHandle {
   void SetConfigurationComplete(bool success, const ResultCallback& callback);
   void ClaimInterfaceOnBlockingThread(int interface_number,
                                       const ResultCallback& callback);
-  void ClaimInterfaceComplete(int interface_number,
-                              bool success,
+  void ClaimInterfaceComplete(scoped_refptr<InterfaceClaimer> interface_claimer,
                               const ResultCallback& callback);
   void SetInterfaceAlternateSettingOnBlockingThread(
       int interface_number,
diff --git a/device/usb/usb_device_handle_unittest.cc b/device/usb/usb_device_handle_unittest.cc
index 1c116fd..5fac40a 100644
--- a/device/usb/usb_device_handle_unittest.cc
+++ b/device/usb/usb_device_handle_unittest.cc
@@ -164,6 +164,10 @@ TEST_F(UsbDeviceHandleTest, InterruptTransfer) {
         << "Mismatch at index " << i << ".";
   }
 
+  TestResultCallback release_interface;
+  handle->ReleaseInterface(0, release_interface.callback());
+  ASSERT_TRUE(release_interface.WaitForResult());
+
   handle->Close();
 }
 
@@ -219,6 +223,10 @@ TEST_F(UsbDeviceHandleTest, BulkTransfer) {
         << "Mismatch at index " << i << ".";
   }
 
+  TestResultCallback release_interface;
+  handle->ReleaseInterface(1, release_interface.callback());
+  ASSERT_TRUE(release_interface.WaitForResult());
+
   handle->Close();
 }
 
@@ -245,6 +253,10 @@ TEST_F(UsbDeviceHandleTest, SetInterfaceAlternateSetting) {
   handle->SetInterfaceAlternateSetting(2, 1, set_interface.callback());
   ASSERT_TRUE(set_interface.WaitForResult());
 
+  TestResultCallback release_interface;
+  handle->ReleaseInterface(2, release_interface.callback());
+  ASSERT_TRUE(release_interface.WaitForResult());
+
   handle->Close();
 }