Reland "[CrOS PhoneHub] Add setter functions to various manager classes"

This is a reland of 6a3c229f

Original change's description:
> [CrOS PhoneHub] Add setter functions to various manager classes
>
> The added setters are going to be used by a future Cl that implements
> the PhoneStatusProcessor class.
>
> Bug: 1106937
> Change-Id: Ie95210cfcedbba2ad3ed5875a34ef21a7486b13f
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2423346
> Commit-Queue: Jimmy Gong <jimmyxgong@chromium.org>
> Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#810084}

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

ash/system/phonehub/phone_hub_notification_controller_unittest.cc

@@ -77,7 +77,7 @@ class PhoneHubNotificationControllerTest : public AshTestBase {
 
 TEST_F(PhoneHubNotificationControllerTest, AddNotifications) {
   EXPECT_FALSE(message_center_->NotificationCount());
-  notification_manager_.SetNotifications(fake_notifications_);
+  notification_manager_.SetNotificationsInternal(fake_notifications_);
   EXPECT_EQ(3u, message_center_->NotificationCount());
 
   ASSERT_TRUE(
@@ -95,7 +95,7 @@ TEST_F(PhoneHubNotificationControllerTest, AddNotifications) {
 
 TEST_F(PhoneHubNotificationControllerTest, UpdateNotifications) {
   EXPECT_FALSE(message_center_->NotificationCount());
-  notification_manager_.SetNotifications(fake_notifications_);
+  notification_manager_.SetNotificationsInternal(fake_notifications_);
   EXPECT_EQ(3u, message_center_->NotificationCount());
 
   auto* notification =
@@ -124,7 +124,7 @@ TEST_F(PhoneHubNotificationControllerTest, UpdateNotifications) {
 
 TEST_F(PhoneHubNotificationControllerTest, RemoveNotifications) {
   EXPECT_FALSE(message_center_->NotificationCount());
-  notification_manager_.SetNotifications(fake_notifications_);
+  notification_manager_.SetNotificationsInternal(fake_notifications_);
   EXPECT_EQ(3u, message_center_->NotificationCount());
 
   notification_manager_.RemoveNotification(kPhoneHubNotificationId0);
@@ -132,13 +132,13 @@ TEST_F(PhoneHubNotificationControllerTest, RemoveNotifications) {
   EXPECT_FALSE(
       message_center_->FindVisibleNotificationById(kCrOSNotificationId0));
 
-  notification_manager_.RemoveNotifications(base::flat_set<int64_t>(
+  notification_manager_.RemoveNotificationsInternal(base::flat_set<int64_t>(
       {kPhoneHubNotificationId1, kPhoneHubNotificationId2}));
   EXPECT_FALSE(message_center_->NotificationCount());
 }
 
 TEST_F(PhoneHubNotificationControllerTest, CloseByUser) {
-  notification_manager_.SetNotifications(fake_notifications_);
+  notification_manager_.SetNotificationsInternal(fake_notifications_);
   EXPECT_EQ(3u, message_center_->NotificationCount());
 
   message_center_->RemoveNotification(kCrOSNotificationId0, /*by_user=*/true);
@@ -152,7 +152,7 @@ TEST_F(PhoneHubNotificationControllerTest, CloseByUser) {
 }
 
 TEST_F(PhoneHubNotificationControllerTest, InlineReply) {
-  notification_manager_.SetNotifications(fake_notifications_);
+  notification_manager_.SetNotificationsInternal(fake_notifications_);
 
   const base::string16 kInlineReply0 = base::UTF8ToUTF16("inline reply 0");
   const base::string16 kInlineReply1 = base::UTF8ToUTF16("inline reply 1");

ash/system/phonehub/phone_hub_tray_unittest.cc

@@ -126,7 +126,7 @@ TEST_F(PhoneHubTrayTest, ClickTrayButton) {
 }
 
 TEST_F(PhoneHubTrayTest, ShowNotificationOptInViewWhenAccessNotGranted) {
-  GetNotificationAccessManager()->SetHasAccessBeenGranted(false);
+  GetNotificationAccessManager()->SetHasAccessBeenGrantedInternal(false);
 
   ClickTrayButton();
 
@@ -141,7 +141,7 @@ TEST_F(PhoneHubTrayTest, ShowNotificationOptInViewWhenAccessNotGranted) {
 }
 
 TEST_F(PhoneHubTrayTest, HideNotificationOptInViewWhenAccessHasBeenGranted) {
-  GetNotificationAccessManager()->SetHasAccessBeenGranted(true);
+  GetNotificationAccessManager()->SetHasAccessBeenGrantedInternal(true);
 
   ClickTrayButton();
 
@@ -149,7 +149,7 @@ TEST_F(PhoneHubTrayTest, HideNotificationOptInViewWhenAccessHasBeenGranted) {
 }
 
 TEST_F(PhoneHubTrayTest, StartNotificationSetUpFlow) {
-  GetNotificationAccessManager()->SetHasAccessBeenGranted(false);
+  GetNotificationAccessManager()->SetHasAccessBeenGrantedInternal(false);
 
   ClickTrayButton();
   EXPECT_TRUE(notification_opt_in_view());

chrome/browser/ui/webui/chromeos/multidevice_internals/multidevice_internals_phone_hub_handler.cc

@@ -266,7 +266,7 @@ void MultidevicePhoneHubHandler::HandleEnableDnd(const base::ListValue* args) {
   CHECK(args->GetBoolean(0, &enabled));
   PA_LOG(VERBOSE) << "Setting Do Not Disturb state to " << enabled;
   fake_phone_hub_manager_->fake_do_not_disturb_controller()
-      ->SetDoNotDisturbState(enabled);
+      ->SetDoNotDisturbStateInternal(enabled);
 }
 
 void MultidevicePhoneHubHandler::HandleSetFindMyDeviceStatus(
@@ -278,7 +278,7 @@ void MultidevicePhoneHubHandler::HandleSetFindMyDeviceStatus(
   bool is_ringing = status_as_int == 2;
   PA_LOG(VERBOSE) << "Setting phone ringing status to " << is_ringing;
   fake_phone_hub_manager_->fake_find_my_device_controller()
-      ->SetPhoneRingingState(is_ringing);
+      ->SetIsPhoneRingingInternal(is_ringing);
 }
 
 void MultidevicePhoneHubHandler::HandleSetTetherStatus(

chrome/browser/ui/webui/settings/chromeos/multidevice_handler_unittest.cc

@@ -236,7 +236,7 @@ class MultideviceHandlerTest : public testing::Test {
   }
 
   void CallAttemptNotificationSetup(bool has_access_been_granted) {
-    fake_notification_access_manager()->SetHasAccessBeenGranted(
+    fake_notification_access_manager()->SetHasAccessBeenGrantedInternal(
         has_access_been_granted);
     base::ListValue empty_args;
     test_web_ui()->HandleReceivedMessage("attemptNotificationSetup",

chromeos/components/phonehub/do_not_disturb_controller.h

@@ -32,7 +32,7 @@ class DoNotDisturbController {
   // Note: Setting DND state is not a synchronous operation, since it requires
   // sending a message to the connected phone. Use the observer interface to be
   // notified of when the state changes.
-  virtual void SetDoNotDisturbState(bool enabled) = 0;
+  virtual void RequestNewDoNotDisturbState(bool enabled) = 0;
 
   void AddObserver(Observer* observer);
   void RemoveObserver(Observer* observer);
@@ -40,6 +40,10 @@ class DoNotDisturbController {
  protected:
   DoNotDisturbController();
 
+  // This only sets the internal state of the DoNotDisturb mode and does not
+  // send a request to set the state of the remote phone device.
+  virtual void SetDoNotDisturbStateInternal(bool is_dnd_enabled) = 0;
+
   void NotifyDndStateChanged();
 
  private:

chromeos/components/phonehub/do_not_disturb_controller_impl.cc

@@ -17,7 +17,12 @@ bool DoNotDisturbControllerImpl::IsDndEnabled() const {
   return is_dnd_enabled_;
 }
 
-void DoNotDisturbControllerImpl::SetDoNotDisturbState(bool enabled) {
+void DoNotDisturbControllerImpl::SetDoNotDisturbStateInternal(
+    bool is_dnd_enabled) {
+  is_dnd_enabled_ = is_dnd_enabled;
+}
+
+void DoNotDisturbControllerImpl::RequestNewDoNotDisturbState(bool enabled) {
   PA_LOG(INFO) << "Attempting to set DND state; new value: " << enabled;
 }
 

chromeos/components/phonehub/do_not_disturb_controller_impl.h

@@ -19,7 +19,8 @@ class DoNotDisturbControllerImpl : public DoNotDisturbController {
  private:
   // DoNotDisturbController:
   bool IsDndEnabled() const override;
-  void SetDoNotDisturbState(bool enabled) override;
+  void SetDoNotDisturbStateInternal(bool is_dnd_enabled) override;
+  void RequestNewDoNotDisturbState(bool enabled) override;
 
   bool is_dnd_enabled_ = false;
 };

chromeos/components/phonehub/fake_do_not_disturb_controller.cc

@@ -15,13 +15,18 @@ bool FakeDoNotDisturbController::IsDndEnabled() const {
   return is_dnd_enabled_;
 }
 
-void FakeDoNotDisturbController::SetDoNotDisturbState(bool enabled) {
-  if (is_dnd_enabled_ == enabled)
+void FakeDoNotDisturbController::SetDoNotDisturbStateInternal(
+    bool is_dnd_enabled) {
+  if (is_dnd_enabled_ == is_dnd_enabled)
     return;
 
-  is_dnd_enabled_ = enabled;
+  is_dnd_enabled_ = is_dnd_enabled;
   NotifyDndStateChanged();
 }
 
+void FakeDoNotDisturbController::RequestNewDoNotDisturbState(bool enabled) {
+  SetDoNotDisturbStateInternal(enabled);
+}
+
 }  // namespace phonehub
 }  // namespace chromeos

chromeos/components/phonehub/fake_do_not_disturb_controller.h

@@ -17,7 +17,8 @@ class FakeDoNotDisturbController : public DoNotDisturbController {
 
   // DoNotDisturbController:
   bool IsDndEnabled() const override;
-  void SetDoNotDisturbState(bool enabled) override;
+  void SetDoNotDisturbStateInternal(bool is_dnd_enabled) override;
+  void RequestNewDoNotDisturbState(bool enabled) override;
 
  private:
   bool is_dnd_enabled_ = false;

chromeos/components/phonehub/fake_find_my_device_controller.cc

@@ -15,13 +15,18 @@ bool FakeFindMyDeviceController::IsPhoneRinging() const {
   return is_phone_ringing_;
 }
 
-void FakeFindMyDeviceController::SetPhoneRingingState(bool ringing) {
-  if (is_phone_ringing_ == ringing)
+void FakeFindMyDeviceController::SetIsPhoneRingingInternal(
+    bool is_phone_ringing) {
+  if (is_phone_ringing_ == is_phone_ringing)
     return;
 
-  is_phone_ringing_ = ringing;
+  is_phone_ringing_ = is_phone_ringing;
   NotifyPhoneRingingStateChanged();
 }
 
+void FakeFindMyDeviceController::RequestNewPhoneRingingState(bool ringing) {
+  SetIsPhoneRingingInternal(ringing);
+}
+
 }  // namespace phonehub
 }  // namespace chromeos

chromeos/components/phonehub/fake_find_my_device_controller.h

@@ -17,7 +17,8 @@ class FakeFindMyDeviceController : public FindMyDeviceController {
 
   // FindMyDeviceController:
   bool IsPhoneRinging() const override;
-  void SetPhoneRingingState(bool ringing) override;
+  void SetIsPhoneRingingInternal(bool is_phone_ringing) override;
+  void RequestNewPhoneRingingState(bool ringing) override;
 
  private:
   bool is_phone_ringing_ = false;

chromeos/components/phonehub/fake_notification_access_manager.cc

@@ -13,7 +13,7 @@ FakeNotificationAccessManager::FakeNotificationAccessManager(
 
 FakeNotificationAccessManager::~FakeNotificationAccessManager() = default;
 
-void FakeNotificationAccessManager::SetHasAccessBeenGranted(
+void FakeNotificationAccessManager::SetHasAccessBeenGrantedInternal(
     bool has_access_been_granted) {
   if (has_access_been_granted_ == has_access_been_granted)
     return;
@@ -30,7 +30,7 @@ void FakeNotificationAccessManager::SetNotificationSetupOperationStatus(
     NotificationAccessSetupOperation::Status new_status) {
   if (new_status ==
       NotificationAccessSetupOperation::Status::kCompletedSuccessfully) {
-    SetHasAccessBeenGranted(true);
+    SetHasAccessBeenGrantedInternal(true);
   }
 
   NotificationAccessManager::SetNotificationSetupOperationStatus(new_status);

chromeos/components/phonehub/fake_notification_access_manager.h

@@ -17,7 +17,7 @@ class FakeNotificationAccessManager : public NotificationAccessManager {
 
   using NotificationAccessManager::IsSetupOperationInProgress;
 
-  void SetHasAccessBeenGranted(bool has_access_been_granted);
+  void SetHasAccessBeenGrantedInternal(bool has_access_been_granted) override;
   void SetNotificationSetupOperationStatus(
       NotificationAccessSetupOperation::Status new_status);
 

chromeos/components/phonehub/fake_notification_manager.cc

@@ -22,10 +22,10 @@ FakeNotificationManager::~FakeNotificationManager() = default;
 
 void FakeNotificationManager::SetNotification(
     const Notification& notification) {
-  SetNotifications(base::flat_set<Notification>{notification});
+  SetNotificationsInternal(base::flat_set<Notification>{notification});
 }
 
-void FakeNotificationManager::SetNotifications(
+void FakeNotificationManager::SetNotificationsInternal(
     const base::flat_set<Notification>& notifications) {
   base::flat_set<int64_t> added_ids;
   base::flat_set<int64_t> updated_ids;
@@ -49,10 +49,10 @@ void FakeNotificationManager::SetNotifications(
 }
 
 void FakeNotificationManager::RemoveNotification(int64_t id) {
-  RemoveNotifications(base::flat_set<int64_t>{id});
+  RemoveNotificationsInternal(base::flat_set<int64_t>{id});
 }
 
-void FakeNotificationManager::RemoveNotifications(
+void FakeNotificationManager::RemoveNotificationsInternal(
     const base::flat_set<int64_t>& ids) {
   for (int64_t id : ids) {
     auto it = id_to_notification_map_.find(id);

chromeos/components/phonehub/fake_notification_manager.h

@@ -21,10 +21,11 @@ class FakeNotificationManager : public NotificationManager {
   ~FakeNotificationManager() override;
 
   void SetNotification(const Notification& notification);
-  void SetNotifications(const base::flat_set<Notification>& notifications);
+  void SetNotificationsInternal(
+      const base::flat_set<Notification>& notifications) override;
 
   void RemoveNotification(int64_t id);
-  void RemoveNotifications(const base::flat_set<int64_t>& ids);
+  void RemoveNotificationsInternal(const base::flat_set<int64_t>& ids) override;
 
   const std::vector<int64_t>& dismissed_notification_ids() const {
     return dismissed_notification_ids_;

chromeos/components/phonehub/find_my_device_controller.h

@@ -34,7 +34,7 @@ class FindMyDeviceController {
   // Note: Ringing the phone via Find My Device is not a synchronous operation,
   // since it requires sending a message to the connected phone. Use the
   // observer interface to be notified of when the state changes.
-  virtual void SetPhoneRingingState(bool ringing) = 0;
+  virtual void RequestNewPhoneRingingState(bool ringing) = 0;
 
   void AddObserver(Observer* observer);
   void RemoveObserver(Observer* observer);
@@ -42,6 +42,9 @@ class FindMyDeviceController {
  protected:
   FindMyDeviceController();
 
+  // This only sets the internal state of the whether the phone is ringin
+  // and does not send a request to start ringing the the remote phone device.
+  virtual void SetIsPhoneRingingInternal(bool is_phone_ringing) = 0;
   void NotifyPhoneRingingStateChanged();
 
  private:

chromeos/components/phonehub/find_my_device_controller_impl.cc

@@ -17,7 +17,12 @@ bool FindMyDeviceControllerImpl::IsPhoneRinging() const {
   return is_phone_ringing_;
 }
 
-void FindMyDeviceControllerImpl::SetPhoneRingingState(bool ringing) {
+void FindMyDeviceControllerImpl::SetIsPhoneRingingInternal(
+    bool is_phone_ringing) {
+  is_phone_ringing_ = is_phone_ringing;
+}
+
+void FindMyDeviceControllerImpl::RequestNewPhoneRingingState(bool ringing) {
   PA_LOG(INFO) << "Attempting to set Find My Device phone ring state; new "
                << "value: " << ringing;
 }

chromeos/components/phonehub/find_my_device_controller_impl.h

@@ -19,7 +19,8 @@ class FindMyDeviceControllerImpl : public FindMyDeviceController {
  private:
   // FindMyDeviceController:
   bool IsPhoneRinging() const override;
-  void SetPhoneRingingState(bool ringing) override;
+  void SetIsPhoneRingingInternal(bool is_phone_ringing) override;
+  void RequestNewPhoneRingingState(bool ringing) override;
 
   bool is_phone_ringing_ = false;
 };

chromeos/components/phonehub/notification_access_manager.h

@@ -72,6 +72,12 @@ class NotificationAccessManager {
  private:
   friend class NotificationAccessManagerImplTest;
 
+  // This only sets the internal state of the whether notification access has
+  // mode been enabled and does not send a request to set the state of the
+  // remote phone device.
+  virtual void SetHasAccessBeenGrantedInternal(
+      bool has_access_been_granted) = 0;
+
   void OnSetupOperationDeleted(int operation_id);
 
   int next_operation_id_ = 0;

chromeos/components/phonehub/notification_access_manager_impl.cc

@@ -28,6 +28,13 @@ bool NotificationAccessManagerImpl::HasAccessBeenGranted() const {
   return pref_service_->GetBoolean(prefs::kNotificationAccessGranted);
 }
 
+void NotificationAccessManagerImpl::SetHasAccessBeenGrantedInternal(
+    bool has_access_been_granted) {
+  PA_LOG(INFO) << "Notification access state has been set to: "
+               << has_access_been_granted;
+  // TODO(jimmyxgong): Implement this stub function.
+}
+
 void NotificationAccessManagerImpl::OnSetupAttemptStarted() {
   PA_LOG(INFO) << "Notification access setup flow started.";
   // TODO(khorimoto): Attempt notification setup flow.

chromeos/components/phonehub/notification_access_manager_impl.h

@@ -28,6 +28,7 @@ class NotificationAccessManagerImpl : public NotificationAccessManager {
  private:
   // NotificationAccessManager:
   bool HasAccessBeenGranted() const override;
+  void SetHasAccessBeenGrantedInternal(bool has_access_been_granted) override;
   void OnSetupAttemptStarted() override;
   void OnSetupAttemptEnded() override;
 

chromeos/components/phonehub/notification_manager.h

@@ -10,6 +10,7 @@
 #include "base/containers/flat_set.h"
 #include "base/observer_list.h"
 #include "base/observer_list_types.h"
+#include "chromeos/components/phonehub/notification.h"
 
 namespace chromeos {
 namespace phonehub {
@@ -62,6 +63,17 @@ class NotificationManager {
  protected:
   NotificationManager();
 
+  // Sets the internal collection of notifications. This does not send any
+  // requests to the remote phone device.
+  virtual void SetNotificationsInternal(
+      const base::flat_set<Notification>& notifications) = 0;
+
+  // Removes the dismissed notifications from the internal collection of
+  // notifications. Does not send a request to remove notifications to the
+  // remote device.
+  virtual void RemoveNotificationsInternal(
+      const base::flat_set<int64_t>& notification_ids) = 0;
+
   void NotifyNotificationsAdded(
       const base::flat_set<int64_t>& notification_ids);
   void NotifyNotificationsUpdated(

chromeos/components/phonehub/notification_manager_impl.cc

@@ -5,6 +5,7 @@
 #include "chromeos/components/phonehub/notification_manager_impl.h"
 
 #include "chromeos/components/multidevice/logging/logging.h"
+#include "chromeos/components/phonehub/notification.h"
 
 namespace chromeos {
 namespace phonehub {
@@ -18,6 +19,18 @@ const Notification* NotificationManagerImpl::GetNotification(
   return nullptr;
 }
 
+void NotificationManagerImpl::SetNotificationsInternal(
+    const base::flat_set<Notification>& notifications) {
+  PA_LOG(INFO) << "Setting notifications internally.";
+  // TODO(jimmyxong): Implement this stub function.
+}
+
+void NotificationManagerImpl::RemoveNotificationsInternal(
+    const base::flat_set<int64_t>& notification_ids) {
+  PA_LOG(INFO) << "Removing notifications internally.";
+  // TODO(jimmyxgong): Implement this stub function.
+}
+
 void NotificationManagerImpl::DismissNotification(int64_t notification_id) {
   PA_LOG(INFO) << "Dismissing notification with ID " << notification_id << ".";
 }

chromeos/components/phonehub/notification_manager_impl.h

@@ -5,6 +5,8 @@
 #ifndef CHROMEOS_COMPONENTS_PHONEHUB_NOTIFICATION_MANAGER_IMPL_H_
 #define CHROMEOS_COMPONENTS_PHONEHUB_NOTIFICATION_MANAGER_IMPL_H_
 
+#include "base/containers/flat_set.h"
+#include "chromeos/components/phonehub/notification.h"
 #include "chromeos/components/phonehub/notification_manager.h"
 
 namespace chromeos {
@@ -19,6 +21,10 @@ class NotificationManagerImpl : public NotificationManager {
  private:
   // NotificationManager:
   const Notification* GetNotification(int64_t notification_id) const override;
+  void SetNotificationsInternal(
+      const base::flat_set<Notification>& notifications) override;
+  void RemoveNotificationsInternal(
+      const base::flat_set<int64_t>& notification_ids) override;
   void DismissNotification(int64_t notification_id) override;
   void SendInlineReply(int64_t notification_id,
                        const base::string16& inline_reply_text) override;