Destroy DeviceImpl when the underlying UsbDevice is disconnected.

A open message pipe to a DeviceImpl represents a physical resource, the
USB device connected to the host. Instead of continuing to support
handling messages sent on this pipe after the device is disconnected it
makes more sense to simply close it.

For efficiency and because DeviceImpls can outlive the DeviceManagerImpl
that created them a new observer interface is added to UsbDevice so that
the DeviceImpl can listed for the removal of the physical device all on
its own.

BUG=492204

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

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

8 files changed, 77 insertions, 7 deletions

diff --git a/device/usb/mock_usb_device.cc b/device/usb/mock_usb_device.cc
index 65c7086..49b2252 100644
--- a/device/usb/mock_usb_device.cc
+++ b/device/usb/mock_usb_device.cc
@@ -61,4 +61,8 @@ MockUsbDevice::MockUsbDevice(
 MockUsbDevice::~MockUsbDevice() {
 }
 
+void MockUsbDevice::NotifyDeviceRemoved() {
+  UsbDevice::NotifyDeviceRemoved();
+}
+
 }  // namespace device
diff --git a/device/usb/mock_usb_device.h b/device/usb/mock_usb_device.h
index 91bb0bb..569a471 100644
--- a/device/usb/mock_usb_device.h
+++ b/device/usb/mock_usb_device.h
@@ -43,6 +43,9 @@ class MockUsbDevice : public UsbDevice {
   MOCK_METHOD1(Open, void(const OpenCallback&));
   MOCK_METHOD0(GetActiveConfiguration, const device::UsbConfigDescriptor*());
 
+  // Public wrapper around UsbDevice::NotifyDeviceRemoved().
+  void NotifyDeviceRemoved();
+
  private:
   ~MockUsbDevice() override;
 };
diff --git a/device/usb/mojo/device_impl.cc b/device/usb/mojo/device_impl.cc
index 0d1c64f..ae5f4bb 100644
--- a/device/usb/mojo/device_impl.cc
+++ b/device/usb/mojo/device_impl.cc
@@ -179,13 +179,17 @@ void OnIsochronousTransferOut(
 DeviceImpl::DeviceImpl(scoped_refptr<UsbDevice> device,
                        PermissionProviderPtr permission_provider,
                        mojo::InterfaceRequest<Device> request)
-    : binding_(this, std::move(request)),
-      device_(device),
+    : device_(device),
+      observer_(this),
       permission_provider_(std::move(permission_provider)),
+      binding_(this, std::move(request)),
       weak_factory_(this) {
-  // This object owns itself and will be destroyed if either the message pipe
-  // it is bound to is closed or the PermissionProvider it depends on is
-  // unavailable.
+  DCHECK(device_);
+  // This object owns itself and will be destroyed if,
+  //  * the device is disconnected,
+  //  * the message pipe it is bound to is closed, or
+  //  * the PermissionProvider it depends on is unavailable.
+  observer_.Add(device_.get());
   binding_.set_connection_error_handler([this]() { delete this; });
   permission_provider_.set_connection_error_handler([this]() { delete this; });
 }
@@ -463,5 +467,10 @@ void DeviceImpl::IsochronousTransferOut(
       base::Bind(&OnIsochronousTransferOut, base::Passed(&callback_ptr)));
 }
 
+void DeviceImpl::OnDeviceRemoved(scoped_refptr<UsbDevice> device) {
+  DCHECK_EQ(device_, device);
+  delete this;
+}
+
 }  // namespace usb
 }  // namespace device
diff --git a/device/usb/mojo/device_impl.h b/device/usb/mojo/device_impl.h
index e19feef..487a7d4 100644
--- a/device/usb/mojo/device_impl.h
+++ b/device/usb/mojo/device_impl.h
@@ -11,8 +11,10 @@
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/weak_ptr.h"
+#include "base/scoped_observer.h"
 #include "device/usb/public/interfaces/device.mojom.h"
 #include "device/usb/public/interfaces/permission_provider.mojom.h"
+#include "device/usb/usb_device.h"
 #include "device/usb/usb_device_handle.h"
 #include "mojo/public/cpp/bindings/binding.h"
 #include "mojo/public/cpp/bindings/callback.h"
