summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorreillyg <reillyg@chromium.org>2015-04-07 15:49:58 -0700
committerCommit bot <commit-bot@chromium.org>2015-04-07 22:50:33 +0000
commit556a9e9fddcc48e9b9650f2ac4a413ce4705efdf (patch)
tree670041bf13e18c4a98716581da2f5e740827e443
parentd6aece7c6748553aa11b9dbacd27b5ec4d2d363c (diff)
downloadchromium_src-556a9e9fddcc48e9b9650f2ac4a413ce4705efdf.zip
chromium_src-556a9e9fddcc48e9b9650f2ac4a413ce4705efdf.tar.gz
chromium_src-556a9e9fddcc48e9b9650f2ac4a413ce4705efdf.tar.bz2
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}
-rw-r--r--chrome/browser/devtools/device/usb/android_usb_browsertest.cc12
-rw-r--r--chrome/browser/devtools/device/usb/android_usb_device.cc5
-rw-r--r--chrome/browser/extensions/api/device_permissions_manager_unittest.cc1
-rw-r--r--chromeos/BUILD.gn2
-rw-r--r--chromeos/chromeos.gyp2
-rw-r--r--chromeos/dbus/fake_permission_broker_client.cc8
-rw-r--r--chromeos/dbus/fake_permission_broker_client.h5
-rw-r--r--chromeos/dbus/mock_permission_broker_client.cc17
-rw-r--r--chromeos/dbus/mock_permission_broker_client.h51
-rw-r--r--chromeos/dbus/permission_broker_client.cc11
-rw-r--r--chromeos/dbus/permission_broker_client.h7
-rw-r--r--chromeos/network/firewall_hole_unittest.cc51
-rw-r--r--device/usb/BUILD.gn1
-rw-r--r--device/usb/usb.gyp1
-rw-r--r--device/usb/usb_device.cc26
-rw-r--r--device/usb/usb_device.h24
-rw-r--r--device/usb/usb_device_filter_unittest.cc1
-rw-r--r--device/usb/usb_device_impl.cc57
-rw-r--r--device/usb/usb_device_impl.h7
-rw-r--r--extensions/browser/api/device_permissions_prompt.cc69
-rw-r--r--extensions/browser/api/device_permissions_prompt.h7
-rw-r--r--extensions/browser/api/usb/usb_api.cc10
-rw-r--r--extensions/browser/api/usb/usb_apitest.cc8
23 files changed, 242 insertions, 141 deletions
diff --git a/chrome/browser/devtools/device/usb/android_usb_browsertest.cc b/chrome/browser/devtools/device/usb/android_usb_browsertest.cc
index 0c96628..c7f99ed 100644
--- a/chrome/browser/devtools/device/usb/android_usb_browsertest.cc
+++ b/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:
diff --git a/chrome/browser/devtools/device/usb/android_usb_device.cc b/chrome/browser/devtools/device/usb/android_usb_device.cc
index 4ced939..9e5d700 100644
--- a/chrome/browser/devtools/device/usb/android_usb_device.cc
+++ b/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;
diff --git a/chrome/browser/extensions/api/device_permissions_manager_unittest.cc b/chrome/browser/extensions/api/device_permissions_manager_unittest.cc
index 4bac4e3..0140bf6 100644
--- a/chrome/browser/extensions/api/device_permissions_manager_unittest.cc
+++ b/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*());
diff --git a/chromeos/BUILD.gn b/chromeos/BUILD.gn
index 40823e6..afa1728 100644
--- a/chromeos/BUILD.gn
+++ b/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",
diff --git a/chromeos/chromeos.gyp b/chromeos/chromeos.gyp
index 1cf4e8e..41a30f8 100644
--- a/chromeos/chromeos.gyp
+++ b/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',
diff --git a/chromeos/dbus/fake_permission_broker_client.cc b/chromeos/dbus/fake_permission_broker_client.cc
index c4b265c..067dca7 100644
--- a/chromeos/dbus/fake_permission_broker_client.cc
+++ b/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(
diff --git a/chromeos/dbus/fake_permission_broker_client.h b/chromeos/dbus/fake_permission_broker_client.h
index 4f7fff4..501fef0 100644
--- a/chromeos/dbus/fake_permission_broker_client.h
+++ b/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;
diff --git a/chromeos/dbus/mock_permission_broker_client.cc b/chromeos/dbus/mock_permission_broker_client.cc
new file mode 100644
index 0000000..00a3014
--- /dev/null
+++ b/chromeos/dbus/mock_permission_broker_client.cc
@@ -0,0 +1,17 @@
+// 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
diff --git a/chromeos/dbus/mock_permission_broker_client.h b/chromeos/dbus/mock_permission_broker_client.h
new file mode 100644
index 0000000..1c764b6
--- /dev/null
+++ b/chromeos/dbus/mock_permission_broker_client.h
@@ -0,0 +1,51 @@
+// 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_
diff --git a/chromeos/dbus/permission_broker_client.cc b/chromeos/dbus/permission_broker_client.cc
index c26d53c..51d8337 100644
--- a/chromeos/dbus/permission_broker_client.cc
+++ b/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 {
diff --git a/chromeos/dbus/permission_broker_client.h b/chromeos/dbus/permission_broker_client.h
index aa14757..4dfe4fa 100644
--- a/chromeos/dbus/permission_broker_client.h
+++ b/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
diff --git a/chromeos/network/firewall_hole_unittest.cc b/chromeos/network/firewall_hole_unittest.cc
index ea5f5ec..3032d5b 100644
--- a/chromeos/network/firewall_hole_unittest.cc
+++ b/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));
diff --git a/device/usb/BUILD.gn b/device/usb/BUILD.gn
index 5a5d449..d989ecb 100644
--- a/device/usb/BUILD.gn
+++ b/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",
diff --git a/device/usb/usb.gyp b/device/usb/usb.gyp
index e8785d6..085b9b3 100644
--- a/device/usb/usb.gyp
+++ b/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',
diff --git a/device/usb/usb_device.cc b/device/usb/usb_device.cc
new file mode 100644
index 0000000..4b9ed1e
--- /dev/null
+++ b/device/usb/usb_device.cc
@@ -0,0 +1,26 @@
+// 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
diff --git a/device/usb/usb_device.h b/device/usb/usb_device.h
index adbd495..e824078 100644
--- a/device/usb/usb_device.h
+++ b/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>;
diff --git a/device/usb/usb_device_filter_unittest.cc b/device/usb/usb_device_filter_unittest.cc
index 1c3fb92..d66a462 100644
--- a/device/usb/usb_device_filter_unittest.cc
+++ b/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*());
diff --git a/device/usb/usb_device_impl.cc b/device/usb/usb_device_impl.cc
index 3aa9fba..02ceb6f 100644
--- a/device/usb/usb_device_impl.cc
+++ b/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
diff --git a/device/usb/usb_device_impl.h b/device/usb/usb_device_impl.h
index 702b20e..897b4910 100644
--- a/device/usb/usb_device_impl.h
+++ b/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;
diff --git a/extensions/browser/api/device_permissions_prompt.cc b/extensions/browser/api/device_permissions_prompt.cc
index afb895c..f45ae56 100644
--- a/extensions/browser/api/device_permissions_prompt.cc
+++ b/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));
}
diff --git a/extensions/browser/api/device_permissions_prompt.h b/extensions/browser/api/device_permissions_prompt.h
index 3ef2587..e7d11ca 100644
--- a/extensions/browser/api/device_permissions_prompt.h
+++ b/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);
diff --git a/extensions/browser/api/usb/usb_api.cc b/extensions/browser/api/usb/usb_api.cc
index e1a4627..f1fcd23 100644
--- a/extensions/browser/api/usb/usb_api.cc
+++ b/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) {
diff --git a/extensions/browser/api/usb/usb_apitest.cc b/extensions/browser/api/usb/usb_apitest.cc
index 2bf42ca..ad36408 100644
--- a/extensions/browser/api/usb/usb_apitest.cc
+++ b/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_));