Phonehub: Clear internal notification when notification settings is disabled

Bug: 1106937
Fixed: 1141604
Test: chromeos_components_unittests
Change-Id: I033f8685061d9e5f2cfc30218786abcf89723f40
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2503895
Commit-Queue: Jimmy Gong <jimmyxgong@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#821991}

chromeos/components/phonehub/notification_manager_impl.cc

@@ -12,12 +12,23 @@
 namespace chromeos {
 namespace phonehub {
 
-NotificationManagerImpl::NotificationManagerImpl(MessageSender* message_sender)
-    : message_sender_(message_sender) {
+using multidevice_setup::mojom::Feature;
+using multidevice_setup::mojom::FeatureState;
+
+NotificationManagerImpl::NotificationManagerImpl(
+    MessageSender* message_sender,
+    multidevice_setup::MultiDeviceSetupClient* multidevice_setup_client)
+    : message_sender_(message_sender),
+      multidevice_setup_client_(multidevice_setup_client) {
   DCHECK(message_sender_);
+  DCHECK(multidevice_setup_client_);
+
+  multidevice_setup_client_->AddObserver(this);
 }
 
-NotificationManagerImpl::~NotificationManagerImpl() = default;
+NotificationManagerImpl::~NotificationManagerImpl() {
+  multidevice_setup_client_->RemoveObserver(this);
+}
 
 void NotificationManagerImpl::DismissNotification(int64_t notification_id) {
   PA_LOG(INFO) << "Dismissing notification with ID " << notification_id << ".";
@@ -47,5 +58,19 @@ void NotificationManagerImpl::SendInlineReply(
                                                       inline_reply_text);
 }
 
+void NotificationManagerImpl::OnFeatureStatesChanged(
+    const multidevice_setup::MultiDeviceSetupClient::FeatureStatesMap&
+        feature_states_map) {
+  FeatureState notifications_feature_state =
+      multidevice_setup_client_->GetFeatureState(
+          Feature::kPhoneHubNotifications);
+  if (notifications_feature_status_ == FeatureState::kEnabledByUser &&
+      notifications_feature_state != FeatureState::kEnabledByUser) {
+    ClearNotificationsInternal();
+  }
+
+  notifications_feature_status_ = notifications_feature_state;
+}
+
 }  // namespace phonehub
 }  // namespace chromeos

chromeos/components/phonehub/notification_manager_impl.h

@@ -9,15 +9,21 @@
 
 #include "chromeos/components/phonehub/notification.h"
 #include "chromeos/components/phonehub/notification_manager.h"
+#include "chromeos/services/multidevice_setup/public/cpp/multidevice_setup_client.h"
+#include "chromeos/services/multidevice_setup/public/mojom/multidevice_setup.mojom.h"
 
 namespace chromeos {
 namespace phonehub {
 
 class MessageSender;
 
-class NotificationManagerImpl : public NotificationManager {
+class NotificationManagerImpl
+    : public NotificationManager,
+      public multidevice_setup::MultiDeviceSetupClient::Observer {
  public:
-  NotificationManagerImpl(MessageSender* message_sender);
+  NotificationManagerImpl(
+      MessageSender* message_sender,
+      multidevice_setup::MultiDeviceSetupClient* multidevice_setup_client);
   ~NotificationManagerImpl() override;
 
  private:
@@ -26,7 +32,14 @@ class NotificationManagerImpl : public NotificationManager {
   void SendInlineReply(int64_t notification_id,
                        const base::string16& inline_reply_text) override;
 
+  // MultiDeviceSetupClient::Observer:
+  void OnFeatureStatesChanged(
+      const multidevice_setup::MultiDeviceSetupClient::FeatureStatesMap&
+          feature_states_map) override;
+
   MessageSender* message_sender_;
+  multidevice_setup::MultiDeviceSetupClient* multidevice_setup_client_;
+  multidevice_setup::mojom::FeatureState notifications_feature_status_;
 };
 
 }  // namespace phonehub

chromeos/components/phonehub/notification_manager_impl_unittest.cc

@@ -10,6 +10,7 @@
 #include "base/optional.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chromeos/components/phonehub/fake_message_sender.h"
+#include "chromeos/services/multidevice_setup/public/cpp/fake_multidevice_setup_client.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace chromeos {
@@ -35,6 +36,9 @@ Notification CreateNotification(int64_t id) {
       base::UTF8ToUTF16(kTextContent));
 }
 
+using multidevice_setup::mojom::Feature;
+using multidevice_setup::mojom::FeatureState;
+
 class FakeObserver : public NotificationManager::Observer {
  public:
   FakeObserver() = default;
@@ -83,8 +87,10 @@ class NotificationManagerImplTest : public testing::Test {
   // testing::Test:
   void SetUp() override {
     fake_message_sender_ = std::make_unique<FakeMessageSender>();
-    manager_ =
-        std::make_unique<NotificationManagerImpl>(fake_message_sender_.get());
+    fake_multidevice_setup_client_ =
+        std::make_unique<multidevice_setup::FakeMultiDeviceSetupClient>();
+    manager_ = std::make_unique<NotificationManagerImpl>(
+        fake_message_sender_.get(), fake_multidevice_setup_client_.get());
     manager_->AddObserver(&fake_observer_);
   }
 
@@ -109,9 +115,16 @@ class NotificationManagerImplTest : public testing::Test {
     return fake_observer_.GetState(notification_id);
   }
 
+  void SetNotificationFeatureStatus(FeatureState feature_state) {
+    fake_multidevice_setup_client_->SetFeatureState(
+        Feature::kPhoneHubNotifications, feature_state);
+  }
+
  private:
   FakeObserver fake_observer_;
   std::unique_ptr<FakeMessageSender> fake_message_sender_;
+  std::unique_ptr<multidevice_setup::FakeMultiDeviceSetupClient>
+      fake_multidevice_setup_client_;
   std::unique_ptr<NotificationManager> manager_;
 };
 
@@ -237,5 +250,20 @@ TEST_F(NotificationManagerImplTest, SendInlineReply) {
   EXPECT_EQ(expected_id1, pair.first);
   EXPECT_EQ(expected_reply, pair.second);
 }
+
+TEST_F(NotificationManagerImplTest, ClearNotificationsOnFeatureStatusChanged) {
+  // Simulate enabling notification feature state.
+  SetNotificationFeatureStatus(FeatureState::kEnabledByUser);
+
+  // Set an internal notification.
+  SetNotificationsInternal(
+      base::flat_set<Notification>{CreateNotification(/*notification_id=*/1)});
+  EXPECT_EQ(1u, GetNumNotifications());
+
+  // Change notification feature state to disabled, expect internal
+  // notifications to be cleared.
+  SetNotificationFeatureStatus(FeatureState::kDisabledByUser);
+  EXPECT_EQ(0u, GetNumNotifications());
+}
 }  // namespace phonehub
 }  // namespace chromeos

chromeos/components/phonehub/phone_hub_manager_impl.cc

@@ -65,7 +65,8 @@ PhoneHubManagerImpl::PhoneHubManagerImpl(
               message_sender_.get(),
               connection_scheduler_.get())),
       notification_manager_(
-          std::make_unique<NotificationManagerImpl>(message_sender_.get())),
+          std::make_unique<NotificationManagerImpl>(message_sender_.get(),
+                                                    multidevice_setup_client)),
       onboarding_ui_tracker_(std::make_unique<OnboardingUiTrackerImpl>(
           pref_service,
           feature_status_provider_.get(),
@@ -143,6 +144,7 @@ void PhoneHubManagerImpl::Shutdown() {
   phone_status_processor_.reset();
   phone_model_.reset();
   onboarding_ui_tracker_.reset();
+  notification_manager_.reset();
   notification_access_manager_.reset();
   find_my_device_controller_.reset();
   connection_scheduler_.reset();