[CrOS PhoneHub] Add the MultideviceSetupStateUpdater

This class enables the PhoneHub Multidevice feature state when the
HostStatus (provided by the MultideviceSetupClient) of the phone is
initially |kHostSetLocallyButWaitingForBackendConfirmation|, and becomes
either 1) |kHostVerified|, or 2) becomes |kHostSetButNotYetVerified|
first, and then later |kHostVerified|. This class also disables the
PhoneHubNotification Multidevice feature state when Notification
access has been revoked by the phone, provided via
NotificationAccessManager.

Bug: 1106938
Change-Id: I01435323b3a3b8ec886ab32de0ba324b9473d2a4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2468265
Commit-Queue: Regan Hsu <hsuregan@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#817313}

chrome/browser/chromeos/phonehub/phone_hub_manager_factory.cc

@@ -15,6 +15,7 @@
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/sync/session_sync_service_factory.h"
 #include "chrome/browser/ui/webui/chromeos/multidevice_setup/multidevice_setup_dialog.h"
+#include "chromeos/components/phonehub/multidevice_setup_state_updater.h"
 #include "chromeos/components/phonehub/notification_access_manager_impl.h"
 #include "chromeos/components/phonehub/onboarding_ui_tracker_impl.h"
 #include "chromeos/components/phonehub/phone_hub_manager_impl.h"
