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}

9 files changed, 112 insertions, 42 deletions

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));