Make //content responsible for generating notification Ids

content/browser/notifications/notification_database.cc

Index: content/browser/notifications/notification_database.cc
diff --git a/content/browser/notifications/notification_database.cc b/content/browser/notifications/notification_database.cc
index e7ac09b3f0ac78d7acb13e237c4a2ed43616317a..62344dd6f807a56f1973e7d12170bd1833afbcf8 100644
--- a/content/browser/notifications/notification_database.cc
+++ b/content/browser/notifications/notification_database.cc
@@ -43,7 +43,7 @@ const char kDataKeyPrefix[] = "DATA:";
 const char kKeySeparator = '\x00';
 
 // The first notification id which to be handed out by the database.
-const int64_t kFirstNotificationId = 1;
+const int64_t kFirstPersistentNotificationId = 1;
 
 // Converts the LevelDB |status| to one of the notification database's values.
 NotificationDatabase::Status LevelDBStatusToStatus(
@@ -75,9 +75,12 @@ std::string CreateDataPrefix(const GURL& origin) {
 }
 
 // Creates the compound data key in which notification data is stored.
-std::string CreateDataKey(const GURL& origin, int64_t notification_id) {
+std::string CreateDataKey(const GURL& origin,
+                          const std::string& notification_id) {
   DCHECK(origin.is_valid());
-  return CreateDataPrefix(origin) + base::Int64ToString(notification_id);
+  DCHECK(!notification_id.empty());
+
+  return CreateDataPrefix(origin) + notification_id;
 }
 
 // Deserializes data in |serialized_data| to |notification_database_data|.
@@ -140,13 +143,17 @@ NotificationDatabase::Status NotificationDatabase::Open(
   return ReadNextNotificationId();
 }
 
+int64_t NotificationDatabase::GetNextNotificationId() {
+  return next_persistent_notification_id_++;
+}
+
 NotificationDatabase::Status NotificationDatabase::ReadNotificationData(
-    int64_t notification_id,
+    const std::string& notification_id,
     const GURL& origin,
     NotificationDatabaseData* notification_database_data) const {
   DCHECK(sequence_checker_.CalledOnValidSequence());
   DCHECK_EQ(STATE_INITIALIZED, state_);
-  DCHECK_GE(notification_id, kFirstNotificationId);
+  DCHECK(!notification_id.empty());
   DCHECK(origin.is_valid());
   DCHECK(notification_database_data);
 
@@ -188,45 +195,39 @@ NotificationDatabase::ReadAllNotificationDataForServiceWorkerRegistration(
 
 NotificationDatabase::Status NotificationDatabase::WriteNotificationData(
     const GURL& origin,
-    const NotificationDatabaseData& notification_database_data,
-    int64_t* notification_id) {
+    const NotificationDatabaseData& notification_data) {
   DCHECK(sequence_checker_.CalledOnValidSequence());
   DCHECK_EQ(STATE_INITIALIZED, state_);
-  DCHECK(notification_id);
   DCHECK(origin.is_valid());
 
-  DCHECK_GE(next_notification_id_, kFirstNotificationId);
-
-  NotificationDatabaseData storage_data = notification_database_data;
-  storage_data.notification_id = next_notification_id_;
+  const std::string& notification_id = notification_data.notification_id;
+  DCHECK(!notification_id.empty());
 
   std::string serialized_data;
-  if (!SerializeNotificationDatabaseData(storage_data, &serialized_data)) {
+  if (!SerializeNotificationDatabaseData(notification_data, &serialized_data)) {
     DLOG(ERROR) << "Unable to serialize data for a notification belonging "
                 << "to: " << origin;
     return STATUS_ERROR_FAILED;
   }
 
   leveldb::WriteBatch batch;
-  batch.Put(CreateDataKey(origin, next_notification_id_), serialized_data);
-  batch.Put(kNextNotificationIdKey,
-            base::Int64ToString(next_notification_id_ + 1));
+  batch.Put(CreateDataKey(origin, notification_id), serialized_data);
 
-  Status status =
-      LevelDBStatusToStatus(db_->Write(leveldb::WriteOptions(), &batch));
-  if (status != STATUS_OK)
-    return status;
+  if (written_persistent_notification_id_ != next_persistent_notification_id_) {

[johnme, 2016/09/07 17:13:13]

I like that if you create two and successfully write only the second one, it
increases the stored next id by 2. If you create two notifications and only
successfully write only the first one, we'll still increase the stored next id
by 2, but nevermind.

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

That's fine. We have space for another 9,223,372,036,854,775,807 notifications
:-).

+    written_persistent_notification_id_ = next_persistent_notification_id_;
+    batch.Put(kNextNotificationIdKey,
+              base::Int64ToString(next_persistent_notification_id_));
+  }
 
-  *notification_id = next_notification_id_++;
-  return STATUS_OK;
+  return LevelDBStatusToStatus(db_->Write(leveldb::WriteOptions(), &batch));
 }
 
 NotificationDatabase::Status NotificationDatabase::DeleteNotificationData(
-    int64_t notification_id,
+    const std::string& notification_id,
     const GURL& origin) {
   DCHECK(sequence_checker_.CalledOnValidSequence());
   DCHECK_EQ(STATE_INITIALIZED, state_);
-  DCHECK_GE(notification_id, kFirstNotificationId);
+  DCHECK(!notification_id.empty());
   DCHECK(origin.is_valid());
 
   std::string key = CreateDataKey(origin, notification_id);
@@ -237,20 +238,20 @@ NotificationDatabase::Status
 NotificationDatabase::DeleteAllNotificationDataForOrigin(
     const GURL& origin,
     const std::string& tag,
-    std::set<int64_t>* deleted_notification_set) {
+    std::set<std::string>* deleted_notification_ids) {
   return DeleteAllNotificationDataInternal(origin, tag,
                                            kInvalidServiceWorkerRegistrationId,
-                                           deleted_notification_set);
+                                           deleted_notification_ids);
 }
 
 NotificationDatabase::Status
 NotificationDatabase::DeleteAllNotificationDataForServiceWorkerRegistration(
     const GURL& origin,
     int64_t service_worker_registration_id,
-    std::set<int64_t>* deleted_notification_set) {
+    std::set<std::string>* deleted_notification_ids) {
   return DeleteAllNotificationDataInternal(origin, "" /* tag */,
                                            service_worker_registration_id,
-                                           deleted_notification_set);
+                                           deleted_notification_ids);
 }
 
 NotificationDatabase::Status NotificationDatabase::Destroy() {
@@ -277,18 +278,21 @@ NotificationDatabase::Status NotificationDatabase::ReadNextNotificationId() {
       db_->Get(leveldb::ReadOptions(), kNextNotificationIdKey, &value));
 
   if (status == STATUS_ERROR_NOT_FOUND) {
-    next_notification_id_ = kFirstNotificationId;
+    next_persistent_notification_id_ = kFirstPersistentNotificationId;
+    written_persistent_notification_id_ = kFirstPersistentNotificationId;
     return STATUS_OK;
   }
 
   if (status != STATUS_OK)
     return status;
 
-  if (!base::StringToInt64(value, &next_notification_id_) ||
-      next_notification_id_ < kFirstNotificationId) {
+  if (!base::StringToInt64(value, &next_persistent_notification_id_) ||
+      next_persistent_notification_id_ < kFirstPersistentNotificationId) {
     return STATUS_ERROR_CORRUPTED;
   }
 
+  written_persistent_notification_id_ = next_persistent_notification_id_;
+
   return STATUS_OK;
 }
 
@@ -333,15 +337,12 @@ NotificationDatabase::DeleteAllNotificationDataInternal(
     const GURL& origin,
     const std::string& tag,
     int64_t service_worker_registration_id,
-    std::set<int64_t>* deleted_notification_set) {
+    std::set<std::string>* deleted_notification_ids) {
   DCHECK(sequence_checker_.CalledOnValidSequence());
-  DCHECK(deleted_notification_set);
+  DCHECK(deleted_notification_ids);
   DCHECK(origin.is_valid());
 
   const std::string prefix = CreateDataPrefix(origin);
-  const bool should_deserialize =
-      service_worker_registration_id != kInvalidServiceWorkerRegistrationId ||
-      !tag.empty();
 
   leveldb::Slice prefix_slice(prefix);
   leveldb::WriteBatch batch;
@@ -353,39 +354,28 @@ NotificationDatabase::DeleteAllNotificationDataInternal(
     if (!iter->key().starts_with(prefix_slice))
       break;
 
-    if (should_deserialize) {
-      Status status = DeserializedNotificationData(iter->value().ToString(),
-                                                   &notification_database_data);
-      if (status != STATUS_OK)
-        return status;
-
-      if (!tag.empty() &&
-          notification_database_data.notification_data.tag != tag) {
-        continue;
-      }
-
-      if (service_worker_registration_id !=
-              kInvalidServiceWorkerRegistrationId &&
-          notification_database_data.service_worker_registration_id !=
-              service_worker_registration_id) {
-        continue;
-      }
-    }
+    Status status = DeserializedNotificationData(iter->value().ToString(),
+                                                 &notification_database_data);
+    if (status != STATUS_OK)
+      return status;
 
-    leveldb::Slice notification_id_slice = iter->key();
-    notification_id_slice.remove_prefix(prefix_slice.size());
+    if (!tag.empty() &&
+        notification_database_data.notification_data.tag != tag) {
+      continue;
+    }
 
-    int64_t notification_id = 0;
-    if (!base::StringToInt64(notification_id_slice.ToString(),
-                             &notification_id)) {
-      return STATUS_ERROR_CORRUPTED;
+    if (service_worker_registration_id != kInvalidServiceWorkerRegistrationId &&
+        notification_database_data.service_worker_registration_id !=
+            service_worker_registration_id) {
+      continue;
     }
 
-    deleted_notification_set->insert(notification_id);
     batch.Delete(iter->key());
+    deleted_notification_ids->insert(
+        notification_database_data.notification_id);
   }
 
-  if (deleted_notification_set->empty())
+  if (deleted_notification_ids->empty())
     return STATUS_OK;
 
   return LevelDBStatusToStatus(db_->Write(leveldb::WriteOptions(), &batch));