Make //content responsible for generating notification Ids

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 6a777cc7949e9b693f55d1ca9dea9babf8332a27..8b2260284e1df1bb809bbc7e7740646028b4b1e5 100644
--- a/content/browser/notifications/platform_notification_context_impl.cc
+++ b/content/browser/notifications/platform_notification_context_impl.cc
@@ -21,19 +21,23 @@
 using base::DoNothing;
 
 namespace content {
+namespace {
 
 // Name of the directory in the user's profile directory where the notification
 // database files should be stored.
 const base::FilePath::CharType kPlatformNotificationsDirectory[] =
     FILE_PATH_LITERAL("Platform Notifications");
 
+}  // namespace
+
 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) {
+      service_worker_context_(service_worker_context),
+      notification_id_generator_(browser_context) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 }
 
@@ -137,10 +141,11 @@ void PlatformNotificationContextImpl::RemoveService(
 }
 
 void PlatformNotificationContextImpl::ReadNotificationData(
-    int64_t notification_id,
+    const std::string& notification_id,
     const GURL& origin,
     const ReadResultCallback& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
+
   LazyInitialize(
       base::Bind(&PlatformNotificationContextImpl::DoReadNotificationData, this,
                  notification_id, origin, callback),
@@ -148,7 +153,7 @@ void PlatformNotificationContextImpl::ReadNotificationData(
 }
 
 void PlatformNotificationContextImpl::DoReadNotificationData(
-    int64_t notification_id,
+    const std::string& notification_id,
     const GURL& origin,
     const ReadResultCallback& callback) {
   DCHECK(task_runner_->RunsTasksOnCurrentThread());
@@ -230,7 +235,7 @@ void PlatformNotificationContextImpl::WriteNotificationData(
   LazyInitialize(
       base::Bind(&PlatformNotificationContextImpl::DoWriteNotificationData,
                  this, origin, database_data, callback),
-      base::Bind(callback, false /* success */, 0 /* notification_id */));
+      base::Bind(callback, false /* success */, "" /* notification_id */));
 }
 
 void PlatformNotificationContextImpl::DoWriteNotificationData(
@@ -241,11 +246,11 @@ void PlatformNotificationContextImpl::DoWriteNotificationData(
 
   // Eagerly delete data for replaced notifications from the database.
   if (!database_data.notification_data.tag.empty()) {
-    std::set<int64_t> deleted_notification_set;
+    std::set<std::string> deleted_notification_ids;
     NotificationDatabase::Status delete_status =
         database_->DeleteAllNotificationDataForOrigin(
             origin, database_data.notification_data.tag,
-            &deleted_notification_set);
+            &deleted_notification_ids);
 
     UMA_HISTOGRAM_ENUMERATION("Notifications.Database.DeleteBeforeWriteResult",
                               delete_status,
@@ -259,23 +264,29 @@ void PlatformNotificationContextImpl::DoWriteNotificationData(
 
       BrowserThread::PostTask(
           BrowserThread::IO, FROM_HERE,
-          base::Bind(callback, false /* success */, 0 /* notification_id */));
+          base::Bind(callback, false /* success */, "" /* notification_id */));
       return;
     }
   }
 
-  int64_t notification_id = 0;
+  // Create a copy of the |database_data| to store a generated notification ID.
+  NotificationDatabaseData write_database_data = database_data;
+  write_database_data.notification_id =
+      notification_id_generator_.GenerateForPersistentNotification(
+          origin, database_data.notification_data.tag,
+          database_->GetNextNotificationId());
+
   NotificationDatabase::Status status =
-      database_->WriteNotificationData(origin, database_data, &notification_id);
+      database_->WriteNotificationData(origin, write_database_data);
 
   UMA_HISTOGRAM_ENUMERATION("Notifications.Database.WriteResult", status,
                             NotificationDatabase::STATUS_COUNT);
 
   if (status == NotificationDatabase::STATUS_OK) {
-    DCHECK_GT(notification_id, 0);
-    BrowserThread::PostTask(
-        BrowserThread::IO, FROM_HERE,
-        base::Bind(callback, true /* success */, notification_id));
+    BrowserThread::PostTask(BrowserThread::IO, FROM_HERE,
+                            base::Bind(callback, true /* success */,
+                                       write_database_data.notification_id));
+
     return;
   }
 
@@ -285,14 +296,15 @@ void PlatformNotificationContextImpl::DoWriteNotificationData(
 
   BrowserThread::PostTask(
       BrowserThread::IO, FROM_HERE,
-      base::Bind(callback, false /* success */, 0 /* notification_id */));
+      base::Bind(callback, false /* success */, "" /* notification_id */));
 }
 
 void PlatformNotificationContextImpl::DeleteNotificationData(
-    int64_t notification_id,
+    const std::string& notification_id,
     const GURL& origin,
     const DeleteResultCallback& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
+
   LazyInitialize(
       base::Bind(&PlatformNotificationContextImpl::DoDeleteNotificationData,
                  this, notification_id, origin, callback),
@@ -300,7 +312,7 @@ void PlatformNotificationContextImpl::DeleteNotificationData(
 }
 
 void PlatformNotificationContextImpl::DoDeleteNotificationData(
-    int64_t notification_id,
+    const std::string& notification_id,
     const GURL& origin,
     const DeleteResultCallback& callback) {
   DCHECK(task_runner_->RunsTasksOnCurrentThread());
@@ -342,10 +354,10 @@ void PlatformNotificationContextImpl::
         int64_t service_worker_registration_id) {
   DCHECK(task_runner_->RunsTasksOnCurrentThread());
 
-  std::set<int64_t> deleted_notifications_set;
+  std::set<std::string> deleted_notification_ids;
   NotificationDatabase::Status status =
       database_->DeleteAllNotificationDataForServiceWorkerRegistration(
-          origin, service_worker_registration_id, &deleted_notifications_set);
+          origin, service_worker_registration_id, &deleted_notification_ids);
 
   UMA_HISTOGRAM_ENUMERATION(
       "Notifications.Database.DeleteServiceWorkerRegistrationResult", status,
@@ -355,7 +367,7 @@ void PlatformNotificationContextImpl::
   if (status == NotificationDatabase::STATUS_ERROR_CORRUPTED)
     DestroyDatabase();
 
-  // TODO(peter): Close the notifications in |deleted_notifications_set|. See
+  // TODO(peter): Close the notifications in |deleted_notification_ids|. See

[johnme, 2016/09/08 15:21:50]

Can this TODO be fixed with the new ID scheme?

[Peter Beverloo, 2016/09/08 18:56:02]

Yes.

   // https://crbug.com/532436.
 }