@@ -111,6 +112,7 @@ bool PhoneHubManagerFactory::ServiceIsCreatedWithBrowserContext() const {
 
 void PhoneHubManagerFactory::RegisterProfilePrefs(
     user_prefs::PrefRegistrySyncable* registry) {
+  MultideviceSetupStateUpdater::RegisterPrefs(registry);
   NotificationAccessManagerImpl::RegisterPrefs(registry);
   OnboardingUiTrackerImpl::RegisterPrefs(registry);
 }

chromeos/components/phonehub/BUILD.gn

@@ -45,6 +45,8 @@ static_library("phonehub") {
     "message_sender.h",
     "message_sender_impl.cc",
     "message_sender_impl.h",
+    "multidevice_setup_state_updater.cc",
+    "multidevice_setup_state_updater.h",
     "mutable_phone_model.cc",
     "mutable_phone_model.h",
     "notification.cc",
@@ -170,6 +172,7 @@ source_set("unit_tests") {
     "find_my_device_controller_impl_unittest.cc",
     "message_receiver_unittest.cc",
     "message_sender_unittest.cc",
+    "multidevice_setup_state_updater_unittest.cc",
     "mutable_phone_model_unittest.cc",
     "notification_access_manager_impl_unittest.cc",
     "notification_manager_impl_unittest.cc",

chromeos/components/phonehub/multidevice_setup_state_updater.cc

+// Copyright 2020 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/components/phonehub/multidevice_setup_state_updater.h"
+
+#include "chromeos/components/multidevice/logging/logging.h"
+#include "chromeos/components/phonehub/pref_names.h"
+#include "components/prefs/pref_registry_simple.h"
+#include "components/prefs/pref_service.h"
+
+namespace chromeos {
+namespace phonehub {
+namespace {
+using multidevice_setup::mojom::Feature;
+using multidevice_setup::mojom::HostStatus;
+}  // namespace
+
+// static
+void MultideviceSetupStateUpdater::RegisterPrefs(PrefRegistrySimple* registry) {
+  registry->RegisterBooleanPref(prefs::kIsAwaitingVerifiedHost, false);
+}
+
+MultideviceSetupStateUpdater::MultideviceSetupStateUpdater(
+    PrefService* pref_service,
+    multidevice_setup::MultiDeviceSetupClient* multidevice_setup_client,
+    NotificationAccessManager* notification_access_manager)
+    : pref_service_(pref_service),
+      multidevice_setup_client_(multidevice_setup_client),
+      notification_access_manager_(notification_access_manager) {
+  multidevice_setup_client_->AddObserver(this);
+  notification_access_manager_->AddObserver(this);
+
+  const HostStatus host_status =
+      multidevice_setup_client_->GetHostStatus().first;
+  UpdateIsAwaitingVerifiedHost(host_status);
+}
+
+MultideviceSetupStateUpdater::~MultideviceSetupStateUpdater() {
+  multidevice_setup_client_->RemoveObserver(this);
+  notification_access_manager_->RemoveObserver(this);
+}
+
+void MultideviceSetupStateUpdater::OnNotificationAccessChanged() {
+  if (notification_access_manager_->HasAccessBeenGranted())
+    return;
+
+  PA_LOG(INFO) << "Disabling PhoneHubNotifications feature.";
+
+  // Disable kPhoneHubNotifications if notification access has been revoked by
+  // the phone.
+  multidevice_setup_client_->SetFeatureEnabledState(
+      Feature::kPhoneHubNotifications, /*enabled=*/false,
+      /*auth_token=*/base::nullopt, base::DoNothing());
+}
+
+void MultideviceSetupStateUpdater::OnHostStatusChanged(
+    const multidevice_setup::MultiDeviceSetupClient::HostStatusWithDevice&
+        host_device_with_status) {
+  const HostStatus host_status = host_device_with_status.first;
+
+  bool is_awaiting_verified_host =
+      pref_service_->GetBoolean(prefs::kIsAwaitingVerifiedHost);
+
+  // Enable the PhoneHub feature if phone is verified and there was an
+  // intent to enable the feature.
+  if (is_awaiting_verified_host && host_status == HostStatus::kHostVerified) {
+    multidevice_setup_client_->SetFeatureEnabledState(
+        Feature::kPhoneHub, /*enabled=*/true, /*auth_token=*/base::nullopt,
+        base::DoNothing());
+  }
+
+  UpdateIsAwaitingVerifiedHost(host_status);
+}
+
+void MultideviceSetupStateUpdater::UpdateIsAwaitingVerifiedHost(
+    HostStatus host_status) {
+  // Keep |prefs::kIsAwaitingVerifiedHost| at the same state.
+  if (host_status == HostStatus::kHostSetButNotYetVerified)
+    return;
+
+  // Begin waiting for a verified host if the phone is
+  // |kHostSetLocallyButWaitingForBackendConfirmation|.
+  bool is_awaiting_verified_host =
+      host_status ==
+      HostStatus::kHostSetLocallyButWaitingForBackendConfirmation;
+
+  // Must be persisted in the event that this class is destroyed
+  // before the phone is verified.
+  pref_service_->SetBoolean(prefs::kIsAwaitingVerifiedHost,
+                            is_awaiting_verified_host);
+}
+
+}  // namespace phonehub
+}  // namespace chromeos

chromeos/components/phonehub/multidevice_setup_state_updater.h

+// Copyright 2020 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_COMPONENTS_PHONEHUB_MULTIDEVICE_SETUP_STATE_UPDATER_H_
+#define CHROMEOS_COMPONENTS_PHONEHUB_MULTIDEVICE_SETUP_STATE_UPDATER_H_
+
+#include "chromeos/components/phonehub/notification_access_manager.h"
+#include "chromeos/services/multidevice_setup/public/cpp/multidevice_setup_client.h"
+
+class PrefRegistrySimple;
+class PrefService;
+
+namespace chromeos {
+namespace phonehub {
+
+// This class enables the PhoneHub Multidevice feature state when the HostStatus
+// (provided by the MultideviceSetupClient) of the phone is initially
+// |kHostSetLocallyButWaitingForBackendConfirmation|, and becomes either 1)
+// |kHostVerified|, or 2) becomes |kHostSetButNotYetVerified| first, and then
+// later |kHostVerified|. This class also disables the PhoneHubNotification
+// Multidevice feature state when Notification access has been revoked by the
+// phone, provided via NotificationAccessManager.
+class MultideviceSetupStateUpdater
+    : public multidevice_setup::MultiDeviceSetupClient::Observer,
+      public NotificationAccessManager::Observer {
+ public:
+  static void RegisterPrefs(PrefRegistrySimple* registry);
+
+  MultideviceSetupStateUpdater(
+      PrefService* pref_service,
+      multidevice_setup::MultiDeviceSetupClient* multidevice_setup_client,
+      NotificationAccessManager* notification_access_manager);
+  ~MultideviceSetupStateUpdater() override;
+
+ private:
+  // multidevice_setup::MultiDeviceSetupClient::Observer:
+  void OnHostStatusChanged(
+      const multidevice_setup::MultiDeviceSetupClient::HostStatusWithDevice&
+          host_device_with_status) override;
+
+  // NotificationAccessManager::Observer:
+  void OnNotificationAccessChanged() override;
+
+  void UpdateIsAwaitingVerifiedHost(
+      multidevice_setup::mojom::HostStatus host_status);
+
+  PrefService* pref_service_;
+  multidevice_setup::MultiDeviceSetupClient* multidevice_setup_client_;
+  NotificationAccessManager* notification_access_manager_;
+};
+
+}  // namespace phonehub
+}  // namespace chromeos
+
+#endif  // CHROMEOS_COMPONENTS_PHONEHUB_MULTIDEVICE_SETUP_STATE_UPDATER_H_

chromeos/components/phonehub/multidevice_setup_state_updater_unittest.cc

+// Copyright 2020 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/components/phonehub/multidevice_setup_state_updater.h"
+
+#include "chromeos/components/phonehub/fake_notification_access_manager.h"
+#include "chromeos/services/multidevice_setup/public/cpp/fake_multidevice_setup_client.h"
+#include "components/prefs/testing_pref_service.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace chromeos {
+namespace phonehub {
+
+using multidevice_setup::mojom::Feature;
+using multidevice_setup::mojom::FeatureState;
+using multidevice_setup::mojom::HostStatus;
+
+class MultideviceSetupStateUpdaterTest : public testing::Test {
+ protected:
+  MultideviceSetupStateUpdaterTest() = default;
+  ~MultideviceSetupStateUpdaterTest() override = default;
+
+  MultideviceSetupStateUpdaterTest(const MultideviceSetupStateUpdaterTest&) =
+      delete;
+  MultideviceSetupStateUpdaterTest& operator=(
+      const MultideviceSetupStateUpdaterTest&) = delete;
+
+  // testing::Test:
+  void SetUp() override {
+    MultideviceSetupStateUpdater::RegisterPrefs(pref_service_.registry());
+
+    updater_ = std::make_unique<MultideviceSetupStateUpdater>(
+        &pref_service_, &fake_multidevice_setup_client_,
+        &fake_notification_access_manager_);
+  }
+
+  void SetNotififcationAccess(bool enabled) {
+    fake_notification_access_manager_.SetHasAccessBeenGrantedInternal(enabled);
+  }
+
+  void SetFeatureState(Feature feature, FeatureState feature_state) {
+    fake_multidevice_setup_client_.SetFeatureState(feature, feature_state);
+  }
+
+  void SetHostStatus(HostStatus host_status) {
+    fake_multidevice_setup_client_.SetHostStatusWithDevice(
+        std::make_pair(host_status, base::nullopt /* host_device */));
+  }
+
+  multidevice_setup::FakeMultiDeviceSetupClient*
+  fake_multidevice_setup_client() {
+    return &fake_multidevice_setup_client_;
+  }
+
+ private:
+  std::unique_ptr<MultideviceSetupStateUpdater> updater_;
+
+  TestingPrefServiceSimple pref_service_;
+  multidevice_setup::FakeMultiDeviceSetupClient fake_multidevice_setup_client_;
+  FakeNotificationAccessManager fake_notification_access_manager_;
+};
+
+TEST_F(MultideviceSetupStateUpdaterTest, EnablePhoneHub) {
+  // Test that there is a call to enable kPhoneHub when host status goes from
+  // kHostSetLocallyButWaitingForBackendConfirmation to kHostVerified.
+  SetHostStatus(HostStatus::kHostSetLocallyButWaitingForBackendConfirmation);
+  SetHostStatus(HostStatus::kHostVerified);
+  fake_multidevice_setup_client()->InvokePendingSetFeatureEnabledStateCallback(
+      /*expected_feature=*/Feature::kPhoneHub,
+      /*expected_enabled=*/true, /*expected_auth_token=*/base::nullopt,
+      /*success=*/true);
+
+  // Test that there is a call to enable kPhoneHub when host status goes from
+  // kHostSetLocallyButWaitingForBackendConfirmation to
+  // kHostSetButNotYetVerified, then finally to kHostVerified.
+  SetHostStatus(HostStatus::kHostSetLocallyButWaitingForBackendConfirmation);
+  SetHostStatus(HostStatus::kHostSetButNotYetVerified);
+  SetHostStatus(HostStatus::kHostVerified);
+  fake_multidevice_setup_client()->InvokePendingSetFeatureEnabledStateCallback(
+      /*expected_feature=*/Feature::kPhoneHub,
+      /*expected_enabled=*/true, /*expected_auth_token=*/base::nullopt,
+      /*success=*/true);
+}
+
+TEST_F(MultideviceSetupStateUpdaterTest, DisablePhoneHubNotifications) {
+  SetNotififcationAccess(true);
+
+  // Test that there is a call to disable kPhoneHubNotifications when
+  // notification access has been revoked.
+  SetNotififcationAccess(false);
+  fake_multidevice_setup_client()->InvokePendingSetFeatureEnabledStateCallback(
+      /*expected_feature=*/Feature::kPhoneHubNotifications,
+      /*expected_enabled=*/false, /*expected_auth_token=*/base::nullopt,
+      /*success=*/true);
+}
+
+}  // namespace phonehub
+}  // namespace chromeos

chromeos/components/phonehub/phone_hub_manager_impl.cc

@@ -15,6 +15,7 @@
 #include "chromeos/components/phonehub/find_my_device_controller_impl.h"
 #include "chromeos/components/phonehub/message_receiver_impl.h"
 #include "chromeos/components/phonehub/message_sender_impl.h"
+#include "chromeos/components/phonehub/multidevice_setup_state_updater.h"
 #include "chromeos/components/phonehub/mutable_phone_model.h"
 #include "chromeos/components/phonehub/notification_access_manager_impl.h"
 #include "chromeos/components/phonehub/notification_manager_impl.h"
@@ -87,7 +88,12 @@ PhoneHubManagerImpl::PhoneHubManagerImpl(
           std::make_unique<BrowserTabsModelController>(
               multidevice_setup_client,
               browser_tabs_model_provider_.get(),
-              phone_model_.get())) {}
+              phone_model_.get())),
+      multidevice_setup_state_updater_(
+          std::make_unique<MultideviceSetupStateUpdater>(
+              pref_service,
+              multidevice_setup_client,
+              notification_access_manager_.get())) {}
 
 PhoneHubManagerImpl::~PhoneHubManagerImpl() = default;
 