@@ -28,7 +30,7 @@ namespace usb {
 // Implementation of the public Device interface. Instances of this class are
 // constructed by DeviceManagerImpl and are strongly bound to their MessagePipe
 // lifetime.
-class DeviceImpl : public Device {
+class DeviceImpl : public Device, public device::UsbDevice::Observer {
  public:
   DeviceImpl(scoped_refptr<UsbDevice> device,
              PermissionProviderPtr permission_provider,
@@ -94,14 +96,18 @@ class DeviceImpl : public Device {
       uint32_t timeout,
       const IsochronousTransferOutCallback& callback) override;
 
-  mojo::Binding<Device> binding_;
+  // device::UsbDevice::Observer implementation:
+  void OnDeviceRemoved(scoped_refptr<device::UsbDevice>) override;
 
   scoped_refptr<UsbDevice> device_;
+  ScopedObserver<device::UsbDevice, device::UsbDevice::Observer> observer_;
+
   // The device handle. Will be null before the device is opened and after it
   // has been closed.
   scoped_refptr<UsbDeviceHandle> device_handle_;
   PermissionProviderPtr permission_provider_;
 
+  mojo::Binding<Device> binding_;
   base::WeakPtrFactory<DeviceImpl> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(DeviceImpl);
diff --git a/device/usb/mojo/device_impl_unittest.cc b/device/usb/mojo/device_impl_unittest.cc
index df23b01..25cc048 100644
--- a/device/usb/mojo/device_impl_unittest.cc
+++ b/device/usb/mojo/device_impl_unittest.cc
@@ -447,6 +447,15 @@ class USBDeviceImplTest : public testing::Test {
 
 }  // namespace
 
+TEST_F(USBDeviceImplTest, Disconnect) {
+  DevicePtr device = GetMockDeviceProxy();
+
+  base::RunLoop loop;
+  device.set_connection_error_handler(loop.QuitClosure());
+  mock_device().NotifyDeviceRemoved();
+  loop.Run();
+}
+
 TEST_F(USBDeviceImplTest, Open) {
   DevicePtr device = GetMockDeviceProxy();
 
diff --git a/device/usb/usb_device.cc b/device/usb/usb_device.cc
index d7ab38b..d34e3e0 100644
--- a/device/usb/usb_device.cc
+++ b/device/usb/usb_device.cc
@@ -9,6 +9,10 @@
 
 namespace device {
 
+UsbDevice::Observer::~Observer() {}
+
+void UsbDevice::Observer::OnDeviceRemoved(scoped_refptr<UsbDevice> device) {}
+
 UsbDevice::UsbDevice(uint16_t vendor_id,
                      uint16_t product_id,
                      const base::string16& manufacturer_string,
@@ -30,4 +34,16 @@ void UsbDevice::CheckUsbAccess(const ResultCallback& callback) {
   callback.Run(true);
 }
 
+void UsbDevice::AddObserver(Observer* observer) {
+  observer_list_.AddObserver(observer);
+}
+
+void UsbDevice::RemoveObserver(Observer* observer) {
+  observer_list_.RemoveObserver(observer);
+}
+
+void UsbDevice::NotifyDeviceRemoved() {
+  FOR_EACH_OBSERVER(Observer, observer_list_, OnDeviceRemoved(this));
+}
+
 }  // namespace device
diff --git a/device/usb/usb_device.h b/device/usb/usb_device.h
index 2745405..b1406d9 100644
--- a/device/usb/usb_device.h
+++ b/device/usb/usb_device.h
@@ -13,6 +13,7 @@
 #include "base/callback.h"
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
+#include "base/observer_list.h"
 #include "base/strings/string16.h"
 #include "device/usb/usb_descriptors.h"
 #include "url/gurl.h"
@@ -32,6 +33,18 @@ class UsbDevice : public base::RefCountedThreadSafe<UsbDevice> {
   using OpenCallback = base::Callback<void(scoped_refptr<UsbDeviceHandle>)>;
   using ResultCallback = base::Callback<void(bool success)>;
 
+  // This observer interface should be used by objects that need only be
+  // notified about the removal of a particular device as it is more efficient
+  // than registering a large number of observers with UsbService::AddObserver.
+  class Observer {
+   public:
+    virtual ~Observer();
+
+    // This method is called when the UsbService that created this object
+    // detects that the device has been disconnected from the host.
+    virtual void OnDeviceRemoved(scoped_refptr<UsbDevice> device);
+  };
+
   // A unique identifier which remains stable for the lifetime of this device
   // object (i.e., until the device is unplugged or the USB service dies.)
   const std::string& guid() const { return guid_; }
@@ -64,7 +77,12 @@ class UsbDevice : public base::RefCountedThreadSafe<UsbDevice> {
   // if the device is unconfigured.
   virtual const UsbConfigDescriptor* GetActiveConfiguration() = 0;
 
+  void AddObserver(Observer* observer);
+  void RemoveObserver(Observer* observer);
+
  protected:
+  friend class UsbService;
+
   UsbDevice(uint16_t vendor_id,
             uint16_t product_id,
             const base::string16& manufacturer_string,
@@ -72,6 +90,8 @@ class UsbDevice : public base::RefCountedThreadSafe<UsbDevice> {
             const base::string16& serial_number);
   virtual ~UsbDevice();
 
+  void NotifyDeviceRemoved();
+
   // These members must be mutable by subclasses as necessary during device
   // enumeration. To preserve the thread safety of this object they must remain
   // constant afterwards.
@@ -91,6 +111,8 @@ class UsbDevice : public base::RefCountedThreadSafe<UsbDevice> {
   const uint16_t vendor_id_;
   const uint16_t product_id_;
 
+  base::ObserverList<Observer, true> observer_list_;
+
   DISALLOW_COPY_AND_ASSIGN(UsbDevice);
 };
 
diff --git a/device/usb/usb_service.cc b/device/usb/usb_service.cc
index 6909d96..d0e9d09 100644
--- a/device/usb/usb_service.cc
+++ b/device/usb/usb_service.cc
@@ -68,6 +68,7 @@ void UsbService::NotifyDeviceRemoved(scoped_refptr<UsbDevice> device) {
   DCHECK(CalledOnValidThread());
 
   FOR_EACH_OBSERVER(Observer, observer_list_, OnDeviceRemoved(device));
+  device->NotifyDeviceRemoved();
   FOR_EACH_OBSERVER(Observer, observer_list_, OnDeviceRemovedCleanup(device));
 }