Beginnings of synchronizing notifications in the notification database.

For both desktop Chrome and Android we need to support a mechanism for
synchronizing open notifications. For desktop the most important case
is where Chrome just started up, where there won't be any notifications
since the message center is owned by Chrome.

For Android, there are more cases, since the platform hosts displaying
the notifications and informs Chrome through Intents of mutations. This
patch leaves synchronization there mostly out of scope, while still
laying the early groundwork towards supporting this.

BUG=442143

Review URL: https://codereview.chromium.org/1127013008

Cr-Commit-Position: refs/heads/master@{#330583}

17 files changed, 161 insertions, 9 deletions

diff --git a/chrome/browser/notifications/message_center_notification_manager.cc b/chrome/browser/notifications/message_center_notification_manager.cc
index 4c755f0..e4c54e7 100644
--- a/chrome/browser/notifications/message_center_notification_manager.cc
+++ b/chrome/browser/notifications/message_center_notification_manager.cc
@@ -237,13 +237,28 @@ MessageCenterNotificationManager::GetAllIdsByProfileAndSourceOrigin(
   // no profile method should be called inside this function.
 
   std::set<std::string> delegate_ids;
-  for (NotificationMap::iterator iter = profile_notifications_.begin();
-       iter != profile_notifications_.end(); iter++) {
-    if ((*iter).second->notification().origin_url() == source &&
-        profile == (*iter).second->profile()) {
-      delegate_ids.insert(iter->second->notification().delegate_id());
+  for (const auto& pair : profile_notifications_) {
+    const Notification& notification = pair.second->notification();
+    if (pair.second->profile() == profile &&
+        notification.origin_url() == source) {
+      delegate_ids.insert(notification.delegate_id());
     }
   }
+
+  return delegate_ids;
+}
+
+std::set<std::string> MessageCenterNotificationManager::GetAllIdsByProfile(
+    Profile* profile) {
+  // The profile pointer can be weak, the instance may have been destroyed, so
+  // no profile method should be called inside this function.
+
+  std::set<std::string> delegate_ids;
+  for (const auto& pair : profile_notifications_) {
+    if (pair.second->profile() == profile)
+      delegate_ids.insert(pair.second->notification().delegate_id());
+  }
+
   return delegate_ids;
 }
 
diff --git a/chrome/browser/notifications/message_center_notification_manager.h b/chrome/browser/notifications/message_center_notification_manager.h
index 986f9cc..a5a2149 100644
--- a/chrome/browser/notifications/message_center_notification_manager.h
+++ b/chrome/browser/notifications/message_center_notification_manager.h
@@ -61,6 +61,7 @@ class MessageCenterNotificationManager
   std::set<std::string> GetAllIdsByProfileAndSourceOrigin(
       Profile* profile,
       const GURL& source) override;
+  std::set<std::string> GetAllIdsByProfile(Profile* profile) override;
   bool CancelAllBySourceOrigin(const GURL& source_origin) override;
   bool CancelAllByProfile(ProfileID profile_id) override;
   void CancelAll() override;
diff --git a/chrome/browser/notifications/notification_test_util.cc b/chrome/browser/notifications/notification_test_util.cc
index 140d727..1cfdf9d 100644
--- a/chrome/browser/notifications/notification_test_util.cc
+++ b/chrome/browser/notifications/notification_test_util.cc
@@ -92,6 +92,16 @@ StubNotificationUIManager::GetAllIdsByProfileAndSourceOrigin(
   return delegate_ids;
 }
 
+std::set<std::string> StubNotificationUIManager::GetAllIdsByProfile(
+    Profile* profile) {
+  std::set<std::string> delegate_ids;
+  for (const auto& pair : notifications_) {
+    if (pair.second == profile)
+      delegate_ids.insert(pair.first.delegate_id());
+  }
+  return delegate_ids;
+}
+
 bool StubNotificationUIManager::CancelAllBySourceOrigin(
     const GURL& source_origin) {
   NOTIMPLEMENTED();
diff --git a/chrome/browser/notifications/notification_test_util.h b/chrome/browser/notifications/notification_test_util.h
index 6eb9d0a..1ea0fc0 100644
--- a/chrome/browser/notifications/notification_test_util.h
+++ b/chrome/browser/notifications/notification_test_util.h
@@ -58,6 +58,7 @@ class StubNotificationUIManager : public NotificationUIManager {
   std::set<std::string> GetAllIdsByProfileAndSourceOrigin(
       Profile* profile,
       const GURL& source) override;
+  std::set<std::string> GetAllIdsByProfile(Profile* profile) override;
   bool CancelAllBySourceOrigin(const GURL& source_origin) override;
   bool CancelAllByProfile(ProfileID profile_id) override;
   void CancelAll() override;
diff --git a/chrome/browser/notifications/notification_ui_manager.h b/chrome/browser/notifications/notification_ui_manager.h
index 182bb44..49d0ed9 100644
--- a/chrome/browser/notifications/notification_ui_manager.h
+++ b/chrome/browser/notifications/notification_ui_manager.h
@@ -68,10 +68,17 @@ class NotificationUIManager {
 
   // Returns the set of all delegate IDs for notifications from the passed
   // |profile| and |source|.
+  // TODO(peter): Change Profile* to ProfileID to clarify that the profile
+  // should not be used for making any calls, as it might be dead.
   virtual std::set<std::string> GetAllIdsByProfileAndSourceOrigin(
       Profile* profile,
       const GURL& source) = 0;
 
+  // Returns the set of all delegate IDs for notifications from |profile|.
+  // TODO(peter): Change Profile* to ProfileID to clarify that the profile
+  // should not be used for making any calls, as it might be dead.
+  virtual std::set<std::string> GetAllIdsByProfile(Profile* profile) = 0;
+
   // Removes notifications matching the |source_origin| (which could be an
   // extension ID). Returns true if anything was removed.
   virtual bool CancelAllBySourceOrigin(const GURL& source_origin) = 0;
diff --git a/chrome/browser/notifications/notification_ui_manager_android.cc b/chrome/browser/notifications/notification_ui_manager_android.cc
index 4ee7219..cee5ffa 100644
--- a/chrome/browser/notifications/notification_ui_manager_android.cc
+++ b/chrome/browser/notifications/notification_ui_manager_android.cc
@@ -205,6 +205,12 @@ NotificationUIManagerAndroid::GetAllIdsByProfileAndSourceOrigin(
   return std::set<std::string>();
 }
 
+std::set<std::string> NotificationUIManagerAndroid::GetAllIdsByProfile(
+    Profile* profile) {
+  NOTREACHED();
+  return std::set<std::string>();
+}
+
 bool NotificationUIManagerAndroid::CancelAllBySourceOrigin(
     const GURL& source_origin) {
   NOTREACHED();
diff --git a/chrome/browser/notifications/notification_ui_manager_android.h b/chrome/browser/notifications/notification_ui_manager_android.h
index 59b5adc..68fd226 100644
--- a/chrome/browser/notifications/notification_ui_manager_android.h
+++ b/chrome/browser/notifications/notification_ui_manager_android.h
@@ -54,6 +54,7 @@ class NotificationUIManagerAndroid : public NotificationUIManager {
   std::set<std::string> GetAllIdsByProfileAndSourceOrigin(Profile* profile,
                                                           const GURL& source)
       override;
+  std::set<std::string> GetAllIdsByProfile(Profile* profile) override;
   bool CancelAllBySourceOrigin(const GURL& source_origin) override;
   bool CancelAllByProfile(ProfileID profile_id) override;
   void CancelAll() override;
diff --git a/chrome/browser/notifications/notification_ui_manager_desktop.cc b/chrome/browser/notifications/notification_ui_manager_desktop.cc
index bca1303..90c2c74 100644
--- a/chrome/browser/notifications/notification_ui_manager_desktop.cc
+++ b/chrome/browser/notifications/notification_ui_manager_desktop.cc
@@ -14,8 +14,12 @@
 
 // static
 NotificationUIManager* NotificationUIManager::Create(PrefService* local_state) {
+  ProfileManager* profile_manager = g_browser_process->profile_manager();
+  if (!profile_manager)
+    return nullptr;
+
   ProfileInfoCache* profile_info_cache =
-      &g_browser_process->profile_manager()->GetProfileInfoCache();
+      &profile_manager->GetProfileInfoCache();
   scoped_ptr<message_center::NotifierSettingsProvider> settings_provider(
       new MessageCenterSettingsController(profile_info_cache));
   return new MessageCenterNotificationManager(
diff --git a/chrome/browser/notifications/platform_notification_service_impl.cc b/chrome/browser/notifications/platform_notification_service_impl.cc
index 21baeb7..56908d8 100644
--- a/chrome/browser/notifications/platform_notification_service_impl.cc
+++ b/chrome/browser/notifications/platform_notification_service_impl.cc
@@ -306,6 +306,28 @@ void PlatformNotificationServiceImpl::ClosePersistentNotification(
 #endif
 }
 
+bool PlatformNotificationServiceImpl::GetDisplayedPersistentNotifications(
+    BrowserContext* browser_context,
+    std::set<std::string>* displayed_notifications) {
+  DCHECK(displayed_notifications);
+
+#if !defined(OS_ANDROID)
+  Profile* profile = Profile::FromBrowserContext(browser_context);
+  if (!profile || profile->AsTestingProfile())
+    return false;  // Tests will not have a message center.
+
+  // TODO(peter): Filter for persistent notifications only.
+  *displayed_notifications =
+      GetNotificationUIManager()->GetAllIdsByProfile(profile);
+
+  return true;
+#else
+  // Android cannot reliably return the notifications that are currently being
+  // displayed on the platform, see the comment in NotificationUIManagerAndroid.
+  return false;
+#endif  // !defined(OS_ANDROID)
+}
+
 Notification PlatformNotificationServiceImpl::CreateNotificationFromData(
     Profile* profile,
     const GURL& origin,
diff --git a/chrome/browser/notifications/platform_notification_service_impl.h b/chrome/browser/notifications/platform_notification_service_impl.h
index bfbad2d..adfff80 100644
--- a/chrome/browser/notifications/platform_notification_service_impl.h
+++ b/chrome/browser/notifications/platform_notification_service_impl.h
@@ -7,6 +7,8 @@
 
 #include <stdint.h>
 #include <map>
+#include <set>
+#include <string>
 
 #include "base/gtest_prod_util.h"
 #include "base/memory/singleton.h"
@@ -78,6 +80,9 @@ class PlatformNotificationServiceImpl
   void ClosePersistentNotification(
       content::BrowserContext* browser_context,
       int64_t persistent_notification_id) override;
+  bool GetDisplayedPersistentNotifications(
+      content::BrowserContext* browser_context,
+      std::set<std::string>* displayed_notifications) override;
 
  private:
   friend struct DefaultSingletonTraits<PlatformNotificationServiceImpl>;
diff --git a/content/browser/notifications/platform_notification_context_impl.cc b/content/browser/notifications/platform_notification_context_impl.cc
index db6f136..bdeabce 100644
--- a/content/browser/notifications/platform_notification_context_impl.cc
+++ b/content/browser/notifications/platform_notification_context_impl.cc
@@ -11,7 +11,9 @@
 #include "content/browser/notifications/notification_database.h"
 #include "content/browser/service_worker/service_worker_context_wrapper.h"
 #include "content/public/browser/browser_thread.h"
+#include "content/public/browser/content_browser_client.h"
 #include "content/public/browser/notification_database_data.h"
+#include "content/public/browser/platform_notification_service.h"
 
 using base::DoNothing;
 
@@ -24,8 +26,10 @@ const base::FilePath::CharType kPlatformNotificationsDirectory[] =
 
 PlatformNotificationContextImpl::PlatformNotificationContextImpl(
     const base::FilePath& path,
+    BrowserContext* browser_context,
     const scoped_refptr<ServiceWorkerContextWrapper>& service_worker_context)
     : path_(path),
+      browser_context_(browser_context),
       service_worker_context_(service_worker_context) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 }
@@ -43,6 +47,30 @@ PlatformNotificationContextImpl::~PlatformNotificationContextImpl() {
 
 void PlatformNotificationContextImpl::Initialize() {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  PlatformNotificationService* service =
+      GetContentClient()->browser()->GetPlatformNotificationService();
+  if (service) {
+    std::set<std::string> displayed_notifications;
+
+    bool notification_synchronization_supported =
+        service->GetDisplayedPersistentNotifications(browser_context_,
+                                                     &displayed_notifications);
+
+    // Synchronize the notifications stored in the database with the set of
+    // displaying notifications in |displayed_notifications|. This is necessary
+    // because flakiness may cause a platform to inform Chrome of a notification
+    // that has since been closed, or because the platform does not support
+    // notifications that exceed the lifetime of the browser process.
+
+    // TODO(peter): Synchronizing the actual notifications will be done when the
+    // persistent notification ids are stable. For M44 we need to support the
+    // case where there may be no notifications after a Chrome restart.
+    if (notification_synchronization_supported &&
+        !displayed_notifications.size()) {
+      prune_database_on_open_ = true;
+    }
+  }
+
   BrowserThread::PostTask(
       BrowserThread::IO,
       FROM_HERE,
@@ -330,10 +358,23 @@ void PlatformNotificationContextImpl::OpenDatabase(
   UMA_HISTOGRAM_ENUMERATION("Notifications.Database.OpenResult",
                             status, NotificationDatabase::STATUS_COUNT);
 
+  // TODO(peter): Do finer-grained synchronization here.
+  if (prune_database_on_open_) {
+    prune_database_on_open_ = false;
+    DestroyDatabase();
+
+    database_.reset(new NotificationDatabase(GetDatabasePath()));
+    status = database_->Open(true /* create_if_missing */);
+
+    // TODO(peter): Find the appropriate UMA to cover in regards to
+    // synchronizing notifications after the implementation is complete.
+  }
+
   // When the database could not be opened due to corruption, destroy it, blow
   // away the contents of the directory and try re-opening the database.
   if (status == NotificationDatabase::STATUS_ERROR_CORRUPTED) {
     if (DestroyDatabase()) {
+      database_.reset(new NotificationDatabase(GetDatabasePath()));
       status = database_->Open(true /* create_if_missing */);
 
       UMA_HISTOGRAM_ENUMERATION(
diff --git a/content/browser/notifications/platform_notification_context_impl.h b/content/browser/notifications/platform_notification_context_impl.h
index c6b370d..a41729b 100644
--- a/content/browser/notifications/platform_notification_context_impl.h
+++ b/content/browser/notifications/platform_notification_context_impl.h
@@ -6,6 +6,8 @@
 #define CONTENT_BROWSER_NOTIFICATIONS_PLATFORM_NOTIFICATION_CONTEXT_IMPL_H_
 
 #include <stdint.h>
+#include <set>
+#include <string>
 
 #include "base/callback.h"
 #include "base/compiler_specific.h"
@@ -25,6 +27,7 @@ class SequencedTaskRunner;
 
 namespace content {
 
+class BrowserContext;
 class NotificationDatabase;
 struct NotificationDatabaseData;
 class ServiceWorkerContextWrapper;
@@ -42,6 +45,7 @@ class CONTENT_EXPORT PlatformNotificationContextImpl
   // constructor must only be called on the IO thread.
   PlatformNotificationContextImpl(
       const base::FilePath& path,
+      BrowserContext* browser_context,
       const scoped_refptr<ServiceWorkerContextWrapper>& service_worker_context);
 
   // To be called on the UI thread to initialize the instance.
@@ -139,12 +143,16 @@ class CONTENT_EXPORT PlatformNotificationContextImpl
       const scoped_refptr<base::SequencedTaskRunner>& task_runner);
 
   base::FilePath path_;
+  BrowserContext* browser_context_;
 
   scoped_refptr<ServiceWorkerContextWrapper> service_worker_context_;
 
   scoped_refptr<base::SequencedTaskRunner> task_runner_;
   scoped_ptr<NotificationDatabase> database_;
 
+  // Indicates whether the database should be pruned when it's opened.
+  bool prune_database_on_open_ = false;
+
   DISALLOW_COPY_AND_ASSIGN(PlatformNotificationContextImpl);
 };
 
diff --git a/content/browser/notifications/platform_notification_context_unittest.cc b/content/browser/notifications/platform_notification_context_unittest.cc
index 6dca0d0..30ab46a 100644
--- a/content/browser/notifications/platform_notification_context_unittest.cc
+++ b/content/browser/notifications/platform_notification_context_unittest.cc
@@ -11,6 +11,7 @@
 #include "content/browser/service_worker/service_worker_context_wrapper.h"
 #include "content/common/service_worker/service_worker_types.h"
 #include "content/public/browser/notification_database_data.h"
+#include "content/public/test/test_browser_context.h"
 #include "content/public/test/test_browser_thread_bundle.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "url/gurl.h"
@@ -86,7 +87,9 @@ class PlatformNotificationContextTest : public ::testing::Test {
   // current message loop proxy will be used as the task runner.
   PlatformNotificationContextImpl* CreatePlatformNotificationContext() {
     PlatformNotificationContextImpl* context =
-        new PlatformNotificationContextImpl(base::FilePath(), nullptr);
+        new PlatformNotificationContextImpl(base::FilePath(),
+                                            &browser_context_,
+                                            nullptr);
     context->Initialize();
 
     OverrideTaskRunnerForTesting(context);
@@ -99,6 +102,9 @@ class PlatformNotificationContextTest : public ::testing::Test {
     context->SetTaskRunnerForTesting(base::MessageLoopProxy::current());
   }
 
+  // Returns the testing browsing context that can be used for this test.
+  BrowserContext* browser_context() { return &browser_context_; }
+
   // Returns whether the last invoked callback finished successfully.
   bool success() const { return success_; }
 
@@ -113,6 +119,7 @@ class PlatformNotificationContextTest : public ::testing::Test {
 
  private:
   TestBrowserThreadBundle thread_bundle_;
+  TestBrowserContext browser_context_;
 
   bool success_;
   NotificationDatabaseData database_data_;
@@ -240,6 +247,7 @@ TEST_F(PlatformNotificationContextTest, ServiceWorkerUnregistered) {
   scoped_refptr<PlatformNotificationContextImpl> notification_context(
       new PlatformNotificationContextImpl(
           base::FilePath(),
+          browser_context(),
           embedded_worker_test_helper->context_wrapper()));
   notification_context->Initialize();
 
@@ -343,7 +351,9 @@ TEST_F(PlatformNotificationContextTest, DestroyOnDiskDatabase) {
   // Manually construct the PlatformNotificationContextImpl because this test
   // requires the database to be created on the filesystem.
   scoped_refptr<PlatformNotificationContextImpl> context(
-      new PlatformNotificationContextImpl(database_dir.path(), nullptr));
+      new PlatformNotificationContextImpl(database_dir.path(),
+                                          browser_context(),
+                                          nullptr));
 
   OverrideTaskRunnerForTesting(context.get());
 
diff --git a/content/browser/storage_partition_impl.cc b/content/browser/storage_partition_impl.cc
index 53c928c8..dc4f55d 100644
--- a/content/browser/storage_partition_impl.cc
+++ b/content/browser/storage_partition_impl.cc
@@ -521,7 +521,8 @@ StoragePartitionImpl* StoragePartitionImpl::Create(
       new NavigatorConnectServiceWorkerServiceFactory(service_worker_context)));
 
   scoped_refptr<PlatformNotificationContextImpl> platform_notification_context =
-      new PlatformNotificationContextImpl(path, service_worker_context);
+      new PlatformNotificationContextImpl(path, context,
+                                          service_worker_context);
   platform_notification_context->Initialize();
 
   scoped_refptr<BackgroundSyncContextImpl> background_sync_context =
diff --git a/content/public/browser/platform_notification_service.h b/content/public/browser/platform_notification_service.h
index afcc319..6c9e509 100644
--- a/content/public/browser/platform_notification_service.h
+++ b/content/public/browser/platform_notification_service.h
@@ -6,6 +6,7 @@
 #define CONTENT_PUBLIC_BROWSER_PLATFORM_NOTIFICATION_SERVICE_H_
 
 #include <stdint.h>
+#include <set>
 #include <string>
 
 #include "base/callback_forward.h"
@@ -72,6 +73,13 @@ class CONTENT_EXPORT PlatformNotificationService {
   virtual void ClosePersistentNotification(
       BrowserContext* browser_context,
       int64_t persistent_notification_id) = 0;
+
+  // Writes the ids of all currently displaying persistent notifications for the
+  // given |browser_context| to |displayed_notifications|. Returns whether the
+  // platform is able to provide such a set.
+  virtual bool GetDisplayedPersistentNotifications(
+      BrowserContext* browser_context,
+      std::set<std::string>* displayed_notifications) = 0;
 };
 
 }  // namespace content
diff --git a/content/shell/browser/layout_test/layout_test_notification_manager.cc b/content/shell/browser/layout_test/layout_test_notification_manager.cc
index a30c719..057cc17 100644
--- a/content/shell/browser/layout_test/layout_test_notification_manager.cc
+++ b/content/shell/browser/layout_test/layout_test_notification_manager.cc
@@ -97,6 +97,15 @@ void LayoutTestNotificationManager::ClosePersistentNotification(
   }
 }
 
+bool LayoutTestNotificationManager::GetDisplayedPersistentNotifications(
+    BrowserContext* browser_context,
+    std::set<std::string>* displayed_notifications) {
+  DCHECK(displayed_notifications);
+
+  // Notifications will never outlive the lifetime of running layout tests.
+  return false;
+}
+
 void LayoutTestNotificationManager::SimulateClick(const std::string& title) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
diff --git a/content/shell/browser/layout_test/layout_test_notification_manager.h b/content/shell/browser/layout_test/layout_test_notification_manager.h
index ac51c5c..53c5930 100644
--- a/content/shell/browser/layout_test/layout_test_notification_manager.h
+++ b/content/shell/browser/layout_test/layout_test_notification_manager.h
@@ -55,6 +55,9 @@ class LayoutTestNotificationManager : public PlatformNotificationService {
   void ClosePersistentNotification(
       BrowserContext* browser_context,
       int64_t persistent_notification_id) override;
+  bool GetDisplayedPersistentNotifications(
+      BrowserContext* browser_context,
+      std::set<std::string>* displayed_notifications) override;
 
  private:
   // Structure to represent the information of a persistent notification.