Fix dismiss Phonehub notification crash

- Crash was due to attempting to remove an previously removed
  notification.
- The fix is to verify that the notification_id is a valid id to remove.

Bug: 1139561, 1106937
Test: chromeos_components_unittests, ash_unittests
Change-Id: Ia606df1dfec13beba9ccc14c2618ec4f6fda7129
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2482428Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Commit-Queue: Jimmy Gong <jimmyxgong@chromium.org>
Cr-Commit-Position: refs/heads/master@{#818372}

ash/system/phonehub/phone_hub_notification_controller_unittest.cc

@@ -136,6 +136,11 @@ TEST_F(PhoneHubNotificationControllerTest, RemoveNotifications) {
   notification_manager_.RemoveNotificationsInternal(base::flat_set<int64_t>(
       {kPhoneHubNotificationId1, kPhoneHubNotificationId2}));
   EXPECT_FALSE(message_center_->NotificationCount());
+
+  // Attempt removing the same notifications again and expect nothing to happen.
+  notification_manager_.RemoveNotificationsInternal(base::flat_set<int64_t>(
+      {kPhoneHubNotificationId1, kPhoneHubNotificationId2}));
+  EXPECT_FALSE(message_center_->NotificationCount());
 }
 
 TEST_F(PhoneHubNotificationControllerTest, CloseByUser) {

chromeos/components/phonehub/notification_manager.cc

@@ -70,7 +70,9 @@ void NotificationManager::RemoveNotificationsInternal(
 
   for (int64_t id : notification_ids) {
     auto it = id_to_notification_map_.find(id);
-    DCHECK(it != id_to_notification_map_.end());
+    if (it == id_to_notification_map_.end())
+      continue;
+
     id_to_notification_map_.erase(it);
   }
 

chromeos/components/phonehub/notification_manager_impl.cc

@@ -22,6 +22,12 @@ NotificationManagerImpl::~NotificationManagerImpl() = default;
 void NotificationManagerImpl::DismissNotification(int64_t notification_id) {
   PA_LOG(INFO) << "Dismissing notification with ID " << notification_id << ".";
 
+  if (!GetNotification(notification_id)) {
+    PA_LOG(WARNING) << "Attempted to dismiss an invalid notification with id: "
+                    << notification_id << ".";
+    return;
+  }
+
   RemoveNotificationsInternal(base::flat_set<int64_t>{notification_id});
   message_sender_->SendDismissNotificationRequest(notification_id);
 }

chromeos/components/phonehub/notification_manager_impl_unittest.cc

@@ -176,6 +176,15 @@ TEST_F(NotificationManagerImplTest, DismissNotifications) {
   EXPECT_EQ(1u, fake_message_sender().GetDismissNotificationRequestCallCount());
   EXPECT_EQ(expected_id2,
             fake_message_sender().GetRecentDismissNotificationRequest());
+
+  // Dismiss the same notification again, verify nothing happens.
+  manager().DismissNotification(expected_id2);
+  EXPECT_EQ(1u, GetNumNotifications());
+  EXPECT_EQ(NotificationState::kAdded, GetNotificationState(expected_id1));
+  EXPECT_EQ(NotificationState::kRemoved, GetNotificationState(expected_id2));
+  EXPECT_EQ(1u, fake_message_sender().GetDismissNotificationRequestCallCount());
+  EXPECT_EQ(expected_id2,
+            fake_message_sender().GetRecentDismissNotificationRequest());
 }
 
 TEST_F(NotificationManagerImplTest, UpdatedNotification) {