Extract PermissionBrokerClient from DBusThreadManager

For minor refactoring in usb_host_bridge,cc:
TBR=yusukes@chromium.org

Bug: 944665
Change-Id: I40a0511a07c911678cbd93b2d6342c1083a75c24
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1535486Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Yusuke Sato <yusukes@chromium.org>
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Commit-Queue: Steven Bennetts <stevenjb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#644154}

chrome/browser/chromeos/dbus/dbus_helper.cc

@@ -12,6 +12,7 @@
 #include "chromeos/dbus/hammerd/hammerd_client.h"
 #include "chromeos/dbus/kerberos/kerberos_client.h"
 #include "chromeos/dbus/machine_learning/machine_learning_client.h"
+#include "chromeos/dbus/permission_broker/permission_broker_client.h"
 #include "chromeos/dbus/power/power_manager_client.h"
 #include "chromeos/dbus/system_clock/system_clock_client.h"
 #include "chromeos/dbus/upstart/upstart_client.h"
@@ -37,6 +38,7 @@ void InitializeDBus() {
     CryptohomeClient::Initialize(bus);
     KerberosClient::Initialize(bus);
     MachineLearningClient::Initialize(bus);
+    PermissionBrokerClient::Initialize(bus);
     PowerManagerClient::Initialize(bus);
     SystemClockClient::Initialize(bus);
     UpstartClient::Initialize(bus);
@@ -46,6 +48,7 @@ void InitializeDBus() {
     CryptohomeClient::InitializeFake();
     KerberosClient::InitializeFake();
     MachineLearningClient::InitializeFake();
+    PermissionBrokerClient::InitializeFake();
     PowerManagerClient::InitializeFake();
     SystemClockClient::InitializeFake();
     UpstartClient::InitializeFake();
@@ -62,6 +65,7 @@ void ShutdownDBus() {
   UpstartClient::Shutdown();
   SystemClockClient::Shutdown();
   PowerManagerClient::Shutdown();
+  PermissionBrokerClient::Shutdown();
   MachineLearningClient::Shutdown();
   KerberosClient::Shutdown();
   CryptohomeClient::Shutdown();

chromeos/dbus/BUILD.gn

@@ -119,8 +119,6 @@ component("dbus") {
     "fake_modem_messaging_client.h",
     "fake_oobe_configuration_client.cc",
     "fake_oobe_configuration_client.h",
-    "fake_permission_broker_client.cc",
-    "fake_permission_broker_client.h",
     "fake_runtime_probe_client.cc",
     "fake_runtime_probe_client.h",
     "fake_seneschal_client.cc",
@@ -163,8 +161,10 @@ component("dbus") {
     "modem_messaging_client.h",
     "oobe_configuration_client.cc",
     "oobe_configuration_client.h",
-    "permission_broker_client.cc",
-    "permission_broker_client.h",
+    "permission_broker/fake_permission_broker_client.cc",
+    "permission_broker/fake_permission_broker_client.h",
+    "permission_broker/permission_broker_client.cc",
+    "permission_broker/permission_broker_client.h",
     "power/fake_power_manager_client.cc",
     "power/fake_power_manager_client.h",
     "power/native_timer.cc",

chromeos/dbus/dbus_clients_common.cc

@@ -12,7 +12,6 @@
 #include "chromeos/dbus/fake_cras_audio_client.h"
 #include "chromeos/dbus/fake_gsm_sms_client.h"
 #include "chromeos/dbus/fake_modem_messaging_client.h"
-#include "chromeos/dbus/fake_permission_broker_client.h"
 #include "chromeos/dbus/fake_shill_device_client.h"
 #include "chromeos/dbus/fake_shill_ipconfig_client.h"
 #include "chromeos/dbus/fake_shill_manager_client.h"
@@ -22,7 +21,6 @@
 #include "chromeos/dbus/fake_sms_client.h"
 #include "chromeos/dbus/gsm_sms_client.h"
 #include "chromeos/dbus/modem_messaging_client.h"
-#include "chromeos/dbus/permission_broker_client.h"
 #include "chromeos/dbus/session_manager/session_manager_client.h"
 #include "chromeos/dbus/shill_device_client.h"
 #include "chromeos/dbus/shill_ipconfig_client.h"
@@ -77,11 +75,6 @@ DBusClientsCommon::DBusClientsCommon(bool use_real_clients) {
   else
     modem_messaging_client_.reset(new FakeModemMessagingClient);
 
-  if (use_real_clients)
-    permission_broker_client_.reset(PermissionBrokerClient::Create());
-  else
-    permission_broker_client_.reset(new FakePermissionBrokerClient);
-
   session_manager_client_.reset(SessionManagerClient::Create(client_impl_type));
 
   if (use_real_clients)
@@ -98,7 +91,6 @@ void DBusClientsCommon::Initialize(dbus::Bus* system_bus) {
   cras_audio_client_->Init(system_bus);
   gsm_sms_client_->Init(system_bus);
   modem_messaging_client_->Init(system_bus);
-  permission_broker_client_->Init(system_bus);
   session_manager_client_->Init(system_bus);
   shill_device_client_->Init(system_bus);
   shill_ipconfig_client_->Init(system_bus);

chromeos/dbus/dbus_clients_common.h

@@ -19,7 +19,6 @@ namespace chromeos {
 class CrasAudioClient;
 class GsmSMSClient;
 class ModemMessagingClient;
-class PermissionBrokerClient;
 class SessionManagerClient;
 class ShillDeviceClient;
 class ShillIPConfigClient;
@@ -54,7 +53,6 @@ class COMPONENT_EXPORT(CHROMEOS_DBUS) DBusClientsCommon {
   std::unique_ptr<ShillProfileClient> shill_profile_client_;
   std::unique_ptr<ShillThirdPartyVpnDriverClient>
       shill_third_party_vpn_driver_client_;
-  std::unique_ptr<PermissionBrokerClient> permission_broker_client_;
   std::unique_ptr<SMSClient> sms_client_;
   std::unique_ptr<SessionManagerClient> session_manager_client_;
 

chromeos/dbus/dbus_thread_manager.cc

@@ -31,7 +31,6 @@
 #include "chromeos/dbus/lorgnette_manager_client.h"
 #include "chromeos/dbus/media_analytics_client.h"
 #include "chromeos/dbus/modem_messaging_client.h"
-#include "chromeos/dbus/permission_broker_client.h"
 #include "chromeos/dbus/runtime_probe_client.h"
 #include "chromeos/dbus/seneschal_client.h"
 #include "chromeos/dbus/session_manager/session_manager_client.h"
@@ -225,10 +224,6 @@ OobeConfigurationClient* DBusThreadManager::GetOobeConfigurationClient() {
   return clients_browser_->oobe_configuration_client_.get();
 }
 
-PermissionBrokerClient* DBusThreadManager::GetPermissionBrokerClient() {
-  return clients_common_->permission_broker_client_.get();
-}
-
 SessionManagerClient* DBusThreadManager::GetSessionManagerClient() {
   return clients_common_->session_manager_client_.get();
 }
@@ -440,12 +435,6 @@ void DBusThreadManagerSetter::SetMediaAnalyticsClient(
       std::move(client);
 }
 
-void DBusThreadManagerSetter::SetPermissionBrokerClient(
-    std::unique_ptr<PermissionBrokerClient> client) {
-  DBusThreadManager::Get()->clients_common_->permission_broker_client_ =
-      std::move(client);
-}
-
 void DBusThreadManagerSetter::SetSessionManagerClient(
     std::unique_ptr<SessionManagerClient> client) {
   DBusThreadManager::Get()->clients_common_->session_manager_client_ =

chromeos/dbus/dbus_thread_manager.h

@@ -46,7 +46,6 @@ class LorgnetteManagerClient;
 class MediaAnalyticsClient;
 class ModemMessagingClient;
 class OobeConfigurationClient;
-class PermissionBrokerClient;
 class RuntimeProbeClient;
 class SeneschalClient;
 class SessionManagerClient;
@@ -138,7 +137,6 @@ class COMPONENT_EXPORT(CHROMEOS_DBUS) DBusThreadManager {
   MediaAnalyticsClient* GetMediaAnalyticsClient();
   ModemMessagingClient* GetModemMessagingClient();
   OobeConfigurationClient* GetOobeConfigurationClient();
-  PermissionBrokerClient* GetPermissionBrokerClient();
   RuntimeProbeClient* GetRuntimeProbeClient();
   SeneschalClient* GetSeneschalClient();
   SessionManagerClient* GetSessionManagerClient();
@@ -193,8 +191,6 @@ class COMPONENT_EXPORT(CHROMEOS_DBUS) DBusThreadManagerSetter {
   void SetImageBurnerClient(std::unique_ptr<ImageBurnerClient> client);
   void SetImageLoaderClient(std::unique_ptr<ImageLoaderClient> client);
   void SetMediaAnalyticsClient(std::unique_ptr<MediaAnalyticsClient> client);
-  void SetPermissionBrokerClient(
-      std::unique_ptr<PermissionBrokerClient> client);
   void SetSeneschalClient(std::unique_ptr<SeneschalClient> client);
   void SetRuntimeProbeClient(std::unique_ptr<RuntimeProbeClient> client);
   void SetSessionManagerClient(std::unique_ptr<SessionManagerClient> client);

chromeos/dbus/dbus_thread_manager_unittest.cc

@@ -33,7 +33,6 @@ TEST(DBusThreadManagerTest, Initialize) {
   EXPECT_TRUE(manager->GetImageBurnerClient());
   EXPECT_TRUE(manager->GetLorgnetteManagerClient());
   EXPECT_TRUE(manager->GetModemMessagingClient());
-  EXPECT_TRUE(manager->GetPermissionBrokerClient());
   EXPECT_TRUE(manager->GetSeneschalClient());
   EXPECT_TRUE(manager->GetSessionManagerClient());
   EXPECT_TRUE(manager->GetShillDeviceClient());
@@ -59,7 +58,6 @@ TEST(DBusThreadManagerTest, InitializeForBrowser) {
   EXPECT_TRUE(manager->GetCrasAudioClient());
   EXPECT_TRUE(manager->GetGsmSMSClient());
   EXPECT_TRUE(manager->GetModemMessagingClient());
-  EXPECT_TRUE(manager->GetPermissionBrokerClient());
   EXPECT_TRUE(manager->GetSessionManagerClient());
   EXPECT_TRUE(manager->GetShillDeviceClient());
   EXPECT_TRUE(manager->GetShillIPConfigClient());
@@ -96,7 +94,6 @@ TEST(DBusThreadManagerTest, InitializeForAsh) {
   EXPECT_TRUE(manager->GetCrasAudioClient());
   EXPECT_TRUE(manager->GetGsmSMSClient());
   EXPECT_TRUE(manager->GetModemMessagingClient());
-  EXPECT_TRUE(manager->GetPermissionBrokerClient());
   EXPECT_TRUE(manager->GetSessionManagerClient());
   EXPECT_TRUE(manager->GetShillDeviceClient());
   EXPECT_TRUE(manager->GetShillIPConfigClient());

chromeos/dbus/fake_permission_broker_client.cc → chromeos/dbus/permission_broker/fake_permission_broker_client.cc

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "chromeos/dbus/fake_permission_broker_client.h"
+#include "chromeos/dbus/permission_broker/fake_permission_broker_client.h"
 
 #include <fcntl.h>
 #include <stdint.h>
@@ -22,6 +22,8 @@ namespace {
 
 const char kOpenFailedError[] = "open_failed";
 
+FakePermissionBrokerClient* g_instance = nullptr;
+
 // So that real devices can be accessed by tests and "Chromium OS on Linux" this
 // function implements a simplified version of the method implemented by the
 // permission broker by opening the path specified and returning the resulting
@@ -48,11 +50,21 @@ void OpenPath(const std::string& path,
 
 }  // namespace
 
-FakePermissionBrokerClient::FakePermissionBrokerClient() = default;
+FakePermissionBrokerClient::FakePermissionBrokerClient() {
+  DCHECK(!g_instance);
+  g_instance = this;
+}
 
-FakePermissionBrokerClient::~FakePermissionBrokerClient() = default;
+FakePermissionBrokerClient::~FakePermissionBrokerClient() {
+  DCHECK_EQ(this, g_instance);
+  g_instance = nullptr;
+}
 
-void FakePermissionBrokerClient::Init(dbus::Bus* bus) {}
+// static
+FakePermissionBrokerClient* FakePermissionBrokerClient::Get() {
+  DCHECK(g_instance);
+  return g_instance;
+}
 
 void FakePermissionBrokerClient::CheckPathAccess(
     const std::string& path,

chromeos/dbus/fake_permission_broker_client.h → chromeos/dbus/permission_broker/fake_permission_broker_client.h

@@ -2,8 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef CHROMEOS_DBUS_FAKE_PERMISSION_BROKER_CLIENT_H_
-#define CHROMEOS_DBUS_FAKE_PERMISSION_BROKER_CLIENT_H_
+#ifndef CHROMEOS_DBUS_PERMISSION_BROKER_FAKE_PERMISSION_BROKER_CLIENT_H_
+#define CHROMEOS_DBUS_PERMISSION_BROKER_FAKE_PERMISSION_BROKER_CLIENT_H_
 
 #include <stdint.h>
 
@@ -13,7 +13,7 @@
 
 #include "base/compiler_specific.h"
 #include "base/macros.h"
-#include "chromeos/dbus/permission_broker_client.h"
+#include "chromeos/dbus/permission_broker/permission_broker_client.h"
 
 namespace chromeos {
 
@@ -23,7 +23,9 @@ class COMPONENT_EXPORT(CHROMEOS_DBUS) FakePermissionBrokerClient
   FakePermissionBrokerClient();
   ~FakePermissionBrokerClient() override;
 
-  void Init(dbus::Bus* bus) override;
+  // Checks that a fake instance was initialized and returns it.
+  static FakePermissionBrokerClient* Get();
+
   void CheckPathAccess(const std::string& path,
                        const ResultCallback& callback) override;
   void OpenPath(const std::string& path,
@@ -76,4 +78,4 @@ class COMPONENT_EXPORT(CHROMEOS_DBUS) FakePermissionBrokerClient
 
 }  // namespace chromeos
 
-#endif  // CHROMEOS_DBUS_FAKE_PERMISSION_BROKER_CLIENT_H_
+#endif  // CHROMEOS_DBUS_PERMISSION_BROKER_FAKE_PERMISSION_BROKER_CLIENT_H_

chromeos/dbus/permission_broker_client.cc → chromeos/dbus/permission_broker/permission_broker_client.cc

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "chromeos/dbus/permission_broker_client.h"
+#include "chromeos/dbus/permission_broker/permission_broker_client.h"
 
 #include <stdint.h>
 #include <utility>
@@ -10,6 +10,7 @@
 #include "base/bind.h"
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
+#include "chromeos/dbus/permission_broker/fake_permission_broker_client.h"
 #include "dbus/bus.h"
 #include "dbus/message.h"
 #include "dbus/object_proxy.h"
@@ -28,12 +29,17 @@ using permission_broker::kRequestUdpPortAccess;
 namespace chromeos {
 
 namespace {
+
 const char kNoResponseError[] = "org.chromium.Error.NoResponse";
-}
+
+PermissionBrokerClient* g_instance = nullptr;
+
+}  // namespace
 
 class PermissionBrokerClientImpl : public PermissionBrokerClient {
  public:
-  PermissionBrokerClientImpl() : proxy_(NULL), weak_ptr_factory_(this) {}
+  PermissionBrokerClientImpl() = default;
+  ~PermissionBrokerClientImpl() override = default;
 
   void CheckPathAccess(const std::string& path,
                        const ResultCallback& callback) override {
@@ -118,8 +124,7 @@ class PermissionBrokerClientImpl : public PermissionBrokerClient {
                        weak_ptr_factory_.GetWeakPtr(), callback));
   }
 
- protected:
-  void Init(dbus::Bus* bus) override {
+  void Init(dbus::Bus* bus) {
     proxy_ =
         bus->GetObjectProxy(kPermissionBrokerServiceName,
                             dbus::ObjectPath(kPermissionBrokerServicePath));
@@ -164,22 +169,46 @@ class PermissionBrokerClientImpl : public PermissionBrokerClient {
     callback.Run(error_name, error_message);
   }
 
-  dbus::ObjectProxy* proxy_;
+  dbus::ObjectProxy* proxy_ = nullptr;
 
   // Note: This should remain the last member so that it will be destroyed
   // first, invalidating its weak pointers, before the other members are
   // destroyed.
-  base::WeakPtrFactory<PermissionBrokerClientImpl> weak_ptr_factory_;
+  base::WeakPtrFactory<PermissionBrokerClientImpl> weak_ptr_factory_{this};
 
   DISALLOW_COPY_AND_ASSIGN(PermissionBrokerClientImpl);
 };
 
-PermissionBrokerClient::PermissionBrokerClient() = default;
+PermissionBrokerClient::PermissionBrokerClient() {
+  DCHECK(!g_instance);
+  g_instance = this;
+}
+
+PermissionBrokerClient::~PermissionBrokerClient() {
+  DCHECK_EQ(this, g_instance);
+  g_instance = nullptr;
+}
+
+// static
+void PermissionBrokerClient::Initialize(dbus::Bus* bus) {
+  DCHECK(bus);
+  (new PermissionBrokerClientImpl())->Init(bus);
+}
 
-PermissionBrokerClient::~PermissionBrokerClient() = default;
+// static
+void PermissionBrokerClient::InitializeFake() {
+  new FakePermissionBrokerClient();
+}
+
+// static
+void PermissionBrokerClient::Shutdown() {
+  DCHECK(g_instance);
+  delete g_instance;
+}
 
-PermissionBrokerClient* PermissionBrokerClient::Create() {
-  return new PermissionBrokerClientImpl();
+// static
+PermissionBrokerClient* PermissionBrokerClient::Get() {
+  return g_instance;
 }
 
 }  // namespace chromeos

chromeos/dbus/permission_broker_client.h → chromeos/dbus/permission_broker/permission_broker_client.h

@@ -2,8 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef CHROMEOS_DBUS_PERMISSION_BROKER_CLIENT_H_
-#define CHROMEOS_DBUS_PERMISSION_BROKER_CLIENT_H_
+#ifndef CHROMEOS_DBUS_PERMISSION_BROKER_PERMISSION_BROKER_CLIENT_H_
+#define CHROMEOS_DBUS_PERMISSION_BROKER_PERMISSION_BROKER_CLIENT_H_
 
 #include <stdint.h>
 
@@ -13,7 +13,10 @@
 #include "base/component_export.h"
 #include "base/files/scoped_file.h"
 #include "base/macros.h"
-#include "chromeos/dbus/dbus_client.h"
+
+namespace dbus {
+class Bus;
+}
 
 namespace chromeos {
 
@@ -24,8 +27,7 @@ namespace chromeos {
 // which the user the browser runs under normally wouldn't have access to. For
 // more details on the permission broker see:
 // http://git.chromium.org/gitweb/?p=chromiumos/platform/permission_broker.git
-class COMPONENT_EXPORT(CHROMEOS_DBUS) PermissionBrokerClient
-    : public DBusClient {
+class COMPONENT_EXPORT(CHROMEOS_DBUS) PermissionBrokerClient {
  public:
   // The ResultCallback is used for both the RequestPathAccess and
   // RequestUsbAccess methods. Its boolean parameter represents the result of
@@ -41,9 +43,17 @@ class COMPONENT_EXPORT(CHROMEOS_DBUS) PermissionBrokerClient
                               const std::string& message)>
       ErrorCallback;
 
-  ~PermissionBrokerClient() override;
+  // Creates and initializes the global instance. |bus| must not be null.
+  static void Initialize(dbus::Bus* bus);
+
+  // Creates and initializes a fake global instance if not already created.
+  static void InitializeFake();
+
+  // Destroys the global instance which must have been initialized.
+  static void Shutdown();
 
-  static PermissionBrokerClient* Create();
+  // Returns the global instance if initialized. May return null.
+  static PermissionBrokerClient* Get();
 
   // CheckPathAccess requests a hint from the permission broker about whether
   // a later call to RequestPathAccess will be successful. It presumes that
@@ -96,7 +106,9 @@ class COMPONENT_EXPORT(CHROMEOS_DBUS) PermissionBrokerClient
                               const ResultCallback& callback) = 0;
 
  protected:
+  // Initialize/Shutdown should be used instead.
   PermissionBrokerClient();
+  virtual ~PermissionBrokerClient();
 
  private:
   DISALLOW_COPY_AND_ASSIGN(PermissionBrokerClient);
@@ -104,4 +116,4 @@ class COMPONENT_EXPORT(CHROMEOS_DBUS) PermissionBrokerClient
 
 }  // namespace chromeos
 
-#endif  // CHROMEOS_DBUS_PERMISSION_BROKER_CLIENT_H_
+#endif  // CHROMEOS_DBUS_PERMISSION_BROKER_PERMISSION_BROKER_CLIENT_H_

chromeos/network/firewall_hole.cc

@@ -14,8 +14,7 @@
 #include "base/location.h"
 #include "base/memory/ptr_util.h"
 #include "base/threading/thread_task_runner_handle.h"
-#include "chromeos/dbus/dbus_thread_manager.h"
-#include "chromeos/dbus/permission_broker_client.h"
+#include "chromeos/dbus/permission_broker/permission_broker_client.h"
 
 namespace chromeos {
 
@@ -65,8 +64,7 @@ void FirewallHole::Open(PortType type,
       base::Bind(&FirewallHole::PortAccessGranted, type, port, interface,
                  base::Passed(&lifeline_local), callback);
 
-  PermissionBrokerClient* client =
-      DBusThreadManager::Get()->GetPermissionBrokerClient();
+  PermissionBrokerClient* client = PermissionBrokerClient::Get();
   DCHECK(client) << "Could not get permission broker client.";
 
   switch (type) {
@@ -85,8 +83,7 @@ FirewallHole::~FirewallHole() {
   base::Callback<void(bool)> port_released_closure = base::Bind(
       &PortReleased, type_, port_, interface_, base::Passed(&lifeline_fd_));
 
-  PermissionBrokerClient* client =
-      DBusThreadManager::Get()->GetPermissionBrokerClient();
+  PermissionBrokerClient* client = PermissionBrokerClient::Get();
   DCHECK(client) << "Could not get permission broker client.";
   switch (type_) {
     case PortType::TCP:

chromeos/network/firewall_hole_unittest.cc

@@ -10,8 +10,7 @@
 #include "base/memory/ptr_util.h"
 #include "base/run_loop.h"
 #include "base/test/scoped_task_environment.h"
-#include "chromeos/dbus/dbus_thread_manager.h"
-#include "chromeos/dbus/fake_permission_broker_client.h"
+#include "chromeos/dbus/permission_broker/fake_permission_broker_client.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -32,14 +31,9 @@ class FirewallHoleTest : public testing::Test {
             base::test::ScopedTaskEnvironment::MainThreadType::UI) {}
   ~FirewallHoleTest() override = default;
 
-  void SetUp() override { DBusThreadManager::Initialize(); }
+  void SetUp() override { PermissionBrokerClient::InitializeFake(); }
 
-  void TearDown() override { DBusThreadManager::Shutdown(); }
-
-  FakePermissionBrokerClient* permission_broker_client() {
-    return static_cast<FakePermissionBrokerClient*>(
-        DBusThreadManager::Get()->GetPermissionBrokerClient());
-  }
+  void TearDown() override { PermissionBrokerClient::Shutdown(); }
 
  private:
   base::test::ScopedTaskEnvironment scoped_task_environment_;
@@ -52,13 +46,13 @@ TEST_F(FirewallHoleTest, GrantTcpPortAccess) {
                      base::Bind(&CopyFirewallHole, &run_loop, &hole));
   run_loop.Run();
   EXPECT_TRUE(hole.get());
-  EXPECT_TRUE(permission_broker_client()->HasTcpHole(1234, "foo0"));
+  EXPECT_TRUE(FakePermissionBrokerClient::Get()->HasTcpHole(1234, "foo0"));
   hole.reset();
-  EXPECT_FALSE(permission_broker_client()->HasTcpHole(1234, "foo0"));
+  EXPECT_FALSE(FakePermissionBrokerClient::Get()->HasTcpHole(1234, "foo0"));
 }
 
 TEST_F(FirewallHoleTest, DenyTcpPortAccess) {
-  permission_broker_client()->AddTcpDenyRule(1234, "foo0");
+  FakePermissionBrokerClient::Get()->AddTcpDenyRule(1234, "foo0");
 
   base::RunLoop run_loop;
   std::unique_ptr<FirewallHole> hole;
@@ -75,13 +69,13 @@ TEST_F(FirewallHoleTest, GrantUdpPortAccess) {
                      base::Bind(&CopyFirewallHole, &run_loop, &hole));
   run_loop.Run();
   EXPECT_TRUE(hole.get());
-  EXPECT_TRUE(permission_broker_client()->HasUdpHole(1234, "foo0"));
+  EXPECT_TRUE(FakePermissionBrokerClient::Get()->HasUdpHole(1234, "foo0"));
   hole.reset();
-  EXPECT_FALSE(permission_broker_client()->HasUdpHole(1234, "foo0"));
+  EXPECT_FALSE(FakePermissionBrokerClient::Get()->HasUdpHole(1234, "foo0"));
 }
 
 TEST_F(FirewallHoleTest, DenyUdpPortAccess) {
-  permission_broker_client()->AddUdpDenyRule(1234, "foo0");
+  FakePermissionBrokerClient::Get()->AddUdpDenyRule(1234, "foo0");
 
   base::RunLoop run_loop;
   std::unique_ptr<FirewallHole> hole;

components/arc/usb/usb_host_bridge.cc

@@ -9,8 +9,7 @@
 #include "base/bind.h"
 #include "base/logging.h"
 #include "base/memory/singleton.h"
-#include "chromeos/dbus/dbus_thread_manager.h"
-#include "chromeos/dbus/permission_broker_client.h"
+#include "chromeos/dbus/permission_broker/permission_broker_client.h"
 #include "components/arc/arc_browser_context_keyed_service_factory_base.h"
 #include "components/arc/arc_features.h"
 #include "components/arc/session/arc_bridge_service.h"
@@ -156,14 +155,11 @@ void ArcUsbHostBridge::OpenDevice(const std::string& guid,
     return;
   }
 
-  chromeos::PermissionBrokerClient* client =
-      chromeos::DBusThreadManager::Get()->GetPermissionBrokerClient();
-  DCHECK(client) << "Could not get permission broker client.";
   auto repeating_callback =
       base::AdaptCallbackForRepeating(std::move(callback));
-  client->OpenPath(device->device_path(),
-                   base::Bind(&OnDeviceOpened, repeating_callback),
-                   base::Bind(&OnDeviceOpenError, repeating_callback));
+  chromeos::PermissionBrokerClient::Get()->OpenPath(
+      device->device_path(), base::Bind(&OnDeviceOpened, repeating_callback),
+      base::Bind(&OnDeviceOpenError, repeating_callback));
 }
 
 void ArcUsbHostBridge::OpenDeviceDeprecated(

device/usb/mojo/device_manager_impl.cc

@@ -24,8 +24,7 @@
 #include "device/usb/usb_service.h"
 
 #if defined(OS_CHROMEOS)
-#include "chromeos/dbus/dbus_thread_manager.h"
-#include "chromeos/dbus/permission_broker_client.h"
+#include "chromeos/dbus/permission_broker/permission_broker_client.h"
 #include "device/usb/usb_device_linux.h"
 #endif  // defined(OS_CHROMEOS)
 
@@ -87,8 +86,6 @@ void DeviceManagerImpl::CheckAccess(const std::string& guid,
 void DeviceManagerImpl::OpenFileDescriptor(
     const std::string& guid,
     OpenFileDescriptorCallback callback) {
-  auto* client =
-      chromeos::DBusThreadManager::Get()->GetPermissionBrokerClient();
   scoped_refptr<UsbDevice> device = usb_service_->GetDevice(guid);
   if (!device) {
     LOG(ERROR) << "Was asked to open non-existent USB device: " << guid;
@@ -98,7 +95,7 @@ void DeviceManagerImpl::OpenFileDescriptor(
         base::AdaptCallbackForRepeating(std::move(callback));
     auto devpath =
         static_cast<device::UsbDeviceLinux*>(device.get())->device_path();
-    client->OpenPath(
+    chromeos::PermissionBrokerClient::Get()->OpenPath(
         devpath,
         base::BindRepeating(&DeviceManagerImpl::OnOpenFileDescriptor,
                             weak_factory_.GetWeakPtr(), copyable_callback),

device/usb/usb_device_linux.cc

@@ -21,8 +21,7 @@
 #include "device/usb/usb_service.h"
 
 #if defined(OS_CHROMEOS)
-#include "chromeos/dbus/dbus_thread_manager.h"
-#include "chromeos/dbus/permission_broker_client.h"
+#include "chromeos/dbus/permission_broker/permission_broker_client.h"
 #endif                             // defined(OS_CHROMEOS)
 
 namespace device {
@@ -49,11 +48,8 @@ UsbDeviceLinux::~UsbDeviceLinux() = default;
 
 void UsbDeviceLinux::CheckUsbAccess(ResultCallback callback) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  chromeos::PermissionBrokerClient* client =
-      chromeos::DBusThreadManager::Get()->GetPermissionBrokerClient();
-  DCHECK(client) << "Could not get permission broker client.";
-  client->CheckPathAccess(device_path_,
-                          base::AdaptCallbackForRepeating(std::move(callback)));
+  chromeos::PermissionBrokerClient::Get()->CheckPathAccess(
+      device_path_, base::AdaptCallbackForRepeating(std::move(callback)));
 }
 
 #endif  // defined(OS_CHROMEOS)
@@ -62,11 +58,8 @@ void UsbDeviceLinux::Open(OpenCallback callback) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
 #if defined(OS_CHROMEOS)
-  chromeos::PermissionBrokerClient* client =
-      chromeos::DBusThreadManager::Get()->GetPermissionBrokerClient();
   auto copyable_callback = base::AdaptCallbackForRepeating(std::move(callback));
-  DCHECK(client) << "Could not get permission broker client.";
-  client->OpenPath(
+  chromeos::PermissionBrokerClient::Get()->OpenPath(
       device_path_,
       base::Bind(&UsbDeviceLinux::OnOpenRequestComplete, this,
                  copyable_callback),

extensions/browser/api/device_permissions_prompt.cc

@@ -33,8 +33,7 @@
 #include "ui/base/l10n/l10n_util.h"
 
 #if defined(OS_CHROMEOS)
-#include "chromeos/dbus/dbus_thread_manager.h"
-#include "chromeos/dbus/permission_broker_client.h"
+#include "chromeos/dbus/permission_broker/permission_broker_client.h"
 #endif  // defined(OS_CHROMEOS)
 
 using device::HidDeviceFilter;
@@ -261,10 +260,7 @@ class HidDevicePermissionsPrompt : public DevicePermissionsPrompt::Prompt,
         (filters_.empty() || HidDeviceFilter::MatchesAny(*device, filters_))) {
       auto device_info = std::make_unique<HidDeviceInfo>(std::move(device));
 #if defined(OS_CHROMEOS)
-      chromeos::PermissionBrokerClient* client =
-          chromeos::DBusThreadManager::Get()->GetPermissionBrokerClient();
-      DCHECK(client) << "Could not get permission broker client.";
-      client->CheckPathAccess(
+      chromeos::PermissionBrokerClient::Get()->CheckPathAccess(
           device_info.get()->device()->device_node,
           base::Bind(&HidDevicePermissionsPrompt::AddCheckedDevice, this,
                      base::Passed(&device_info)));

extensions/browser/api/hid/hid_apitest.cc

@@ -21,6 +21,10 @@
 #include "services/device/public/mojom/hid.mojom.h"
 #include "services/service_manager/public/cpp/service_binding.h"
 
+#if defined(OS_CHROMEOS)
+#include "chromeos/dbus/permission_broker/fake_permission_broker_client.h"
+#endif  // defined(OS_CHROMEOS)
+
 using base::ThreadTaskRunnerHandle;
 using device::FakeHidManager;
 using device::HidReportDescriptor;
@@ -95,6 +99,10 @@ class TestExtensionsAPIClient : public ShellExtensionsAPIClient {
 class HidApiTest : public ShellApiTest {
  public:
   HidApiTest() {
+#if defined(OS_CHROMEOS)
+    // Required for DevicePermissionsPrompt:
+    chromeos::PermissionBrokerClient::InitializeFake();
+#endif
     // Because Device Service also runs in this process (browser process), we
     // can set our binder to intercept requests for HidManager interface to it.
     fake_hid_manager_ = std::make_unique<FakeHidManager>();
@@ -107,6 +115,9 @@ class HidApiTest : public ShellApiTest {
   ~HidApiTest() override {
     service_manager::ServiceBinding::ClearInterfaceBinderOverrideForTesting<
         device::mojom::HidManager>(device::mojom::kServiceName);
+#if defined(OS_CHROMEOS)
+    chromeos::PermissionBrokerClient::Shutdown();
+#endif
   }
 
   void SetUpOnMainThread() override {

services/device/hid/hid_service_linux.cc

@@ -34,8 +34,7 @@
 
 #if defined(OS_CHROMEOS)
 #include "base/system/sys_info.h"
-#include "chromeos/dbus/dbus_thread_manager.h"
-#include "chromeos/dbus/permission_broker_client.h"
+#include "chromeos/dbus/permission_broker/permission_broker_client.h"
 #endif  // defined(OS_CHROMEOS)
 
 namespace device {
@@ -224,13 +223,10 @@ void HidServiceLinux::Connect(const std::string& device_guid,
   auto params = std::make_unique<ConnectParams>(device_info, callback);
 
 #if defined(OS_CHROMEOS)
-  chromeos::PermissionBrokerClient* client =
-      chromeos::DBusThreadManager::Get()->GetPermissionBrokerClient();
-  DCHECK(client) << "Could not get permission broker client.";
   chromeos::PermissionBrokerClient::ErrorCallback error_callback =
       base::Bind(&HidServiceLinux::OnPathOpenError,
                  params->device_info->device_node(), params->callback);
-  client->OpenPath(
+  chromeos::PermissionBrokerClient::Get()->OpenPath(
       device_info->device_node(),
       base::Bind(&HidServiceLinux::OnPathOpenComplete, base::Passed(&params)),
       error_callback);

services/device/serial/serial_io_handler.cc

@@ -16,8 +16,7 @@
 #include "build/build_config.h"
 
 #if defined(OS_CHROMEOS)
-#include "chromeos/dbus/dbus_thread_manager.h"
-#include "chromeos/dbus/permission_broker_client.h"
+#include "chromeos/dbus/permission_broker/permission_broker_client.h"
 #endif  // defined(OS_CHROMEOS)
 
 namespace device {
@@ -49,8 +48,9 @@ void SerialIoHandler::Open(const mojom::SerialConnectionOptions& options,
   MergeConnectionOptions(options);
 
 #if defined(OS_CHROMEOS)
-  chromeos::PermissionBrokerClient* client =
-      chromeos::DBusThreadManager::Get()->GetPermissionBrokerClient();
+  // Note: dbus clients are destroyed in PostDestroyThreads so passing |client|
+  // as unretained is safe.
+  auto* client = chromeos::PermissionBrokerClient::Get();
   DCHECK(client) << "Could not get permission_broker client.";
   // PermissionBrokerClient should be called on the UI thread.
   scoped_refptr<base::SingleThreadTaskRunner> task_runner =