Introduce the NotificationDatabase class.

content/browser/notifications/notification_database.h

Index: content/browser/notifications/notification_database.h
diff --git a/content/browser/notifications/notification_database.h b/content/browser/notifications/notification_database.h
new file mode 100644
index 0000000000000000000000000000000000000000..3a1aed5d57f80814d505b8bf0989373d2be76bf8
--- /dev/null
+++ b/content/browser/notifications/notification_database.h
@@ -0,0 +1,72 @@
+// 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 "base/callback.h"
+#include "base/files/file_path.h"
+#include "base/sequence_checker.h"
+#include "content/common/content_export.h"
+
+namespace leveldb {
+class DB;
+class Env;
+}
+
+namespace content {
+
+// Implementation of the persistent notification database.
+//
+// This class must be constructed on the IO thread. All other methods must be

[Bernhard Bauer, 2015/03/11 17:15:22]

There doesn't seem to anything that requires the construction thread to be the
IO thread. You could just say that this class can be constructed on any thread.

[Peter Beverloo, 2015/03/11 18:01:26]

Done.

+// called on a sequenced task runner because they may cause file I/O, which

[Bernhard Bauer, 2015/03/11 17:15:22]

Nit: I would phrase it as "All other methods must be called on a thread or
sequenced task runner that allows I/O" (there are default threads that can do
I/O, and sequenced task runners that can't). And we should also express somehow
that it has to be the same sequenced task runner / thread... 

[Peter Beverloo, 2015/03/11 18:01:26]

Done.

+// should not be done on any of the default threads.
+class CONTENT_EXPORT NotificationDatabase {
+  using InitCallback = base::Callback<void(bool /* success */)>;

[Bernhard Bauer, 2015/03/11 17:15:22]

Is this used somewhere? And in any case, public: should come first in the class.

[Peter Beverloo, 2015/03/11 18:01:26]

Removed.

+
+ public:
+  explicit NotificationDatabase(const base::FilePath& path);
+  ~NotificationDatabase();
+
+  enum Status {

[Bernhard Bauer, 2015/03/11 17:15:22]

Enum declarations should come before methods.

[Peter Beverloo, 2015/03/11 18:01:26]

Done.

+    STATUS_OK,
+    STATUS_ERROR_NOT_FOUND,
+    STATUS_ERROR_FAILED

[Bernhard Bauer, 2015/03/11 17:15:22]

Can you explain what these mean? Also, you might want to add a trailing comma,
to reduce edit churn when adding more values later.

[Peter Beverloo, 2015/03/11 18:01:26]

Done.

+  };
+
+  // Opens the database located in |path|. If |path| does not contain a valid
+  // path, the database will be opened in memory instead. |create_if_missing|

[Bernhard Bauer, 2015/03/11 17:15:22]

"If path is non-empty, [...]"?

[Peter Beverloo, 2015/03/11 18:01:26]

Done, rephrased this comment.

+  // indicates whether to create the database if it does not exist.
+  Status Open(bool create_if_missing);
+
+  // Completely destroys the contents of this database.
+  Status Destroy();
+
+ private:
+  friend class NotificationDatabaseTest;
+
+  // 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(); }
+
+  base::FilePath path_;
+  scoped_ptr<leveldb::Env> env_;
+  scoped_ptr<leveldb::DB> db_;
+
+  enum State {

[Bernhard Bauer, 2015/03/11 17:15:22]

Move the enum to the beginning of the private section (after the friend
declaration). You could also make this an enum class, then you could
forward-declare it.

[Peter Beverloo, 2015/03/11 18:01:26]

It seems like we can't use enum class values in DCHECK_EQ without casting:

logging.h:503:23: error: invalid operands to binary expression
('basic_ostream<char, std::char_traits<char> >' and
'content::NotificationDatabase::State')
  ss << names << " (" << v1 << " vs. " << v2 << ")";

I've kept the enum in the header for now, with a TODO to update this when we do
support this. A bug already existed.

+    STATE_UNINITIALIZED,
+    STATE_INITIALIZED,
+    STATE_DISABLED

[Bernhard Bauer, 2015/03/11 17:15:22]

Also add a trailing comma here.

[Peter Beverloo, 2015/03/11 18:01:26]

Done.

+  };
+
+  State state_;
+
+  base::SequenceChecker sequence_checker_;

[Bernhard Bauer, 2015/03/11 17:15:22]

DISALLOW_COPY_AND_ASSIGN?

[Peter Beverloo, 2015/03/11 18:01:26]

Oops. Done.

+};
+
+}  // namespace content
+
+#endif  // CONTENT_BROWSER_NOTIFICATIONS_NOTIFICATION_DATABASE_H_