Use USB mojom interfaces in UsbChooserController.

This CL converted USB requests in UsbChooserController to mojom
interfaces through UsbChooserContext.

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

chrome/browser/usb/usb_chooser_context.cc

@@ -87,11 +87,7 @@ UsbChooserContext::UsbChooserContext(Profile* profile)
                          CONTENT_SETTINGS_TYPE_USB_CHOOSER_DATA),
       is_incognito_(profile->IsOffTheRecord()),
       client_binding_(this),
-      weak_factory_(this) {
-  // TODO(donna.wu@intel.com) set up connection with the device manager in
-  // AddObserver() after converting USB interfaces in UsbChooserController.
-  EnsureConnectionWithDeviceManager();
-}
+      weak_factory_(this) {}
 
 void UsbChooserContext::EnsureConnectionWithDeviceManager() {
   if (device_manager_)
@@ -271,6 +267,7 @@ void UsbChooserContext::GetDevice(
 }
 
 void UsbChooserContext::AddObserver(Observer* observer) {
+  EnsureConnectionWithDeviceManager();
   observer_list_.AddObserver(observer);
 }
 

chrome/browser/usb/usb_chooser_controller.cc

@@ -13,17 +13,13 @@
 #include "chrome/browser/net/referrer.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/usb/usb_blocklist.h"
-#include "chrome/browser/usb/usb_chooser_context.h"
 #include "chrome/browser/usb/usb_chooser_context_factory.h"
 #include "chrome/browser/usb/web_usb_histograms.h"
 #include "chrome/common/url_constants.h"
 #include "chrome/grit/generated_resources.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/web_contents.h"
-#include "device/base/device_client.h"
-#include "device/usb/mojo/type_converters.h"
 #include "device/usb/public/cpp/filter_utils.h"
-#include "device/usb/usb_device.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "url/gurl.h"
 
@@ -33,7 +29,6 @@
 
 using content::RenderFrameHost;
 using content::WebContents;
-using device::UsbDevice;
 
 namespace {
 
@@ -78,16 +73,8 @@ UsbChooserController::UsbChooserController(
       filters_(std::move(device_filters)),
       callback_(std::move(callback)),
       web_contents_(WebContents::FromRenderFrameHost(render_frame_host)),
-      usb_service_observer_(this),
+      observer_(this),
       weak_factory_(this) {
-  device::UsbService* usb_service =
-      device::DeviceClient::Get()->GetUsbService();
-  if (usb_service) {
-    usb_service_observer_.Add(usb_service);
-    usb_service->GetDevices(base::Bind(&UsbChooserController::GotUsbDeviceList,
-                                       weak_factory_.GetWeakPtr()));
-  }
-
   RenderFrameHost* main_frame = web_contents_->GetMainFrame();
   requesting_origin_ = render_frame_host->GetLastCommittedURL().GetOrigin();
   embedding_origin_ = main_frame->GetLastCommittedURL().GetOrigin();
@@ -95,6 +82,9 @@ UsbChooserController::UsbChooserController(
       Profile::FromBrowserContext(web_contents_->GetBrowserContext());
   chooser_context_ =
       UsbChooserContextFactory::GetForProfile(profile)->AsWeakPtr();
+  DCHECK(chooser_context_);
+  chooser_context_->GetDevices(base::BindOnce(
+      &UsbChooserController::GotUsbDeviceList, weak_factory_.GetWeakPtr()));
 }
 
 UsbChooserController::~UsbChooserController() {
@@ -172,9 +162,8 @@ void UsbChooserController::OpenHelpCenterUrl() const {
       ui::PAGE_TRANSITION_AUTO_TOPLEVEL, false /* is_renderer_initialized */));
 }
 
-void UsbChooserController::OnDeviceAdded(scoped_refptr<UsbDevice> device) {
-  auto device_info = device::mojom::UsbDeviceInfo::From(*device);
-  DCHECK(device_info);
+void UsbChooserController::OnDeviceAdded(
+    device::mojom::UsbDeviceInfoPtr device_info) {
   if (DisplayDevice(*device_info)) {
     base::string16 device_name = FormatUsbDeviceName(*device_info);
     devices_.push_back(std::make_pair(std::move(device_info), device_name));
@@ -184,9 +173,8 @@ void UsbChooserController::OnDeviceAdded(scoped_refptr<UsbDevice> device) {
   }
 }
 
-void UsbChooserController::OnDeviceRemoved(scoped_refptr<UsbDevice> device) {
-  auto device_info = device::mojom::UsbDeviceInfo::From(*device);
-  DCHECK(device_info);
+void UsbChooserController::OnDeviceRemoved(
+    device::mojom::UsbDeviceInfoPtr device_info) {
   for (auto it = devices_.begin(); it != devices_.end(); ++it) {
     if (it->first->guid == device_info->guid) {
       size_t index = it - devices_.begin();
@@ -201,12 +189,15 @@ void UsbChooserController::OnDeviceRemoved(scoped_refptr<UsbDevice> device) {
   }
 }
 
+void UsbChooserController::OnDeviceManagerConnectionError() {
+  observer_.RemoveAll();
+}
+
 // Get a list of devices that can be shown in the chooser bubble UI for
 // user to grant permsssion.
 void UsbChooserController::GotUsbDeviceList(
-    const std::vector<scoped_refptr<UsbDevice>>& devices) {
-  for (const auto& device : devices) {
-    auto device_info = device::mojom::UsbDeviceInfo::From(*device);
+    std::vector<::device::mojom::UsbDeviceInfoPtr> devices) {
+  for (auto& device_info : devices) {
     DCHECK(device_info);
     if (DisplayDevice(*device_info)) {
       base::string16 device_name = FormatUsbDeviceName(*device_info);
@@ -214,6 +205,12 @@ void UsbChooserController::GotUsbDeviceList(
       ++device_name_map_[device_name];
     }
   }
+
+  // Listen to UsbChooserContext for OnDeviceAdded/Removed events after the
+  // enumeration.
+  if (chooser_context_)
+    observer_.Add(chooser_context_.get());
+
   if (view())
     view()->OnOptionsInitialized();
 }

chrome/browser/usb/usb_chooser_controller.h

@@ -14,8 +14,8 @@
 #include "base/memory/weak_ptr.h"
 #include "base/scoped_observer.h"
 #include "chrome/browser/chooser_controller/chooser_controller.h"
+#include "chrome/browser/usb/usb_chooser_context.h"
 #include "device/usb/public/mojom/device.mojom.h"
-#include "device/usb/usb_service.h"
 #include "third_party/blink/public/mojom/usb/web_usb_service.mojom.h"
 #include "url/gurl.h"
 
@@ -24,16 +24,10 @@ class RenderFrameHost;
 class WebContents;
 }
 
-namespace device {
-class UsbDevice;
-}
-
-class UsbChooserContext;
-
 // UsbChooserController creates a chooser for WebUSB.
 // It is owned by ChooserBubbleDelegate.
 class UsbChooserController : public ChooserController,
-                             public device::UsbService::Observer {
+                             public UsbChooserContext::Observer {
  public:
   UsbChooserController(
       content::RenderFrameHost* render_frame_host,
@@ -52,13 +46,13 @@ class UsbChooserController : public ChooserController,
   void Close() override;
   void OpenHelpCenterUrl() const override;
 
-  // device::UsbService::Observer:
-  void OnDeviceAdded(scoped_refptr<device::UsbDevice> device) override;
-  void OnDeviceRemoved(scoped_refptr<device::UsbDevice> device) override;
+  // UsbChooserContext::Observer implementation:
+  void OnDeviceAdded(device::mojom::UsbDeviceInfoPtr device_info) override;
+  void OnDeviceRemoved(device::mojom::UsbDeviceInfoPtr device_info) override;
+  void OnDeviceManagerConnectionError() override;
 
  private:
-  void GotUsbDeviceList(
-      const std::vector<scoped_refptr<device::UsbDevice>>& devices);
+  void GotUsbDeviceList(std::vector<device::mojom::UsbDeviceInfoPtr> devices);
   bool DisplayDevice(const device::mojom::UsbDeviceInfo& device) const;
 
   std::vector<device::mojom::UsbDeviceFilterPtr> filters_;
@@ -68,8 +62,7 @@ class UsbChooserController : public ChooserController,
 
   content::WebContents* const web_contents_;
   base::WeakPtr<UsbChooserContext> chooser_context_;
-  ScopedObserver<device::UsbService, device::UsbService::Observer>
-      usb_service_observer_;
+  ScopedObserver<UsbChooserContext, UsbChooserContext::Observer> observer_;
 
   // Each pair is a (device, device name).
   std::vector<std::pair<device::mojom::UsbDeviceInfoPtr, base::string16>>

chrome/browser/usb/usb_chooser_controller_unittest.cc

@@ -56,6 +56,9 @@ class UsbChooserControllerTest : public ChromeRenderViewHostTestHarness {
         main_rfh(), std::move(device_filters), std::move(callback)));
     mock_usb_chooser_view_.reset(new MockUsbChooserView());
     usb_chooser_controller_->set_view(mock_usb_chooser_view_.get());
+    // Make sure the device::mojom::UsbDeviceManager::SetClient() call has
+    // been received.
+    base::RunLoop().RunUntilIdle();
   }
 
  protected:
@@ -80,6 +83,7 @@ TEST_F(UsbChooserControllerTest, AddDevice) {
       CreateMockUsbDevice("a", "001");
   EXPECT_CALL(*mock_usb_chooser_view_, OnOptionAdded(0)).Times(1);
   device_client_.usb_service()->AddDevice(device_a);
+  base::RunLoop().RunUntilIdle();
   EXPECT_EQ(1u, usb_chooser_controller_->NumOptions());
   EXPECT_EQ(base::ASCIIToUTF16("a"), usb_chooser_controller_->GetOption(0));
 
@@ -87,6 +91,7 @@ TEST_F(UsbChooserControllerTest, AddDevice) {
       CreateMockUsbDevice("b", "002");
   EXPECT_CALL(*mock_usb_chooser_view_, OnOptionAdded(1)).Times(1);
   device_client_.usb_service()->AddDevice(device_b);
+  base::RunLoop().RunUntilIdle();
   EXPECT_EQ(2u, usb_chooser_controller_->NumOptions());
   EXPECT_EQ(base::ASCIIToUTF16("b"), usb_chooser_controller_->GetOption(1));
 
@@ -94,6 +99,7 @@ TEST_F(UsbChooserControllerTest, AddDevice) {
       CreateMockUsbDevice("c", "003");
   EXPECT_CALL(*mock_usb_chooser_view_, OnOptionAdded(2)).Times(1);
   device_client_.usb_service()->AddDevice(device_c);
+  base::RunLoop().RunUntilIdle();
   EXPECT_EQ(3u, usb_chooser_controller_->NumOptions());
   EXPECT_EQ(base::ASCIIToUTF16("c"), usb_chooser_controller_->GetOption(2));
 }
@@ -108,9 +114,11 @@ TEST_F(UsbChooserControllerTest, RemoveDevice) {
   scoped_refptr<device::MockUsbDevice> device_c =
       CreateMockUsbDevice("c", "003");
   device_client_.usb_service()->AddDevice(device_c);
+  base::RunLoop().RunUntilIdle();
 
   EXPECT_CALL(*mock_usb_chooser_view_, OnOptionRemoved(1)).Times(1);
   device_client_.usb_service()->RemoveDevice(device_b);
+  base::RunLoop().RunUntilIdle();
   EXPECT_EQ(2u, usb_chooser_controller_->NumOptions());
   EXPECT_EQ(base::ASCIIToUTF16("a"), usb_chooser_controller_->GetOption(0));
   EXPECT_EQ(base::ASCIIToUTF16("c"), usb_chooser_controller_->GetOption(1));
@@ -119,17 +127,20 @@ TEST_F(UsbChooserControllerTest, RemoveDevice) {
   scoped_refptr<device::MockUsbDevice> device_non_existent =
       CreateMockUsbDevice("d", "001");
   device_client_.usb_service()->RemoveDevice(device_non_existent);
+  base::RunLoop().RunUntilIdle();
   EXPECT_EQ(2u, usb_chooser_controller_->NumOptions());
   EXPECT_EQ(base::ASCIIToUTF16("a"), usb_chooser_controller_->GetOption(0));
   EXPECT_EQ(base::ASCIIToUTF16("c"), usb_chooser_controller_->GetOption(1));
 
   EXPECT_CALL(*mock_usb_chooser_view_, OnOptionRemoved(0)).Times(1);
   device_client_.usb_service()->RemoveDevice(device_a);
+  base::RunLoop().RunUntilIdle();
   EXPECT_EQ(1u, usb_chooser_controller_->NumOptions());
   EXPECT_EQ(base::ASCIIToUTF16("c"), usb_chooser_controller_->GetOption(0));
 
   EXPECT_CALL(*mock_usb_chooser_view_, OnOptionRemoved(0)).Times(1);
   device_client_.usb_service()->RemoveDevice(device_c);
+  base::RunLoop().RunUntilIdle();
   EXPECT_EQ(0u, usb_chooser_controller_->NumOptions());
 }
 
@@ -137,13 +148,16 @@ TEST_F(UsbChooserControllerTest, AddAndRemoveDeviceWithSameName) {
   scoped_refptr<device::MockUsbDevice> device_a_1 =
       CreateMockUsbDevice("a", "001");
   device_client_.usb_service()->AddDevice(device_a_1);
+  base::RunLoop().RunUntilIdle();
   EXPECT_EQ(base::ASCIIToUTF16("a"), usb_chooser_controller_->GetOption(0));
+
   scoped_refptr<device::MockUsbDevice> device_b =
       CreateMockUsbDevice("b", "002");
   device_client_.usb_service()->AddDevice(device_b);
   scoped_refptr<device::MockUsbDevice> device_a_2 =
       CreateMockUsbDevice("a", "002");
   device_client_.usb_service()->AddDevice(device_a_2);
+  base::RunLoop().RunUntilIdle();
   EXPECT_EQ(base::ASCIIToUTF16("a (001)"),
             usb_chooser_controller_->GetOption(0));
   EXPECT_EQ(base::ASCIIToUTF16("b"), usb_chooser_controller_->GetOption(1));
@@ -151,6 +165,7 @@ TEST_F(UsbChooserControllerTest, AddAndRemoveDeviceWithSameName) {
             usb_chooser_controller_->GetOption(2));
 
   device_client_.usb_service()->RemoveDevice(device_a_1);
+  base::RunLoop().RunUntilIdle();
   EXPECT_EQ(base::ASCIIToUTF16("b"), usb_chooser_controller_->GetOption(0));
   EXPECT_EQ(base::ASCIIToUTF16("a"), usb_chooser_controller_->GetOption(1));
 }
@@ -161,6 +176,7 @@ TEST_F(UsbChooserControllerTest, UnknownDeviceName) {
   scoped_refptr<device::MockUsbDevice> device =
       new device::MockUsbDevice(vendor_id, product_id);
   device_client_.usb_service()->AddDevice(device);
+  base::RunLoop().RunUntilIdle();
   EXPECT_EQ(base::ASCIIToUTF16("Unknown device [007b:01c8]"),
             usb_chooser_controller_->GetOption(0));
 }