Make //content responsible for generating notification Ids

content/browser/notifications/notification_database.cc

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/notifications/notification_database.h"
 
 #include <string>
 
 #include "base/files/file_util.h"
 #include "base/strings/string_number_conversions.h"
@@ skipping 12 matching lines @@
 #include "url/gurl.h"
 
 // Notification LevelDB database schema (in alphabetized order)
 // =======================
 //
 // key: "DATA:" <origin identifier> '\x00' <notification_id>
 // value: String containing the NotificationDatabaseDataProto protocol buffer
 //        in serialized form.
 //
 // key: "NEXT_NOTIFICATION_ID"
 // value: Decimal string which fits into an int64_t.

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

"Decimal string that fits into an int64_t, containing the next persistent
notification id."

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

Done.

 
 namespace content {
 namespace {
 
 // Keys of the fields defined in the database.
 const char kNextNotificationIdKey[] = "NEXT_NOTIFICATION_ID";

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

kNextPersistentNotificationIdKey?

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

The name ideally should match the key. We shouldn't rename the key, so let's
keep as-is.

 const char kDataKeyPrefix[] = "DATA:";
 
 // Separates the components of compound keys.
 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(
     const leveldb::Status& status) {
   if (status.ok())
     return NotificationDatabase::STATUS_OK;
   else if (status.IsNotFound())
     return NotificationDatabase::STATUS_ERROR_NOT_FOUND;
   else if (status.IsCorruption())
     return NotificationDatabase::STATUS_ERROR_CORRUPTED;
   else if (status.IsIOError())
     return NotificationDatabase::STATUS_IO_ERROR;
   else if (status.IsNotSupportedError())
     return NotificationDatabase::STATUS_NOT_SUPPORTED;
   else if (status.IsInvalidArgument())
     return NotificationDatabase::STATUS_INVALID_ARGUMENT;
 
   return NotificationDatabase::STATUS_ERROR_FAILED;
 }
 
 // Creates a prefix for the data entries based on |origin|.
 std::string CreateDataPrefix(const GURL& origin) {
   if (!origin.is_valid())
     return kDataKeyPrefix;
 
   return base::StringPrintf("%s%s%c", kDataKeyPrefix,
                             storage::GetIdentifierFromOrigin(origin).c_str(),

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

Drive-by: DatabaseIdentifier::CreateFromOrigin has some quirks, for example it
returns DatabaseIdentifier::UniqueFileIdentifier() for file:// URLs. Though I
guess we don't currently support persistent notifications on file:// URLs, since
we don't support SWs there?

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

Yes.

                             kKeySeparator);
 }
 
 // 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|.
 // Will return if the deserialization was successful.
 NotificationDatabase::Status DeserializedNotificationData(
     const std::string& serialized_data,
     NotificationDatabaseData* notification_database_data) {
   DCHECK(notification_database_data);
   if (DeserializeNotificationDatabaseData(serialized_data,
                                           notification_database_data)) {
@@ skipping 42 matching lines @@
       leveldb::DB::Open(options, path_.AsUTF8Unsafe(), &db));
   if (status != STATUS_OK)
     return status;
 
   state_ = STATE_INITIALIZED;
   db_.reset(db);
 
   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);
 
   std::string key = CreateDataKey(origin, notification_id);
   std::string serialized_data;
 
   Status status = LevelDBStatusToStatus(
       db_->Get(leveldb::ReadOptions(), key, &serialized_data));
   if (status != STATUS_OK)
     return status;
@@ skipping 21 matching lines @@
 NotificationDatabase::ReadAllNotificationDataForServiceWorkerRegistration(
     const GURL& origin,
     int64_t service_worker_registration_id,
     std::vector<NotificationDatabaseData>* notification_data_vector) const {
   return ReadAllNotificationDataInternal(origin, service_worker_registration_id,
                                          notification_data_vector);
 }
 
 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);
   return LevelDBStatusToStatus(db_->Delete(leveldb::WriteOptions(), key));
 }
 
 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() {
   DCHECK(sequence_checker_.CalledOnValidSequence());
 
   leveldb::Options options;
   if (IsInMemoryDatabase()) {
     if (!env_)
       return STATUS_OK;  // The database has not been initialized.
 
     options.env = env_.get();
   }
 
   state_ = STATE_DISABLED;
   db_.reset();
 
   return LevelDBStatusToStatus(
       leveldb::DestroyDB(path_.AsUTF8Unsafe(), options));
 }
 
 NotificationDatabase::Status NotificationDatabase::ReadNextNotificationId() {
   std::string value;
   Status status = LevelDBStatusToStatus(
       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;
 }
 
 NotificationDatabase::Status
 NotificationDatabase::ReadAllNotificationDataInternal(
     const GURL& origin,
     int64_t service_worker_registration_id,
     std::vector<NotificationDatabaseData>* notification_data_vector) const {
   DCHECK(sequence_checker_.CalledOnValidSequence());
   DCHECK(notification_data_vector);
@@ skipping 24 matching lines @@
   }
 
   return LevelDBStatusToStatus(iter->status());
 }
 
 NotificationDatabase::Status
 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;
 
   NotificationDatabaseData notification_database_data;
   std::unique_ptr<leveldb::Iterator> iter(
       db_->NewIterator(leveldb::ReadOptions()));
   for (iter->Seek(prefix_slice); iter->Valid(); iter->Next()) {
     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;
+    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;
-      }
+    if (!tag.empty() &&
+        notification_database_data.notification_data.tag != tag) {
+      continue;
     }
 
-    leveldb::Slice notification_id_slice = iter->key();
-    notification_id_slice.remove_prefix(prefix_slice.size());
-
-    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));
 }
 
 }  // namespace content