Make //content responsible for generating notification Ids

content/browser/notifications/notification_database.h

 // 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.
 
 #ifndef CONTENT_BROWSER_NOTIFICATIONS_NOTIFICATION_DATABASE_H_
 #define CONTENT_BROWSER_NOTIFICATIONS_NOTIFICATION_DATABASE_H_
 
 #include <stdint.h>
 
 #include <memory>
@@ skipping 60 matching lines @@
 
   explicit NotificationDatabase(const base::FilePath& path);
   ~NotificationDatabase();
 
   // Opens the database. If |path| is non-empty, it will be created on the given
   // directory on the filesystem. If |path| is empty, the database will be
   // created in memory instead, and its lifetime will be tied to this instance.
   // |create_if_missing| determines whether to create the database if necessary.
   Status Open(bool create_if_missing);
 
+  // Gets the next assignable notification ID. Subsequent calls to this method

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

persistent notification ID

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

Done.

+  // will yield unique identifiers for the same database. The last used ID will
+  // be written to the database when a notification is created.
+  int64_t GetNextNotificationId();

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

GetNextPersistentNotificationId

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

Done.

+
   // Reads the notification data for the notification identified by
   // |notification_id| and belonging to |origin| from the database, and stores
-  // it in |notification_database_data|. Returns the status code.
+  // it in |*notification_database_data|. Returns the status code.
   Status ReadNotificationData(
-      int64_t notification_id,
+      const std::string& notification_id,
       const GURL& origin,
       NotificationDatabaseData* notification_database_data) const;

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

Nit: notification_data since you renamed it in WriteNotificationData (ditto in
comment above)

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

Done.

 
   // Reads all notification data for all origins from the database, and appends
   // the data to |notification_data_vector|. Returns the status code.
   Status ReadAllNotificationData(
       std::vector<NotificationDatabaseData>* notification_data_vector) const;
 
   // Reads all notification data associated with |origin| from the database, and
   // appends the data to |notification_data_vector|. Returns the status code.
   Status ReadAllNotificationDataForOrigin(
       const GURL& origin,
       std::vector<NotificationDatabaseData>* notification_data_vector) const;
 
   // Reads all notification data associated to |service_worker_registration_id|
   // belonging to |origin| from the database, and appends the data to the
   // |notification_data_vector|. Returns the status code.
   Status ReadAllNotificationDataForServiceWorkerRegistration(
       const GURL& origin,
       int64_t service_worker_registration_id,
       std::vector<NotificationDatabaseData>* notification_data_vector) const;
 
-  // Writes the |notification_database_data| for a new notification belonging to
-  // |origin| to the database, and returns the status code of the writing
-  // operation. The id of the new notification will be set in |notification_id|.
+  // Writes the |notification_data| for a new notification belonging to |origin|
+  // to the database, and returns the status code of the writing operation. The
+  // notification's ID must have been set in the |notification_data|.
   Status WriteNotificationData(
       const GURL& origin,
-      const NotificationDatabaseData& notification_database_data,
-      int64_t* notification_id);
+      const NotificationDatabaseData& notification_data);
 
   // Deletes all data associated with the notification identified by
   // |notification_id| belonging to |origin| from the database. Returns the
   // status code of the deletion operation. Note that it is not considered a
   // failure if the to-be-deleted notification does not exist.
-  Status DeleteNotificationData(int64_t notification_id, const GURL& origin);
+  Status DeleteNotificationData(const std::string& notification_id,
+                                const GURL& origin);
 
   // Deletes all data associated with |origin| from the database, optionally
   // filtered by the |tag|, and appends the deleted notification ids to
-  // |deleted_notification_set|. Returns the status code of the deletion
+  // |deleted_notification_ids|. Returns the status code of the deletion
   // operation.
   Status DeleteAllNotificationDataForOrigin(
       const GURL& origin,
       const std::string& tag,
-      std::set<int64_t>* deleted_notification_set);
+      std::set<std::string>* deleted_notification_ids);
 
   // Deletes all data associated with the |service_worker_registration_id|
   // belonging to |origin| from the database, and appends the deleted
   // notification ids to |deleted_notification_set|. Returns the status code
   // of the deletion operation.
   Status DeleteAllNotificationDataForServiceWorkerRegistration(
       const GURL& origin,
       int64_t service_worker_registration_id,
-      std::set<int64_t>* deleted_notification_set);
+      std::set<std::string>* deleted_notification_ids);
 
   // Completely destroys the contents of this database.
   Status Destroy();
 
  private:
   friend class NotificationDatabaseTest;
 
   // TODO(peter): Convert to an enum class when DCHECK_EQ supports this.
   // See https://crbug.com/463869.
   enum State {
     STATE_UNINITIALIZED,
     STATE_INITIALIZED,
     STATE_DISABLED,
   };
 
   // Reads the next available notification id from the database and returns

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

persistent notification id

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

Done.

   // the status code of the reading operation. The value will be stored in
   // the |next_notification_id_| member.

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

next_persistent_notification_id

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

Done.

   Status ReadNextNotificationId();

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

ReadNextPersistentNotificationId

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

Done.

 
   // Reads all notification data with the given constraints. |origin| may be
   // empty to read all notification data from all origins. If |origin| is
   // set, but |service_worker_registration_id| is invalid, then all notification
   // data for |origin| will be read. If both are set, then all notification data
   // for the given |service_worker_registration_id| will be read.
   Status ReadAllNotificationDataInternal(
       const GURL& origin,
       int64_t service_worker_registration_id,
       std::vector<NotificationDatabaseData>* notification_data_vector) const;
 
   // Deletes all notification data with the given constraints. |origin| must
   // always be set - use Destroy() when the goal is to empty the database. If
   // |service_worker_registration_id| is invalid, all notification data for the
   // |origin| will be deleted, optionally filtered by the |tag| when non-empty.
-  // All deleted notification ids will be written to |deleted_notification_set|.
+  // All deleted notification ids will be written to |deleted_notification_ids|.
   Status 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);
 
   // Returns whether the database has been opened.
   bool IsOpen() const { return db_ != nullptr; }
 
   // Returns whether the database should only exist in memory.
   bool IsInMemoryDatabase() const { return path_.empty(); }
 
   // Exposes the LevelDB database used to back this notification database.
   // Should only be used for testing purposes.
   leveldb::DB* GetDBForTesting() const { return db_.get(); }
 
   base::FilePath path_;
 
-  int64_t next_notification_id_ = 0;
+  int64_t next_persistent_notification_id_ = 0;
+  int64_t written_persistent_notification_id_ = 0;
 
   std::unique_ptr<const leveldb::FilterPolicy> filter_policy_;
 
   // The declaration order for these members matters, as |db_| depends on |env_|
   // and thus has to be destructed first.
   std::unique_ptr<leveldb::Env> env_;
   std::unique_ptr<leveldb::DB> db_;
 
   State state_ = STATE_UNINITIALIZED;
 
   base::SequenceChecker sequence_checker_;
 
   DISALLOW_COPY_AND_ASSIGN(NotificationDatabase);
 };
 
 }  // namespace content
 
 #endif  // CONTENT_BROWSER_NOTIFICATIONS_NOTIFICATION_DATABASE_H_