[CrOS MultiDevice] Re-land HostVerifierOperation.

Original CL (uploaded as patchset 1):
https://chromium-review.googlesource.com/c/chromium/src/+/1119466

Reverted CL:
https://chromium-review.googlesource.com/c/chromium/src/+/1125479

The CL failed on some bots due to trying to set an invalid response code
for a proto (the test was trying to test an invalid response code). I've
removed that test in patchset 2.

Original CL description below:

This class represents an operation for completing the verification step
for the current host device. A HostVerifierOperation instance is meant
to be used for a single verification attempt; if verification needs to
be retried, a new instance should be created for the next attempt.

This class completes the following steps to verify a device:
(1) Call FindEligibleDevices(). This step sends a message to the host
    device, which in turn enables background advertising.
(2) Creates a connection to the device using the BLE listener role.
(3) Sends an EnableBetterTogetherRequest message to the host device.
(4) Waits for an EnableBetterTogetherResponse messages to be returned by
    the host device.

Bug: 824568
Change-Id: Iee9084763e0f3163baed06f36cc2e357e6313f64
Reviewed-on: https://chromium-review.googlesource.com/1125500Reviewed-by: Jeremy Klein <jlklein@chromium.org>
Commit-Queue: Kyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#572427}

chromeos/services/multidevice_setup/BUILD.gn