@@ -130,6 +136,7 @@ TetherController* PhoneHubManagerImpl::GetTetherController() {
 // These should be destroyed in the opposite order of how these objects are
 // initialized in the constructor.
 void PhoneHubManagerImpl::Shutdown() {
+  multidevice_setup_state_updater_.reset();
   browser_tabs_model_controller_.reset();
   browser_tabs_model_provider_.reset();
   tether_controller_.reset();

chromeos/components/phonehub/phone_hub_manager_impl.h

@@ -35,6 +35,7 @@ class ConnectionManager;
 class CrosStateSender;
 class MessageSender;
 class MessageReceiver;
+class MultideviceSetupStateUpdater;
 class MutablePhoneModel;
 class PhoneStatusProcessor;
 
@@ -82,6 +83,8 @@ class PhoneHubManagerImpl : public PhoneHubManager, public KeyedService {
   std::unique_ptr<TetherController> tether_controller_;
   std::unique_ptr<BrowserTabsModelProvider> browser_tabs_model_provider_;
   std::unique_ptr<BrowserTabsModelController> browser_tabs_model_controller_;
+  std::unique_ptr<MultideviceSetupStateUpdater>
+      multidevice_setup_state_updater_;
 };
 
 }  // namespace phonehub

chromeos/components/phonehub/pref_names.cc

@@ -16,6 +16,11 @@ const char kNotificationAccessGranted[] =
 const char kHasDismissedUiAfterCompletingOnboarding[] =
     "cros.phonehub.has_completed_onboarding_before";
 
+// Whether the MultideviceSetupStateUpdater is waiting for a verified host
+// in order to enable the Multidevice PhoneHub feature.
+const char kIsAwaitingVerifiedHost[] =
+    "cros.phonehub.is_awaiting_verified_host";
+
 }  // namespace prefs
 }  // namespace phonehub
 }  // namespace chromeos

