[CrOS MultiDevice] Complete SecureChannel initialization flow.

Initialization of the service is asynchronous due to the need to
fetch the Bluetooth adapter asynchronously. This class allows
clients to make requests of the service before it is fully
initializes; queued requests are then passed on to the rest of
the service once initialization completes.

Bug: 824568, 752273
Change-Id: I79b5940070b7c3ff6e0a9ed3f7ef4dff8f260038
Reviewed-on: https://chromium-review.googlesource.com/1105616Reviewed-by: Ryan Hansberry <hansberry@chromium.org>
Commit-Queue: Kyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#568568}

chromeos/services/secure_channel/BUILD.gn

@@ -95,6 +95,8 @@ static_library("secure_channel") {
     "secure_channel_disconnector_impl.h",
     "secure_channel_impl.cc",
     "secure_channel_impl.h",
+    "secure_channel_initializer.cc",
+    "secure_channel_initializer.h",
     "secure_channel_service.cc",
     "secure_channel_service.h",
     "shared_resource_scheduler.cc",

chromeos/services/secure_channel/public/cpp/client/secure_channel_client_impl_unittest.cc

@@ -21,7 +21,7 @@
 #include "chromeos/services/secure_channel/public/cpp/client/fake_connection_attempt.h"
 #include "chromeos/services/secure_channel/public/mojom/constants.mojom.h"
 #include "chromeos/services/secure_channel/public/mojom/secure_channel.mojom.h"
-#include "chromeos/services/secure_channel/secure_channel_impl.h"
+#include "chromeos/services/secure_channel/secure_channel_initializer.h"
 #include "chromeos/services/secure_channel/secure_channel_service.h"
 #include "components/cryptauth/remote_device_test_util.h"
 #include "services/service_manager/public/cpp/test/test_connector_factory.h"
@@ -35,16 +35,18 @@ namespace {
 
 const size_t kNumTestDevices = 5u;
 
-class FakeSecureChannelImplFactory : public SecureChannelImpl::Factory {
+class FakeSecureChannelInitializerFactory
+    : public SecureChannelInitializer::Factory {
  public:
-  explicit FakeSecureChannelImplFactory(
+  explicit FakeSecureChannelInitializerFactory(
       std::unique_ptr<FakeSecureChannel> fake_secure_channel)
       : fake_secure_channel_(std::move(fake_secure_channel)) {}
 
-  ~FakeSecureChannelImplFactory() override = default;
+  ~FakeSecureChannelInitializerFactory() override = default;
 
-  // SecureChannelImpl::Factory:
-  std::unique_ptr<SecureChannelBase> BuildInstance() override {
+  // SecureChannelInitializer::Factory:
+  std::unique_ptr<SecureChannelBase> BuildInstance(
+      scoped_refptr<base::TaskRunner> task_runner) override {
     EXPECT_TRUE(fake_secure_channel_);
     return std::move(fake_secure_channel_);
   }
@@ -126,11 +128,11 @@ class SecureChannelClientImplTest : public testing::Test {
   void SetUp() override {
     auto fake_secure_channel = std::make_unique<FakeSecureChannel>();
     fake_secure_channel_ = fake_secure_channel.get();
-    fake_secure_channel_impl_factory_ =
-        std::make_unique<FakeSecureChannelImplFactory>(
+    fake_secure_channel_initializer_factory_ =
+        std::make_unique<FakeSecureChannelInitializerFactory>(
             std::move(fake_secure_channel));
-    SecureChannelImpl::Factory::SetFactoryForTesting(
-        fake_secure_channel_impl_factory_.get());
+    SecureChannelInitializer::Factory::SetFactoryForTesting(
+        fake_secure_channel_initializer_factory_.get());
 
     fake_connection_attempt_factory_ =
         std::make_unique<FakeConnectionAttemptFactory>();
@@ -158,7 +160,7 @@ class SecureChannelClientImplTest : public testing::Test {
   }
 
   void TearDown() override {
-    SecureChannelImpl::Factory::SetFactoryForTesting(nullptr);
+    SecureChannelInitializer::Factory::SetFactoryForTesting(nullptr);
   }
 
   std::unique_ptr<FakeConnectionAttempt> CallListenForConnectionFromDevice(
@@ -206,8 +208,8 @@ class SecureChannelClientImplTest : public testing::Test {
   const base::test::ScopedTaskEnvironment scoped_task_environment_;
 
   FakeSecureChannel* fake_secure_channel_;
-  std::unique_ptr<FakeSecureChannelImplFactory>
-      fake_secure_channel_impl_factory_;
+  std::unique_ptr<FakeSecureChannelInitializerFactory>
+      fake_secure_channel_initializer_factory_;
   std::unique_ptr<FakeConnectionAttemptFactory>
       fake_connection_attempt_factory_;
   std::unique_ptr<FakeClientChannelImplFactory>

chromeos/services/secure_channel/secure_channel_impl.cc

@@ -12,10 +12,14 @@
 #include "base/stl_util.h"
 #include "chromeos/components/proximity_auth/logging/logging.h"
 #include "chromeos/services/secure_channel/active_connection_manager_impl.h"
+#include "chromeos/services/secure_channel/ble_connection_manager_impl.h"
+#include "chromeos/services/secure_channel/ble_service_data_helper_impl.h"
 #include "chromeos/services/secure_channel/client_connection_parameters_impl.h"
 #include "chromeos/services/secure_channel/device_id_pair.h"
 #include "chromeos/services/secure_channel/pending_connection_manager_impl.h"
 #include "chromeos/services/secure_channel/public/cpp/shared/authenticated_channel.h"
+#include "chromeos/services/secure_channel/timer_factory_impl.h"
+#include "device/bluetooth/bluetooth_adapter.h"
 
 namespace chromeos {
 
@@ -40,8 +44,9 @@ void SecureChannelImpl::Factory::SetFactoryForTesting(Factory* test_factory) {
 
 SecureChannelImpl::Factory::~Factory() = default;
 
-std::unique_ptr<SecureChannelBase> SecureChannelImpl::Factory::BuildInstance() {
-  return base::WrapUnique(new SecureChannelImpl());
+std::unique_ptr<mojom::SecureChannel> SecureChannelImpl::Factory::BuildInstance(
+    scoped_refptr<device::BluetoothAdapter> bluetooth_adapter) {
+  return base::WrapUnique(new SecureChannelImpl(bluetooth_adapter));
 }
 
 SecureChannelImpl::ConnectionRequestWaitingForDisconnection::
@@ -64,17 +69,26 @@ SecureChannelImpl::ConnectionRequestWaitingForDisconnection::operator=(
 SecureChannelImpl::ConnectionRequestWaitingForDisconnection::
     ~ConnectionRequestWaitingForDisconnection() = default;
 
-SecureChannelImpl::SecureChannelImpl()
-    : active_connection_manager_(
-          ActiveConnectionManagerImpl::Factory::Get()->BuildInstance(
-              this /* delegate */)),
+SecureChannelImpl::SecureChannelImpl(
+    scoped_refptr<device::BluetoothAdapter> bluetooth_adapter)
+    : timer_factory_(TimerFactoryImpl::Factory::Get()->BuildInstance()),
+      remote_device_cache_(
+          cryptauth::RemoteDeviceCache::Factory::Get()->BuildInstance()),
+      ble_service_data_helper_(
+          BleServiceDataHelperImpl::Factory::Get()->BuildInstance(
+              remote_device_cache_.get())),
+      ble_connection_manager_(
+          BleConnectionManagerImpl::Factory::Get()->BuildInstance(
+              bluetooth_adapter,
+              ble_service_data_helper_.get(),
+              timer_factory_.get())),
       pending_connection_manager_(
           PendingConnectionManagerImpl::Factory::Get()->BuildInstance(
               this /* delegate */,
-              // TODO(khorimoto): Pass the actual BleConnectionManager here.
-              nullptr /* ble_connection_manager */)),
-      remote_device_cache_(
-          cryptauth::RemoteDeviceCache::Factory::Get()->BuildInstance()) {}
+              ble_connection_manager_.get())),
+      active_connection_manager_(
+          ActiveConnectionManagerImpl::Factory::Get()->BuildInstance(
+              this /* delegate */)) {}
 
 SecureChannelImpl::~SecureChannelImpl() = default;
 

chromeos/services/secure_channel/secure_channel_impl.h

@@ -17,20 +17,27 @@
 #include "chromeos/services/secure_channel/pending_connection_manager.h"
 #include "chromeos/services/secure_channel/public/cpp/shared/connection_priority.h"
 #include "chromeos/services/secure_channel/public/mojom/secure_channel.mojom.h"
-#include "chromeos/services/secure_channel/secure_channel_base.h"
 #include "components/cryptauth/remote_device_cache.h"
 
+namespace device {
+class BluetoothAdapter;
+}  // namespace device
+
 namespace chromeos {
 
 namespace secure_channel {
 
+class BleConnectionManager;
+class BleServiceDataHelper;
+class TimerFactory;
+
 // Concrete SecureChannelImpl implementation, which contains three pieces:
 // (1) PendingConnectionManager: Attempts to create connections to remote
 //     devices.
 // (2) ActiveConnectionManager: Maintains connections to remote devices, sharing
 //     a single connection with multiple clients when appropriate.
 // (3) RemoteDeviceCache: Caches devices within this service.
-class SecureChannelImpl : public SecureChannelBase,
+class SecureChannelImpl : public mojom::SecureChannel,
                           public ActiveConnectionManager::Delegate,
                           public PendingConnectionManager::Delegate {
  public:
@@ -39,7 +46,8 @@ class SecureChannelImpl : public SecureChannelBase,
     static Factory* Get();
     static void SetFactoryForTesting(Factory* test_factory);
     virtual ~Factory();
-    virtual std::unique_ptr<SecureChannelBase> BuildInstance();
+    virtual std::unique_ptr<mojom::SecureChannel> BuildInstance(
+        scoped_refptr<device::BluetoothAdapter> bluetooth_adapter);
 
    private:
     static Factory* test_factory_;
@@ -48,7 +56,7 @@ class SecureChannelImpl : public SecureChannelBase,
   ~SecureChannelImpl() override;
 
  private:
-  SecureChannelImpl();
+  SecureChannelImpl(scoped_refptr<device::BluetoothAdapter> bluetooth_adapter);
 
   enum class InvalidRemoteDeviceReason { kInvalidPublicKey, kInvalidPsk };
 
@@ -135,9 +143,12 @@ class SecureChannelImpl : public SecureChannelBase,
       ApiFunctionName api_fn_name,
       const cryptauth::RemoteDevice& device);
 
-  std::unique_ptr<ActiveConnectionManager> active_connection_manager_;
-  std::unique_ptr<PendingConnectionManager> pending_connection_manager_;
+  std::unique_ptr<TimerFactory> timer_factory_;
   std::unique_ptr<cryptauth::RemoteDeviceCache> remote_device_cache_;
+  std::unique_ptr<BleServiceDataHelper> ble_service_data_helper_;
+  std::unique_ptr<BleConnectionManager> ble_connection_manager_;
+  std::unique_ptr<PendingConnectionManager> pending_connection_manager_;
+  std::unique_ptr<ActiveConnectionManager> active_connection_manager_;
 
   base::flat_map<ConnectionDetails,
                  std::vector<ConnectionRequestWaitingForDisconnection>>

chromeos/services/secure_channel/secure_channel_initializer.cc

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chromeos/services/secure_channel/secure_channel_initializer.h"
+
+#include "base/memory/ptr_util.h"
+#include "base/no_destructor.h"
+#include "chromeos/components/proximity_auth/logging/logging.h"
+#include "chromeos/services/secure_channel/secure_channel_impl.h"
+#include "device/bluetooth/bluetooth_adapter_factory.h"
+
+namespace chromeos {
+
+namespace secure_channel {
+
+// static
+SecureChannelInitializer::Factory*
+    SecureChannelInitializer::Factory::test_factory_ = nullptr;
+
+// static
+SecureChannelInitializer::Factory* SecureChannelInitializer::Factory::Get() {
+  if (test_factory_)
+    return test_factory_;
+
+  static base::NoDestructor<Factory> factory;
+  return factory.get();
+}
+
+// static
+void SecureChannelInitializer::Factory::SetFactoryForTesting(
+    Factory* test_factory) {
+  test_factory_ = test_factory;
+}
+
+SecureChannelInitializer::Factory::~Factory() = default;
+
+std::unique_ptr<SecureChannelBase>
+SecureChannelInitializer::Factory::BuildInstance(
+    scoped_refptr<base::TaskRunner> task_runner) {
+  return base::WrapUnique(new SecureChannelInitializer(task_runner));
+}
+
+SecureChannelInitializer::ConnectionRequestArgs::ConnectionRequestArgs(
+    const cryptauth::RemoteDevice& device_to_connect,
+    const cryptauth::RemoteDevice& local_device,
+    const std::string& feature,
+    ConnectionPriority connection_priority,
+    mojom::ConnectionDelegatePtr delegate,
+    bool is_listen_request)
+    : device_to_connect(device_to_connect),
+      local_device(local_device),
+      feature(feature),
+      connection_priority(connection_priority),
+      delegate(std::move(delegate)),
+      is_listen_request(is_listen_request) {}
+
+SecureChannelInitializer::ConnectionRequestArgs::~ConnectionRequestArgs() =
+    default;
+
+SecureChannelInitializer::SecureChannelInitializer(
+    scoped_refptr<base::TaskRunner> task_runner)
+    : weak_ptr_factory_(this) {
+  PA_LOG(INFO) << "SecureChannelInitializer::SecureChannelInitializer(): "
+               << "Fetching Bluetooth adapter. All requests received before "
+               << "the adapter is fetched will be queued.";
+
+  // device::BluetoothAdapterFactory::SetAdapterForTesting() causes the
+  // GetAdapter() callback to return synchronously. Thus, post the GetAdapter()
+  // call as a task to ensure that it is returned asynchronously, even in tests.
+  task_runner->PostTask(
+      FROM_HERE,
+      base::Bind(
+          device::BluetoothAdapterFactory::GetAdapter,
+          base::Bind(&SecureChannelInitializer::OnBluetoothAdapterReceived,
+                     weak_ptr_factory_.GetWeakPtr())));
+}
+
+SecureChannelInitializer::~SecureChannelInitializer() = default;
+
+void SecureChannelInitializer::ListenForConnectionFromDevice(
+    const cryptauth::RemoteDevice& device_to_connect,
+    const cryptauth::RemoteDevice& local_device,
+    const std::string& feature,
+    ConnectionPriority connection_priority,
+    mojom::ConnectionDelegatePtr delegate) {
+  if (secure_channel_impl_) {
+    secure_channel_impl_->ListenForConnectionFromDevice(
+        device_to_connect, local_device, feature, connection_priority,
+        std::move(delegate));
+    return;
+  }
+
+  pending_args_.push(std::make_unique<ConnectionRequestArgs>(
+      device_to_connect, local_device, feature, connection_priority,
+      std::move(delegate), true /* is_listen_request */));
+}
+
+void SecureChannelInitializer::InitiateConnectionToDevice(
+    const cryptauth::RemoteDevice& device_to_connect,
+    const cryptauth::RemoteDevice& local_device,
+    const std::string& feature,
+    ConnectionPriority connection_priority,
+    mojom::ConnectionDelegatePtr delegate) {
+  if (secure_channel_impl_) {
+    secure_channel_impl_->InitiateConnectionToDevice(
+        device_to_connect, local_device, feature, connection_priority,
+        std::move(delegate));
+    return;
+  }
+
+  pending_args_.push(std::make_unique<ConnectionRequestArgs>(
+      device_to_connect, local_device, feature, connection_priority,
+      std::move(delegate), false /* is_listen_request */));
+}
+
+void SecureChannelInitializer::OnBluetoothAdapterReceived(
+    scoped_refptr<device::BluetoothAdapter> bluetooth_adapter) {
+  PA_LOG(INFO) << "SecureChannelInitializer::OnBluetoothAdapterReceived(): "
+               << "Bluetooth adapter has been fetched. Passing all queued "
+               << "requests to the service.";
+
+  secure_channel_impl_ =
+      SecureChannelImpl::Factory::Get()->BuildInstance(bluetooth_adapter);
+
+  while (!pending_args_.empty()) {
+    std::unique_ptr<ConnectionRequestArgs> args_to_pass =
+        std::move(pending_args_.front());
+    pending_args_.pop();
+
+    if (args_to_pass->is_listen_request) {
+      secure_channel_impl_->ListenForConnectionFromDevice(
+          args_to_pass->device_to_connect, args_to_pass->local_device,
+          args_to_pass->feature, args_to_pass->connection_priority,
+          std::move(args_to_pass->delegate));
+      continue;
+    }
+
+    secure_channel_impl_->InitiateConnectionToDevice(
+        args_to_pass->device_to_connect, args_to_pass->local_device,
+        args_to_pass->feature, args_to_pass->connection_priority,
+        std::move(args_to_pass->delegate));
+  }
+}
+
+}  // namespace secure_channel
+
+}  // namespace chromeos

chromeos/services/secure_channel/secure_channel_initializer.h

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROMEOS_SERVICES_SECURE_CHANNEL_SECURE_CHANNEL_INITIALIZER_H_
+#define CHROMEOS_SERVICES_SECURE_CHANNEL_SECURE_CHANNEL_INITIALIZER_H_
+
+#include <memory>
+#include <queue>
+#include <string>
+
+#include "base/macros.h"
+#include "base/memory/ref_counted.h"
+#include "base/memory/weak_ptr.h"
+#include "base/threading/thread_task_runner_handle.h"
+#include "chromeos/services/secure_channel/public/mojom/secure_channel.mojom.h"
+#include "chromeos/services/secure_channel/secure_channel_base.h"
+
+namespace device {
+class BluetoothAdapter;
+}  // namespace device
+
+namespace chromeos {
+
+namespace secure_channel {
+
+// SecureChannelBase implementation which fetches the Bluetooth adapter, then
+// initializes the rest of the service. Initialization of the service is
+// asynchronous due to the need to fetch the Bluetooth adapter asynchronously.
+// This class allows clients to make requests of the service before it is fully
+// initializes; queued requests are then passed on to the rest of the service
+// once initialization completes.
+class SecureChannelInitializer : public SecureChannelBase {
+ public:
+  class Factory {
+   public:
+    static Factory* Get();
+    static void SetFactoryForTesting(Factory* test_factory);
+    virtual ~Factory();
+    virtual std::unique_ptr<SecureChannelBase> BuildInstance(
+        scoped_refptr<base::TaskRunner> task_runner =
+            base::ThreadTaskRunnerHandle::Get());
+
+   private:
+    static Factory* test_factory_;
+  };
+
+  ~SecureChannelInitializer() override;
+
+ private:
+  SecureChannelInitializer(scoped_refptr<base::TaskRunner> task_runner);
+
+  struct ConnectionRequestArgs {
+    ConnectionRequestArgs(const cryptauth::RemoteDevice& device_to_connect,
+                          const cryptauth::RemoteDevice& local_device,
+                          const std::string& feature,
+                          ConnectionPriority connection_priority,
+                          mojom::ConnectionDelegatePtr delegate,
+                          bool is_listen_request);
+    ~ConnectionRequestArgs();
+
+    cryptauth::RemoteDevice device_to_connect;
+    cryptauth::RemoteDevice local_device;
+    std::string feature;
+    ConnectionPriority connection_priority;
+    mojom::ConnectionDelegatePtr delegate;
+    bool is_listen_request;
+  };
+
+  // mojom::SecureChannel:
+  void ListenForConnectionFromDevice(
+      const cryptauth::RemoteDevice& device_to_connect,
+      const cryptauth::RemoteDevice& local_device,
+      const std::string& feature,
+      ConnectionPriority connection_priority,
+      mojom::ConnectionDelegatePtr delegate) override;
+  void InitiateConnectionToDevice(
+      const cryptauth::RemoteDevice& device_to_connect,
+      const cryptauth::RemoteDevice& local_device,
+      const std::string& feature,
+      ConnectionPriority connection_priority,
+      mojom::ConnectionDelegatePtr delegate) override;
+
+  void OnBluetoothAdapterReceived(
+      scoped_refptr<device::BluetoothAdapter> bluetooth_adapter);
+
+  std::queue<std::unique_ptr<ConnectionRequestArgs>> pending_args_;
+  std::unique_ptr<mojom::SecureChannel> secure_channel_impl_;
+
+  base::WeakPtrFactory<SecureChannelInitializer> weak_ptr_factory_;
+
+  DISALLOW_COPY_AND_ASSIGN(SecureChannelInitializer);
+};
+
+}  // namespace secure_channel
+
+}  // namespace chromeos
+
+#endif  // CHROMEOS_SERVICES_SECURE_CHANNEL_SECURE_CHANNEL_INITIALIZER_H_

chromeos/services/secure_channel/secure_channel_service.cc

@@ -5,7 +5,7 @@
 #include "chromeos/services/secure_channel/secure_channel_service.h"
 
 #include "chromeos/components/proximity_auth/logging/logging.h"
-#include "chromeos/services/secure_channel/secure_channel_impl.h"
+#include "chromeos/services/secure_channel/secure_channel_initializer.h"
 
 namespace chromeos {
 
@@ -24,7 +24,7 @@ SecureChannelService::~SecureChannelService() = default;
 void SecureChannelService::OnStart() {
   PA_LOG(INFO) << "SecureChannelService::OnStart()";
 
-  secure_channel_ = SecureChannelImpl::Factory::Get()->BuildInstance();
+  secure_channel_ = SecureChannelInitializer::Factory::Get()->BuildInstance();
 
   registry_.AddInterface(
       base::BindRepeating(&SecureChannelBase::BindRequest,