Support multiple bindings in device::usb::DeviceManagerImpl.

Support multiple bindings in DeviceManagerImpl so that the Device
Service can own a unique DeviceManagerImpl instance easily and
the unique UsbService instance will be owned by DeviceManagerImpl,
then we can git rid of DeviceClient after all users being converted
to mojom interface.

Bug: 699790
Change-Id: I8a62739796f99a98bf87a1ed71562c09cba9e050
Reviewed-on: https://chromium-review.googlesource.com/1221475
Commit-Queue: Donna Wu <donna.wu@intel.com>
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#592706}

chrome/browser/usb/usb_chooser_context.cc

@@ -99,8 +99,8 @@ void UsbChooserContext::EnsureConnectionWithDeviceManager() {
 
   // TODO(donna.wu@intel.com): Request UsbDeviceManagerPtr from DeviceService
   // after moving //device/usb to //services/device.
-  device_manager_instance_ = device::usb::DeviceManagerImpl::Create(
-      mojo::MakeRequest(&device_manager_));
+  device_manager_instance_ = std::make_unique<device::usb::DeviceManagerImpl>();
+  device_manager_instance_->AddBinding(mojo::MakeRequest(&device_manager_));
   device_manager_.set_connection_error_handler(
       base::BindOnce(&UsbChooserContext::OnDeviceManagerConnectionError,
                      base::Unretained(this)));

device/usb/mojo/device_manager_impl.cc

@@ -24,32 +24,19 @@
 namespace device {
 namespace usb {
 
-// static
-std::unique_ptr<DeviceManagerImpl> DeviceManagerImpl::Create(
-    mojom::UsbDeviceManagerRequest request) {
-  DCHECK(DeviceClient::Get());
-  UsbService* service = DeviceClient::Get()->GetUsbService();
-  if (!service)
-    return nullptr;
-
-  auto* device_manager = new DeviceManagerImpl(service);
-  device_manager->binding_.Bind(std::move(request));
-  return base::WrapUnique(device_manager);
-}
-
-DeviceManagerImpl::DeviceManagerImpl(UsbService* usb_service)
-    : usb_service_(usb_service),
-      observer_(this),
-      binding_(this),
-      weak_factory_(this) {
-  // This object owns itself and will be destroyed if the message pipe it is
-  // bound to is closed, the message loop is destructed, or the UsbService is
-  // shut down.
+DeviceManagerImpl::DeviceManagerImpl() : observer_(this), weak_factory_(this) {
+  usb_service_ = DeviceClient::Get()->GetUsbService();
+  if (usb_service_)
     observer_.Add(usb_service_);
 }
 
 DeviceManagerImpl::~DeviceManagerImpl() = default;
 
+void DeviceManagerImpl::AddBinding(mojom::UsbDeviceManagerRequest request) {
+  if (usb_service_)
+    bindings_.AddBinding(this, std::move(request));
+}
+
 void DeviceManagerImpl::GetDevices(mojom::UsbEnumerationOptionsPtr options,
                                    GetDevicesCallback callback) {
   usb_service_->GetDevices(
@@ -71,7 +58,9 @@ void DeviceManagerImpl::GetDevice(const std::string& guid,
 void DeviceManagerImpl::SetClient(
     mojom::UsbDeviceManagerClientAssociatedPtrInfo client) {
   DCHECK(client);
-  client_.Bind(std::move(client));
+  mojom::UsbDeviceManagerClientAssociatedPtr client_ptr;
+  client_ptr.Bind(std::move(client));
+  clients_.AddPtr(std::move(client_ptr));
 }
 
 void DeviceManagerImpl::OnGetDevices(
@@ -93,19 +82,28 @@ void DeviceManagerImpl::OnGetDevices(
 }
 
 void DeviceManagerImpl::OnDeviceAdded(scoped_refptr<UsbDevice> device) {
-  if (client_)
-    client_->OnDeviceAdded(mojom::UsbDeviceInfo::From(*device));
+  auto device_info = device::mojom::UsbDeviceInfo::From(*device);
+  DCHECK(device_info);
+  clients_.ForAllPtrs([&device_info](mojom::UsbDeviceManagerClient* client) {
+    client->OnDeviceAdded(device_info->Clone());
+  });
 }
 
 void DeviceManagerImpl::OnDeviceRemoved(scoped_refptr<UsbDevice> device) {
-  if (client_)
-    client_->OnDeviceRemoved(mojom::UsbDeviceInfo::From(*device));
+  auto device_info = device::mojom::UsbDeviceInfo::From(*device);
+  DCHECK(device_info);
+  clients_.ForAllPtrs([&device_info](mojom::UsbDeviceManagerClient* client) {
+    client->OnDeviceRemoved(device_info->Clone());
+  });
 }
 
 void DeviceManagerImpl::WillDestroyUsbService() {
   observer_.RemoveAll();
-  binding_.Close();
-  client_.reset();
+  usb_service_ = nullptr;
+
+  // Close all the connections.
+  bindings_.CloseAllBindings();
+  clients_.CloseAll();
 }
 
 }  // namespace usb

device/usb/mojo/device_manager_impl.h

@@ -18,7 +18,8 @@
 #include "base/scoped_observer.h"
 #include "device/usb/public/mojom/device_manager.mojom.h"
 #include "device/usb/usb_service.h"
-#include "mojo/public/cpp/bindings/binding.h"
+#include "mojo/public/cpp/bindings/binding_set.h"
+#include "mojo/public/cpp/bindings/interface_ptr_set.h"
 
 namespace device {
 
@@ -31,14 +32,12 @@ namespace usb {
 class DeviceManagerImpl : public mojom::UsbDeviceManager,
                           public UsbService::Observer {
  public:
-  static std::unique_ptr<DeviceManagerImpl> Create(
-      mojom::UsbDeviceManagerRequest request);
-
+  DeviceManagerImpl();
   ~DeviceManagerImpl() override;
 
- private:
-  explicit DeviceManagerImpl(UsbService* usb_service);
+  void AddBinding(mojom::UsbDeviceManagerRequest request);
 
+ private:
   // DeviceManager implementation:
   void GetDevices(mojom::UsbEnumerationOptionsPtr options,
                   GetDevicesCallback callback) override;
@@ -63,8 +62,8 @@ class DeviceManagerImpl : public mojom::UsbDeviceManager,
   UsbService* usb_service_;
   ScopedObserver<UsbService, UsbService::Observer> observer_;
 
-  mojo::Binding<mojom::UsbDeviceManager> binding_;
-  mojom::UsbDeviceManagerClientAssociatedPtr client_;
+  mojo::BindingSet<mojom::UsbDeviceManager> bindings_;
+  mojo::AssociatedInterfacePtrSet<mojom::UsbDeviceManagerClient> clients_;
 
   base::WeakPtrFactory<DeviceManagerImpl> weak_factory_;
 

device/usb/mojo/device_manager_impl_unittest.cc

@@ -54,15 +54,12 @@ class USBDeviceManagerImplTest : public testing::Test {
   ~USBDeviceManagerImplTest() override = default;
 
  protected:
-  void TearDown() override {
-    // Clean up the device manager for next test case.
-    device_manager_instance_.reset();
-  }
-
   UsbDeviceManagerPtr ConnectToDeviceManager() {
     UsbDeviceManagerPtr device_manager;
-    device_manager_instance_ =
-        DeviceManagerImpl::Create(mojo::MakeRequest(&device_manager));
+    if (!device_manager_instance_)
+      device_manager_instance_ = std::make_unique<DeviceManagerImpl>();
+
+    device_manager_instance_->AddBinding(mojo::MakeRequest(&device_manager));
     return device_manager;
   }