[CrOS MultiDevice] Create ConnectToDeviceOperationFactory.

The base class for this factory ensures that only one operation instance
is active at any time.

Bug: 824568, 752273
Change-Id: Ia0b4220aef2f021802b7f43ead7b2915a7cf983a
Reviewed-on: https://chromium-review.googlesource.com/1054097
Commit-Queue: Ryan Hansberry <hansberry@chromium.org>
Reviewed-by: Ryan Hansberry <hansberry@chromium.org>
Cr-Commit-Position: refs/heads/master@{#558382}

chromeos/services/secure_channel/BUILD.gn

@@ -8,6 +8,8 @@ static_library("secure_channel") {
   sources = [
     "connect_to_device_operation.h",
     "connect_to_device_operation_base.h",
+    "connect_to_device_operation_factory.h",
+    "connect_to_device_operation_factory_base.h",
     "pending_connection_request.h",
     "pending_connection_request_base.h",
     "pending_connection_request_delegate.cc",
@@ -28,6 +30,8 @@ static_library("test_support") {
   sources = [
     "fake_connect_to_device_operation.cc",
     "fake_connect_to_device_operation.h",
+    "fake_connect_to_device_operation_factory.cc",
+    "fake_connect_to_device_operation_factory.h",
     "fake_connection_delegate.cc",
     "fake_connection_delegate.h",
     "fake_pending_connection_request.cc",
@@ -48,6 +52,7 @@ source_set("unit_tests") {
 
   sources = [
     "connect_to_device_operation_base_unittest.cc",
+    "connect_to_device_operation_factory_base_unittest.cc",
     "pending_connection_request_base_unittest.cc",
   ]
 

chromeos/services/secure_channel/connect_to_device_operation_factory.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_CONNECT_TO_DEVICE_OPERATION_FACTORY_H_
+#define CHROMEOS_SERVICES_SECURE_CHANNEL_CONNECT_TO_DEVICE_OPERATION_FACTORY_H_
+
+#include "base/callback_forward.h"
+#include "base/macros.h"
+#include "chromeos/services/secure_channel/connect_to_device_operation.h"
+
+namespace chromeos {
+
+namespace secure_channel {
+
+// Factory for creating ConnectToDeviceOperation instances.
+template <typename FailureDetailType>
+class ConnectToDeviceOperationFactory {
+ public:
+  virtual ~ConnectToDeviceOperationFactory() = default;
+
+  virtual std::unique_ptr<ConnectToDeviceOperation<FailureDetailType>>
+  CreateOperation(
+      typename ConnectToDeviceOperation<
+          FailureDetailType>::ConnectionSuccessCallback success_callback,
+      typename ConnectToDeviceOperation<
+          FailureDetailType>::ConnectionFailedCallback failure_callback) = 0;
+
+ protected:
+  ConnectToDeviceOperationFactory() = default;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(ConnectToDeviceOperationFactory);
+};
+
+}  // namespace secure_channel
+
+}  // namespace chromeos
+
+#endif  // CHROMEOS_SERVICES_SECURE_CHANNEL_CONNECT_TO_DEVICE_OPERATION_FACTORY_H_

chromeos/services/secure_channel/connect_to_device_operation_factory_base.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_CONNECT_TO_DEVICE_OPERATION_FACTORY_BASE_H_
+#define CHROMEOS_SERVICES_SECURE_CHANNEL_CONNECT_TO_DEVICE_OPERATION_FACTORY_BASE_H_
+
+#include "base/bind.h"
+#include "base/callback_forward.h"
+#include "base/logging.h"
+#include "base/macros.h"
+#include "base/memory/weak_ptr.h"
+#include "chromeos/services/secure_channel/connect_to_device_operation_factory.h"
+#include "components/cryptauth/remote_device.h"
+
+namespace chromeos {
+
+namespace secure_channel {
+
+// ConnectToDeviceOperationFactory implementation, which ensures that only one
+// operation can be active at a time.
+template <typename FailureDetailType>
+class ConnectToDeviceOperationFactoryBase
+    : public ConnectToDeviceOperationFactory<FailureDetailType> {
+ public:
+  ~ConnectToDeviceOperationFactoryBase() override = default;
+
+ protected:
+  ConnectToDeviceOperationFactoryBase(
+      const cryptauth::RemoteDevice& device_to_connect_to)
+      : device_to_connect_to_(device_to_connect_to), weak_ptr_factory_(this) {}
+
+  // Derived types should overload this function, passing the provided
+  // parameters to the constructor of a type derived from
+  // ConnectToDeviceOperationBase.
+  virtual std::unique_ptr<ConnectToDeviceOperation<FailureDetailType>>
+  PerformCreateOperation(
+      typename ConnectToDeviceOperation<
+          FailureDetailType>::ConnectionSuccessCallback success_callback,
+      typename ConnectToDeviceOperation<
+          FailureDetailType>::ConnectionFailedCallback failure_callback,
+      const cryptauth::RemoteDevice& device_to_connect_to,
+      base::OnceClosure destructor_callback) = 0;
+
+ private:
+  // ConnectToDeviceOperationFactory<FailureDetailType>:
+  std::unique_ptr<ConnectToDeviceOperation<FailureDetailType>> CreateOperation(
+      typename ConnectToDeviceOperation<
+          FailureDetailType>::ConnectionSuccessCallback success_callback,
+      typename ConnectToDeviceOperation<FailureDetailType>::
+          ConnectionFailedCallback failure_callback) override {
+    if (is_last_operation_active_) {
+      PA_LOG(ERROR) << "ConnectToDeviceOperationFactoryBase::CreateOperation():"
+                    << " Requested new operation before the previous one was "
+                    << "finished.";
+      NOTREACHED();
+      return nullptr;
+    }
+
+    is_last_operation_active_ = true;
+    return PerformCreateOperation(
+        std::move(success_callback), std::move(failure_callback),
+        device_to_connect_to_,
+        base::BindOnce(&ConnectToDeviceOperationFactoryBase<
+                           FailureDetailType>::OnPreviousOperationDeleted,
+                       weak_ptr_factory_.GetWeakPtr()));
+  }
+
+  void OnPreviousOperationDeleted() { is_last_operation_active_ = false; }
+
+  const cryptauth::RemoteDevice device_to_connect_to_;
+  bool is_last_operation_active_ = false;
+
+  base::WeakPtrFactory<ConnectToDeviceOperationFactoryBase> weak_ptr_factory_;
+
+  DISALLOW_COPY_AND_ASSIGN(ConnectToDeviceOperationFactoryBase);
+};
+
+}  // namespace secure_channel
+
+}  // namespace chromeos
+
+#endif  // CHROMEOS_SERVICES_SECURE_CHANNEL_CONNECT_TO_DEVICE_OPERATION_FACTORY_BASE_H_

chromeos/services/secure_channel/connect_to_device_operation_factory_base_unittest.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/connect_to_device_operation_factory_base.h"
+
+#include <memory>
+
+#include "base/bind_helpers.h"
+#include "base/test/gtest_util.h"
+#include "chromeos/services/secure_channel/fake_connect_to_device_operation.h"
+#include "components/cryptauth/remote_device_test_util.h"
+#include "components/cryptauth/secure_channel.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace chromeos {
+
+namespace secure_channel {
+
+namespace {
+
+// Since ConnectToDeviceOperationFactoryBase is templatized, a concrete
+// implementation is needed for its test.
+class TestConnectToDeviceOperationFactory
+    : public ConnectToDeviceOperationFactoryBase<std::string> {
+ public:
+  TestConnectToDeviceOperationFactory(
+      const cryptauth::RemoteDevice& device_to_connect_to)
+      : ConnectToDeviceOperationFactoryBase<std::string>(device_to_connect_to),
+        remote_device_(device_to_connect_to) {}
+
+  ~TestConnectToDeviceOperationFactory() override = default;
+
+  void InvokePendingDestructorCallback() {
+    EXPECT_TRUE(last_destructor_callback_);
+    std::move(last_destructor_callback_).Run();
+  }
+
+  // ConnectToDeviceOperationFactoryBase<std::string>:
+  std::unique_ptr<ConnectToDeviceOperation<std::string>> PerformCreateOperation(
+      typename ConnectToDeviceOperation<std::string>::ConnectionSuccessCallback
+          success_callback,
+      typename ConnectToDeviceOperation<std::string>::ConnectionFailedCallback
+          failure_callback,
+      const cryptauth::RemoteDevice& device_to_connect_to,
+      base::OnceClosure destructor_callback) override {
+    // The previous destructor callback should have been invoked by the time
+    // this function runs.
+    EXPECT_FALSE(last_destructor_callback_);
+    last_destructor_callback_ = std::move(destructor_callback);
+
+    EXPECT_EQ(remote_device_, device_to_connect_to);
+
+    return std::make_unique<FakeConnectToDeviceOperation>(
+        std::move(success_callback), std::move(failure_callback));
+  }
+
+ private:
+  const cryptauth::RemoteDevice remote_device_;
+
+  base::OnceClosure last_destructor_callback_;
+};
+
+}  // namespace
+
+class SecureChannelConnectToDeviceOperationFactoryBaseTest
+    : public testing::Test {
+ protected:
+  SecureChannelConnectToDeviceOperationFactoryBaseTest()
+      : test_device_(cryptauth::CreateRemoteDeviceForTest()) {}
+
+  ~SecureChannelConnectToDeviceOperationFactoryBaseTest() override = default;
+
+  void SetUp() override {
+    test_factory_ =
+        std::make_unique<TestConnectToDeviceOperationFactory>(test_device_);
+  }
+
+  std::unique_ptr<ConnectToDeviceOperation<std::string>> CallCreateOperation() {
+    // Use a pointer to ConnectToDeviceOperationFactory, since
+    // ConnectToDeviceOperationFactoryBase makes CreateOperation private.
+    ConnectToDeviceOperationFactory<std::string>* factory = test_factory_.get();
+    return factory->CreateOperation(base::DoNothing() /* success_callback */,
+                                    base::DoNothing() /* failure_callback */);
+  }
+
+  void FinishActiveOperation(
+      std::unique_ptr<ConnectToDeviceOperation<std::string>>
+          operation_to_finish) {
+    EXPECT_TRUE(operation_to_finish);
+
+    // Use a pointer to FakeConnectToDeviceOperation, since
+    // ConnectToDeviceOperation makes OnSuccessfulConnectionAttempt() protected.
+    FakeConnectToDeviceOperation* operation =
+        static_cast<FakeConnectToDeviceOperation*>(operation_to_finish.get());
+    operation->OnFailedConnectionAttempt(
+        "arbitraryFailureDetail" /* failure_detail */);
+
+    test_factory_->InvokePendingDestructorCallback();
+  }
+
+ private:
+  const cryptauth::RemoteDevice test_device_;
+
+  std::unique_ptr<TestConnectToDeviceOperationFactory> test_factory_;
+
+  DISALLOW_COPY_AND_ASSIGN(
+      SecureChannelConnectToDeviceOperationFactoryBaseTest);
+};
+
+TEST_F(SecureChannelConnectToDeviceOperationFactoryBaseTest,
+       UseFactoryCorrectly) {
+  auto operation1 = CallCreateOperation();
+  EXPECT_TRUE(operation1);
+
+  // Invoke the destructor callback and try again.
+  FinishActiveOperation(std::move(operation1));
+  auto operation2 = CallCreateOperation();
+  EXPECT_TRUE(operation2);
+
+  // Invoke the destructor callback and try again.
+  FinishActiveOperation(std::move(operation2));
+  auto operation3 = CallCreateOperation();
+  EXPECT_TRUE(operation3);
+
+  FinishActiveOperation(std::move(operation3));
+}
+
+TEST_F(SecureChannelConnectToDeviceOperationFactoryBaseTest,
+       UseFactoryIncorrectly) {
+  // First operation should always complete successfully.
+  auto operation = CallCreateOperation();
+  EXPECT_TRUE(operation);
+
+  // The first operation's destructor callback has not yet been invoked, so
+  // creating another should fail.
+  EXPECT_DCHECK_DEATH(CallCreateOperation());
+
+  FinishActiveOperation(std::move(operation));
+}
+
+}  // namespace secure_channel
+
+}  // namespace chromeos

chromeos/services/secure_channel/fake_connect_to_device_operation_factory.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/fake_connect_to_device_operation_factory.h"
+
+#include "chromeos/services/secure_channel/fake_connect_to_device_operation.h"
+
+namespace chromeos {
+
+namespace secure_channel {
+
+FakeConnectToDeviceOperationFactory::FakeConnectToDeviceOperationFactory() =
+    default;
+
+FakeConnectToDeviceOperationFactory::~FakeConnectToDeviceOperationFactory() =
+    default;
+
+std::unique_ptr<ConnectToDeviceOperation<std::string>>
+FakeConnectToDeviceOperationFactory::CreateOperation(
+    ConnectToDeviceOperation<std::string>::ConnectionSuccessCallback
+        success_callback,
+    ConnectToDeviceOperation<std::string>::ConnectionFailedCallback
+        failure_callback) {
+  auto instance = std::make_unique<FakeConnectToDeviceOperation>(
+      std::move(success_callback), std::move(failure_callback));
+  last_created_instance_ = instance.get();
+  ++num_instances_created_;
+  return instance;
+}
+
+}  // namespace secure_channel
+
+}  // namespace chromeos

chromeos/services/secure_channel/fake_connect_to_device_operation_factory.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_FAKE_CONNECT_TO_DEVICE_OPERATION_FACTORY_H_
+#define CHROMEOS_SERVICES_SECURE_CHANNEL_FAKE_CONNECT_TO_DEVICE_OPERATION_FACTORY_H_
+
+#include <string>
+
+#include "base/callback_forward.h"
+#include "base/macros.h"
+#include "chromeos/services/secure_channel/connect_to_device_operation_factory.h"
+
+namespace chromeos {
+
+namespace secure_channel {
+
+class FakeConnectToDeviceOperation;
+
+// Fake ConnectToDeviceOperationFactory implementation, whose FailureDetailType
+// is std::string.
+class FakeConnectToDeviceOperationFactory
+    : public ConnectToDeviceOperationFactory<std::string> {
+ public:
+  FakeConnectToDeviceOperationFactory();
+  ~FakeConnectToDeviceOperationFactory() override;
+
+  FakeConnectToDeviceOperation* last_created_instance() {
+    return last_created_instance_;
+  }
+
+  size_t num_instances_created() { return num_instances_created_; }
+
+  // ConnectToDeviceOperationFactory<std::string>:
+  std::unique_ptr<ConnectToDeviceOperation<std::string>> CreateOperation(
+      ConnectToDeviceOperation<std::string>::ConnectionSuccessCallback
+          success_callback,
+      ConnectToDeviceOperation<std::string>::ConnectionFailedCallback
+          failure_callback) override;
+
+ private:
+  FakeConnectToDeviceOperation* last_created_instance_ = nullptr;
+  size_t num_instances_created_ = 0u;
+
+  DISALLOW_COPY_AND_ASSIGN(FakeConnectToDeviceOperationFactory);
+};
+
+}  // namespace secure_channel
+
+}  // namespace chromeos
+
+#endif  // CHROMEOS_SERVICES_SECURE_CHANNEL_FAKE_CONNECT_TO_DEVICE_OPERATION_FACTORY_H_