leveldb: Add DBTracker for exposing databases to Chrome's memory-infra.

third_party/leveldatabase/env_chromium.h

Index: third_party/leveldatabase/env_chromium.h
diff --git a/third_party/leveldatabase/env_chromium.h b/third_party/leveldatabase/env_chromium.h
index d4335810b24496abaf1835a01ada393d9bdbbc43..a20b0cc767a72be719833332aa057ff7aee148bc 100644
--- a/third_party/leveldatabase/env_chromium.h
+++ b/third_party/leveldatabase/env_chromium.h
@@ -6,13 +6,18 @@
 #define THIRD_PARTY_LEVELDATABASE_ENV_CHROMIUM_H_
 
 #include <deque>
+#include <functional>
+#include <memory>
 #include <set>
 #include <string>
 #include <vector>
 
+#include "base/containers/linked_list.h"
 #include "base/files/file.h"
 #include "base/files/file_path.h"
+#include "base/macros.h"
 #include "base/metrics/histogram.h"
+#include "leveldb/db.h"
 #include "leveldb/env.h"
 #include "port/port_chromium.h"
 #include "util/mutexlock.h"
@@ -230,6 +235,57 @@ class ChromiumEnv : public leveldb::Env,
   LockTable locks_;
 };
 
+// Keeps track of live databases (opened via Open() method) and

[pwnall, 2017/06/26 23:37:14]

"Tracks database open via .... " seems shorter.

IIUC, while this class exists to expose the information to memory-infra, it
doens't do the exposing directly, so I'd remove that sentence.

[DmitrySkiba, 2017/06/28 20:35:28]

As you mention in the next commend, GetMemoryDumpName() (which I'll re-introduce
in the next CL) couples DBTracker to memory-infra. I.e. the main purpose of this
class is indeed to expose databases to memory-infra.

+// reports them to memory-infra. The class is thread safe.
+class DBTracker {
+ public:
+  // Opens a database and starts tracking it. Until the database is closed
+  // (i.e. database object deleted) it's exposed to memory-infra and is
+  // enumerated by VisitDatabases() method.
+  static leveldb::Status Open(const leveldb::Options& options,
+                              const std::string& name,
+                              leveldb::DB** dbptr);
+
+  // Returns name of memory-infra dump for |database|. Can be used to attach
+  // additional info to a database dump, or to attribute memory usage via
+  // ProcessMemoryDump::AddSuballocation().
+  static std::string GetMemoryDumpName(leveldb::DB* db);

[pwnall, 2017/06/26 23:37:14]

FWIW, this method makes DBTracker depend on memory-infra, conceptually. Without
it, DBTracker is an abstract layer that helps inspect LevelDB. I don't know how
much you value this layering, but I figured it's worth noting.

Separately, it might be worth noting if this only works correctly when used with
leveldb::DB instances produced by Open(). IIUC, that seems to be the case.
Assuming I'm correct, I think it'd also be worth adding a DCHECK verifying the
pre-condition. (the best solution I can think of is checking that the DB
instance is in the linked list)

Given the extra complexity around this method, I think it'd make more sense to
introduce it in a separate CL that adds at least one MDP user, so it's easier to
see what the best design would be.

[DmitrySkiba, 2017/06/28 20:35:28]

I'll move the method to the next CL, and we can debate there.

+
+  // DBTracker singleton instance.
+  static DBTracker* GetInstance();
+
+  // DB wrapper that provides extra information about the database.
+  class DBWrapper : public leveldb::DB {
+   public:
+    // Name that OpenDatabase() was called with.
+    virtual const std::string& name() const = 0;
+  };
+
+  using DatabaseVisitor = std::function<void(DBWrapper*)>;
+
+  // Calls |visitor| for each live database. Takes a lock, preventing
+  // any database from being created or destroyed (but doesn't lock the
+  // databases themselves). Note that visiting order is not specified.

[pwnall, 2017/06/26 23:37:14]

"The databases may be visited in an arbitrary order." to also waive the
guarantee for a consistent order?

[DmitrySkiba, 2017/06/28 20:35:28]

Done.

+  void VisitDatabases(const DatabaseVisitor& visitor);
+
+ private:
+  class DBWrapperImpl;
+  class MemoryDumpProvider;
+
+  DBTracker();
+  ~DBTracker();
+
+  void DatabaseOpened(DBWrapperImpl* database);
+  void DatabaseClosed(DBWrapperImpl* database);
+
+  std::unique_ptr<MemoryDumpProvider> mdp_;
+
+  base::Lock databases_lock_;
+  base::LinkedList<DBWrapperImpl> databases_;
+
+  DISALLOW_COPY_AND_ASSIGN(DBTracker);
+};
+
 }  // namespace leveldb_env
 
 #endif  // THIRD_PARTY_LEVELDATABASE_ENV_CHROMIUM_H_