[CrOS MultiDevice] Update MultiDeviceSetupClient w/ feature APIs.

This CL adds SetFeatureEnabledState() and GetFeatureState() functions to
MultiDeviceSetupClient.

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

chromeos/services/multidevice_setup/public/cpp/fake_multidevice_setup_client.cc

@@ -14,6 +14,8 @@ FakeMultiDeviceSetupClient::~FakeMultiDeviceSetupClient() {
   DCHECK(get_eligible_host_devices_callback_queue_.empty());
   DCHECK(set_host_device_id_and_callback_queue_.empty());
   DCHECK(get_host_status_callback_queue_.empty());
+  DCHECK(set_feature_enabled_state_args_queue_.empty());
+  DCHECK(get_feature_states_args_queue_.empty());
   DCHECK(retry_set_host_now_callback_queue_.empty());
   DCHECK(trigger_event_for_debugging_type_and_callback_queue_.empty());
 }
@@ -42,6 +44,23 @@ void FakeMultiDeviceSetupClient::InvokePendingGetHostStatusCallback(
   get_host_status_callback_queue_.pop();
 }
 
+void FakeMultiDeviceSetupClient::InvokePendingSetFeatureEnabledStateCallback(
+    mojom::Feature expected_feature,
+    bool expected_enabled,
+    bool success) {
+  auto& tuple = set_feature_enabled_state_args_queue_.front();
+  DCHECK_EQ(expected_feature, std::get<0>(tuple));
+  DCHECK_EQ(expected_enabled, std::get<1>(tuple));
+  std::move(std::get<2>(tuple)).Run(success);
+  set_feature_enabled_state_args_queue_.pop();
+}
+
+void FakeMultiDeviceSetupClient::InvokePendingGetFeatureStatesCallback(
+    const FeatureStatesMap& feature_states_map) {
+  std::move(get_feature_states_args_queue_.front()).Run(feature_states_map);
+  get_feature_states_args_queue_.pop();
+}
+
 void FakeMultiDeviceSetupClient::InvokePendingRetrySetHostNowCallback(
     bool success) {
   std::move(retry_set_host_now_callback_queue_.front()).Run(success);
@@ -78,6 +97,19 @@ void FakeMultiDeviceSetupClient::GetHostStatus(GetHostStatusCallback callback) {
   get_host_status_callback_queue_.push(std::move(callback));
 }
 
+void FakeMultiDeviceSetupClient::SetFeatureEnabledState(
+    mojom::Feature feature,
+    bool enabled,
+    mojom::MultiDeviceSetup::SetFeatureEnabledStateCallback callback) {
+  set_feature_enabled_state_args_queue_.emplace(feature, enabled,
+                                                std::move(callback));
+}
+
+void FakeMultiDeviceSetupClient::GetFeatureStates(
+    mojom::MultiDeviceSetup::GetFeatureStatesCallback callback) {
+  get_feature_states_args_queue_.emplace(std::move(callback));
+}
+
 void FakeMultiDeviceSetupClient::RetrySetHostNow(
     mojom::MultiDeviceSetup::RetrySetHostNowCallback callback) {
   retry_set_host_now_callback_queue_.push(std::move(callback));

chromeos/services/multidevice_setup/public/cpp/fake_multidevice_setup_client.h

@@ -8,6 +8,7 @@
 #include <memory>
 #include <queue>
 #include <string>
+#include <tuple>
 
 #include "base/callback.h"
 #include "base/macros.h"
@@ -33,6 +34,12 @@ class FakeMultiDeviceSetupClient : public MultiDeviceSetupClient {
   void InvokePendingGetHostStatusCallback(
       mojom::HostStatus host_status,
       const base::Optional<cryptauth::RemoteDeviceRef>& host_device);
+  void InvokePendingSetFeatureEnabledStateCallback(
+      mojom::Feature expected_feature,
+      bool expected_enabled,
+      bool success);
+  void InvokePendingGetFeatureStatesCallback(
+      const FeatureStatesMap& feature_states_map);
   void InvokePendingRetrySetHostNowCallback(bool success);
   void InvokePendingTriggerEventForDebuggingCallback(
       mojom::EventTypeForDebugging expected_type,
@@ -43,6 +50,7 @@ class FakeMultiDeviceSetupClient : public MultiDeviceSetupClient {
   }
 
   using MultiDeviceSetupClient::NotifyHostStatusChanged;
+  using MultiDeviceSetupClient::NotifyFeatureStateChanged;
 
  private:
   void GetEligibleHostDevices(GetEligibleHostDevicesCallback callback) override;
@@ -51,6 +59,13 @@ class FakeMultiDeviceSetupClient : public MultiDeviceSetupClient {
       mojom::MultiDeviceSetup::SetHostDeviceCallback callback) override;
   void RemoveHostDevice() override;
   void GetHostStatus(GetHostStatusCallback callback) override;
+  void SetFeatureEnabledState(
+      mojom::Feature feature,
+      bool enabled,
+      mojom::MultiDeviceSetup::SetFeatureEnabledStateCallback callback)
+      override;
+  void GetFeatureStates(
+      mojom::MultiDeviceSetup::GetFeatureStatesCallback callback) override;
   void RetrySetHostNow(
       mojom::MultiDeviceSetup::RetrySetHostNowCallback callback) override;
   void TriggerEventForDebugging(
@@ -66,6 +81,13 @@ class FakeMultiDeviceSetupClient : public MultiDeviceSetupClient {
       std::pair<std::string, mojom::MultiDeviceSetup::SetHostDeviceCallback>>
       set_host_device_id_and_callback_queue_;
   std::queue<GetHostStatusCallback> get_host_status_callback_queue_;
+  std::queue<
+      std::tuple<mojom::Feature,
+                 bool,
+                 mojom::MultiDeviceSetup::SetFeatureEnabledStateCallback>>
+      set_feature_enabled_state_args_queue_;
+  std::queue<mojom::MultiDeviceSetup::GetFeatureStatesCallback>
+      get_feature_states_args_queue_;
   std::queue<mojom::MultiDeviceSetup::RetrySetHostNowCallback>
       retry_set_host_now_callback_queue_;
   std::queue<

chromeos/services/multidevice_setup/public/cpp/multidevice_setup_client.cc

@@ -27,6 +27,12 @@ void MultiDeviceSetupClient::NotifyHostStatusChanged(
     observer.OnHostStatusChanged(host_status, host_device);
 }
 
+void MultiDeviceSetupClient::NotifyFeatureStateChanged(
+    const FeatureStatesMap& feature_states_map) {
+  for (auto& observer : observer_list_)
+    observer.OnFeatureStatesChanged(feature_states_map);
+}
+
 }  // namespace multidevice_setup
 
 }  // namespace chromeos

chromeos/services/multidevice_setup/public/cpp/multidevice_setup_client.h

@@ -9,6 +9,7 @@
 #include <string>
 
 #include "base/callback.h"
+#include "base/containers/flat_map.h"
 #include "base/macros.h"
 #include "base/observer_list.h"
 #include "base/optional.h"
@@ -22,6 +23,8 @@ namespace multidevice_setup {
 // Provides clients access to the MultiDeviceSetup API.
 class MultiDeviceSetupClient {
  public:
+  using FeatureStatesMap = base::flat_map<mojom::Feature, mojom::FeatureState>;
+
   class Observer {
    public:
     // Called whenever the host status changes. If the host status is
@@ -29,7 +32,11 @@ class MultiDeviceSetupClient {
     // HostStatus::kEligibleHostExistsButNoHostSet, |host_device| is null.
     virtual void OnHostStatusChanged(
         mojom::HostStatus host_status,
-        const base::Optional<cryptauth::RemoteDeviceRef>& host_device) = 0;
+        const base::Optional<cryptauth::RemoteDeviceRef>& host_device) {}
+
+    // Called whenever the state of any feature has changed.
+    virtual void OnFeatureStatesChanged(
+        const FeatureStatesMap& feature_states_map) {}
 
    protected:
     virtual ~Observer() = default;
@@ -54,6 +61,12 @@ class MultiDeviceSetupClient {
       mojom::MultiDeviceSetup::SetHostDeviceCallback callback) = 0;
   virtual void RemoveHostDevice() = 0;
   virtual void GetHostStatus(GetHostStatusCallback callback) = 0;
+  virtual void SetFeatureEnabledState(
+      mojom::Feature feature,
+      bool enabled,
+      mojom::MultiDeviceSetup::SetFeatureEnabledStateCallback callback) = 0;
+  virtual void GetFeatureStates(
+      mojom::MultiDeviceSetup::GetFeatureStatesCallback callback) = 0;
   virtual void RetrySetHostNow(
       mojom::MultiDeviceSetup::RetrySetHostNowCallback callback) = 0;
   virtual void TriggerEventForDebugging(
@@ -64,6 +77,7 @@ class MultiDeviceSetupClient {
   void NotifyHostStatusChanged(
       mojom::HostStatus host_status,
       const base::Optional<cryptauth::RemoteDeviceRef>& host_device);
+  void NotifyFeatureStateChanged(const FeatureStatesMap& feature_states_map);
 
  private:
   base::ObserverList<Observer> observer_list_;

chromeos/services/multidevice_setup/public/cpp/multidevice_setup_client_impl.cc

@@ -47,11 +47,15 @@ MultiDeviceSetupClientImpl::Factory::BuildInstance(
 
 MultiDeviceSetupClientImpl::MultiDeviceSetupClientImpl(
     service_manager::Connector* connector)
-    : binding_(this),
+    : host_status_observer_binding_(this),
+      feature_state_observer_binding_(this),
       remote_device_cache_(
           cryptauth::RemoteDeviceCache::Factory::Get()->BuildInstance()) {
   connector->BindInterface(mojom::kServiceName, &multidevice_setup_ptr_);
-  multidevice_setup_ptr_->AddHostStatusObserver(GenerateInterfacePtr());
+  multidevice_setup_ptr_->AddHostStatusObserver(
+      GenerateHostStatusObserverInterfacePtr());
+  multidevice_setup_ptr_->AddFeatureStateObserver(
+      GenerateFeatureStatesObserverInterfacePtr());
 }
 
 MultiDeviceSetupClientImpl::~MultiDeviceSetupClientImpl() = default;
@@ -79,6 +83,19 @@ void MultiDeviceSetupClientImpl::GetHostStatus(GetHostStatusCallback callback) {
                      base::Unretained(this), std::move(callback)));
 }
 
+void MultiDeviceSetupClientImpl::SetFeatureEnabledState(
+    mojom::Feature feature,
+    bool enabled,
+    mojom::MultiDeviceSetup::SetFeatureEnabledStateCallback callback) {
+  multidevice_setup_ptr_->SetFeatureEnabledState(feature, enabled,
+                                                 std::move(callback));
+}
+
+void MultiDeviceSetupClientImpl::GetFeatureStates(
+    mojom::MultiDeviceSetup::GetFeatureStatesCallback callback) {
+  multidevice_setup_ptr_->GetFeatureStates(std::move(callback));
+}
+
 void MultiDeviceSetupClientImpl::RetrySetHostNow(
     mojom::MultiDeviceSetup::RetrySetHostNowCallback callback) {
   multidevice_setup_ptr_->RetrySetHostNow(std::move(callback));
@@ -102,6 +119,11 @@ void MultiDeviceSetupClientImpl::OnHostStatusChanged(
   }
 }
 
+void MultiDeviceSetupClientImpl::OnFeatureStatesChanged(
+    const FeatureStatesMap& feature_states_map) {
+  NotifyFeatureStateChanged(feature_states_map);
+}
+
 void MultiDeviceSetupClientImpl::OnGetEligibleHostDevicesCompleted(
     GetEligibleHostDevicesCallback callback,
     const cryptauth::RemoteDeviceList& eligible_host_devices) {
@@ -132,9 +154,16 @@ void MultiDeviceSetupClientImpl::OnGetHostStatusCompleted(
 }
 
 mojom::HostStatusObserverPtr
-MultiDeviceSetupClientImpl::GenerateInterfacePtr() {
+MultiDeviceSetupClientImpl::GenerateHostStatusObserverInterfacePtr() {
   mojom::HostStatusObserverPtr interface_ptr;
-  binding_.Bind(mojo::MakeRequest(&interface_ptr));
+  host_status_observer_binding_.Bind(mojo::MakeRequest(&interface_ptr));
+  return interface_ptr;
+}
+
+mojom::FeatureStateObserverPtr
+MultiDeviceSetupClientImpl::GenerateFeatureStatesObserverInterfacePtr() {
+  mojom::FeatureStateObserverPtr interface_ptr;
+  feature_state_observer_binding_.Bind(mojo::MakeRequest(&interface_ptr));
   return interface_ptr;
 }
 

chromeos/services/multidevice_setup/public/cpp/multidevice_setup_client_impl.h

@@ -28,7 +28,8 @@ namespace multidevice_setup {
 
 // Concrete implementation of MultiDeviceSetupClient.
 class MultiDeviceSetupClientImpl : public MultiDeviceSetupClient,
-                                   public mojom::HostStatusObserver {
+                                   public mojom::HostStatusObserver,
+                                   public mojom::FeatureStateObserver {
  public:
   class Factory {
    public:
@@ -51,6 +52,13 @@ class MultiDeviceSetupClientImpl : public MultiDeviceSetupClient,
       mojom::MultiDeviceSetup::SetHostDeviceCallback callback) override;
   void RemoveHostDevice() override;
   void GetHostStatus(GetHostStatusCallback callback) override;
+  void SetFeatureEnabledState(
+      mojom::Feature feature,
+      bool enabled,
+      mojom::MultiDeviceSetup::SetFeatureEnabledStateCallback callback)
+      override;
+  void GetFeatureStates(
+      mojom::MultiDeviceSetup::GetFeatureStatesCallback callback) override;
   void RetrySetHostNow(
       mojom::MultiDeviceSetup::RetrySetHostNowCallback callback) override;
   void TriggerEventForDebugging(
@@ -63,6 +71,10 @@ class MultiDeviceSetupClientImpl : public MultiDeviceSetupClient,
       mojom::HostStatus host_status,
       const base::Optional<cryptauth::RemoteDevice>& host_device) override;
 
+  // mojom::FeatureStateObserver:
+  void OnFeatureStatesChanged(
+      const FeatureStatesMap& feature_states_map) override;
+
  private:
   friend class MultiDeviceSetupClientImplTest;
 
@@ -76,12 +88,14 @@ class MultiDeviceSetupClientImpl : public MultiDeviceSetupClient,
       mojom::HostStatus host_status,
       const base::Optional<cryptauth::RemoteDevice>& host_device);
 
-  mojom::HostStatusObserverPtr GenerateInterfacePtr();
+  mojom::HostStatusObserverPtr GenerateHostStatusObserverInterfacePtr();
+  mojom::FeatureStateObserverPtr GenerateFeatureStatesObserverInterfacePtr();
 
   void FlushForTesting();
 
   mojom::MultiDeviceSetupPtr multidevice_setup_ptr_;
-  mojo::Binding<mojom::HostStatusObserver> binding_;
+  mojo::Binding<mojom::HostStatusObserver> host_status_observer_binding_;
+  mojo::Binding<mojom::FeatureStateObserver> feature_state_observer_binding_;
   std::unique_ptr<cryptauth::RemoteDeviceCache> remote_device_cache_;
 
   DISALLOW_COPY_AND_ASSIGN(MultiDeviceSetupClientImpl);

chromeos/services/multidevice_setup/public/cpp/multidevice_setup_client_impl_unittest.cc

@@ -217,6 +217,57 @@ class MultiDeviceSetupClientImplTest : public testing::Test {
     }
   }
 
+  void CallSetFeatureEnabledState(mojom::Feature feature,
+                                  bool enabled,
+                                  bool should_succeed) {
+    size_t num_set_feature_enabled_args_before_call =
+        fake_multidevice_setup_->set_feature_enabled_args().size();
+
+    base::RunLoop run_loop;
+    client_->SetFeatureEnabledState(
+        feature, enabled,
+        base::BindOnce(
+            &MultiDeviceSetupClientImplTest::OnSetFeatureEnabledStateCompleted,
+            base::Unretained(this), run_loop.QuitClosure()));
+    SendPendingMojoMessages();
+
+    EXPECT_EQ(num_set_feature_enabled_args_before_call + 1u,
+              fake_multidevice_setup_->set_feature_enabled_args().size());
+    EXPECT_EQ(feature,
+              std::get<0>(
+                  fake_multidevice_setup_->set_feature_enabled_args().back()));
+    EXPECT_EQ(enabled,
+              std::get<1>(
+                  fake_multidevice_setup_->set_feature_enabled_args().back()));
+    std::move(
+        std::get<2>(fake_multidevice_setup_->set_feature_enabled_args().back()))
+        .Run(should_succeed /* success */);
+
+    run_loop.Run();
+    EXPECT_EQ(should_succeed, *set_feature_enabled_state_success_);
+  }
+
+  void CallGetFeatureStates(
+      const base::flat_map<mojom::Feature, mojom::FeatureState>&
+          expected_feature_states_map) {
+    size_t num_get_feature_states_args_before_call =
+        fake_multidevice_setup_->get_feature_states_args().size();
+
+    base::RunLoop run_loop;
+    client_->GetFeatureStates(base::BindOnce(
+        &MultiDeviceSetupClientImplTest::OnGetFeatureStatesCompleted,
+        base::Unretained(this), run_loop.QuitClosure()));
+    SendPendingMojoMessages();
+
+    EXPECT_EQ(num_get_feature_states_args_before_call + 1u,
+              fake_multidevice_setup_->get_feature_states_args().size());
+    std::move(fake_multidevice_setup_->get_feature_states_args().back())
+        .Run(expected_feature_states_map);
+
+    run_loop.Run();
+    EXPECT_EQ(expected_feature_states_map, *get_feature_states_result_);
+  }
+
   void CallRetrySetHostNow(bool expect_success) {
     base::RunLoop run_loop;
 
@@ -305,6 +356,20 @@ class MultiDeviceSetupClientImplTest : public testing::Test {
     std::move(quit_closure).Run();
   }
 
+  void OnSetFeatureEnabledStateCompleted(base::OnceClosure quit_closure,
+                                         bool success) {
+    set_feature_enabled_state_success_ = success;
+    std::move(quit_closure).Run();
+  }
+
+  void OnGetFeatureStatesCompleted(
+      base::OnceClosure quit_closure,
+      const base::flat_map<mojom::Feature, mojom::FeatureState>&
+          feature_states_map) {
+    get_feature_states_result_ = feature_states_map;
+    std::move(quit_closure).Run();
+  }
+
   void OnRetrySetHostNowCompleted(base::OnceClosure quit_closure,
                                   bool success) {
     retry_set_host_now_success_ = success;
@@ -330,6 +395,9 @@ class MultiDeviceSetupClientImplTest : public testing::Test {
   base::Optional<
       std::pair<mojom::HostStatus, base::Optional<cryptauth::RemoteDeviceRef>>>
       get_host_status_result_;
+  base::Optional<bool> set_feature_enabled_state_success_;
+  base::Optional<base::flat_map<mojom::Feature, mojom::FeatureState>>
+      get_feature_states_result_;
   base::Optional<bool> retry_set_host_now_success_;
   base::Optional<bool> trigger_event_for_debugging_success_;
 
@@ -374,6 +442,19 @@ TEST_F(MultiDeviceSetupClientImplTest, TestGetHostStatus_NoHost) {
                     base::nullopt /* expected_host_device */);
 }
 
+TEST_F(MultiDeviceSetupClientImplTest, SetFeatureEnabledState) {
+  CallSetFeatureEnabledState(mojom::Feature::kBetterTogetherSuite,
+                             true /* enabled */, true /* should_succeed */);
+  CallSetFeatureEnabledState(mojom::Feature::kBetterTogetherSuite,
+                             false /* enabled */, false /* should_succeed */);
+  CallSetFeatureEnabledState(mojom::Feature::kBetterTogetherSuite,
+                             false /* enabled */, true /* should_succeed */);
+}
+
+TEST_F(MultiDeviceSetupClientImplTest, GetFeatureState) {
+  CallGetFeatureStates(base::flat_map<mojom::Feature, mojom::FeatureState>());
+}
+
 TEST_F(MultiDeviceSetupClientImplTest, TestRetrySetHostNow_Success) {
   CallRetrySetHostNow(true /* expect_success */);
 }