Store the actual notification data in the notification database.

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 4d8b4f9ca9a94a00a49455b33b75c8c0d4298872..c4f3c4bb3937f71eebbc2b5916328f00ccf4535d 100644
--- a/content/browser/notifications/notification_database.cc
+++ b/content/browser/notifications/notification_database.cc
@@ -8,24 +8,37 @@
 
 #include "base/files/file_util.h"
 #include "base/strings/string_number_conversions.h"
+#include "base/strings/stringprintf.h"
+#include "content/browser/notifications/notification_database_data.h"
 #include "content/public/browser/browser_thread.h"
+#include "storage/common/database/database_identifier.h"
 #include "third_party/leveldatabase/src/helpers/memenv/memenv.h"
 #include "third_party/leveldatabase/src/include/leveldb/db.h"
 #include "third_party/leveldatabase/src/include/leveldb/env.h"
 #include "third_party/leveldatabase/src/include/leveldb/write_batch.h"
+#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.
 //
+//
 
 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;
@@ -43,6 +56,15 @@ NotificationDatabase::Status LevelDBStatusToStatus(
   return NotificationDatabase::STATUS_ERROR_FAILED;
 }
 
+// Creates the compound data key in which notification data is stored.
+std::string CreateDataKey(int64_t notification_id, const GURL& origin) {
+  return base::StringPrintf("%s%s%c%s",
+                            kDataKeyPrefix,
+                            storage::GetIdentifierFromOrigin(origin).c_str(),
+                            kKeySeparator,
+                            base::Int64ToString(notification_id).c_str());
+}
+
 }  // namespace
 
 NotificationDatabase::NotificationDatabase(const base::FilePath& path)
@@ -88,32 +110,79 @@ NotificationDatabase::Status NotificationDatabase::Open(
   return STATUS_OK;
 }
 
-NotificationDatabase::Status NotificationDatabase::GetNextNotificationId(
-    int64_t* notification_id) const {
+NotificationDatabase::Status NotificationDatabase::ReadNotificationData(
+    int64_t notification_id,
+    const GURL& origin,
+    NotificationDatabaseData* notification_database_data) const {
   DCHECK(sequence_checker_.CalledOnValidSequencedThread());
-  DCHECK_EQ(state_, STATE_INITIALIZED);
-  DCHECK(notification_id);
+  DCHECK_EQ(STATE_INITIALIZED, state_);
+  DCHECK_GE(notification_id, kFirstNotificationId);
+  DCHECK(origin.is_valid());
+  DCHECK(notification_database_data);
+
+  std::string key = CreateDataKey(notification_id, origin);
+  std::string serialized_data;
 
-  std::string value;
   Status status = LevelDBStatusToStatus(
-      db_->Get(leveldb::ReadOptions(), kNextNotificationIdKey, &value));
+      db_->Get(leveldb::ReadOptions(), key, &serialized_data));
+  if (status != STATUS_OK)
+    return status;
 
-  if (status == STATUS_ERROR_NOT_FOUND) {
-    *notification_id = kFirstNotificationId;
+  if (notification_database_data->ParseFromString(serialized_data))
     return STATUS_OK;
+
+  DLOG(ERROR) << "Unable to deserialize data for notification "
+              << notification_id << " belonging to " << origin << ".";
+  return STATUS_ERROR_CORRUPTED;
+}
+
+NotificationDatabase::Status NotificationDatabase::WriteNotificationData(
+    const GURL& origin,
+    const NotificationDatabaseData& notification_database_data,
+    int64_t* notification_id) {
+  DCHECK(sequence_checker_.CalledOnValidSequencedThread());
+  DCHECK_EQ(STATE_INITIALIZED, state_);
+  DCHECK(notification_id);
+  DCHECK(origin.is_valid());
+
+  std::string serialized_data;
+  if (!notification_database_data.SerializeToString(&serialized_data)) {
+    DLOG(ERROR) << "Unable to serialize data for notification "
+                << notification_id << " belonging to " << origin << ".";
+    return STATUS_ERROR_FAILED;
   }
 
+  leveldb::WriteBatch batch;
+  Status status = GetAndIncrementNextNotificationId(&batch, notification_id);

[cmumford, 2015/03/13 21:03:40]

What about just getting the next notification ID once, when the db is opened,
and keeping in a member variable. And then when WriteNotificationData is called,
you'd increment and write it out in the batch. Basically what you do now, but
w/o a db.get on every write?

[Peter Beverloo, 2015/03/16 11:56:45]

Awesome, even better :) Thanks for the suggestion - done!

   if (status != STATUS_OK)
     return status;
 
-  int64_t next_notification_id;
-  if (!base::StringToInt64(value, &next_notification_id) ||
-      next_notification_id < kFirstNotificationId) {
-    return STATUS_ERROR_CORRUPTED;
-  }
+  batch.Put(CreateDataKey(*notification_id, origin), serialized_data);
 
-  *notification_id = next_notification_id;
-  return STATUS_OK;
+  return LevelDBStatusToStatus(db_->Write(leveldb::WriteOptions(), &batch));
+}
+
+NotificationDatabase::Status NotificationDatabase::DeleteNotificationData(
+    int64_t notification_id,
+    const GURL& origin) {
+  DCHECK(sequence_checker_.CalledOnValidSequencedThread());
+  DCHECK_EQ(STATE_INITIALIZED, state_);
+  DCHECK_GE(notification_id, kFirstNotificationId);
+  DCHECK(origin.is_valid());
+
+  std::string key = CreateDataKey(notification_id, origin);
+
+  // TODO(peter): Instead of validating the existence of the data by reading the
+  // notification data, the notification list for |origin| should be considered
+  // instead. That list hasn't been implemented yet, however.
+
+  std::string value;
+  Status status = LevelDBStatusToStatus(
+      db_->Get(leveldb::ReadOptions(), key, &value));
+  if (status != STATUS_OK)
+    return status;
+
+  return LevelDBStatusToStatus(db_->Delete(leveldb::WriteOptions(), key));
 }
 
 NotificationDatabase::Status NotificationDatabase::Destroy() {
@@ -134,13 +203,36 @@ NotificationDatabase::Status NotificationDatabase::Destroy() {
       leveldb::DestroyDB(path_.AsUTF8Unsafe(), options));
 }
 
-void NotificationDatabase::WriteNextNotificationId(
+NotificationDatabase::Status
+NotificationDatabase::GetAndIncrementNextNotificationId(
     leveldb::WriteBatch* batch,
-    int64_t next_notification_id) const {
-  DCHECK_GE(next_notification_id, kFirstNotificationId);
+    int64_t* notification_id) const {
   DCHECK(batch);
+  DCHECK(notification_id);
+
+  std::string value;
+  Status status = LevelDBStatusToStatus(
+      db_->Get(leveldb::ReadOptions(), kNextNotificationIdKey, &value));
+
+  if (status == STATUS_ERROR_NOT_FOUND) {
+    *notification_id = kFirstNotificationId;
+
+  } else if (status == STATUS_OK) {
+    int64_t next_notification_id;
+    if (!base::StringToInt64(value, &next_notification_id) ||
+        next_notification_id < kFirstNotificationId) {
+      return STATUS_ERROR_CORRUPTED;
+    }
+
+    *notification_id = next_notification_id;
+  } else {
+    return status;
+  }
+
+  batch->Put(kNextNotificationIdKey,
+             base::Int64ToString(1 + *notification_id));
 
-  batch->Put(kNextNotificationIdKey, base::Int64ToString(next_notification_id));
+  return STATUS_OK;
 }
 
 }  // namespace content