@@ -29,6 +29,10 @@ static_library("multidevice_setup") {
     "host_verifier.h",
     "host_verifier_impl.cc",
     "host_verifier_impl.h",
+    "host_verifier_operation.cc",
+    "host_verifier_operation.h",
+    "host_verifier_operation_impl.cc",
+    "host_verifier_operation_impl.h",
     "multidevice_setup_base.cc",
     "multidevice_setup_base.h",
     "multidevice_setup_impl.cc",
@@ -47,6 +51,7 @@ static_library("multidevice_setup") {
     "//chromeos/components/proximity_auth/logging",
     "//chromeos/services/device_sync/public/cpp",
     "//chromeos/services/device_sync/public/mojom",
+    "//chromeos/services/multidevice_setup/proto",
     "//chromeos/services/multidevice_setup/public/mojom",
     "//chromeos/services/secure_channel/public/cpp/client",
     "//chromeos/services/secure_channel/public/mojom",
@@ -77,6 +82,8 @@ static_library("test_support") {
     "fake_host_status_provider.h",
     "fake_host_verifier.cc",
     "fake_host_verifier.h",
+    "fake_host_verifier_operation.cc",
+    "fake_host_verifier_operation.h",
     "fake_setup_flow_completion_recorder.cc",
     "fake_setup_flow_completion_recorder.h",
   ]
@@ -100,6 +107,7 @@ source_set("unit_tests") {
     "host_backend_delegate_impl_unittest.cc",
     "host_status_provider_impl_unittest.cc",
     "host_verifier_impl_unittest.cc",
+    "host_verifier_operation_impl_unittest.cc",
     "multidevice_setup_impl_unittest.cc",
     "multidevice_setup_service_unittest.cc",
     "setup_flow_completion_recorder_impl_unittest.cc",

chromeos/services/multidevice_setup/fake_host_verifier_operation.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/multidevice_setup/fake_host_verifier_operation.h"
+
+namespace chromeos {
+
+namespace multidevice_setup {
+
+FakeHostVerifierOperation::FakeHostVerifierOperation(Delegate* delegate)
+    : HostVerifierOperation(delegate) {}
+
+FakeHostVerifierOperation::~FakeHostVerifierOperation() = default;
+
+void FakeHostVerifierOperation::PerformCancelOperation() {}
+
+FakeHostVerifierOperationDelegate::FakeHostVerifierOperationDelegate() =
+    default;
+
+FakeHostVerifierOperationDelegate::~FakeHostVerifierOperationDelegate() =
+    default;
+
+void FakeHostVerifierOperationDelegate::OnOperationFinished(
+    HostVerifierOperation::Result result) {
+  DCHECK(!result_);
+  result_ = result;
+}
+
+}  // namespace multidevice_setup
+
+}  // namespace chromeos

chromeos/services/multidevice_setup/fake_host_verifier_operation.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_MULTIDEVICE_SETUP_FAKE_HOST_VERIFIER_OPERATION_H_
+#define CHROMEOS_SERVICES_MULTIDEVICE_SETUP_FAKE_HOST_VERIFIER_OPERATION_H_
+
+#include "base/macros.h"
+#include "base/optional.h"
+#include "chromeos/services/multidevice_setup/host_verifier_operation.h"
+
+namespace chromeos {
+
+namespace multidevice_setup {
+
+// Test HostVerifierOperation implementation.
+class FakeHostVerifierOperation : public HostVerifierOperation {
+ public:
+  FakeHostVerifierOperation(Delegate* delegate);
+  ~FakeHostVerifierOperation() override;
+
+  using HostVerifierOperation::NotifyOperationFinished;
+
+ private:
+  // HostVerifierOperation:
+  void PerformCancelOperation() override;
+
+  DISALLOW_COPY_AND_ASSIGN(FakeHostVerifierOperation);
+};
+
+// Test HostVerifierOperation::Delegate implementation.
+class FakeHostVerifierOperationDelegate
+    : public HostVerifierOperation::Delegate {
+ public:
+  FakeHostVerifierOperationDelegate();
+  ~FakeHostVerifierOperationDelegate() override;
+
+  const base::Optional<HostVerifierOperation::Result>& result() const {
+    return result_;
+  }
+
+ private:
+  // HostVerifierOperation::Delegate:
+  void OnOperationFinished(HostVerifierOperation::Result result) override;
+
+  base::Optional<HostVerifierOperation::Result> result_;
+
+  DISALLOW_COPY_AND_ASSIGN(FakeHostVerifierOperationDelegate);
+};
+
+}  // namespace multidevice_setup
+
+}  // namespace chromeos
+
+#endif  // CHROMEOS_SERVICES_MULTIDEVICE_SETUP_FAKE_HOST_VERIFIER_OPERATION_H_

chromeos/services/multidevice_setup/host_verifier_operation.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/multidevice_setup/host_verifier_operation.h"
+
+#include "chromeos/components/proximity_auth/logging/logging.h"
+
+namespace chromeos {
+
+namespace multidevice_setup {
+
+HostVerifierOperation::HostVerifierOperation(Delegate* delegate)
+    : delegate_(delegate) {}
+
+HostVerifierOperation::~HostVerifierOperation() = default;
+
+void HostVerifierOperation::CancelOperation() {
+  if (result_) {
+    PA_LOG(ERROR) << "HostVerifierOperation::CancelOperation(): Tried to "
+                  << "cancel operation, but it was already finished. Result: "
+                  << *result_;
+    NOTREACHED();
+  }
+
+  PerformCancelOperation();
+  NotifyOperationFinished(Result::kCanceled);
+}
+
+void HostVerifierOperation::NotifyOperationFinished(Result result) {
+  if (result_) {
+    PA_LOG(ERROR) << "HostVerifierOperation::NotifyOperationFinished(): Tried "
+                  << "to finish operation, but it was already finished. "
+                  << "Result: " << *result_;
+  }
+  result_ = result;
+
+  delegate_->OnOperationFinished(*result_);
+}
+
+std::ostream& operator<<(std::ostream& stream,
+                         const HostVerifierOperation::Result& result) {
+  switch (result) {
+    case HostVerifierOperation::Result::kTimeoutFindingEligibleDevices:
+      stream << "[timeout calling FindEligibleDevices()]";
+      break;
+    case HostVerifierOperation::Result::kErrorCallingFindEligibleDevices:
+      stream << "[error calling FindEligibleDevices()]";
+      break;
+    case HostVerifierOperation::Result::kDeviceToVerifyIsNotEligible:
+      stream << "[device to verify was not included in FindEligibleDevices() "
+             << "response];";
+      break;
+    case HostVerifierOperation::Result::kTimeoutFindingConnection:
+      stream << "[timeout finding connection]";
+      break;
+    case HostVerifierOperation::Result::kConnectionAttemptFailed:
+      stream << "[connection attempt failed]";
+      break;
+    case HostVerifierOperation::Result::kConnectionDisconnectedUnexpectedly:
+      stream << "[connection disconnected unexpectedly]";
+      break;
+    case HostVerifierOperation::Result::kTimeoutReceivingResponse:
+      stream << "[timeout receiving EnableBetterTogetherResponse]";
+      break;
+    case HostVerifierOperation::Result::kReceivedInvalidResponse:
+      stream << "[received invalid EnableBetterTogetherResponse message]";
+      break;
+    case HostVerifierOperation::Result::kReceivedErrorResponse:
+      stream << "[received EnableBetterTogetherResponse with error]";
+      break;
+    case HostVerifierOperation::Result::kCanceled:
+      stream << "[request canceled]";
+      break;
+    case HostVerifierOperation::Result::kSuccess:
+      stream << "[success]";
+      break;
+  }
+  return stream;
+}
+
+}  // namespace multidevice_setup
+
+}  // namespace chromeos

chromeos/services/multidevice_setup/host_verifier_operation.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_MULTIDEVICE_SETUP_HOST_VERIFIER_OPERATION_H_
+#define CHROMEOS_SERVICES_MULTIDEVICE_SETUP_HOST_VERIFIER_OPERATION_H_
+
+#include <ostream>
+
+#include "base/macros.h"
+#include "base/optional.h"
+
+namespace chromeos {
+
+namespace multidevice_setup {
+
+// Operation for completing the verification step for the current host device.
+// A HostVerifierOperation instance is meant to be used for a single
+// verification attempt; if verification needs to be retried, a new instance
+// should be created for the next attempt.
+class HostVerifierOperation {
+ public:
+  enum class Result {
+    kTimeoutFindingEligibleDevices,
+    kErrorCallingFindEligibleDevices,
+    kDeviceToVerifyIsNotEligible,
+    kTimeoutFindingConnection,
+    kConnectionAttemptFailed,
+    kConnectionDisconnectedUnexpectedly,
+    kTimeoutReceivingResponse,
+    kReceivedInvalidResponse,
+    kReceivedErrorResponse,
+    kCanceled,
+    kSuccess
+  };
+
+  class Delegate {
+   public:
+    virtual ~Delegate() = default;
+    virtual void OnOperationFinished(Result result) = 0;
+  };
+
+  virtual ~HostVerifierOperation();
+
+  // Cancels the operation, triggering a delegate callback with the kCanceled
+  // result.
+  //
+  // It is invalid to call this function after the operation has already
+  // completed.
+  void CancelOperation();
+
+  // Returns the result of the operation. If the operation has not yet finished,
+  // null is returned.
+  const base::Optional<Result>& result() const { return result_; }
+
+ protected:
+  HostVerifierOperation(Delegate* delegate);
+
+  // Derived types should use this function to cancel the operation, but they
+  // should not call NotifyOperationFinished() during cancellation.
+  virtual void PerformCancelOperation() = 0;
+
+  void NotifyOperationFinished(Result result);
+
+ private:
+  Delegate* delegate_;
+  base::Optional<Result> result_;
+
+  DISALLOW_COPY_AND_ASSIGN(HostVerifierOperation);
+};
+
+std::ostream& operator<<(std::ostream& stream,
+                         const HostVerifierOperation::Result& result);
+
+}  // namespace multidevice_setup
+
+}  // namespace chromeos
+
+#endif  // CHROMEOS_SERVICES_MULTIDEVICE_SETUP_HOST_VERIFIER_OPERATION_H_

chromeos/services/multidevice_setup/host_verifier_operation_impl.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_MULTIDEVICE_SETUP_HOST_VERIFIER_OPERATION_IMPL_H_
+#define CHROMEOS_SERVICES_MULTIDEVICE_SETUP_HOST_VERIFIER_OPERATION_IMPL_H_
+
+#include <memory>
+#include <ostream>
+#include <string>
+
+#include "base/macros.h"
+#include "base/memory/weak_ptr.h"
+#include "base/optional.h"
+#include "base/timer/timer.h"
+#include "chromeos/services/multidevice_setup/host_verifier_operation.h"
+#include "chromeos/services/multidevice_setup/proto/multidevice_setup.pb.h"
+#include "chromeos/services/secure_channel/public/cpp/client/client_channel.h"
+#include "chromeos/services/secure_channel/public/cpp/client/connection_attempt.h"
+#include "chromeos/services/secure_channel/public/mojom/secure_channel.mojom.h"
+#include "components/cryptauth/remote_device_ref.h"
+
+namespace chromeos {
+
+namespace device_sync {
+class DeviceSyncClient;
+}  // namespace device_sync
+
+namespace secure_channel {
+class SecureChannelClient;
+}  // namespace secure_channel
+
+namespace multidevice_setup {
+
+// Concrete HostVerifierOperation implementation. To verify the host, this class
+// performs the following steps:
+// (1) Call FindEligibleDevices(). This step sends a message to the host device,
+//     which in turn enables background advertising.
+// (2) Creates a connection to the device using the BLE listener role.
+// (3) Sends an EnableBetterTogetherRequest message to the host device.
+// (4) Waits for an EnableBetterTogetherResponse messages to be returned by the
+//     host device.
+class HostVerifierOperationImpl
+    : public HostVerifierOperation,
+      public secure_channel::ConnectionAttempt::Delegate,
+      public secure_channel::ClientChannel::Observer {
+ public:
+  class Factory {
+   public:
+    static Factory* Get();
+    static void SetFactoryForTesting(Factory* test_factory);
+    virtual ~Factory();
+    virtual std::unique_ptr<HostVerifierOperation> BuildInstance(
+        HostVerifierOperation::Delegate* delegate,
+        cryptauth::RemoteDeviceRef device_to_connect,
+        cryptauth::RemoteDeviceRef local_device,
+        device_sync::DeviceSyncClient* device_sync_client,
+        secure_channel::SecureChannelClient* secure_channel_client,
+        std::unique_ptr<base::OneShotTimer> timer =
+            std::make_unique<base::OneShotTimer>());
+
+   private:
+    static Factory* test_factory_;
+  };
+
+  ~HostVerifierOperationImpl() override;
+
+ private:
+  friend class MultiDeviceSetupHostVerifierOperationImplTest;
+
+  enum class Status {
+    kWaitingForFindEligibleDevicesResponse,
+    kWaitingForConnection,
+    kWaitingForResponse,
+    kFinished
+  };
+  friend std::ostream& operator<<(std::ostream& stream, const Status& status);
+
+  HostVerifierOperationImpl(
+      HostVerifierOperation::Delegate* delegate,
+      cryptauth::RemoteDeviceRef device_to_connect,
+      cryptauth::RemoteDeviceRef local_device,
+      device_sync::DeviceSyncClient* device_sync_client,
+      secure_channel::SecureChannelClient* secure_channel_client,
+      std::unique_ptr<base::OneShotTimer> timer);
+
+  static BetterTogetherSetupMessageWrapper
+  CreateWrappedEnableBetterTogetherRequest();
+
+  // HostVerifierOperation:
+  void PerformCancelOperation() override;
+
+  // secure_channel::ConnectionAttempt::Delegate:
+  void OnConnectionAttemptFailure(
+      secure_channel::mojom::ConnectionAttemptFailureReason reason) override;
+  void OnConnection(
+      std::unique_ptr<secure_channel::ClientChannel> channel) override;
+
+  // secure_channel::ClientChannel::Observer:
+  void OnDisconnected() override;
+  void OnMessageReceived(const std::string& payload) override;
+
+  void OnTimeout();
+  void OnFindEligibleDevicesResponse(
+      const base::Optional<std::string>& error_code,
+      cryptauth::RemoteDeviceRefList eligible_devices,
+      cryptauth::RemoteDeviceRefList ineligible_devices);
+  void FinishOperation(Status expected_current_status, Result result);
+  void TransitionStatus(Status expected_current_status, Status new_status);
+
+  cryptauth::RemoteDeviceRef device_to_connect_;
+  cryptauth::RemoteDeviceRef local_device_;
+  device_sync::DeviceSyncClient* device_sync_client_;
+  secure_channel::SecureChannelClient* secure_channel_client_;
+  std::unique_ptr<base::OneShotTimer> timer_;
+
+  Status status_ = Status::kWaitingForFindEligibleDevicesResponse;
+  std::unique_ptr<secure_channel::ConnectionAttempt> connection_attempt_;
+  std::unique_ptr<secure_channel::ClientChannel> client_channel_;
+
+  base::WeakPtrFactory<HostVerifierOperationImpl> weak_ptr_factory_;
+
+  DISALLOW_COPY_AND_ASSIGN(HostVerifierOperationImpl);
+};
+
+std::ostream& operator<<(std::ostream& stream,
+                         const HostVerifierOperationImpl::Status& status);
+
+}  // namespace multidevice_setup
+
+}  // namespace chromeos
+
+#endif  // CHROMEOS_SERVICES_MULTIDEVICE_SETUP_HOST_VERIFIER_OPERATION_IMPL_H_

chromeos/services/multidevice_setup/proto/BUILD.gn

+# 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.
+
+import("//third_party/protobuf/proto_library.gni")
+
+proto_library("proto") {
+  sources = [
+    "multidevice_setup.proto",
+  ]
+}

chromeos/services/multidevice_setup/proto/multidevice_setup.proto

+// 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.
+
+syntax = "proto2";
+
+package chromeos.multidevice_setup;
+
+option optimize_for = LITE_RUNTIME;
+
+enum MessageType {
+  UNKNOWN_TYPE = 0;
+  ENABLE_BETTER_TOGETHER_REQUEST = 1;
+  ENABLE_BETTER_TOGETHER_RESPONSE = 2;
+}
+
+// Client to host, indicating that the client is requesting Better Together
+// setup.
+message EnableBetterTogetherRequest {}
+
+// Host to client, indicating that Better Together setup was completed.
+// Next id: 2
+message EnableBetterTogetherResponse {
+  enum ResultCode {
+    NORMAL = 0;
+    ERROR = 1;
+  }
+
+  optional ResultCode result_code = 1;
+}
+
+// Wrapper that Better Together setup messages use to explicitly indicate
+// message type.
+// Next id: 3
+message BetterTogetherSetupMessageWrapper {
+  required MessageType type = 1;
+  optional bytes payload = 2;
+}