Integrate the notification database with the normal code path.

This patch actually integrates the Web Notification database with the
existing code paths, which means that it will be used for any persistent
notification being shown from this patch forward.

The database is completely tested with a series of unit tests, whereas
all existing browser tests, layout tests and instrumentation tests will
continue to exercise the full flows.

Please mind that this is the first part in a series. I realize that
there's a fair amount of TODOs in the code. Resolving these takes larger
refactorings (in case of reducing PlatformNotificationService
knowledge) or three-sided patches (in case of the Blink API). These
will be addressed in follow-ups.

Design document:
  http://goo.gl/TciXVp

BUG=447628

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

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

chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationConstants.java

@@ -14,13 +14,19 @@ public class NotificationConstants {
     public static final String ACTION_CLOSE_NOTIFICATION =
             "org.chromium.chrome.browser.notifications.CLOSE_NOTIFICATION";
 
-    public static final String EXTRA_NOTIFICATION_ID = "notification_id";
+    /**
+     * Name of the Intent extra set by the framework when a notification preferences intent has
+     * been triggered from there, which could be one of the setting gears in system UI.
+     */
     public static final String EXTRA_NOTIFICATION_TAG = "notification_tag";
 
-    // TODO(peter): Remove these extras once Notifications are powered by a database on the
-    // native side, that contains all the additional information.
-    public static final String EXTRA_NOTIFICATION_PLATFORM_ID = "notification_platform_id";
-    public static final String EXTRA_NOTIFICATION_DATA = "notification_data";
+    /**
+     * Names of the Intent extras used for Intents related to notifications. These intents are set
+     * and owned by Chromium.
+     */
+    public static final String EXTRA_PERSISTENT_NOTIFICATION_ID = "notification_persistent_id";
+    public static final String EXTRA_NOTIFICATION_INFO_ORIGIN = "notification_info_origin";
+    public static final String EXTRA_NOTIFICATION_INFO_TAG = "notification_info_tag";
 
     /**
      * Unique identifier for a single sync notification. Since the notification ID is reused,

chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationService.java

@@ -52,7 +52,11 @@ public class NotificationService extends IntentService {
      */
     @Override
     public void onHandleIntent(final Intent intent) {
-        if (!intent.hasExtra(NotificationConstants.EXTRA_NOTIFICATION_ID)) return;
+        if (!intent.hasExtra(NotificationConstants.EXTRA_PERSISTENT_NOTIFICATION_ID)
+                || !intent.hasExtra(NotificationConstants.EXTRA_NOTIFICATION_INFO_ORIGIN)
+                || !intent.hasExtra(NotificationConstants.EXTRA_NOTIFICATION_INFO_TAG)) {
+            return;
+        }
 
         ThreadUtils.runOnUiThread(new Runnable() {
             @Override

chrome/android/javatests/src/org/chromium/chrome/browser/notifications/NotificationUIManagerTest.java

@@ -7,7 +7,6 @@ package org.chromium.chrome.browser.notifications;
 import android.app.Notification;
 import android.graphics.Bitmap;
 import android.os.Build;
-import android.test.FlakyTest;
 import android.test.suitebuilder.annotation.LargeTest;
 import android.test.suitebuilder.annotation.MediumTest;
 import android.test.suitebuilder.annotation.SmallTest;
@@ -232,28 +231,6 @@ public class NotificationUIManagerTest extends ChromeShellTestBase {
         assertEquals(generatedIcon.getHeight(), notification.largeIcon.getHeight());
     }
 
-    /*
-     * Verifies that starting the PendingIntent stored as the notification's delete intent will
-     * close the notification.
-     */
-    /* @MediumTest */
-    @FlakyTest
-    @Feature({"Browser", "Notifications"})
-    public void testNotificationDeleteIntentClosesNotification() throws Exception {
-        setNotificationContentSettingForCurrentOrigin(ContentSetting.ALLOW);
-
-        Notification notification = showAndGetNotification("MyNotification", "{}");
-        assertEquals(1, mMockNotificationManager.getNotifications().size());
-
-        // Sending the PendingIntent simulates dismissing (swiping away) the notification.
-        assertNotNull(notification.deleteIntent);
-        notification.deleteIntent.send();
-        // The deleteIntent should trigger a call to cancel() in the NotificationManager.
-        assertTrue(waitForNotificationManagerMutation());
-
-        assertEquals(0, mMockNotificationManager.getNotifications().size());
-    }
-
     /*
      * Verifies that starting the PendingIntent stored as the notification's content intent will
      * start up the associated Service Worker, where the JavaScript code will close the notification

chrome/browser/notifications/notification_ui_manager_android.h

@@ -17,21 +17,27 @@ class ProfileNotification;
 
 // Implementation of the Notification UI Manager for Android, which defers to
 // the Android framework for displaying notifications.
+//
+// The Android notification manager only operates reliably on notifications
+// that were opened or interacted with since the last restart of Chrome.
 class NotificationUIManagerAndroid : public NotificationUIManager {
  public:
   NotificationUIManagerAndroid();
   ~NotificationUIManagerAndroid() override;
 
-  // Called by the Java implementation when a notification has been clicked on.
+  // Called by the Java implementation when the notification has been clicked.
   bool OnNotificationClicked(JNIEnv* env,
                              jobject java_object,
-                             jstring notification_id,
-                             jbyteArray serialized_notification_data);
+                             jlong persistent_notification_id,
+                             jstring java_origin,
+                             jstring java_tag);
 
-  // Called by the Java implementation when a notification has been closed.
+  // Called by the Java implementation when the notification has been closed.
   bool OnNotificationClosed(JNIEnv* env,
                             jobject java_object,
-                            jstring notification_id);
+                            jlong persistent_notification_id,
+                            jstring java_origin,
+                            jstring java_tag);
 
   // NotificationUIManager implementation;
   void Add(const Notification& notification, Profile* profile) override;
@@ -75,11 +81,14 @@ class NotificationUIManagerAndroid : public NotificationUIManager {
   // Map from a notification id to the associated ProfileNotification*.
   std::map<std::string, ProfileNotification*> profile_notifications_;
 
-  // Holds the tag of a notification, and its origin.
+  // Pair containing the information necessary in order to enable closing
+  // notifications that were not created by this instance of the manager: the
+  // notification's origin and tag. This list may not contain the notifications
+  // that have not been interacted with since the last restart of Chrome.
   using RegeneratedNotificationInfo = std::pair<std::string, std::string>;
 
-  // Map from notification id to RegeneratedNotificationInfo.
-  std::map<std::string, RegeneratedNotificationInfo>
+  // Map from persistent notification id to RegeneratedNotificationInfo.
+  std::map<int64_t, RegeneratedNotificationInfo>
       regenerated_notification_infos_;
 
   base::android::ScopedJavaGlobalRef<jobject> java_object_;

chrome/browser/notifications/persistent_notification_delegate.cc

@@ -19,13 +19,11 @@ void OnEventDispatchComplete(content::PersistentNotificationStatus status) {}
 
 PersistentNotificationDelegate::PersistentNotificationDelegate(
     content::BrowserContext* browser_context,
-    int64 service_worker_registration_id,
-    const GURL& origin,
-    const content::PlatformNotificationData& notification_data)
+    int64_t persistent_notification_id,
+    const GURL& origin)
     : browser_context_(browser_context),
-      service_worker_registration_id_(service_worker_registration_id),
+      persistent_notification_id_(persistent_notification_id),
       origin_(origin),
-      notification_data_(notification_data),
       id_(base::GenerateGUID()) {}
 
 PersistentNotificationDelegate::~PersistentNotificationDelegate() {}
@@ -37,10 +35,8 @@ void PersistentNotificationDelegate::Close(bool by_user) {}
 void PersistentNotificationDelegate::Click() {
   PlatformNotificationServiceImpl::GetInstance()->OnPersistentNotificationClick(
       browser_context_,
-      service_worker_registration_id_,
-      id_,
+      persistent_notification_id_,
       origin_,
-      notification_data_,
       base::Bind(&OnEventDispatchComplete));
 }
 

chrome/browser/notifications/persistent_notification_delegate.h

@@ -5,10 +5,10 @@
 #ifndef CHROME_BROWSER_NOTIFICATIONS_PERSISTENT_NOTIFICATION_DELEGATE_H_
 #define CHROME_BROWSER_NOTIFICATIONS_PERSISTENT_NOTIFICATION_DELEGATE_H_
 
+#include <stdint.h>
 #include <string>
 
 #include "chrome/browser/notifications/notification_delegate.h"
-#include "content/public/common/platform_notification_data.h"
 #include "url/gurl.h"
 
 namespace content {
@@ -22,16 +22,13 @@ class PersistentNotificationDelegate : public NotificationDelegate {
  public:
   PersistentNotificationDelegate(
       content::BrowserContext* browser_context,
-      int64 service_worker_registration_id,
-      const GURL& origin,
-      const content::PlatformNotificationData& notification_data);
+      int64_t persistent_notification_id,
+      const GURL& origin);
 
-  int64 service_worker_registration_id() const {
-    return service_worker_registration_id_;
-  }
-
-  const content::PlatformNotificationData& notification_data() const {
-    return notification_data_;
+  // Persistent id of this notification in the notification database. To be
+  // used when retrieving all information associated with it.
+  int64_t persistent_notification_id() const {
+    return persistent_notification_id_;
   }
 
   // NotificationDelegate implementation.
@@ -45,9 +42,8 @@ class PersistentNotificationDelegate : public NotificationDelegate {
 
  private:
   content::BrowserContext* browser_context_;
-  int64 service_worker_registration_id_;
+  int64_t persistent_notification_id_;
   GURL origin_;
-  content::PlatformNotificationData notification_data_;
 
   std::string id_;
 };

chrome/browser/notifications/platform_notification_service_impl.cc

@@ -36,6 +36,10 @@
 #include "extensions/common/permissions/permissions_data.h"
 #endif
 
+#if defined(OS_ANDROID)
+#include "base/strings/string_number_conversions.h"
+#endif
+
 using content::BrowserThread;
 using message_center::NotifierId;
 
@@ -61,20 +65,16 @@ PlatformNotificationServiceImpl::~PlatformNotificationServiceImpl() {}
 
 void PlatformNotificationServiceImpl::OnPersistentNotificationClick(
     content::BrowserContext* browser_context,
-    int64 service_worker_registration_id,
-    const std::string& notification_id,
+    int64_t persistent_notification_id,
     const GURL& origin,
-    const content::PlatformNotificationData& notification_data,
     const base::Callback<void(content::PersistentNotificationStatus)>&
         callback) const {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   content::NotificationEventDispatcher::GetInstance()
       ->DispatchNotificationClickEvent(
             browser_context,
+            persistent_notification_id,
             origin,
-            service_worker_registration_id,
-            notification_id,
-            notification_data,
             callback);
 }
 
@@ -218,7 +218,7 @@ void PlatformNotificationServiceImpl::DisplayNotification(
 
 void PlatformNotificationServiceImpl::DisplayPersistentNotification(
     content::BrowserContext* browser_context,
-    int64 service_worker_registration_id,
+    int64_t persistent_notification_id,
     const GURL& origin,
     const SkBitmap& icon,
     const content::PlatformNotificationData& notification_data) {
@@ -228,14 +228,15 @@ void PlatformNotificationServiceImpl::DisplayPersistentNotification(
   DCHECK(profile);
 
   PersistentNotificationDelegate* delegate = new PersistentNotificationDelegate(
-      browser_context,
-      service_worker_registration_id,
-      origin,
-      notification_data);
+      browser_context, persistent_notification_id, origin);
 
   Notification notification = CreateNotificationFromData(
       profile, origin, icon, notification_data, delegate);
 
+  // TODO(peter): Remove this mapping when we have reliable id generation for
+  // the message_center::Notification objects.
+  persistent_notifications_[persistent_notification_id] = notification.id();
+
   GetNotificationUIManager()->Add(notification, profile);
 
   profile->GetHostContentSettingsMap()->UpdateLastUsage(
@@ -244,14 +245,30 @@ void PlatformNotificationServiceImpl::DisplayPersistentNotification(
 
 void PlatformNotificationServiceImpl::ClosePersistentNotification(
     content::BrowserContext* browser_context,
-    const std::string& persistent_notification_id) {
+    int64_t persistent_notification_id) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
   Profile* profile = Profile::FromBrowserContext(browser_context);
   DCHECK(profile);
 
+#if defined(OS_ANDROID)
+  // TODO(peter): Remove this conversion when the notification ids are being
+  // generated by the caller of this method.
+  std::string textual_persistent_notification_id =
+      base::Int64ToString(persistent_notification_id);
   GetNotificationUIManager()->CancelById(
-      persistent_notification_id, NotificationUIManager::GetProfileID(profile));
+      textual_persistent_notification_id,
+      NotificationUIManager::GetProfileID(profile));
+#else
+  auto iter = persistent_notifications_.find(persistent_notification_id);
+  if (iter == persistent_notifications_.end())
+    return;
+
+  GetNotificationUIManager()->CancelById(
+      iter->second, NotificationUIManager::GetProfileID(profile));
+
+  persistent_notifications_.erase(iter);
+#endif
 }
 
 Notification PlatformNotificationServiceImpl::CreateNotificationFromData(

chrome/browser/notifications/platform_notification_service_impl.h

@@ -5,6 +5,9 @@
 #ifndef CHROME_BROWSER_NOTIFICATIONS_PLATFORM_NOTIFICATION_SERVICE_IMPL_H_
 #define CHROME_BROWSER_NOTIFICATIONS_PLATFORM_NOTIFICATION_SERVICE_IMPL_H_
 
+#include <stdint.h>
+#include <map>
+
 #include "base/gtest_prod_util.h"
 #include "base/memory/singleton.h"
 #include "base/strings/string16.h"
@@ -34,10 +37,8 @@ class PlatformNotificationServiceImpl
   // needed, on which the event will be fired. Must be called on the UI thread.
   void OnPersistentNotificationClick(
       content::BrowserContext* browser_context,
-      int64 service_worker_registration_id,
-      const std::string& notification_id,
+      int64_t persistent_notification_id,
       const GURL& origin,
-      const content::PlatformNotificationData& notification_data,
       const base::Callback<void(content::PersistentNotificationStatus)>&
           callback) const;
 
@@ -63,13 +64,13 @@ class PlatformNotificationServiceImpl
       base::Closure* cancel_callback) override;
   void DisplayPersistentNotification(
       content::BrowserContext* browser_context,
-      int64 service_worker_registration_id,
+      int64_t persistent_notification_id,
       const GURL& origin,
       const SkBitmap& icon,
       const content::PlatformNotificationData& notification_data) override;
   void ClosePersistentNotification(
       content::BrowserContext* browser_context,
-      const std::string& persistent_notification_id) override;
+      int64_t persistent_notification_id) override;
 
  private:
   friend struct DefaultSingletonTraits<PlatformNotificationServiceImpl>;
@@ -114,6 +115,10 @@ class PlatformNotificationServiceImpl
   // Weak reference. Ownership maintains with the test.
   NotificationUIManager* notification_ui_manager_for_tests_;
 
+  // Mapping between a persistent notification id and the id of the associated
+  // message_center::Notification object. Must only be used on the UI thread.
+  std::map<int64_t, std::string> persistent_notifications_;
+
   DISALLOW_COPY_AND_ASSIGN(PlatformNotificationServiceImpl);
 };
 

chrome/browser/notifications/platform_notification_service_unittest.cc

@@ -17,6 +17,10 @@
 
 namespace {
 
+#if !defined(OS_ANDROID)
+const int64_t kPersistentNotificationId = 42;
+#endif
+
 class MockDesktopNotificationDelegate
     : public content::DesktopNotificationDelegate {
  public:
@@ -126,13 +130,16 @@ TEST_F(PlatformNotificationServiceTest, DisplayPageCloseClosure) {
   // delegate given that it'd result in a use-after-free.
 }
 
+// TODO(peter): Re-enable this test when //content is responsible for creating
+// the notification delegate ids.
+#if !defined(OS_ANDROID)
 TEST_F(PlatformNotificationServiceTest, PersistentNotificationDisplay) {
   content::PlatformNotificationData notification_data;
   notification_data.title = base::ASCIIToUTF16("My notification's title");
   notification_data.body = base::ASCIIToUTF16("Hello, world!");
 
   service()->DisplayPersistentNotification(
-      profile(), 42 /* sw_registration_id */, GURL("https://chrome.com/"),
+      profile(), kPersistentNotificationId, GURL("https://chrome.com/"),
       SkBitmap(), notification_data);
 
   ASSERT_EQ(1u, ui_manager()->GetNotificationCount());
@@ -144,10 +151,10 @@ TEST_F(PlatformNotificationServiceTest, PersistentNotificationDisplay) {
   EXPECT_EQ("Hello, world!",
       base::UTF16ToUTF8(notification.message()));
 
-  service()->ClosePersistentNotification(profile(),
-                                         notification.delegate_id());
+  service()->ClosePersistentNotification(profile(), kPersistentNotificationId);
   EXPECT_EQ(0u, ui_manager()->GetNotificationCount());
 }
+#endif  // !defined(OS_ANDROID)
 
 TEST_F(PlatformNotificationServiceTest, DisplayPageNotificationMatches) {
   content::PlatformNotificationData notification_data;

content/browser/notifications/notification_event_dispatcher_impl.cc

@@ -5,11 +5,14 @@
 #include "content/browser/notifications/notification_event_dispatcher_impl.h"
 
 #include "base/callback.h"
+#include "base/strings/string_number_conversions.h"
+#include "content/browser/notifications/platform_notification_context_impl.h"
 #include "content/browser/service_worker/service_worker_context_wrapper.h"
 #include "content/browser/service_worker/service_worker_registration.h"
 #include "content/browser/service_worker/service_worker_storage.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/browser_thread.h"
+#include "content/public/browser/notification_database_data.h"
 #include "content/public/browser/storage_partition.h"
 #include "content/public/common/platform_notification_data.h"
 
@@ -61,8 +64,7 @@ void NotificationClickEventFinished(
 // Dispatches the notificationclick on |service_worker_registration| if the
 // registration was available. Must be called on the IO thread.
 void DispatchNotificationClickEventOnRegistration(
-    const std::string& notification_id,
-    const PlatformNotificationData& notification_data,
+    const NotificationDatabaseData& notification_database_data,
     const NotificationClickDispatchCompleteCallback& dispatch_complete_callback,
     ServiceWorkerStatusCode service_worker_status,
     const scoped_refptr<ServiceWorkerRegistration>&
@@ -73,10 +75,17 @@ void DispatchNotificationClickEventOnRegistration(
         base::Bind(&NotificationClickEventFinished,
                    dispatch_complete_callback,
                    service_worker_registration);
-    service_worker_registration->active_version()
-        ->DispatchNotificationClickEvent(dispatch_event_callback,
-                                         notification_id,
-                                         notification_data);
+
+    // TODO(peter): Pass the persistent notification id as an int64_t rather
+    // than as a string. This depends on the Blink API being updated.
+    std::string persistent_notification_id_string =
+        base::Int64ToString(notification_database_data.notification_id);
+
+    service_worker_registration->active_version()->
+        DispatchNotificationClickEvent(
+            dispatch_event_callback,
+            persistent_notification_id_string,
+            notification_database_data.notification_data);
     return;
   }
 
@@ -115,21 +124,44 @@ void DispatchNotificationClickEventOnRegistration(
 // |service_worker_registration_id|. Must be called on the IO thread.
 void FindServiceWorkerRegistration(
     const GURL& origin,
-    int64 service_worker_registration_id,
-    const std::string& notification_id,
-    const PlatformNotificationData& notification_data,
     const NotificationClickDispatchCompleteCallback& dispatch_complete_callback,
-    scoped_refptr<ServiceWorkerContextWrapper> service_worker_context) {
+    scoped_refptr<ServiceWorkerContextWrapper> service_worker_context,
+    bool success,
+    const NotificationDatabaseData& notification_database_data) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  if (!success) {
+    BrowserThread::PostTask(
+        BrowserThread::UI,
+        FROM_HERE,
+        base::Bind(dispatch_complete_callback,
+                   PERSISTENT_NOTIFICATION_STATUS_DATABASE_ERROR));
+    return;
+  }
+
   service_worker_context->context()->storage()->FindRegistrationForId(
-      service_worker_registration_id,
+      notification_database_data.service_worker_registration_id,
       origin,
       base::Bind(&DispatchNotificationClickEventOnRegistration,
-                 notification_id,
-                 notification_data,
+                 notification_database_data,
                  dispatch_complete_callback));
 }
 
+// Reads the data associated with the |persistent_notification_id| belonging to
+// |origin| from the notification context.
+void ReadNotificationDatabaseData(
+    int64_t persistent_notification_id,
+    const GURL& origin,
+    const NotificationClickDispatchCompleteCallback& dispatch_complete_callback,
+    scoped_refptr<ServiceWorkerContextWrapper> service_worker_context,
+    scoped_refptr<PlatformNotificationContextImpl> notification_context) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  notification_context->ReadNotificationData(
+      persistent_notification_id,
+      origin,
+      base::Bind(&FindServiceWorkerRegistration,
+                 origin, dispatch_complete_callback, service_worker_context));
+}
+
 }  // namespace
 
 // static
@@ -149,29 +181,33 @@ NotificationEventDispatcherImpl::~NotificationEventDispatcherImpl() {}
 
 void NotificationEventDispatcherImpl::DispatchNotificationClickEvent(
     BrowserContext* browser_context,
+    int64_t persistent_notification_id,
     const GURL& origin,
-    int64 service_worker_registration_id,
-    const std::string& notification_id,
-    const PlatformNotificationData& notification_data,
     const NotificationClickDispatchCompleteCallback&
         dispatch_complete_callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  DCHECK_GT(persistent_notification_id, 0);
+  DCHECK(origin.is_valid());
 
   StoragePartition* partition =
       BrowserContext::GetStoragePartitionForSite(browser_context, origin);
+
   scoped_refptr<ServiceWorkerContextWrapper> service_worker_context =
       static_cast<ServiceWorkerContextWrapper*>(
           partition->GetServiceWorkerContext());
+  scoped_refptr<PlatformNotificationContextImpl> notification_context =
+      static_cast<PlatformNotificationContextImpl*>(
+          partition->GetPlatformNotificationContext());
+
   BrowserThread::PostTask(
       BrowserThread::IO,
       FROM_HERE,
-      base::Bind(&FindServiceWorkerRegistration,
+      base::Bind(&ReadNotificationDatabaseData,
+                 persistent_notification_id,
                  origin,
-                 service_worker_registration_id,
-                 notification_id,
-                 notification_data,
                  dispatch_complete_callback,
-                 service_worker_context));
+                 service_worker_context,
+                 notification_context));
 }
 
 }  // namespace content

content/browser/notifications/notification_event_dispatcher_impl.h

@@ -19,10 +19,8 @@ class NotificationEventDispatcherImpl : public NotificationEventDispatcher {
   // NotificationEventDispatcher implementation.
   void DispatchNotificationClickEvent(
       BrowserContext* browser_context,
+      int64_t persistent_notification_id,
       const GURL& origin,
-      int64 service_worker_registration_id,
-      const std::string& notification_id,
-      const PlatformNotificationData& notification_data,
       const NotificationClickDispatchCompleteCallback&
           dispatch_complete_callback) override;
 

content/browser/notifications/notification_message_filter.cc

@@ -12,6 +12,7 @@
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/content_browser_client.h"
 #include "content/public/browser/desktop_notification_delegate.h"
+#include "content/public/browser/notification_database_data.h"
 #include "content/public/browser/platform_notification_service.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/common/content_client.h"
@@ -27,14 +28,21 @@ NotificationMessageFilter::NotificationMessageFilter(
       process_id_(process_id),
       notification_context_(notification_context),
       resource_context_(resource_context),
-      browser_context_(browser_context) {}
+      browser_context_(browser_context),
+      weak_factory_io_(this) {}
 
 NotificationMessageFilter::~NotificationMessageFilter() {}
 
 void NotificationMessageFilter::DidCloseNotification(int notification_id) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+
   close_closures_.erase(notification_id);
 }
 
+void NotificationMessageFilter::OnDestruct() const {
+  BrowserThread::DeleteOnIOThread::Destruct(this);
+}
+
 bool NotificationMessageFilter::OnMessageReceived(const IPC::Message& message) {
   bool handled = true;
   IPC_BEGIN_MESSAGE_MAP(NotificationMessageFilter, message)
@@ -59,24 +67,15 @@ bool NotificationMessageFilter::OnMessageReceived(const IPC::Message& message) {
 void NotificationMessageFilter::OverrideThreadForMessage(
     const IPC::Message& message, content::BrowserThread::ID* thread) {
   if (message.type() == PlatformNotificationHostMsg_Show::ID ||
-      message.type() == PlatformNotificationHostMsg_ShowPersistent::ID ||
-      message.type() == PlatformNotificationHostMsg_Close::ID ||
-      message.type() == PlatformNotificationHostMsg_ClosePersistent::ID)
+      message.type() == PlatformNotificationHostMsg_Close::ID)
     *thread = BrowserThread::UI;
 }
 
 void NotificationMessageFilter::OnCheckNotificationPermission(
     const GURL& origin, blink::WebNotificationPermission* permission) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
-  PlatformNotificationService* service =
-      GetContentClient()->browser()->GetPlatformNotificationService();
-  if (service) {
-    *permission = service->CheckPermissionOnIOThread(resource_context_,
-                                                     origin,
-                                                     process_id_);
-  } else {
-    *permission = blink::WebNotificationPermissionDenied;
-  }
+
+  *permission = GetPermissionForOriginOnIO(origin);
 }
 
 void NotificationMessageFilter::OnShowPlatformNotification(
@@ -116,25 +115,58 @@ void NotificationMessageFilter::OnShowPersistentNotification(
     const GURL& origin,
     const SkBitmap& icon,
     const PlatformNotificationData& notification_data) {
-  DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  if (!RenderProcessHost::FromID(process_id_))
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  if (GetPermissionForOriginOnIO(origin) !=
+          blink::WebNotificationPermissionAllowed) {
+    BadMessageReceived();
     return;
+  }
 
-  PlatformNotificationService* service =
-      GetContentClient()->browser()->GetPlatformNotificationService();
-  DCHECK(service);
+  NotificationDatabaseData database_data;
+  database_data.origin = origin;
+  database_data.service_worker_registration_id = service_worker_registration_id;
+  database_data.notification_data = notification_data;
+
+  // TODO(peter): Significantly reduce the amount of information we need to
+  // retain outside of the database for displaying notifications.
+  notification_context_->WriteNotificationData(
+      origin,
+      database_data,
+      base::Bind(&NotificationMessageFilter::DidWritePersistentNotificationData,
+                 weak_factory_io_.GetWeakPtr(),
+                 request_id,
+                 origin,
+                 icon,
+                 notification_data));
+}
 
-  if (!VerifyNotificationPermissionGranted(service, origin))
-    return;
+void NotificationMessageFilter::DidWritePersistentNotificationData(
+    int request_id,
+    const GURL& origin,
+    const SkBitmap& icon,
+    const PlatformNotificationData& notification_data,
+    bool success,
+    int64_t persistent_notification_id) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
-  service->DisplayPersistentNotification(browser_context_,
-                                         service_worker_registration_id, origin,
-                                         icon, notification_data);
+  if (success) {
+    PlatformNotificationService* service =
+      GetContentClient()->browser()->GetPlatformNotificationService();
+    DCHECK(service);
+
+    BrowserThread::PostTask(
+        BrowserThread::UI,
+        FROM_HERE,
+        base::Bind(&PlatformNotificationService::DisplayPersistentNotification,
+                   base::Unretained(service),  // The service is a singleton.
+                   browser_context_,
+                   persistent_notification_id,
+                   origin,
+                   icon,
+                   notification_data));
+  }
 
-  // TODO(peter): Confirm display of the persistent notification after the
-  // data has been stored using the |notification_context_|.
-  Send(new PlatformNotificationMsg_DidShowPersistent(request_id,
-                                                     true /* success */));
+  Send(new PlatformNotificationMsg_DidShowPersistent(request_id, success));
 }
 
 void NotificationMessageFilter::OnGetNotifications(
@@ -168,27 +200,68 @@ void NotificationMessageFilter::OnClosePlatformNotification(
 
 void NotificationMessageFilter::OnClosePersistentNotification(
     const GURL& origin,
-    const std::string& persistent_notification_id) {
-  DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  if (!RenderProcessHost::FromID(process_id_))
+    int64_t persistent_notification_id) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  if (GetPermissionForOriginOnIO(origin) !=
+          blink::WebNotificationPermissionAllowed) {
+    BadMessageReceived();
     return;
+  }
 
   PlatformNotificationService* service =
       GetContentClient()->browser()->GetPlatformNotificationService();
   DCHECK(service);
 
-  // TODO(peter): Use |service_worker_registration_id| and |origin| when feeding
-  // the close event through the notification database.
+  // There's no point in waiting until the database data has been removed before
+  // closing the notification presented to the user. Post that task immediately.
+  BrowserThread::PostTask(
+      BrowserThread::UI,
+      FROM_HERE,
+      base::Bind(&PlatformNotificationService::ClosePersistentNotification,
+                 base::Unretained(service),  // The service is a singleton.
+                 browser_context_,
+                 persistent_notification_id));
+
+  notification_context_->DeleteNotificationData(
+      persistent_notification_id,
+      origin,
+      base::Bind(&NotificationMessageFilter::
+                     DidDeletePersistentNotificationData,
+                 weak_factory_io_.GetWeakPtr()));
+}
+
+void NotificationMessageFilter::DidDeletePersistentNotificationData(
+    bool success) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  // TODO(peter): Consider feeding back to the renderer that the notification
+  // has been closed.
+}
+
+blink::WebNotificationPermission
+NotificationMessageFilter::GetPermissionForOriginOnIO(
+    const GURL& origin) const {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+
+  PlatformNotificationService* service =
+      GetContentClient()->browser()->GetPlatformNotificationService();
+  if (!service)
+    return blink::WebNotificationPermissionDenied;
 
-  service->ClosePersistentNotification(browser_context_,
-                                       persistent_notification_id);
+  return service->CheckPermissionOnIOThread(resource_context_,
+                                            origin,
+                                            process_id_);
 }
 
 bool NotificationMessageFilter::VerifyNotificationPermissionGranted(
     PlatformNotificationService* service,
     const GURL& origin) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+
   blink::WebNotificationPermission permission =
-      service->CheckPermissionOnUIThread(browser_context_, origin, process_id_);
+      service->CheckPermissionOnUIThread(browser_context_,
+                                         origin,
+                                         process_id_);
+
   if (permission == blink::WebNotificationPermissionAllowed)
     return true;
 

content/browser/notifications/notification_message_filter.h

@@ -8,6 +8,7 @@
 #include <map>
 
 #include "base/callback_forward.h"
+#include "base/memory/weak_ptr.h"
 #include "content/public/browser/browser_message_filter.h"
 #include "third_party/WebKit/public/platform/modules/notifications/WebNotificationPermission.h"
 
@@ -35,6 +36,7 @@ class NotificationMessageFilter : public BrowserMessageFilter {
   void DidCloseNotification(int notification_id);
 
   // BrowserMessageFilter implementation. Called on the UI thread.
+  void OnDestruct() const override;
   bool OnMessageReceived(const IPC::Message& message) override;
   void OverrideThreadForMessage(
       const IPC::Message& message, content::BrowserThread::ID* thread) override;
@@ -43,6 +45,9 @@ class NotificationMessageFilter : public BrowserMessageFilter {
   ~NotificationMessageFilter() override;
 
  private:
+  friend class base::DeleteHelper<NotificationMessageFilter>;
+  friend class BrowserThread;
+
   void OnCheckNotificationPermission(
       const GURL& origin, blink::WebNotificationPermission* permission);
   void OnShowPlatformNotification(
@@ -63,7 +68,29 @@ class NotificationMessageFilter : public BrowserMessageFilter {
   void OnClosePlatformNotification(int notification_id);
   void OnClosePersistentNotification(
       const GURL& origin,
-      const std::string& persistent_notification_id);
+      int64_t persistent_notification_id);
+
+  // Callback to be invoked by the notification context when the notification
+  // data for the persistent notification may have been written, as indicated by
+  // |success|. Will present the notification to the user when successful.
+  void DidWritePersistentNotificationData(
+      int request_id,
+      const GURL& origin,
+      const SkBitmap& icon,
+      const PlatformNotificationData& notification_data,
+      bool success,
+      int64_t persistent_notification_id);
+
+  // Callback to be invoked when the data associated with a persistent
+  // notification has been removed by the database, unless an error occurred,
+  // which will be indicated by |success|.
+  void DidDeletePersistentNotificationData(bool success);
+
+  // Returns the permission status for |origin|. Must only be used on the IO
+  // thread. If the PlatformNotificationService is unavailable, permission will
+  // assumed to be denied.
+  blink::WebNotificationPermission GetPermissionForOriginOnIO(
+      const GURL& origin) const;
 
   // Verifies that Web Notification permission has been granted for |origin| in
   // cases where the renderer shouldn't send messages if it weren't the case. If
@@ -80,6 +107,10 @@ class NotificationMessageFilter : public BrowserMessageFilter {
 
   // Map mapping notification ids to their associated close closures.
   std::map<int, base::Closure> close_closures_;
+
+  base::WeakPtrFactory<NotificationMessageFilter> weak_factory_io_;
+
+  DISALLOW_COPY_AND_ASSIGN(NotificationMessageFilter);
 };
 
 }  // namespace content

content/child/notifications/notification_manager.cc

@@ -5,6 +5,7 @@
 #include "content/child/notifications/notification_manager.h"
 
 #include "base/lazy_instance.h"
+#include "base/strings/string_number_conversions.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/thread_task_runner_handle.h"
 #include "base/threading/thread_local.h"
@@ -164,10 +165,23 @@ void NotificationManager::close(blink::WebNotificationDelegate* delegate) {
 
 void NotificationManager::closePersistent(
     const blink::WebSerializedOrigin& origin,
-    const blink::WebString& persistent_notification_id) {
+    const blink::WebString& persistent_notification_id_string) {
+  // TODO(peter): Blink should store the persistent_notification_id as an
+  // int64_t instead of a string. The id, created by Chromium, is a decimal
+  // number that fits in an int64_t, so convert it until the API updates.
+  base::string16 string_value = persistent_notification_id_string;
+
+  int64_t persistent_notification_id = 0;
+  if (!base::StringToInt64(string_value,
+                           &persistent_notification_id)) {
+    NOTREACHED() << "Unable to close persistent notification; invalid id: "
+                  << string_value;
+    return;
+  }
+
   thread_safe_sender_->Send(new PlatformNotificationHostMsg_ClosePersistent(
       GURL(origin.string()),
-      base::UTF16ToUTF8(persistent_notification_id)));
+      persistent_notification_id));
 }
 
 void NotificationManager::notifyDelegateDestroyed(

content/common/platform_notification_messages.h

@@ -99,7 +99,7 @@ IPC_MESSAGE_CONTROL1(PlatformNotificationHostMsg_Close,
 
 IPC_MESSAGE_CONTROL2(PlatformNotificationHostMsg_ClosePersistent,
                      GURL /* origin */,
-                     std::string /* persistent_notification_id */)
+                     int64_t /* persistent_notification_id */)
 
 IPC_SYNC_MESSAGE_CONTROL1_1(PlatformNotificationHostMsg_CheckPermission,
                             GURL /* origin */,

content/public/browser/notification_event_dispatcher.h

@@ -5,6 +5,7 @@
 #ifndef CONTENT_PUBLIC_BROWSER_NOTIFICATION_EVENT_DISPATCHER_H_
 #define CONTENT_PUBLIC_BROWSER_NOTIFICATION_EVENT_DISPATCHER_H_
 
+#include <stdint.h>
 #include <string>
 
 #include "base/callback_forward.h"
@@ -30,14 +31,12 @@ class CONTENT_EXPORT NotificationEventDispatcher {
       base::Callback<void(PersistentNotificationStatus)>;
 
   // Dispatches the "notificationclick" event on the Service Worker associated
-  // with |origin| and |service_worker_registration_id|. The |callback| will
-  // be invoked when it's known whether the event successfully executed.
+  // with |persistent_notification_id| belonging to |origin|. The |callback|
+  // will be invoked when it's known whether the event successfully executed.
   virtual void DispatchNotificationClickEvent(
       BrowserContext* browser_context,
+      int64_t persistent_notification_id,
       const GURL& origin,
-      int64 service_worker_registration_id,
-      const std::string& notification_id,
-      const PlatformNotificationData& notification_data,
       const NotificationClickDispatchCompleteCallback&
           dispatch_complete_callback) = 0;
 

content/public/browser/platform_notification_service.h

@@ -5,6 +5,7 @@
 #ifndef CONTENT_PUBLIC_BROWSER_PLATFORM_NOTIFICATION_SERVICE_H_
 #define CONTENT_PUBLIC_BROWSER_PLATFORM_NOTIFICATION_SERVICE_H_
 
+#include <stdint.h>
 #include <string>
 
 #include "base/callback_forward.h"
@@ -61,7 +62,7 @@ class CONTENT_EXPORT PlatformNotificationService {
   // the user. This method must be called on the UI thread.
   virtual void DisplayPersistentNotification(
       BrowserContext* browser_context,
-      int64 service_worker_registration_id,
+      int64_t persistent_notification_id,
       const GURL& origin,
       const SkBitmap& icon,
       const PlatformNotificationData& notification_data) = 0;
@@ -70,7 +71,7 @@ class CONTENT_EXPORT PlatformNotificationService {
   // |persistent_notification_id|. This method must be called on the UI thread.
   virtual void ClosePersistentNotification(
       BrowserContext* browser_context,
-      const std::string& persistent_notification_id) = 0;
+      int64_t persistent_notification_id) = 0;
 };
 
 }  // namespace content

content/public/common/persistent_notification_status.h

@@ -20,7 +20,11 @@ enum PersistentNotificationStatus {
 
   // The event has been delivered, but the developer extended the event with a
   // promise that has been rejected.
-  PERSISTENT_NOTIFICATION_STATUS_EVENT_WAITUNTIL_REJECTED
+  PERSISTENT_NOTIFICATION_STATUS_EVENT_WAITUNTIL_REJECTED,
+
+  // The event could not be delivered because the data associated with the
+  // notification could not be read from the database.
+  PERSISTENT_NOTIFICATION_STATUS_DATABASE_ERROR,
 };
 
 }  // content

content/shell/browser/layout_test/layout_test_notification_manager.cc

@@ -24,10 +24,6 @@ void OnEventDispatchComplete(PersistentNotificationStatus status) {}
 LayoutTestNotificationManager::LayoutTestNotificationManager()
     : weak_factory_(this) {}
 
-LayoutTestNotificationManager::PersistentNotification::PersistentNotification()
-    : browser_context(nullptr),
-      service_worker_registration_id(0) {}
-
 LayoutTestNotificationManager::~LayoutTestNotificationManager() {}
 
 PermissionStatus LayoutTestNotificationManager::RequestPermission(
@@ -84,7 +80,7 @@ void LayoutTestNotificationManager::DisplayNotification(
 
 void LayoutTestNotificationManager::DisplayPersistentNotification(
     BrowserContext* browser_context,
-    int64 service_worker_registration_id,
+    int64_t persistent_notification_id,
     const GURL& origin,
     const SkBitmap& icon,
     const PlatformNotificationData& notification_data) {
@@ -96,16 +92,14 @@ void LayoutTestNotificationManager::DisplayPersistentNotification(
   PersistentNotification notification;
   notification.browser_context = browser_context;
   notification.origin = origin;
-  notification.notification_data = notification_data;
-  notification.service_worker_registration_id = service_worker_registration_id;
-  notification.persistent_id = base::GenerateGUID();
+  notification.persistent_id = persistent_notification_id;
 
   persistent_notifications_[title] = notification;
 }
 
 void LayoutTestNotificationManager::ClosePersistentNotification(
     BrowserContext* browser_context,
-    const std::string& persistent_notification_id) {
+    int64_t persistent_notification_id) {
   for (const auto& iter : persistent_notifications_) {
     if (iter.second.persistent_id != persistent_notification_id)
       continue;
@@ -134,10 +128,8 @@ void LayoutTestNotificationManager::SimulateClick(const std::string& title) {
   content::NotificationEventDispatcher::GetInstance()
       ->DispatchNotificationClickEvent(
           notification.browser_context,
-          notification.origin,
-          notification.service_worker_registration_id,
           notification.persistent_id,
-          notification.notification_data,
+          notification.origin,
           base::Bind(&OnEventDispatchComplete));
 }
 

content/shell/browser/layout_test/layout_test_notification_manager.h

@@ -5,6 +5,7 @@
 #ifndef CONTENT_SHELL_BROWSER_LAYOUT_TEST_LAYOUT_TEST_NOTIFICATION_MANAGER_H_
 #define CONTENT_SHELL_BROWSER_LAYOUT_TEST_LAYOUT_TEST_NOTIFICATION_MANAGER_H_
 
+#include <stdint.h>
 #include <map>
 #include <string>
 
@@ -13,7 +14,6 @@
 #include "base/synchronization/lock.h"
 #include "content/public/browser/platform_notification_service.h"
 #include "content/public/common/permission_status.mojom.h"
-#include "content/public/common/platform_notification_data.h"
 #include "third_party/WebKit/public/platform/modules/notifications/WebNotificationPermission.h"
 #include "url/gurl.h"
 
@@ -70,13 +70,13 @@ class LayoutTestNotificationManager : public PlatformNotificationService {
                            base::Closure* cancel_callback) override;
   void DisplayPersistentNotification(
       BrowserContext* browser_context,
-      int64 service_worker_registration_id,
+      int64_t persistent_notification_id,
       const GURL& origin,
       const SkBitmap& icon,
       const PlatformNotificationData& notification_data) override;
   void ClosePersistentNotification(
       BrowserContext* browser_context,
-      const std::string& persistent_notification_id) override;
+      int64_t persistent_notification_id) override;
 
  private:
   // Closes the notification titled |title|. Must be called on the UI thread.
@@ -90,13 +90,9 @@ class LayoutTestNotificationManager : public PlatformNotificationService {
 
   // Structure to represent the information of a persistent notification.
   struct PersistentNotification {
-    PersistentNotification();
-
-    BrowserContext* browser_context;
+    BrowserContext* browser_context = nullptr;
     GURL origin;
-    int64 service_worker_registration_id;
-    PlatformNotificationData notification_data;
-    std::string persistent_id;
+    int64_t persistent_id = 0;
   };
 
   std::map<GURL, blink::WebNotificationPermission> permission_map_;