Implement NotificationAccessManager

- Responsible for sending requests for displaying the notification
  access setup UI to the user's phone.

Bug: 1106937
Test: chromeos_components_unittests

Change-Id: If00b19664b4dd6240558e48b4bf5bb45c98e2723
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2453511
Commit-Queue: Jimmy Gong <jimmyxgong@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#815448}

chromeos/components/phonehub/notification_access_manager.cc

@@ -30,7 +30,7 @@ NotificationAccessManager::AttemptNotificationSetup(
                      weak_ptr_factory_.GetWeakPtr(), operation_id)));
   id_to_operation_map_.emplace(operation_id, operation.get());
 
-  OnSetupAttemptStarted();
+  OnSetupRequested();
   return operation;
 }
 
@@ -70,8 +70,9 @@ void NotificationAccessManager::OnSetupOperationDeleted(int operation_id) {
     return;
 
   id_to_operation_map_.erase(it);
+
   if (id_to_operation_map_.empty())
-    OnSetupAttemptEnded();
+    PA_LOG(INFO) << "Notification access setup operation has ended.";
 }
 
 }  // namespace phonehub

chromeos/components/phonehub/notification_access_manager.h

@@ -66,8 +66,7 @@ class NotificationAccessManager {
 
   bool IsSetupOperationInProgress() const;
 
-  virtual void OnSetupAttemptStarted() {}
-  virtual void OnSetupAttemptEnded() {}
+  virtual void OnSetupRequested() {}
 
  private:
   friend class NotificationAccessManagerImplTest;

chromeos/components/phonehub/notification_access_manager_impl.cc

@@ -5,6 +5,8 @@
 #include "chromeos/components/phonehub/notification_access_manager_impl.h"
 
 #include "chromeos/components/multidevice/logging/logging.h"
+#include "chromeos/components/phonehub/connection_scheduler.h"
+#include "chromeos/components/phonehub/message_sender.h"
 #include "chromeos/components/phonehub/pref_names.h"
 #include "components/prefs/pref_registry_simple.h"
 #include "components/prefs/pref_service.h"
@@ -19,10 +21,24 @@ void NotificationAccessManagerImpl::RegisterPrefs(
 }
 
 NotificationAccessManagerImpl::NotificationAccessManagerImpl(
-    PrefService* pref_service)
-    : pref_service_(pref_service) {}
+    PrefService* pref_service,
+    FeatureStatusProvider* feature_status_provider,
+    MessageSender* message_sender,
+    ConnectionScheduler* connection_scheduler)
+    : pref_service_(pref_service),
+      feature_status_provider_(feature_status_provider),
+      message_sender_(message_sender),
+      connection_scheduler_(connection_scheduler) {
+  DCHECK(feature_status_provider_);
+  DCHECK(message_sender_);
 
-NotificationAccessManagerImpl::~NotificationAccessManagerImpl() = default;
+  current_feature_status_ = feature_status_provider_->GetStatus();
+  feature_status_provider_->AddObserver(this);
+}
+
+NotificationAccessManagerImpl::~NotificationAccessManagerImpl() {
+  feature_status_provider_->RemoveObserver(this);
+}
 
 bool NotificationAccessManagerImpl::HasAccessBeenGranted() const {
   return pref_service_->GetBoolean(prefs::kNotificationAccessGranted);
@@ -30,19 +46,84 @@ bool NotificationAccessManagerImpl::HasAccessBeenGranted() const {
 
 void NotificationAccessManagerImpl::SetHasAccessBeenGrantedInternal(
     bool has_access_been_granted) {
+  if (has_access_been_granted == HasAccessBeenGranted())
+    return;
+
   PA_LOG(INFO) << "Notification access state has been set to: "
                << has_access_been_granted;
-  // TODO(jimmyxgong): Implement this stub function.
+
+  pref_service_->SetBoolean(prefs::kNotificationAccessGranted,
+                            has_access_been_granted);
+  NotifyNotificationAccessChanged();
+
+  if (IsSetupOperationInProgress() && has_access_been_granted) {
+    SetNotificationSetupOperationStatus(
+        NotificationAccessSetupOperation::Status::kCompletedSuccessfully);
+  }
 }
 
-void NotificationAccessManagerImpl::OnSetupAttemptStarted() {
+void NotificationAccessManagerImpl::OnSetupRequested() {
   PA_LOG(INFO) << "Notification access setup flow started.";
-  // TODO(khorimoto): Attempt notification setup flow.
+
+  switch (feature_status_provider_->GetStatus()) {
+    // We're already connected, so request that the UI be shown on the phone.
+    case FeatureStatus::kEnabledAndConnected:
+      SendShowNotificationAccessSetupRequest();
+      break;
+    // We're already connecting, so wait until a connection succeeds before
+    // trying to send a message
+    case FeatureStatus::kEnabledAndConnecting:
+      break;
+    // We are not connected, so schedule a connection; once the
+    // connection succeeds, we'll send the message in OnFeatureStatusChanged().
+    case FeatureStatus::kEnabledButDisconnected:
+      connection_scheduler_->ScheduleConnectionNow();
+      break;
+    default:
+      NOTREACHED();
+      break;
+  }
+}
+
+void NotificationAccessManagerImpl::OnFeatureStatusChanged() {
+  if (!IsSetupOperationInProgress())
+    return;
+
+  const FeatureStatus previous_feature_status = current_feature_status_;
+  current_feature_status_ = feature_status_provider_->GetStatus();
+
+  if (previous_feature_status == current_feature_status_)
+    return;
+
+  // If we were previously connecting and could not establish a connection,
+  // send a timeout state.
+  if (previous_feature_status == FeatureStatus::kEnabledAndConnecting &&
+      current_feature_status_ != FeatureStatus::kEnabledAndConnected) {
+    SetNotificationSetupOperationStatus(
+        NotificationAccessSetupOperation::Status::kTimedOutConnecting);
+    return;
+  }
+
+  // If we were previously connected and are now no longer connected, send a
+  // connection disconnected state.
+  if (previous_feature_status == FeatureStatus::kEnabledAndConnected &&
+      current_feature_status_ != FeatureStatus::kEnabledAndConnected) {
+    SetNotificationSetupOperationStatus(
+        NotificationAccessSetupOperation::Status::kConnectionDisconnected);
+    return;
+  }
+
+  if (current_feature_status_ == FeatureStatus::kEnabledAndConnected) {
+    SendShowNotificationAccessSetupRequest();
+    return;
+  }
 }
 
-void NotificationAccessManagerImpl::OnSetupAttemptEnded() {
-  PA_LOG(INFO) << "Notification access setup flow ended.";
-  // TODO(khorimoto): Stop ongoing notification setup flow.
+void NotificationAccessManagerImpl::SendShowNotificationAccessSetupRequest() {
+  message_sender_->SendShowNotificationAccessSetupRequest();
+  SetNotificationSetupOperationStatus(
+      NotificationAccessSetupOperation::Status::
+          kSentMessageToPhoneAndWaitingForResponse);
 }
 
 }  // namespace phonehub

chromeos/components/phonehub/notification_access_manager_impl.h

@@ -7,32 +7,52 @@
 
 #include "chromeos/components/phonehub/notification_access_manager.h"
 
+#include "chromeos/components/phonehub/feature_status_provider.h"
+
 class PrefRegistrySimple;
 class PrefService;
 
 namespace chromeos {
 namespace phonehub {
 
+class MessageSender;
+class ConnectionScheduler;
+
 // Implements NotificationAccessManager by persisting the last-known
 // notification access value to user prefs.
 // TODO(khorimoto): Currently HasAccessBeenGranted() always returns false. Have
 // it return true once the phone has sent a message indicating that it has
 // granted access.
-class NotificationAccessManagerImpl : public NotificationAccessManager {
+class NotificationAccessManagerImpl : public NotificationAccessManager,
+                                      public FeatureStatusProvider::Observer {
  public:
   static void RegisterPrefs(PrefRegistrySimple* registry);
 
-  explicit NotificationAccessManagerImpl(PrefService* pref_service);
+  explicit NotificationAccessManagerImpl(
+      PrefService* pref_service,
+      FeatureStatusProvider* feature_status_provider,
+      MessageSender* message_sender,
+      ConnectionScheduler* connection_scheduler);
   ~NotificationAccessManagerImpl() override;
 
  private:
+  friend class NotificationAccessManagerImplTest;
+
   // NotificationAccessManager:
   bool HasAccessBeenGranted() const override;
   void SetHasAccessBeenGrantedInternal(bool has_access_been_granted) override;
-  void OnSetupAttemptStarted() override;
-  void OnSetupAttemptEnded() override;
+  void OnSetupRequested() override;
+
+  // FeatureStatusProvider::Observer:
+  void OnFeatureStatusChanged() override;
+
+  void SendShowNotificationAccessSetupRequest();
 
+  FeatureStatus current_feature_status_;
   PrefService* pref_service_;
+  FeatureStatusProvider* feature_status_provider_;
+  MessageSender* message_sender_;
+  ConnectionScheduler* connection_scheduler_;
 };
 
 }  // namespace phonehub

chromeos/components/phonehub/notification_access_manager_impl_unittest.cc

@@ -6,6 +6,9 @@
 
 #include <memory>
 
+#include "chromeos/components/phonehub/fake_connection_scheduler.h"
+#include "chromeos/components/phonehub/fake_feature_status_provider.h"
+#include "chromeos/components/phonehub/fake_message_sender.h"
 #include "chromeos/components/phonehub/notification_access_setup_operation.h"
 #include "chromeos/components/phonehub/pref_names.h"
 #include "components/prefs/testing_pref_service.h"
@@ -62,23 +65,62 @@ class NotificationAccessManagerImplTest : public testing::Test {
   // testing::Test:
   void SetUp() override {
     NotificationAccessManagerImpl::RegisterPrefs(pref_service_.registry());
+    fake_feature_status_provider_ =
+        std::make_unique<FakeFeatureStatusProvider>();
+    fake_message_sender_ = std::make_unique<FakeMessageSender>();
+    fake_connection_scheduler_ = std::make_unique<FakeConnectionScheduler>();
   }
 
+  void TearDown() override { manager_->RemoveObserver(&fake_observer_); }
+
   void Initialize(bool initial_has_access_been_granted) {
     pref_service_.SetBoolean(prefs::kNotificationAccessGranted,
                              initial_has_access_been_granted);
-    manager_ = std::make_unique<NotificationAccessManagerImpl>(&pref_service_);
+    manager_ = std::make_unique<NotificationAccessManagerImpl>(
+        &pref_service_, fake_feature_status_provider_.get(),
+        fake_message_sender_.get(), fake_connection_scheduler_.get());
+    manager_->AddObserver(&fake_observer_);
+  }
+
+  NotificationAccessSetupOperation::Status
+  GetNotifcationAccessSetupOperationStatus() {
+    return fake_delegate_.status();
+  }
+
+  void VerifyNotificationAccessGrantedState(bool expected_value) {
+    EXPECT_EQ(expected_value,
+              pref_service_.GetBoolean(prefs::kNotificationAccessGranted));
+    EXPECT_EQ(expected_value, manager_->HasAccessBeenGranted());
   }
 
   std::unique_ptr<NotificationAccessSetupOperation> StartSetupOperation() {
     return manager_->AttemptNotificationSetup(&fake_delegate_);
   }
 
-  bool GetHasAccessBeenGranted() { return manager_->HasAccessBeenGranted(); }
   bool IsSetupOperationInProgress() {
     return manager_->IsSetupOperationInProgress();
   }
 
+  void SetHasAccessBeenGrantedInternal(bool has_access_been_granted) {
+    manager_->SetHasAccessBeenGrantedInternal(has_access_been_granted);
+  }
+
+  void SetFeatureStatus(FeatureStatus status) {
+    fake_feature_status_provider_->SetStatus(status);
+  }
+
+  FeatureStatus GetFeatureStatus() {
+    return fake_feature_status_provider_->GetStatus();
+  }
+
+  size_t GetNumScheduleConnectionNowCalls() const {
+    return fake_connection_scheduler_->num_schedule_connection_now_calls();
+  }
+
+  size_t GetNumShowNotificationAccessSetupRequestCount() const {
+    return fake_message_sender_->show_notification_access_setup_request_count();
+  }
+
   size_t GetNumObserverCalls() const { return fake_observer_.num_calls(); }
 
  private:
@@ -86,12 +128,15 @@ class NotificationAccessManagerImplTest : public testing::Test {
 
   FakeObserver fake_observer_;
   FakeOperationDelegate fake_delegate_;
+  std::unique_ptr<FakeFeatureStatusProvider> fake_feature_status_provider_;
+  std::unique_ptr<FakeMessageSender> fake_message_sender_;
+  std::unique_ptr<FakeConnectionScheduler> fake_connection_scheduler_;
   std::unique_ptr<NotificationAccessManager> manager_;
 };
 
 TEST_F(NotificationAccessManagerImplTest, InitiallyGranted) {
   Initialize(/*initial_has_access_been_granted=*/true);
-  EXPECT_TRUE(GetHasAccessBeenGranted());
+  VerifyNotificationAccessGrantedState(/*expected_value=*/true);
 
   // Cannot start the notification access setup flow if access has already been
   // granted.
@@ -99,17 +144,183 @@ TEST_F(NotificationAccessManagerImplTest, InitiallyGranted) {
   EXPECT_FALSE(operation);
 }
 
-TEST_F(NotificationAccessManagerImplTest, InitiallyNotGranted) {
+TEST_F(NotificationAccessManagerImplTest, OnFeatureStatusChanged) {
+  Initialize(/*initial_has_access_been_granted=*/false);
+  VerifyNotificationAccessGrantedState(/*expected_value=*/false);
+
+  // Set initial state to disconnected.
+  SetFeatureStatus(FeatureStatus::kEnabledButDisconnected);
+  EXPECT_EQ(0u, GetNumShowNotificationAccessSetupRequestCount());
+  EXPECT_EQ(NotificationAccessSetupOperation::Status::kConnecting,
+            GetNotifcationAccessSetupOperationStatus());
+
+  // Simulate feature status to be enabled and connected. SetupOperation is also
+  // not in progress, so expect no new requests to be sent.
+  SetFeatureStatus(FeatureStatus::kEnabledAndConnected);
+  EXPECT_EQ(0u, GetNumShowNotificationAccessSetupRequestCount());
+  EXPECT_EQ(NotificationAccessSetupOperation::Status::kConnecting,
+            GetNotifcationAccessSetupOperationStatus());
+
+  // Simulate setup operation is in progress. This will trigger a sent request.
+  auto operation = StartSetupOperation();
+  EXPECT_TRUE(operation);
+  EXPECT_EQ(1u, GetNumShowNotificationAccessSetupRequestCount());
+  EXPECT_EQ(NotificationAccessSetupOperation::Status::
+                kSentMessageToPhoneAndWaitingForResponse,
+            GetNotifcationAccessSetupOperationStatus());
+
+  // Set another feature status, expect status to be updated.
+  SetFeatureStatus(FeatureStatus::kEnabledButDisconnected);
+  EXPECT_EQ(1u, GetNumShowNotificationAccessSetupRequestCount());
+  EXPECT_EQ(NotificationAccessSetupOperation::Status::kConnectionDisconnected,
+            GetNotifcationAccessSetupOperationStatus());
+}
+
+TEST_F(NotificationAccessManagerImplTest, StartDisconnectedAndNoAccess) {
+  // Set initial state to disconnected.
+  SetFeatureStatus(FeatureStatus::kEnabledButDisconnected);
+
   Initialize(/*initial_has_access_been_granted=*/false);
-  EXPECT_FALSE(GetHasAccessBeenGranted());
+  VerifyNotificationAccessGrantedState(/*expected_value=*/false);
 
+  // Start a setup operation with enabled but disconnected status and access
+  // not granted.
   auto operation = StartSetupOperation();
   EXPECT_TRUE(operation);
-  EXPECT_TRUE(IsSetupOperationInProgress());
+  EXPECT_EQ(1u, GetNumScheduleConnectionNowCalls());
+
+  // Simulate changing states from connecting to connected.
+  SetFeatureStatus(FeatureStatus::kEnabledAndConnecting);
+  SetFeatureStatus(FeatureStatus::kEnabledAndConnected);
+
+  // Verify that the request message has been sent and our operation status
+  // is updated.
+  EXPECT_EQ(1u, GetNumShowNotificationAccessSetupRequestCount());
+  EXPECT_EQ(NotificationAccessSetupOperation::Status::
+                kSentMessageToPhoneAndWaitingForResponse,
+            GetNotifcationAccessSetupOperationStatus());
 
-  operation.reset();
-  EXPECT_FALSE(IsSetupOperationInProgress());
+  // Simulate getting a response back from the phone.
+  SetHasAccessBeenGrantedInternal(/*has_access_been_granted=*/true);
+  VerifyNotificationAccessGrantedState(/*expected_value=*/true);
+  EXPECT_EQ(NotificationAccessSetupOperation::Status::kCompletedSuccessfully,
+            GetNotifcationAccessSetupOperationStatus());
 }
 
+TEST_F(NotificationAccessManagerImplTest, StartConnectingAndNoAccess) {
+  // Set initial state to connecting.
+  SetFeatureStatus(FeatureStatus::kEnabledAndConnecting);
+
+  Initialize(/*initial_has_access_been_granted=*/false);
+  VerifyNotificationAccessGrantedState(/*expected_value=*/false);
+
+  // Start a setup operation with enabled and connecting status and access
+  // not granted.
+  auto operation = StartSetupOperation();
+  EXPECT_TRUE(operation);
+
+  // Simulate changing states from connecting to connected.
+  SetFeatureStatus(FeatureStatus::kEnabledAndConnected);
+
+  // Verify that the request message has been sent and our operation status
+  // is updated.
+  EXPECT_EQ(1u, GetNumShowNotificationAccessSetupRequestCount());
+  EXPECT_EQ(NotificationAccessSetupOperation::Status::
+                kSentMessageToPhoneAndWaitingForResponse,
+            GetNotifcationAccessSetupOperationStatus());
+
+  // Simulate getting a response back from the phone.
+  SetHasAccessBeenGrantedInternal(/*has_access_been_granted=*/true);
+  VerifyNotificationAccessGrantedState(/*expected_value=*/true);
+  EXPECT_EQ(NotificationAccessSetupOperation::Status::kCompletedSuccessfully,
+            GetNotifcationAccessSetupOperationStatus());
+}
+
+TEST_F(NotificationAccessManagerImplTest, StartConnectedAndNoAccess) {
+  // Set initial state to connected.
+  SetFeatureStatus(FeatureStatus::kEnabledAndConnected);
+
+  Initialize(/*initial_has_access_been_granted=*/false);
+  VerifyNotificationAccessGrantedState(/*expected_value=*/false);
+
+  // Start a setup operation with enabled and connected status and access
+  // not granted.
+  auto operation = StartSetupOperation();
+  EXPECT_TRUE(operation);
+
+  // Verify that the request message has been sent and our operation status
+  // is updated.
+  EXPECT_EQ(1u, GetNumShowNotificationAccessSetupRequestCount());
+  EXPECT_EQ(NotificationAccessSetupOperation::Status::
+                kSentMessageToPhoneAndWaitingForResponse,
+            GetNotifcationAccessSetupOperationStatus());
+
+  // Simulate getting a response back from the phone.
+  SetHasAccessBeenGrantedInternal(/*has_access_been_granted=*/true);
+  VerifyNotificationAccessGrantedState(/*expected_value=*/true);
+  EXPECT_EQ(NotificationAccessSetupOperation::Status::kCompletedSuccessfully,
+            GetNotifcationAccessSetupOperationStatus());
+}
+
+TEST_F(NotificationAccessManagerImplTest, SimulateConnectingToDisconnected) {
+  // Set initial state to connecting.
+  SetFeatureStatus(FeatureStatus::kEnabledAndConnecting);
+
+  Initialize(/*initial_has_access_been_granted=*/false);
+  VerifyNotificationAccessGrantedState(/*expected_value=*/false);
+
+  auto operation = StartSetupOperation();
+  EXPECT_TRUE(operation);
+
+  // Simulate a disconnection and expect that status has been updated.
+  SetFeatureStatus(FeatureStatus::kEnabledButDisconnected);
+  EXPECT_EQ(NotificationAccessSetupOperation::Status::kTimedOutConnecting,
+            GetNotifcationAccessSetupOperationStatus());
+}
+
+TEST_F(NotificationAccessManagerImplTest, SimulateConnectedToDisconnected) {
+  // Simulate connected state.
+  SetFeatureStatus(FeatureStatus::kEnabledAndConnected);
+
+  Initialize(/*initial_has_access_been_granted=*/false);
+  VerifyNotificationAccessGrantedState(/*expected_value=*/false);
+
+  auto operation = StartSetupOperation();
+  EXPECT_TRUE(operation);
+
+  EXPECT_EQ(1u, GetNumShowNotificationAccessSetupRequestCount());
+
+  // Simulate a disconnection, expect status update.
+  SetFeatureStatus(FeatureStatus::kEnabledButDisconnected);
+  EXPECT_EQ(NotificationAccessSetupOperation::Status::kConnectionDisconnected,
+            GetNotifcationAccessSetupOperationStatus());
+}
+
+TEST_F(NotificationAccessManagerImplTest, SimulateConnectedToDisabled) {
+  // Simulate connected state.
+  SetFeatureStatus(FeatureStatus::kEnabledAndConnected);
+
+  Initialize(/*initial_has_access_been_granted=*/false);
+  VerifyNotificationAccessGrantedState(/*expected_value=*/false);
+
+  auto operation = StartSetupOperation();
+  EXPECT_TRUE(operation);
+
+  EXPECT_EQ(1u, GetNumShowNotificationAccessSetupRequestCount());
+
+  // Simulate disabling the feature, expect status update.
+  SetFeatureStatus(FeatureStatus::kDisabled);
+  EXPECT_EQ(NotificationAccessSetupOperation::Status::kConnectionDisconnected,
+            GetNotifcationAccessSetupOperationStatus());
+}
+
+TEST_F(NotificationAccessManagerImplTest, FlipAccessGrantedToNotGranted) {
+  Initialize(/*initial_has_access_been_granted=*/true);
+  VerifyNotificationAccessGrantedState(/*expected_value=*/true);
+
+  // Simulate flipping the access state to no granted.
+  SetHasAccessBeenGrantedInternal(/*has_access_been_granted=*/false);
+  VerifyNotificationAccessGrantedState(/*expected_value=*/false);
+}
 }  // namespace phonehub
 }  // namespace chromeos

chromeos/components/phonehub/phone_hub_manager_impl.cc

@@ -58,7 +58,11 @@ PhoneHubManagerImpl::PhoneHubManagerImpl(
           do_not_disturb_controller_.get(),
           message_sender_.get())),
       notification_access_manager_(
-          std::make_unique<NotificationAccessManagerImpl>(pref_service)),
+          std::make_unique<NotificationAccessManagerImpl>(
+              pref_service,
+              feature_status_provider_.get(),
+              message_sender_.get(),
+              connection_scheduler_.get())),
       notification_manager_(std::make_unique<NotificationManagerImpl>()),
       onboarding_ui_tracker_(std::make_unique<OnboardingUiTrackerImpl>(
           pref_service,