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.
+// value: Decimal string which fits into an int64_t, containing the next
+//        persistent notification ID.
 
 namespace content {
 namespace {
 
 // Keys of the fields defined in the database.
 const char kNextNotificationIdKey[] = "NEXT_NOTIFICATION_ID";
 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;
@@ skipping 11 matching lines @@
 std::string CreateDataPrefix(const GURL& origin) {
   if (!origin.is_valid())
     return kDataKeyPrefix;
 
   return base::StringPrintf("%s%s%c", kDataKeyPrefix,
                             storage::GetIdentifierFromOrigin(origin).c_str(),
                             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 39 matching lines @@
 
   leveldb::DB* db = nullptr;
   Status status = LevelDBStatusToStatus(
       leveldb::DB::Open(options, path_.AsUTF8Unsafe(), &db));
   if (status != STATUS_OK)
     return status;
 
   state_ = STATE_INITIALIZED;
   db_.reset(db);
 
-  return ReadNextNotificationId();
+  return ReadNextPersistentNotificationId();
+}
+
+int64_t NotificationDatabase::GetNextPersistentNotificationId() {
+  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_) {
+    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() {
+NotificationDatabase::Status
+NotificationDatabase::ReadNextPersistentNotificationId() {
   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);
 
   const std::string prefix = CreateDataPrefix(origin);
 
   leveldb::Slice prefix_slice(prefix);
 
   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;
 
     Status status = DeserializedNotificationData(iter->value().ToString(),
                                                  &notification_database_data);
     if (status != STATUS_OK)
       return status;

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

I'm concerned that notifications that pre-date the Chrome upgrade which
introduces this patch may remain in the database, and fail to be deserialized
here. What happens when you read a non-existent string value from a proto?

Ditto below in DeleteAllNotificationDataInternal.

It seems both cases would both be self-fixing (since the database would be
considered corrupted), but they may cause some post-upgrade notifications to be
lost as a result.

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

Why would it fail to deserialize? That'd work just fine, because that's how
protobufs work. The string would be the empty string.

This issue indeed is self-correcting. I've added two conditionals in the cases
where we *might* hit this to make sure they're not exposed anywhere until they
get removed.

 
     if (service_worker_registration_id != kInvalidServiceWorkerRegistrationId &&
         notification_database_data.service_worker_registration_id !=
             service_worker_registration_id) {
       continue;
     }
 
     notification_data_vector->push_back(notification_database_data);
   }
 
   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