Support notification synchronization on CrOS

Synchronizing displayed notifications helps to clean up already closed
notifications from the NotificationDatabase. This implements this
functionality for ChromeOS via the MessageCenter.

Bug: None
Change-Id: I660f2263451ca734887da63f0c7ab43cd12ab2b5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2391087
Commit-Queue: Richard Knoll <knollr@chromium.org>
Reviewed-by: Peter Beverloo <peter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#806155}

chrome/browser/notifications/chrome_ash_message_center_client.cc

@@ -152,10 +152,14 @@ void ChromeAshMessageCenterClient::Close(Profile* profile,
 void ChromeAshMessageCenterClient::GetDisplayed(
     Profile* profile,
     GetDisplayedNotificationsCallback callback) const {
-  // Right now, this is only used to get web notifications that were created by
-  // and have outlived a previous browser process. Ash itself doesn't outlive
-  // the browser process, so there's no need to implement.
-  std::move(callback).Run(/*notification_ids=*/{}, /*supports_sync=*/false);
+  message_center::NotificationList::Notifications notifications =
+      MessageCenter::Get()->GetNotifications();
+
+  std::set<std::string> notification_ids;
+  for (message_center::Notification* notification : notifications)
+    notification_ids.insert(notification->id());
+
+  std::move(callback).Run(std::move(notification_ids), /*supports_sync=*/true);
 }
 
 void ChromeAshMessageCenterClient::SetReadyCallback(

chrome/browser/notifications/notification_platform_bridge_chromeos.cc

@@ -77,7 +77,10 @@ void NotificationPlatformBridgeChromeOs::Close(
 void NotificationPlatformBridgeChromeOs::GetDisplayed(
     Profile* profile,
     GetDisplayedNotificationsCallback callback) const {
-  impl_->GetDisplayed(profile, std::move(callback));
+  impl_->GetDisplayed(
+      profile,
+      base::BindOnce(&NotificationPlatformBridgeChromeOs::OnGetDisplayed,
+                     weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
 }
 
 void NotificationPlatformBridgeChromeOs::SetReadyCallback(
@@ -201,3 +204,17 @@ ProfileNotification* NotificationPlatformBridgeChromeOs::GetProfileNotification(
     return nullptr;
   return iter->second.get();
 }
+
+void NotificationPlatformBridgeChromeOs::OnGetDisplayed(
+    GetDisplayedNotificationsCallback callback,
+    std::set<std::string> notification_ids,
+    bool supports_synchronization) const {
+  std::set<std::string> original_notification_ids;
+  for (const auto& id : notification_ids) {
+    auto iter = active_notifications_.find(id);
+    if (iter != active_notifications_.end())
+      original_notification_ids.insert(iter->second->original_id());
+  }
+  std::move(callback).Run(std::move(original_notification_ids),
+                          supports_synchronization);
+}

chrome/browser/notifications/notification_platform_bridge_chromeos.h

@@ -9,6 +9,7 @@
 #include <memory>
 #include <string>
 
+#include "base/memory/weak_ptr.h"
 #include "chrome/browser/notifications/notification_platform_bridge.h"
 #include "chrome/browser/notifications/notification_platform_bridge_delegate.h"
 #include "chrome/browser/notifications/profile_notification.h"
@@ -57,6 +58,12 @@ class NotificationPlatformBridgeChromeOs
   ProfileNotification* GetProfileNotification(
       const std::string& profile_notification_id);
 
+  // Callback after getting displayed notifications from the |impl_| to convert
+  // profile IDs back to the original notification IDs.
+  void OnGetDisplayed(GetDisplayedNotificationsCallback callback,
+                      std::set<std::string> notification_ids,
+                      bool supports_synchronization) const;
+
   // Helper implementation.
   std::unique_ptr<NotificationPlatformBridge> impl_;
 
@@ -65,6 +72,9 @@ class NotificationPlatformBridgeChromeOs
   // the permuted ID.
   std::map<std::string, std::unique_ptr<ProfileNotification>>
       active_notifications_;
+
+  base::WeakPtrFactory<NotificationPlatformBridgeChromeOs> weak_ptr_factory_{
+      this};
 };
 
 #endif  // CHROME_BROWSER_NOTIFICATIONS_NOTIFICATION_PLATFORM_BRIDGE_CHROMEOS_H_

ui/message_center/fake_message_center.cc

@@ -13,7 +13,7 @@ namespace message_center {
 
 FakeMessageCenter::FakeMessageCenter() : notifications_(this) {}
 
-FakeMessageCenter::~FakeMessageCenter() {}
+FakeMessageCenter::~FakeMessageCenter() = default;
 
 void FakeMessageCenter::AddObserver(MessageCenterObserver* observer) {
   observers_.AddObserver(observer);
@@ -60,6 +60,10 @@ NotificationList::Notifications FakeMessageCenter::FindNotificationsByAppId(
   return notifications_.GetNotificationsByAppId(app_id);
 }
 
+NotificationList::Notifications FakeMessageCenter::GetNotifications() {
+  return notifications_.GetNotifications();
+}
+
 const NotificationList::Notifications&
 FakeMessageCenter::GetVisibleNotifications() {
   return visible_notifications_;

ui/message_center/fake_message_center.h

@@ -36,6 +36,7 @@ class FakeMessageCenter : public MessageCenter {
   Notification* FindVisibleNotificationById(const std::string& id) override;
   NotificationList::Notifications FindNotificationsByAppId(
       const std::string& app_id) override;
+  NotificationList::Notifications GetNotifications() override;
   const NotificationList::Notifications& GetVisibleNotifications() override;
   NotificationList::PopupNotifications GetPopupNotifications() override;
   void AddNotification(std::unique_ptr<Notification> notification) override;

ui/message_center/message_center.h

@@ -86,6 +86,10 @@ class MESSAGE_CENTER_EXPORT MessageCenter {
   virtual NotificationList::Notifications FindNotificationsByAppId(
       const std::string& app_id) = 0;
 
+  // Gets all notifications the message center knows about. These might contain
+  // currently hidden ones due to any active NotificationBlockers.
+  virtual NotificationList::Notifications GetNotifications() = 0;
+
   // Gets all notifications to be shown to the user in the message center.  Note
   // that queued changes due to the message center being open are not reflected
   // in this list.

ui/message_center/message_center_impl.h

@@ -53,6 +53,7 @@ class MessageCenterImpl : public MessageCenter,
   Notification* FindVisibleNotificationById(const std::string& id) override;
   NotificationList::Notifications FindNotificationsByAppId(
       const std::string& app_id) override;
+  NotificationList::Notifications GetNotifications() override;
   const NotificationList::Notifications& GetVisibleNotifications() override;
   NotificationList::PopupNotifications GetPopupNotifications() override;
   void AddNotification(std::unique_ptr<Notification> notification) override;

ui/message_center/notification_list.cc

@@ -65,8 +65,7 @@ NotificationList::NotificationList(MessageCenter* message_center)
       quiet_mode_(false) {
 }
 
-NotificationList::~NotificationList() {
-}
+NotificationList::~NotificationList() = default;
 
 void NotificationList::SetNotificationsShown(
     const NotificationBlockers& blockers,
@@ -116,8 +115,15 @@ void NotificationList::RemoveNotification(const std::string& id) {
   EraseNotification(GetNotification(id));
 }
 
+NotificationList::Notifications NotificationList::GetNotifications() const {
+  Notifications notifications;
+  for (const auto& tuple : notifications_)
+    notifications.insert(tuple.first.get());
+  return notifications;
+}
+
 NotificationList::Notifications NotificationList::GetNotificationsByNotifierId(
-        const NotifierId& notifier_id) {
+    const NotifierId& notifier_id) const {
   Notifications notifications;
   for (const auto& tuple : notifications_) {
     Notification* notification = tuple.first.get();
@@ -128,7 +134,7 @@ NotificationList::Notifications NotificationList::GetNotificationsByNotifierId(
 }
 
 NotificationList::Notifications NotificationList::GetNotificationsByAppId(
-    const std::string& app_id) {
+    const std::string& app_id) const {
   Notifications notifications;
   for (const auto& tuple : notifications_) {
     Notification* notification = tuple.first.get();
@@ -156,8 +162,9 @@ bool NotificationList::SetNotificationImage(const std::string& notification_id,
   return true;
 }
 
-bool NotificationList::HasNotificationOfType(const std::string& id,
-                                             const NotificationType type) {
+bool NotificationList::HasNotificationOfType(
+    const std::string& id,
+    const NotificationType type) const {
   auto iter = GetNotification(id);
   if (iter == notifications_.end())
     return false;
@@ -166,7 +173,7 @@ bool NotificationList::HasNotificationOfType(const std::string& id,
 }
 
 bool NotificationList::HasPopupNotifications(
-    const NotificationBlockers& blockers) {
+    const NotificationBlockers& blockers) const {
   for (const auto& tuple : notifications_) {
     if (tuple.first->priority() < DEFAULT_PRIORITY)
       break;
@@ -307,6 +314,16 @@ NotificationList::GetNotification(const std::string& id) {
   return notifications_.end();
 }
 
+NotificationList::OwnedNotifications::const_iterator
+NotificationList::GetNotification(const std::string& id) const {
+  for (auto iter = notifications_.begin(); iter != notifications_.end();
+       ++iter) {
+    if (iter->first->id() == id)
+      return iter;
+  }
+  return notifications_.end();
+}
+
 void NotificationList::EraseNotification(OwnedNotifications::iterator iter) {
   notifications_.erase(iter);
 }

ui/message_center/notification_list.h

@@ -93,11 +93,15 @@ class MESSAGE_CENTER_EXPORT NotificationList {
 
   void RemoveNotification(const std::string& id);
 
+  // Returns all notifications in this list.
+  Notifications GetNotifications() const;
+
   // Returns all notifications that have a matching |notifier_id|.
-  Notifications GetNotificationsByNotifierId(const NotifierId& notifier_id);
+  Notifications GetNotificationsByNotifierId(
+      const NotifierId& notifier_id) const;
 
   // Returns all notifications that have a matching |app_id|.
-  Notifications GetNotificationsByAppId(const std::string& app_id);
+  Notifications GetNotificationsByAppId(const std::string& app_id) const;
 
   // Returns true if the notification exists and was updated.
   bool SetNotificationIcon(const std::string& notification_id,
@@ -110,11 +114,11 @@ class MESSAGE_CENTER_EXPORT NotificationList {
   // Returns true if |id| matches a notification in the list and that
   // notification's type matches the given type.
   bool HasNotificationOfType(const std::string& id,
-                             const NotificationType type);
+                             const NotificationType type) const;
 
   // Returns false if the first notification has been shown as a popup (which
   // means that all notifications have been shown).
-  bool HasPopupNotifications(const NotificationBlockers& blockers);
+  bool HasPopupNotifications(const NotificationBlockers& blockers) const;
 
   // Returns the recent notifications of the priority higher then LOW,
   // that have not been shown as a popup. kMaxVisiblePopupNotifications are
@@ -163,6 +167,8 @@ class MESSAGE_CENTER_EXPORT NotificationList {
 
   // Iterates through the list and returns the first notification matching |id|.
   OwnedNotifications::iterator GetNotification(const std::string& id);
+  OwnedNotifications::const_iterator GetNotification(
+      const std::string& id) const;
 
   void EraseNotification(OwnedNotifications::iterator iter);
 

ui/message_center/notification_list_unittest.cc

@@ -450,6 +450,17 @@ TEST_F(NotificationListTest, HasPopupsWithSystemPriority) {
   EXPECT_EQ(0u, GetPopupCounts());
 }
 
+TEST_F(NotificationListTest, GetNotifications) {
+  ASSERT_EQ(0u, notification_list_->NotificationCount(blockers_));
+  EXPECT_EQ(0u, notification_list_->GetNotifications().size());
+
+  AddPriorityNotification(MIN_PRIORITY);
+  AddPriorityNotification(MAX_PRIORITY);
+
+  EXPECT_EQ(1u, GetPopupCounts());
+  EXPECT_EQ(2u, notification_list_->GetNotifications().size());
+}
+
 // Verifies that notification updates will re-show the toast when there is no
 // message center view (i.e. the bubble anchored to the status bar).
 TEST_F(NotificationListTest, UpdateWithoutMessageCenterView) {