chromeos/components/phonehub/pref_names.h

@@ -11,6 +11,7 @@ namespace prefs {
 
 extern const char kNotificationAccessGranted[];
 extern const char kHasDismissedUiAfterCompletingOnboarding[];
+extern const char kIsAwaitingVerifiedHost[];
 
 }  // namespace prefs
 }  // namespace phonehub

chromeos/services/multidevice_setup/feature_state_manager_impl_unittest.cc

@@ -489,6 +489,9 @@ TEST_F(MultiDeviceSetupFeatureStateManagerImplTest, PhoneHub) {
   VerifyFeatureStateChange(1u /* expected_index */, mojom::Feature::kPhoneHub,
                            mojom::FeatureState::kNotSupportedByPhone);
 
+  // This pref should is disabled for existing Better Together users;
+  // they must go to settings to explicitly enable PhoneHub.
+  test_pref_service()->SetBoolean(kPhoneHubEnabledPrefName, true);
   SetSoftwareFeatureState(false /* use_local_device */,
                           multidevice::SoftwareFeature::kPhoneHubHost,
                           multidevice::SoftwareFeatureState::kEnabled);

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

@@ -55,7 +55,10 @@ void RegisterFeaturePrefs(PrefRegistrySimple* registry) {
   registry->RegisterBooleanPref(kMessagesEnabledPrefName, true);
   registry->RegisterBooleanPref(kSmartLockEnabledDeprecatedPrefName, true);
   registry->RegisterBooleanPref(kSmartLockEnabledPrefName, true);
-  registry->RegisterBooleanPref(kPhoneHubEnabledPrefName, true);
+
+  // This pref should be disabled for existing Better Together users;
+  // they must go to settings to explicitly enable PhoneHub.
+  registry->RegisterBooleanPref(kPhoneHubEnabledPrefName, false);
   registry->RegisterBooleanPref(kPhoneHubNotificationsEnabledPrefName, true);
   registry->RegisterBooleanPref(kPhoneHubTaskContinuationEnabledPrefName, true);
 }