Add WebUsbDeviceManager for permission moving out of device/usb.

This is a preparing CL for USB servicification which introduced
WebUsbDeviceManager and its unit test. As designed, all permission
management for USB devices will be consolidated within //chrome.
Permission checking code in //device/usb will be moved out to this
new class WebUsbDeviceManager.


BUG=699790

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

chrome/browser/BUILD.gn

@@ -1583,6 +1583,8 @@ jumbo_split_static_library("browser") {
     "usb/usb_tab_helper.h",
     "usb/web_usb_chooser_service.cc",
     "usb/web_usb_chooser_service.h",
+    "usb/web_usb_device_manager.cc",
+    "usb/web_usb_device_manager.h",
     "usb/web_usb_histograms.cc",
     "usb/web_usb_histograms.h",
     "usb/web_usb_permission_provider.cc",

chrome/browser/usb/usb_tab_helper.cc

@@ -9,11 +9,11 @@
 
 #include "chrome/browser/ui/browser_finder.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "chrome/browser/usb/web_usb_device_manager.h"
 #include "chrome/browser/usb/web_usb_permission_provider.h"
 #include "content/public/browser/navigation_handle.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/common/content_features.h"
-#include "device/usb/mojo/device_manager_impl.h"
 #include "mojo/public/cpp/bindings/message.h"
 #include "third_party/blink/public/mojom/feature_policy/feature_policy.mojom.h"
 
@@ -65,8 +65,8 @@ void UsbTabHelper::CreateDeviceManager(
     mojo::ReportBadMessage(kFeaturePolicyViolation);
     return;
   }
-  device::usb::DeviceManagerImpl::Create(
-      GetPermissionProvider(render_frame_host), std::move(request));
+  WebUsbDeviceManager::Create(GetPermissionProvider(render_frame_host),
+                              std::move(request));
 }
 
 void UsbTabHelper::CreateChooserService(

chrome/browser/usb/web_usb_device_manager.cc

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/usb/web_usb_device_manager.h"
+
+#include <utility>
+
+#include "base/bind.h"
+#include "device/base/device_client.h"
+#include "device/usb/mojo/device_manager_impl.h"
+#include "device/usb/mojo/type_converters.h"
+#include "device/usb/usb_device.h"
+
+// static
+void WebUsbDeviceManager::Create(
+    base::WeakPtr<device::usb::PermissionProvider> permission_provider,
+    device::mojom::UsbDeviceManagerRequest request) {
+  // Bind the Blink request with WebUsbDeviceManager.
+  auto* web_usb_device_manager =
+      new WebUsbDeviceManager(std::move(permission_provider));
+  web_usb_device_manager->binding_ = mojo::MakeStrongBinding(
+      base::WrapUnique(web_usb_device_manager), std::move(request));
+}
+
+WebUsbDeviceManager::WebUsbDeviceManager(
+    base::WeakPtr<device::usb::PermissionProvider> permission_provider)
+    : permission_provider_(std::move(permission_provider)),
+      observer_(this),
+      weak_factory_(this) {
+  // Bind |device_manager_| to UsbDeviceManager and set error handler.
+  // TODO(donna.wu@intel.com): Request UsbDeviceManagerPtr from the Device
+  // Service after moving //device/usb to //services/device.
+  device::usb::DeviceManagerImpl::Create(permission_provider_,
+                                         mojo::MakeRequest(&device_manager_));
+  device_manager_.set_connection_error_handler(base::BindOnce(
+      &WebUsbDeviceManager::OnConnectionError, base::Unretained(this)));
+  // Listen for add/remove device events from UsbService.
+  // TODO(donna.wu@intel.com): Listen to |device_manager_| in the future.
+  // We can't set WebUsbDeviceManager as a UsbDeviceManagerClient because
+  // the OnDeviceRemoved event will be delivered here after it is delivered
+  // to UsbChooserContext, meaning that all ephemeral permission checks in
+  // OnDeviceRemoved() will fail.
+  auto* usb_service = device::DeviceClient::Get()->GetUsbService();
+  if (usb_service)
+    observer_.Add(usb_service);
+}
+
+WebUsbDeviceManager::~WebUsbDeviceManager() = default;
+
+void WebUsbDeviceManager::GetDevices(
+    device::mojom::UsbEnumerationOptionsPtr options,
+    GetDevicesCallback callback) {
+  device_manager_->GetDevices(std::move(options), std::move(callback));
+}
+
+void WebUsbDeviceManager::GetDevice(
+    const std::string& guid,
+    device::mojom::UsbDeviceRequest device_request) {
+  device_manager_->GetDevice(guid, std::move(device_request));
+}
+
+void WebUsbDeviceManager::SetClient(
+    device::mojom::UsbDeviceManagerClientPtr client) {
+  client_ = std::move(client);
+}
+
+void WebUsbDeviceManager::OnDeviceAdded(
+    scoped_refptr<device::UsbDevice> device) {
+  if (client_ && permission_provider_ &&
+      permission_provider_->HasDevicePermission(device)) {
+    client_->OnDeviceAdded(device::mojom::UsbDeviceInfo::From(*device));
+  }
+}
+
+void WebUsbDeviceManager::OnDeviceRemoved(
+    scoped_refptr<device::UsbDevice> device) {
+  if (client_ && permission_provider_ &&
+      permission_provider_->HasDevicePermission(device)) {
+    client_->OnDeviceRemoved(device::mojom::UsbDeviceInfo::From(*device));
+  }
+}
+
+void WebUsbDeviceManager::OnConnectionError() {
+  device_manager_.reset();
+
+  // Close the connection with blink.
+  client_.reset();
+  binding_->Close();
+}
+
+void WebUsbDeviceManager::WillDestroyUsbService() {
+  OnConnectionError();
+}

chrome/browser/usb/web_usb_device_manager.h

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_USB_WEB_USB_DEVICE_MANAGER_H_
+#define CHROME_BROWSER_USB_WEB_USB_DEVICE_MANAGER_H_
+
+#include <string>
+
+#include "base/macros.h"
+#include "base/memory/weak_ptr.h"
+#include "base/scoped_observer.h"
+#include "device/usb/mojo/permission_provider.h"
+#include "device/usb/public/mojom/device.mojom.h"
+#include "device/usb/public/mojom/device_manager.mojom.h"
+#include "device/usb/usb_service.h"
+#include "mojo/public/cpp/bindings/strong_binding.h"
+
+namespace device {
+class UsbDevice;
+}
+
+// Implements a restricted device::mojom::UsbDeviceManager interface by wrapping
+// another UsbDeviceManager instance and checking requests with the provided
+// device::usb::PermissionProvider.
+class WebUsbDeviceManager : public device::mojom::UsbDeviceManager,
+                            public device::UsbService::Observer {
+ public:
+  static void Create(
+      base::WeakPtr<device::usb::PermissionProvider> permission_provider,
+      device::mojom::UsbDeviceManagerRequest request);
+
+  ~WebUsbDeviceManager() override;
+
+ private:
+  WebUsbDeviceManager(
+      base::WeakPtr<device::usb::PermissionProvider> permission_provider);
+
+  // DeviceManager implementation:
+  void GetDevices(device::mojom::UsbEnumerationOptionsPtr options,
+                  GetDevicesCallback callback) override;
+  void GetDevice(const std::string& guid,
+                 device::mojom::UsbDeviceRequest device_request) override;
+  void SetClient(device::mojom::UsbDeviceManagerClientPtr client) override;
+
+  // device::UsbService::Observer implementation:
+  void OnDeviceAdded(scoped_refptr<device::UsbDevice> device) override;
+  void OnDeviceRemoved(scoped_refptr<device::UsbDevice> device) override;
+  void WillDestroyUsbService() override;
+
+  void OnConnectionError();
+
+  base::WeakPtr<device::usb::PermissionProvider> permission_provider_;
+
+  // Used to bind with Blink.
+  mojo::StrongBindingPtr<device::mojom::UsbDeviceManager> binding_;
+  device::mojom::UsbDeviceManagerClientPtr client_;
+
+  device::mojom::UsbDeviceManagerPtr device_manager_;
+  ScopedObserver<device::UsbService, device::UsbService::Observer> observer_;
+
+  base::WeakPtrFactory<WebUsbDeviceManager> weak_factory_;
+
+  DISALLOW_COPY_AND_ASSIGN(WebUsbDeviceManager);
+};
+
+#endif  // CHROME_BROWSER_USB_WEB_USB_DEVICE_MANAGER_H_

chrome/browser/usb/web_usb_device_manager_unittest.cc

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/usb/web_usb_device_manager.h"
+
+#include <memory>
+#include <set>
+#include <string>
+#include <vector>
+
+#include "base/barrier_closure.h"
+#include "base/bind.h"
+#include "base/run_loop.h"
+#include "base/test/scoped_task_environment.h"
+#include "device/base/mock_device_client.h"
+#include "device/usb/mock_usb_device.h"
+#include "device/usb/mock_usb_service.h"
+#include "device/usb/mojo/device_impl.h"
+#include "device/usb/mojo/mock_permission_provider.h"
+#include "mojo/public/cpp/bindings/binding.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+using ::testing::_;
+using ::testing::AtMost;
+
+using device::mojom::UsbDeviceInfo;
+using device::mojom::UsbDeviceInfoPtr;
+using device::mojom::UsbDeviceManagerClient;
+using device::mojom::UsbDeviceManagerClientPtr;
+using device::mojom::UsbDeviceManagerPtr;
+using device::MockUsbDevice;
+using device::usb::MockPermissionProvider;
+
+namespace {
+
+ACTION_P2(ExpectGuidAndThen, expected_guid, callback) {
+  ASSERT_TRUE(arg0);
+  EXPECT_EQ(expected_guid, arg0->guid);
+  if (!callback.is_null())
+    callback.Run();
+};
+
+class WebUsbDeviceManagerTest : public testing::Test {
+ public:
+  WebUsbDeviceManagerTest() = default;
+  ~WebUsbDeviceManagerTest() override = default;
+
+ protected:
+  UsbDeviceManagerPtr ConnectToDeviceManager() {
+    UsbDeviceManagerPtr manager;
+    WebUsbDeviceManager::Create(permission_provider_.GetWeakPtr(),
+                                mojo::MakeRequest(&manager));
+    return manager;
+  }
+
+  device::MockDeviceClient device_client_;
+
+ private:
+  MockPermissionProvider permission_provider_;
+  base::test::ScopedTaskEnvironment task_environment_;
+};
+
+class MockDeviceManagerClient : public UsbDeviceManagerClient {
+ public:
+  MockDeviceManagerClient() : binding_(this) {}
+  ~MockDeviceManagerClient() override = default;
+
+  UsbDeviceManagerClientPtr CreateInterfacePtrAndBind() {
+    UsbDeviceManagerClientPtr client;
+    binding_.Bind(mojo::MakeRequest(&client));
+    return client;
+  }
+
+  MOCK_METHOD1(DoOnDeviceAdded, void(UsbDeviceInfo*));
+  void OnDeviceAdded(UsbDeviceInfoPtr device_info) override {
+    DoOnDeviceAdded(device_info.get());
+  }
+
+  MOCK_METHOD1(DoOnDeviceRemoved, void(UsbDeviceInfo*));
+  void OnDeviceRemoved(UsbDeviceInfoPtr device_info) override {
+    DoOnDeviceRemoved(device_info.get());
+  }
+
+ private:
+  mojo::Binding<UsbDeviceManagerClient> binding_;
+};
+
+void ExpectDevicesAndThen(const std::set<std::string>& expected_guids,
+                          base::OnceClosure continuation,
+                          std::vector<UsbDeviceInfoPtr> results) {
+  EXPECT_EQ(expected_guids.size(), results.size());
+  std::set<std::string> actual_guids;
+  for (size_t i = 0; i < results.size(); ++i)
+    actual_guids.insert(results[i]->guid);
+  EXPECT_EQ(expected_guids, actual_guids);
+  std::move(continuation).Run();
+}
+
+}  // namespace
+
+// Test requesting device enumeration updates with GetDeviceChanges.
+TEST_F(WebUsbDeviceManagerTest, NoPermissionDevice) {
+  scoped_refptr<MockUsbDevice> device0 =
+      new MockUsbDevice(0x1234, 0x5678, "ACME", "Frobinator", "ABCDEF");
+  scoped_refptr<MockUsbDevice> device1 =
+      new MockUsbDevice(0x1234, 0x5679, "ACME", "Frobinator+", "GHIJKL");
+  scoped_refptr<MockUsbDevice> no_permission_device1 =
+      new MockUsbDevice(0xffff, 0x567b, "ACME", "Frobinator II",
+                        MockPermissionProvider::kRestrictedSerialNumber);
+  scoped_refptr<MockUsbDevice> no_permission_device2 =
+      new MockUsbDevice(0xffff, 0x567c, "ACME", "Frobinator Xtreme",
+                        MockPermissionProvider::kRestrictedSerialNumber);
+
+  device_client_.usb_service()->AddDevice(device0);
+  device_client_.usb_service()->AddDevice(no_permission_device1);
+
+  UsbDeviceManagerPtr device_manager = ConnectToDeviceManager();
+  MockDeviceManagerClient mock_client;
+  device_manager->SetClient(mock_client.CreateInterfacePtrAndBind());
+
+  {
+    // Call GetDevices once to make sure the device manager is up and running
+    // and the client is set or else we could block forever waiting for calls.
+    // The site has no permission to access |no_permission_device1| and
+    // |no_permission_device2|, so result of GetDevices() should only contain
+    // the |guid| of |device0|.
+    std::set<std::string> guids;
+    guids.insert(device0->guid());
+    base::RunLoop loop;
+    device_manager->GetDevices(
+        nullptr,
+        base::BindOnce(&ExpectDevicesAndThen, guids, loop.QuitClosure()));
+    loop.Run();
+  }
+
+  device_client_.usb_service()->AddDevice(device1);
+  device_client_.usb_service()->AddDevice(no_permission_device2);
+  device_client_.usb_service()->RemoveDevice(device0);
+  device_client_.usb_service()->RemoveDevice(device1);
+  device_client_.usb_service()->RemoveDevice(no_permission_device1);
+  device_client_.usb_service()->RemoveDevice(no_permission_device2);
+
+  {
+    base::RunLoop loop;
+    base::RepeatingClosure barrier =
+        base::BarrierClosure(3, loop.QuitClosure());
+    testing::InSequence s;
+    EXPECT_CALL(mock_client, DoOnDeviceAdded(_))
+        .Times(1)
+        .WillOnce(ExpectGuidAndThen(device1->guid(), barrier));
+    EXPECT_CALL(mock_client, DoOnDeviceRemoved(_))
+        .Times(2)
+        .WillOnce(ExpectGuidAndThen(device0->guid(), barrier))
+        .WillOnce(ExpectGuidAndThen(device1->guid(), barrier));
+    loop.Run();
+  }
+}

chrome/test/BUILD.gn

@@ -2837,6 +2837,7 @@ test("unit_tests") {
     "//device/base:mocks",
     "//device/bluetooth:mocks",
     "//device/usb:test_support",
+    "//device/usb/mojo:test_support",
     "//extensions/buildflags",
     "//google_apis",
     "//gpu:test_support",
@@ -3138,6 +3139,7 @@ test("unit_tests") {
       "../browser/usb/usb_chooser_context_unittest.cc",
       "../browser/usb/usb_chooser_controller_unittest.cc",
       "../browser/usb/web_usb_detector_unittest.cc",
+      "../browser/usb/web_usb_device_manager_unittest.cc",
 
       # The importer code is not used on Android.
       "../common/importer/firefox_importer_utils_unittest.cc",

device/BUILD.gn

@@ -163,8 +163,6 @@ test("device_unittests") {
       "test/test_device_client.h",
       "usb/mojo/device_impl_unittest.cc",
       "usb/mojo/device_manager_impl_unittest.cc",
-      "usb/mojo/mock_permission_provider.cc",
-      "usb/mojo/mock_permission_provider.h",
       "usb/public/cpp/filter_utils_unittest.cc",
       "usb/usb_descriptors_unittest.cc",
       "usb/usb_device_handle_unittest.cc",
@@ -178,6 +176,7 @@ test("device_unittests") {
       "//device/usb",
       "//device/usb:test_support",
       "//device/usb/mojo",
+      "//device/usb/mojo:test_support",
       "//device/usb/public/cpp",
       "//device/usb/public/mojom",
       "//net:test_support",

device/usb/mojo/BUILD.gn

@@ -23,3 +23,24 @@ source_set("mojo") {
     "//net",
   ]
 }
+
+static_library("test_support") {
+  testonly = true
+
+  sources = [
+    "mock_permission_provider.cc",
+    "mock_permission_provider.h",
+  ]
+
+  deps = [
+    "//base",
+    "//device/usb:usb",
+    "//device/usb/mojo:mojo",
+    "//device/usb/public/mojom",
+    "//testing/gtest",
+  ]
+
+  public_deps = [
+    "//testing/gmock",
+  ]
+}

device/usb/mojo/mock_permission_provider.cc

@@ -7,6 +7,7 @@
 #include <stddef.h>
 #include <utility>
 
+#include "base/strings/utf_string_conversions.h"
 #include "device/usb/public/mojom/device.mojom.h"
 
 using ::testing::Return;
@@ -15,12 +16,19 @@ using ::testing::_;
 namespace device {
 namespace usb {
 
-MockPermissionProvider::MockPermissionProvider() : weak_factory_(this) {
-  ON_CALL(*this, HasDevicePermission(_)).WillByDefault(Return(true));
-}
+const char MockPermissionProvider::kRestrictedSerialNumber[] = "no_permission";
+
+MockPermissionProvider::MockPermissionProvider() : weak_factory_(this) {}
 
 MockPermissionProvider::~MockPermissionProvider() = default;
 
+bool MockPermissionProvider::HasDevicePermission(
+    scoped_refptr<const UsbDevice> device) const {
+  return device->serial_number() == base::ASCIIToUTF16(kRestrictedSerialNumber)
+             ? false
+             : true;
+}
+
 base::WeakPtr<PermissionProvider> MockPermissionProvider::GetWeakPtr() {
   return weak_factory_.GetWeakPtr();
 }

device/usb/mojo/mock_permission_provider.h

@@ -17,12 +17,14 @@ namespace usb {
 
 class MockPermissionProvider : public PermissionProvider {
  public:
+  static const char kRestrictedSerialNumber[];
+
   MockPermissionProvider();
   ~MockPermissionProvider() override;
 
   base::WeakPtr<PermissionProvider> GetWeakPtr();
-  MOCK_CONST_METHOD1(HasDevicePermission,
-                     bool(scoped_refptr<const UsbDevice> device));
+  bool HasDevicePermission(
+      scoped_refptr<const UsbDevice> device) const override;
 
   MOCK_METHOD0(IncrementConnectionCount, void());
   MOCK_METHOD0(DecrementConnectionCount, void());
@@ -34,4 +36,4 @@ class MockPermissionProvider : public PermissionProvider {
 }  // namespace usb
 }  // namespace device
 
-#endif  // DEVICE_USB_MOCK_MOJO_PERMISSION_PROVIDER_H_
+#endif  // DEVICE_USB_MOJO_MOCK_PERMISSION_PROVIDER_H_