Check USB device path access when prompting users to select a device.

Use the permission_broker's new CheckPathAccess method to filter
devices presented to the user by whether or not Chrome will be able to
open them later.

By having FakePermissionBrokerClient and the base UsbDevice class always
return true to access requests a number of #if defined(OS_CHROMEOS)
checks can be removed.

//third_party/cros_system_api is rolled from 95c486f3 to aea83430 to
pull in the new kCheckPathAccess definition in service_constants.h.

BUG=441526

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

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

chrome/browser/devtools/device/usb/android_usb_browsertest.cc

@@ -433,18 +433,6 @@ class MockUsbDevice : public UsbDevice {
     return true;
   }
 
-#if defined(OS_CHROMEOS)
-  // On ChromeOS, if an interface of a claimed device is not claimed, the
-  // permission broker can change the owner of the device so that the unclaimed
-  // interfaces can be used. If this argument is missing, permission broker will
-  // not be used and this method fails if the device is claimed.
-  virtual void RequestUsbAccess(
-      int interface_id,
-      const base::Callback<void(bool success)>& callback) override {
-    callback.Run(true);
-  }
-#endif  // OS_CHROMEOS
-
   std::set<int> claimed_interfaces_;
 
  protected:

chrome/browser/devtools/device/usb/android_usb_device.cc

@@ -265,14 +265,9 @@ static void EnumerateOnFileThread(
         continue;
       }
 
-      // Request permission on Chrome OS.
-#if defined(OS_CHROMEOS)
       device->RequestUsbAccess(
           j, base::Bind(&OpenAndroidDeviceOnFileThread, devices, rsa_key,
                         barrier, device, j));
-#else
-      OpenAndroidDeviceOnFileThread(devices, rsa_key, barrier, device, j, true);
-#endif  // defined(OS_CHROMEOS)
 
       has_android_interface = true;
       break;

chrome/browser/extensions/api/device_permissions_manager_unittest.cc

@@ -80,7 +80,6 @@ class MockUsbDevice : public UsbDevice {
                   Return(true)));
   }
 
-  MOCK_METHOD2(RequestUsbAccess, void(int, const base::Callback<void(bool)>&));
   MOCK_METHOD0(Open, scoped_refptr<UsbDeviceHandle>());
   MOCK_METHOD1(Close, bool(scoped_refptr<UsbDeviceHandle>));
   MOCK_METHOD0(GetConfiguration, const device::UsbConfigDescriptor*());

chromeos/BUILD.gn

