[CrOS PhoneHub] Silently add notifications after device suspension

This CL fixes an issue which re-shows notifications which have already
been shown to the user after the user has (1) suspended their device,
(2) locked their device, or (3) turned Bluetooth off.

When these situations occur, we silently add the notification to the
notification shade but do not pop it up. This fixes an issue which
caused notifications to appear spammy when we'd show them multiple
times.

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

ash/system/phonehub/phone_hub_notification_controller.cc

@@ -265,13 +265,20 @@ void PhoneHubNotificationController::OnFeatureStatusChanged() {
   DCHECK(feature_status_provider_);
 
   auto status = feature_status_provider_->GetStatus();
+
+  // Various states in which the feature is enabled, even if it is not actually
+  // in use (e.g., if Bluetooth is disabled or if the screen is locked).
   bool is_feature_enabled =
+      status == chromeos::phonehub::FeatureStatus::kUnavailableBluetoothOff ||
+      status == chromeos::phonehub::FeatureStatus::kLockOrSuspended ||
       status == chromeos::phonehub::FeatureStatus::kEnabledButDisconnected ||
       status == chromeos::phonehub::FeatureStatus::kEnabledAndConnecting ||
       status == chromeos::phonehub::FeatureStatus::kEnabledAndConnected;
 
-  // Reset the set of shown notifications when PhoneHub is no longer available
-  // (e.g. when the screen is locked).
+  // Reset the set of shown notifications when Phone Hub is disabled. If it is
+  // enabled, we skip this step to ensure that notifications that have already
+  // been shown do not pop up again and spam the user. See
+  // https://crbug.com/1157523 for details.
   if (!is_feature_enabled)
     shown_notification_ids_.clear();
 }

ash/system/phonehub/phone_hub_notification_controller_unittest.cc

@@ -307,8 +307,14 @@ TEST_F(PhoneHubNotificationControllerTest, DoNotReshowPopupNotification) {
   ASSERT_TRUE(cros_notification);
   EXPECT_EQ(message_center::MAX_PRIORITY, cros_notification->priority());
 
+  feature_status_provider_->SetStatus(
+      chromeos::phonehub::FeatureStatus::kEnabledButDisconnected);
   feature_status_provider_->SetStatus(
       chromeos::phonehub::FeatureStatus::kEnabledAndConnecting);
+  feature_status_provider_->SetStatus(
+      chromeos::phonehub::FeatureStatus::kUnavailableBluetoothOff);
+  feature_status_provider_->SetStatus(
+      chromeos::phonehub::FeatureStatus::kLockOrSuspended);
   feature_status_provider_->SetStatus(
       chromeos::phonehub::FeatureStatus::kEnabledAndConnected);
 
@@ -321,9 +327,9 @@ TEST_F(PhoneHubNotificationControllerTest, DoNotReshowPopupNotification) {
   ASSERT_TRUE(cros_notification);
   EXPECT_EQ(message_center::LOW_PRIORITY, cros_notification->priority());
 
-  // Disconnect due to screen lock.
+  // Disable the feature..
   feature_status_provider_->SetStatus(
-      chromeos::phonehub::FeatureStatus::kLockOrSuspended);
+      chromeos::phonehub::FeatureStatus::kDisabled);
   notification_manager_->RemoveNotification(kPhoneHubNotificationId0);
   ASSERT_FALSE(FindNotification(kCrOSNotificationId0));