Use notification display service to collect persistent notifications.

content/browser/notifications/platform_notification_context_impl.cc

Index: content/browser/notifications/platform_notification_context_impl.cc
diff --git a/content/browser/notifications/platform_notification_context_impl.cc b/content/browser/notifications/platform_notification_context_impl.cc
index fa5c1387e1ccacdb81ebfc43ce707dca5e069afe..c0b5ef2ab1403db8268b5974bc3afee326fb1566 100644
--- a/content/browser/notifications/platform_notification_context_impl.cc
+++ b/content/browser/notifications/platform_notification_context_impl.cc
@@ -60,8 +60,8 @@ void PlatformNotificationContextImpl::Initialize() {
     std::set<std::string> displayed_notifications;
 
     bool notification_synchronization_supported =
-        service->GetDisplayedPersistentNotifications(browser_context_,
-                                                     &displayed_notifications);
+        service->GetDisplayedNotifications(browser_context_,
+                                           &displayed_notifications);
 
     // Synchronize the notifications stored in the database with the set of
     // displaying notifications in |displayed_notifications|. This is necessary
@@ -202,6 +202,17 @@ void PlatformNotificationContextImpl::
         const ReadAllResultCallback& callback) {
   DCHECK(task_runner_->RunsTasksOnCurrentThread());
 
+  PlatformNotificationService* service =
+      GetContentClient()->browser()->GetPlatformNotificationService();
+
+  // Ask the underlying implementation what notifications are still being
+  // displayed.
+  std::set<std::string> displayed_notifications;
+  bool notification_synchronization_supported =
+      service &&
+      service->GetDisplayedNotifications(browser_context_,
+                                         &displayed_notifications);

[Peter Beverloo, 2016/12/06 13:04:02]

We're not only calling GetDisplayedNotifications() on the UI thread anymore,
we're now calling it from any arbitrary thread - this method will be executed on
a sequenced task runner created through a blocking thread pool.

That violates a whole series of assumptions - ContentBrowserClient that's
assumed to live on the UI thread, access to the message center (through the
NotificationUIManager), access to the native notification centers. That could be
a potential source of bugs - who knows what operating systems do for concurrent
calls to the system APIs?

I'm not sure what to suggest. If at all possible, I'd like to limit the method
to the UI thread. That would imply that we get a jump like this:

PNCI::ReadAllNotificationDataForServiceWorkerRegistration() (IO thread)
--> Moves the LazyInitialize() call on line 190 to a base::Callback passed to...

PNCI::GetDisplayedNotificationsOnUiThread() (UI thread)
--> Gets the notifications (async if needed - something you'll likely need
anyway)
--> Posts the passed base::Callback to the IO thread.

PNCI::LazyInitialize() and beyond (IO thread) -- the existing execution path

You'd need to do something silly like allocating the std::set<> on the heap,
passing ownership to the base::Callback to
PNCI::DoReadAllNotificationDataForServiceWorkerRegistration() and passing it
weakly to the PNCI::GetDisplayedNotificationsOnUiThread() function, so that both
can access the same memory...

Let's chat? :)

+
   std::vector<NotificationDatabaseData> notification_datas;
 
   NotificationDatabase::Status status =
@@ -212,6 +223,21 @@ void PlatformNotificationContextImpl::
                             status, NotificationDatabase::STATUS_COUNT);
 
   if (status == NotificationDatabase::STATUS_OK) {
+    if (notification_synchronization_supported) {
+      // Filter out notifications that are not actually on display anymore.
+      // TODO(miguelg) synchronize the database if there are inconsistencies.
+      for (auto it = notification_datas.begin();
+           it != notification_datas.end();) {
+        if (NotificationIdGenerator::IsPersistentNotification(
+                it->notification_id) &&
+            displayed_notifications.count(it->notification_id)) {
+          ++it;
+        } else {
+          it = notification_datas.erase(it);
+        }
+      }
+    }
+
     BrowserThread::PostTask(
         BrowserThread::IO, FROM_HERE,
         base::Bind(callback, true /* success */, notification_datas));