@@ -66,6 +66,8 @@ static_library("test_support") {
     "dbus/mock_cryptohome_client.h",
     "dbus/mock_lorgnette_manager_client.cc",
     "dbus/mock_lorgnette_manager_client.h",
+    "dbus/mock_permission_broker_client.cc",
+    "dbus/mock_permission_broker_client.h",
     "dbus/mock_session_manager_client.cc",
     "dbus/mock_session_manager_client.h",
     "dbus/mock_shill_manager_client.cc",

chromeos/chromeos.gyp

@@ -579,6 +579,8 @@
         'dbus/mock_cryptohome_client.h',
         'dbus/mock_lorgnette_manager_client.cc',
         'dbus/mock_lorgnette_manager_client.h',
+        'dbus/mock_permission_broker_client.cc',
+        'dbus/mock_permission_broker_client.h',
         'dbus/mock_session_manager_client.cc',
         'dbus/mock_session_manager_client.h',
         'dbus/mock_shill_manager_client.cc',

chromeos/dbus/fake_permission_broker_client.cc

@@ -16,11 +16,17 @@ FakePermissionBrokerClient::~FakePermissionBrokerClient() {}
 
 void FakePermissionBrokerClient::Init(dbus::Bus* bus) {}
 
+void FakePermissionBrokerClient::CheckPathAccess(
+    const std::string& path,
+    const ResultCallback& callback) {
+  callback.Run(true);
+}
+
 void FakePermissionBrokerClient::RequestPathAccess(
     const std::string& path,
     int interface_id,
     const ResultCallback& callback) {
-  callback.Run(false);
+  callback.Run(true);
 }
 
 void FakePermissionBrokerClient::RequestTcpPortAccess(

chromeos/dbus/fake_permission_broker_client.h

@@ -11,12 +11,15 @@
 
 namespace chromeos {
 
-class FakePermissionBrokerClient : public PermissionBrokerClient {
+class CHROMEOS_EXPORT FakePermissionBrokerClient
+    : public PermissionBrokerClient {
  public:
   FakePermissionBrokerClient();
   ~FakePermissionBrokerClient() override;
 
   void Init(dbus::Bus* bus) override;
+  void CheckPathAccess(const std::string& path,
+                       const ResultCallback& callback) override;
   void RequestPathAccess(const std::string& path,
                          int interface_id,
                          const ResultCallback& callback) override;

chromeos/dbus/mock_permission_broker_client.cc

+// Copyright 2015 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 "chromeos/dbus/mock_permission_broker_client.h"
+
+#include "dbus/file_descriptor.h"
+
+namespace chromeos {
+
+MockPermissionBrokerClient::MockPermissionBrokerClient() {
+}
+
+MockPermissionBrokerClient::~MockPermissionBrokerClient() {
+}
+
+}  // chromeos

chromeos/dbus/mock_permission_broker_client.h

+// Copyright 2015 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 CHROMEOS_DBUS_MOCK_PERMISSION_BROKER_CLIENT_H_
+#define CHROMEOS_DBUS_MOCK_PERMISSION_BROKER_CLIENT_H_
+
+#include "chromeos/dbus/permission_broker_client.h"
+#include "testing/gmock/include/gmock/gmock.h"
+
+namespace dbus {
+class FileDescriptor;
+}  // namespace dbus
+
+namespace chromeos {
+
+class MockPermissionBrokerClient : public PermissionBrokerClient {
+ public:
+  MockPermissionBrokerClient();
+  ~MockPermissionBrokerClient() override;
+
+  MOCK_METHOD1(Init, void(dbus::Bus* bus));
+  MOCK_METHOD2(CheckPathAccess,
+               void(const std::string& path, const ResultCallback& callback));
+  MOCK_METHOD3(RequestPathAccess,
+               void(const std::string& path,
+                    int interface_id,
+                    const ResultCallback& callback));
+  MOCK_METHOD4(RequestTcpPortAccess,
+               void(uint16 port,
+                    const std::string& interface,
+                    const dbus::FileDescriptor& lifeline_fd,
+                    const ResultCallback& callback));
+  MOCK_METHOD4(RequestUdpPortAccess,
+               void(uint16 port,
+                    const std::string& interface,
+                    const dbus::FileDescriptor& lifeline_fd,
+                    const ResultCallback& callback));
+  MOCK_METHOD3(ReleaseTcpPort,
+               void(uint16 port,
+                    const std::string& interface,
+                    const ResultCallback& callback));
+  MOCK_METHOD3(ReleaseUdpPort,
+               void(uint16 port,
+                    const std::string& interface,
+                    const ResultCallback& callback));
+};
+
+}  // namespace chromeos
+
+#endif  // CHROMEOS_DBUS_MOCK_PERMISSION_BROKER_CLIENT_H_

chromeos/dbus/permission_broker_client.cc

@@ -11,6 +11,7 @@
 #include "dbus/object_proxy.h"
 #include "third_party/cros_system_api/dbus/service_constants.h"
 
+using permission_broker::kCheckPathAccess;
 using permission_broker::kPermissionBrokerInterface;
 using permission_broker::kPermissionBrokerServiceName;
 using permission_broker::kPermissionBrokerServicePath;
@@ -26,6 +27,16 @@ class PermissionBrokerClientImpl : public PermissionBrokerClient {
  public:
   PermissionBrokerClientImpl() : proxy_(NULL), weak_ptr_factory_(this) {}
 
+  void CheckPathAccess(const std::string& path,
+                       const ResultCallback& callback) override {
+    dbus::MethodCall method_call(kPermissionBrokerInterface, kCheckPathAccess);
+    dbus::MessageWriter writer(&method_call);
+    writer.AppendString(path);
+    proxy_->CallMethod(&method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
+                       base::Bind(&PermissionBrokerClientImpl::OnResponse,
+                                  weak_ptr_factory_.GetWeakPtr(), callback));
+  }
+
   void RequestPathAccess(const std::string& path,
                          const int interface_id,
                          const ResultCallback& callback) override {

chromeos/dbus/permission_broker_client.h

@@ -36,6 +36,13 @@ class CHROMEOS_EXPORT PermissionBrokerClient : public DBusClient {
 
   static PermissionBrokerClient* Create();
 
+  // CheckPathAccess requests a hint from the permission broker about whether
+  // a later call to RequestPathAccess will be successful. It presumes that
+  // the |interface_id| value passed to RequestPathAccess will be
+  // UsbDevicePermissionsData::ANY_INTERFACE).
+  virtual void CheckPathAccess(const std::string& path,
+                               const ResultCallback& callback) = 0;
+
   // RequestPathAccess requests access to a single device node identified by
   // |path|. If |interface_id| value is passed (different than
   // UsbDevicePermissionData::ANY_INTERFACE), the request will check if a

chromeos/network/firewall_hole_unittest.cc

@@ -5,7 +5,7 @@
 #include "base/bind.h"
 #include "base/run_loop.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
-#include "chromeos/dbus/fake_permission_broker_client.h"
+#include "chromeos/dbus/mock_permission_broker_client.h"
 #include "chromeos/network/firewall_hole.h"
 #include "dbus/file_descriptor.h"
 #include "testing/gmock/include/gmock/gmock.h"
@@ -13,6 +13,7 @@
 
 using chromeos::DBusThreadManager;
 using chromeos::FirewallHole;
+using chromeos::MockPermissionBrokerClient;
 using testing::_;
 
 namespace {
@@ -23,36 +24,6 @@ ACTION_TEMPLATE(InvokeCallback,
   ::std::tr1::get<k>(args).Run(p1);
 }
 
-class MockPermissionsBrokerClient : public chromeos::PermissionBrokerClient {
- public:
-  MockPermissionsBrokerClient() {}
-  ~MockPermissionsBrokerClient() override {}
-
-  MOCK_METHOD1(Init, void(dbus::Bus* bus));
-  MOCK_METHOD3(RequestPathAccess,
-               void(const std::string& path,
-                    int interface_id,
-                    const ResultCallback& callback));
-  MOCK_METHOD4(RequestTcpPortAccess,
-               void(uint16 port,
-                    const std::string& interface,
-                    const dbus::FileDescriptor& lifeline_fd,
-                    const ResultCallback& callback));
-  MOCK_METHOD4(RequestUdpPortAccess,
-               void(uint16 port,
-                    const std::string& interface,
-                    const dbus::FileDescriptor& lifeline_fd,
-                    const ResultCallback& callback));
-  MOCK_METHOD3(ReleaseTcpPort,
-               void(uint16 port,
-                    const std::string& interface,
-                    const ResultCallback& callback));
-  MOCK_METHOD3(ReleaseUdpPort,
-               void(uint16 port,
-                    const std::string& interface,
-                    const ResultCallback& callback));
-};
-
 }  // namespace
 
 class FirewallHoleTest : public testing::Test {
@@ -61,9 +32,9 @@ class FirewallHoleTest : public testing::Test {
   ~FirewallHoleTest() override {}
 
   void SetUp() override {
-    mock_permissions_broker_client_ = new MockPermissionsBrokerClient();
+    mock_permission_broker_client_ = new MockPermissionBrokerClient();
     DBusThreadManager::GetSetterForTesting()->SetPermissionBrokerClient(
-        make_scoped_ptr(mock_permissions_broker_client_));
+        make_scoped_ptr(mock_permission_broker_client_));
   }
 
   void TearDown() override { DBusThreadManager::Shutdown(); }
@@ -83,14 +54,14 @@ class FirewallHoleTest : public testing::Test {
 
  protected:
   base::RunLoop run_loop_;
-  MockPermissionsBrokerClient* mock_permissions_broker_client_ = nullptr;
+  MockPermissionBrokerClient* mock_permission_broker_client_ = nullptr;
 };
 
 TEST_F(FirewallHoleTest, GrantTcpPortAccess) {
-  EXPECT_CALL(*mock_permissions_broker_client_,
+  EXPECT_CALL(*mock_permission_broker_client_,
               RequestTcpPortAccess(1234, "foo0", _, _))
       .WillOnce(InvokeCallback<3>(true));
-  EXPECT_CALL(*mock_permissions_broker_client_, ReleaseTcpPort(1234, "foo0", _))
+  EXPECT_CALL(*mock_permission_broker_client_, ReleaseTcpPort(1234, "foo0", _))
       .WillOnce(InvokeCallback<2>(true));
 
   FirewallHole::Open(
@@ -100,7 +71,7 @@ TEST_F(FirewallHoleTest, GrantTcpPortAccess) {
 }
 
 TEST_F(FirewallHoleTest, DenyTcpPortAccess) {
-  EXPECT_CALL(*mock_permissions_broker_client_,
+  EXPECT_CALL(*mock_permission_broker_client_,
               RequestTcpPortAccess(1234, "foo0", _, _))
       .WillOnce(InvokeCallback<3>(false));
 
@@ -111,10 +82,10 @@ TEST_F(FirewallHoleTest, DenyTcpPortAccess) {
 }
 
 TEST_F(FirewallHoleTest, GrantUdpPortAccess) {
-  EXPECT_CALL(*mock_permissions_broker_client_,
+  EXPECT_CALL(*mock_permission_broker_client_,
               RequestUdpPortAccess(1234, "foo0", _, _))
       .WillOnce(InvokeCallback<3>(true));
-  EXPECT_CALL(*mock_permissions_broker_client_, ReleaseUdpPort(1234, "foo0", _))
+  EXPECT_CALL(*mock_permission_broker_client_, ReleaseUdpPort(1234, "foo0", _))
       .WillOnce(InvokeCallback<2>(true));
 
   FirewallHole::Open(
@@ -124,7 +95,7 @@ TEST_F(FirewallHoleTest, GrantUdpPortAccess) {
 }
 
 TEST_F(FirewallHoleTest, DenyUdpPortAccess) {
-  EXPECT_CALL(*mock_permissions_broker_client_,
+  EXPECT_CALL(*mock_permission_broker_client_,
               RequestUdpPortAccess(1234, "foo0", _, _))
       .WillOnce(InvokeCallback<3>(false));
 

device/usb/BUILD.gn

@@ -13,6 +13,7 @@ source_set("usb") {
     "usb_context.h",
     "usb_descriptors.cc",
     "usb_descriptors.h",
+    "usb_device.cc",
     "usb_device.h",
     "usb_device_filter.cc",
     "usb_device_filter.h",

device/usb/usb.gyp

@@ -25,6 +25,7 @@
         'usb_descriptors.h',
         'usb_device_impl.cc',
         'usb_device_impl.h',
+        'usb_device.cc',
         'usb_device.h',
         'usb_device_filter.cc',
         'usb_device_filter.h',

device/usb/usb_device.cc

+// Copyright 2015 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 "device/usb/usb_device.h"
+
+namespace device {
+
+UsbDevice::UsbDevice(uint16 vendor_id, uint16 product_id, uint32 unique_id)
+    : vendor_id_(vendor_id), product_id_(product_id), unique_id_(unique_id) {
+}
+
+UsbDevice::~UsbDevice() {
+}
+
+void UsbDevice::CheckUsbAccess(const ResultCallback& callback) {
+  callback.Run(true);
+}
+
+// Like CheckUsbAccess but actually changes the ownership of the device node.
+void UsbDevice::RequestUsbAccess(int interface_id,
+                                 const ResultCallback& callback) {
+  callback.Run(true);
+}
+
+}  // namespace device

device/usb/usb_device.h

@@ -20,20 +20,21 @@ struct UsbConfigDescriptor;
 // UsbDeviceHandle must be created from Open() method.
 class UsbDevice : public base::RefCountedThreadSafe<UsbDevice> {
  public:
+  typedef base::Callback<void(bool success)> ResultCallback;
+
   // Accessors to basic information.
   uint16 vendor_id() const { return vendor_id_; }
   uint16 product_id() const { return product_id_; }
   uint32 unique_id() const { return unique_id_; }
 
-#if defined(OS_CHROMEOS)
-  // On ChromeOS, if an interface of a claimed device is not claimed, the
-  // permission broker can change the owner of the device so that the unclaimed
-  // interfaces can be used. If this argument is missing, permission broker will
-  // not be used and this method fails if the device is claimed.
-  virtual void RequestUsbAccess(
-      int interface_id,
-      const base::Callback<void(bool success)>& callback) = 0;
-#endif  // OS_CHROMEOS
+  // On ChromeOS the permission_broker service is used to change the ownership
+  // of USB device nodes so that Chrome can open them. On other platforms these
+  // functions are no-ops and always return true.
+  virtual void CheckUsbAccess(const ResultCallback& callback);
+
+  // Like CheckUsbAccess but actually changes the ownership of the device node.
+  virtual void RequestUsbAccess(int interface_id,
+                                const ResultCallback& callback);
 
   // Creates a UsbDeviceHandle for further manipulation.
   // Blocking method. Must be called on FILE thread.
@@ -66,9 +67,8 @@ class UsbDevice : public base::RefCountedThreadSafe<UsbDevice> {
   virtual bool GetSerialNumber(base::string16* serial) = 0;
 
  protected:
-  UsbDevice(uint16 vendor_id, uint16 product_id, uint32 unique_id)
-      : vendor_id_(vendor_id), product_id_(product_id), unique_id_(unique_id) {}
-  virtual ~UsbDevice() {}
+  UsbDevice(uint16 vendor_id, uint16 product_id, uint32 unique_id);
+  virtual ~UsbDevice();
 
  private:
   friend class base::RefCountedThreadSafe<UsbDevice>;

device/usb/usb_device_filter_unittest.cc

@@ -23,7 +23,6 @@ class MockUsbDevice : public UsbDevice {
   MockUsbDevice(uint16 vendor_id, uint16 product_id, uint32 unique_id)
       : UsbDevice(vendor_id, product_id, unique_id) {}
 
-  MOCK_METHOD2(RequestUsbAccess, void(int, const base::Callback<void(bool)>&));
   MOCK_METHOD0(Open, scoped_refptr<UsbDeviceHandle>());
   MOCK_METHOD1(Close, bool(scoped_refptr<UsbDeviceHandle>));
   MOCK_METHOD0(GetConfiguration, const device::UsbConfigDescriptor*());

device/usb/usb_device_impl.cc

@@ -21,7 +21,6 @@
 #include "third_party/libusb/src/libusb/libusb.h"
 
 #if defined(OS_CHROMEOS)
-#include "base/sys_info.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
 #include "chromeos/dbus/permission_broker_client.h"
 #endif  // defined(OS_CHROMEOS)
@@ -35,12 +34,14 @@ namespace device {
 namespace {
 
 #if defined(OS_CHROMEOS)
-void OnRequestUsbAccessReplied(
+
+void PostResultOnTaskRunner(
     scoped_refptr<base::SingleThreadTaskRunner> task_runner,
     const base::Callback<void(bool success)>& callback,
     bool success) {
   task_runner->PostTask(FROM_HERE, base::Bind(callback, success));
 }
+
 #endif  // defined(OS_CHROMEOS)
 
 UsbEndpointDirection GetDirection(
@@ -179,35 +180,35 @@ UsbDeviceImpl::~UsbDeviceImpl() {
 
 #if defined(OS_CHROMEOS)
 
-void UsbDeviceImpl::RequestUsbAccess(
-    int interface_id,
-    const base::Callback<void(bool success)>& callback) {
+void UsbDeviceImpl::CheckUsbAccess(const ResultCallback& callback) {
   DCHECK(thread_checker_.CalledOnValidThread());
 
-  // ChromeOS builds on non-ChromeOS machines (dev) should not attempt to
-  // use permission broker.
-  if (base::SysInfo::IsRunningOnChromeOS()) {
-    chromeos::PermissionBrokerClient* client =
-        chromeos::DBusThreadManager::Get()->GetPermissionBrokerClient();
-    DCHECK(client) << "Could not get permission broker client.";
-    if (!client) {
-      callback.Run(false);
-      return;
-    }
+  chromeos::PermissionBrokerClient* client =
+      chromeos::DBusThreadManager::Get()->GetPermissionBrokerClient();
+  DCHECK(client) << "Could not get permission broker client.";
 
-    ui_task_runner_->PostTask(
-        FROM_HERE,
-        base::Bind(&chromeos::PermissionBrokerClient::RequestPathAccess,
-                   base::Unretained(client),
-                   devnode_,
-                   interface_id,
-                   base::Bind(&OnRequestUsbAccessReplied,
-                              base::ThreadTaskRunnerHandle::Get(),
-                              callback)));
-  } else {
-    // Not really running on Chrome OS, declare success.
-    callback.Run(true);
-  }
+  ui_task_runner_->PostTask(
+      FROM_HERE,
+      base::Bind(&chromeos::PermissionBrokerClient::CheckPathAccess,
+                 base::Unretained(client), devnode_,
+                 base::Bind(&PostResultOnTaskRunner,
+                            base::ThreadTaskRunnerHandle::Get(), callback)));
+}
+
+void UsbDeviceImpl::RequestUsbAccess(int interface_id,
+                                     const ResultCallback& callback) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+
+  chromeos::PermissionBrokerClient* client =
+      chromeos::DBusThreadManager::Get()->GetPermissionBrokerClient();
+  DCHECK(client) << "Could not get permission broker client.";
+
+  ui_task_runner_->PostTask(
+      FROM_HERE,
+      base::Bind(&chromeos::PermissionBrokerClient::RequestPathAccess,
+                 base::Unretained(client), devnode_, interface_id,
+                 base::Bind(&PostResultOnTaskRunner,
+                            base::ThreadTaskRunnerHandle::Get(), callback)));
 }
 
 #endif

device/usb/usb_device_impl.h

@@ -32,9 +32,10 @@ class UsbDeviceImpl : public UsbDevice {
  public:
 // UsbDevice implementation:
 #if defined(OS_CHROMEOS)
-  void RequestUsbAccess(
-      int interface_id,
-      const base::Callback<void(bool success)>& callback) override;
+  // Only overridden on Chrome OS.
+  void CheckUsbAccess(const ResultCallback& callback) override;
+  void RequestUsbAccess(int interface_id,
+                        const ResultCallback& callback) override;
 #endif  // OS_CHROMEOS
   scoped_refptr<UsbDeviceHandle> Open() override;
   bool Close(scoped_refptr<UsbDeviceHandle> handle) override;

extensions/browser/api/device_permissions_prompt.cc

@@ -4,6 +4,7 @@
 
 #include "extensions/browser/api/device_permissions_prompt.h"
 
+#include "base/barrier_closure.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "content/public/browser/browser_thread.h"
@@ -17,6 +18,7 @@
 #include "extensions/strings/grit/extensions_strings.h"
 #include "ui/base/l10n/l10n_util.h"
 
+using content::BrowserThread;
 using device::UsbDevice;
 using device::UsbDeviceFilter;
 using device::UsbService;
@@ -81,8 +83,8 @@ void DevicePermissionsPrompt::Prompt::SetObserver(Observer* observer) {
   observer_ = observer;
 
   if (observer_) {
-    content::BrowserThread::PostTask(
-        content::BrowserThread::FILE, FROM_HERE,
+    BrowserThread::PostTask(
+        BrowserThread::FILE, FROM_HERE,
         base::Bind(&DevicePermissionsPrompt::Prompt::DoDeviceQuery, this));
   }
 }
@@ -136,24 +138,55 @@ void DevicePermissionsPrompt::Prompt::DoDeviceQuery() {
   std::vector<scoped_refptr<UsbDevice>> devices;
   service->GetDevices(&devices);
 
-  std::vector<DeviceInfo> device_info;
+  if (!usb_service_observer_.IsObserving(service)) {
+    usb_service_observer_.Add(service);
+  }
+
+  std::vector<DeviceInfo>* device_info = new std::vector<DeviceInfo>();
+  base::Closure barrier = base::BarrierClosure(
+      devices.size(),
+      base::Bind(&DevicePermissionsPrompt::Prompt::DeviceQueryComplete, this,
+                 base::Owned(device_info)));
+
   for (const auto& device : devices) {
-    if (!(filters_.empty() || UsbDeviceFilter::MatchesAny(device, filters_))) {
-      continue;
+    if (filters_.empty() || UsbDeviceFilter::MatchesAny(device, filters_)) {
+      device->CheckUsbAccess(
+          base::Bind(&DevicePermissionsPrompt::Prompt::AppendCheckedUsbDevice,
+                     this, device_info, device, barrier));
+    } else {
+      barrier.Run();
     }
+  }
+}
 
-    device_info.push_back(DeviceInfo(device));
+void DevicePermissionsPrompt::Prompt::AppendCheckedUsbDevice(
+    std::vector<DeviceInfo>* device_info,
+    scoped_refptr<UsbDevice> device,
+    const base::Closure& callback,
+    bool allowed) {
+  if (allowed) {
+    device_info->push_back(DeviceInfo(device));
   }
+  callback.Run();
+}
 
-  if (!usb_service_observer_.IsObserving(service)) {
-    usb_service_observer_.Add(service);
+void DevicePermissionsPrompt::Prompt::AddCheckedUsbDevice(
+    scoped_refptr<UsbDevice> device,
+    bool allowed) {
+  if (allowed) {
+    BrowserThread::PostTask(
+        BrowserThread::UI, FROM_HERE,
+        base::Bind(&DevicePermissionsPrompt::Prompt::AddDevice, this,
+                   DeviceInfo(device)));
   }
+}
 
-  content::BrowserThread::PostTask(
-      content::BrowserThread::UI,
-      FROM_HERE,
-      base::Bind(
-          &DevicePermissionsPrompt::Prompt::SetDevices, this, device_info));
+void DevicePermissionsPrompt::Prompt::DeviceQueryComplete(
+    std::vector<DeviceInfo>* device_info) {
+  BrowserThread::PostTask(
+      BrowserThread::UI, FROM_HERE,
+      base::Bind(&DevicePermissionsPrompt::Prompt::SetDevices, this,
+                 *device_info));
 }
 
 void DevicePermissionsPrompt::Prompt::SetDevices(
@@ -193,16 +226,14 @@ void DevicePermissionsPrompt::Prompt::OnDeviceAdded(
     return;
   }
 
-  content::BrowserThread::PostTask(
-      content::BrowserThread::UI, FROM_HERE,
-      base::Bind(&DevicePermissionsPrompt::Prompt::AddDevice, this,
-                 DeviceInfo(device)));
+  device->CheckUsbAccess(base::Bind(
+      &DevicePermissionsPrompt::Prompt::AddCheckedUsbDevice, this, device));
 }
 
 void DevicePermissionsPrompt::Prompt::OnDeviceRemoved(
     scoped_refptr<UsbDevice> device) {
-  content::BrowserThread::PostTask(
-      content::BrowserThread::UI, FROM_HERE,
+  BrowserThread::PostTask(
+      BrowserThread::UI, FROM_HERE,
       base::Bind(&DevicePermissionsPrompt::Prompt::RemoveDevice, this, device));
 }
 

extensions/browser/api/device_permissions_prompt.h

@@ -106,6 +106,13 @@ class DevicePermissionsPrompt {
 
     // Querying for devices must be done asynchronously on the FILE thread.
     void DoDeviceQuery();
+    void AppendCheckedUsbDevice(std::vector<DeviceInfo>* device_info,
+                                scoped_refptr<device::UsbDevice> device,
+                                const base::Closure& callback,
+                                bool allowed);
+    void AddCheckedUsbDevice(scoped_refptr<device::UsbDevice> device,
+                             bool allowed);
+    void DeviceQueryComplete(std::vector<DeviceInfo>* device_info);
     void SetDevices(const std::vector<DeviceInfo>& devices);
     void AddDevice(const DeviceInfo& device);
     void RemoveDevice(scoped_refptr<device::UsbDevice> device);

extensions/browser/api/usb/usb_api.cc

@@ -242,7 +242,6 @@ const char* ConvertTransferStatusToApi(const UsbTransferStatus status) {
   }
 }
 
-#if defined(OS_CHROMEOS)
 void RequestUsbDevicesAccessHelper(
     ScopedDeviceVector devices,
     std::vector<scoped_refptr<UsbDevice> >::iterator i,
@@ -282,7 +281,6 @@ void RequestUsbDevicesAccess(
                                     interface_id,
                                     callback));
 }
-#endif  // OS_CHROMEOS
 
 base::DictionaryValue* CreateTransferInfo(UsbTransferStatus status,
                                           scoped_refptr<net::IOBuffer> data,
@@ -600,14 +598,10 @@ void UsbFindDevicesFunction::AsyncWorkStart() {
     }
   }
 
-#if defined(OS_CHROMEOS)
   RequestUsbDevicesAccess(
       devices.Pass(),
       interface_id,
       base::Bind(&UsbFindDevicesFunction::OpenDevices, this));
-#else
-  OpenDevices(devices.Pass());
-#endif  // OS_CHROMEOS
 }
 
 void UsbFindDevicesFunction::OpenDevices(ScopedDeviceVector devices) {
@@ -792,13 +786,9 @@ void UsbOpenDeviceFunction::AsyncWorkStart() {
     return;
   }
 
-#if defined(OS_CHROMEOS)
   device_->RequestUsbAccess(
       -1, /* any interface, unused by the permission broker */
       base::Bind(&UsbOpenDeviceFunction::OnRequestAccessComplete, this));
-#else
-  OnRequestAccessComplete(true);
-#endif  // OS_CHROMEOS
 }
 
 void UsbOpenDeviceFunction::OnRequestAccessComplete(bool success) {

extensions/browser/api/usb/usb_apitest.cc

@@ -39,11 +39,6 @@ ACTION_TEMPLATE(InvokeUsbTransferCallback,
   ::std::tr1::get<k>(args).Run(p1, io_buffer, 1);
 }
 
-void RequestUsbAccess(int interface_id,
-                      const base::Callback<void(bool success)>& callback) {
-  base::MessageLoop::current()->PostTask(FROM_HERE, base::Bind(callback, true));
-}
-
 class TestDevicePermissionsPrompt
     : public DevicePermissionsPrompt,
       public DevicePermissionsPrompt::Prompt::Observer {
@@ -145,7 +140,6 @@ class MockUsbDevice : public UsbDevice {
   MockUsbDevice(uint16 vendor_id, uint16 product_id, uint32 unique_id)
       : UsbDevice(vendor_id, product_id, unique_id) {}
 
-  MOCK_METHOD2(RequestUsbAccess, void(int, const base::Callback<void(bool)>&));
   MOCK_METHOD0(Open, scoped_refptr<UsbDeviceHandle>());
   MOCK_METHOD1(Close, bool(scoped_refptr<UsbDeviceHandle>));
   MOCK_METHOD0(GetConfiguration, const device::UsbConfigDescriptor*());
@@ -200,8 +194,6 @@ class UsbApiTest : public ShellApiTest {
 
     mock_device_handle_ = new MockUsbDeviceHandle();
     mock_device_handle_->set_device(mock_device_.get());
-    EXPECT_CALL(*mock_device_.get(), RequestUsbAccess(_, _))
-        .WillRepeatedly(Invoke(RequestUsbAccess));
     EXPECT_CALL(*mock_device_.get(), Open())
         .WillRepeatedly(Return(mock_device_handle_));