Extension activity log database refactoring (step 1)

chrome/browser/extensions/activity_log/activity_database.h

Index: chrome/browser/extensions/activity_log/activity_database.h
diff --git a/chrome/browser/extensions/activity_log/activity_database.h b/chrome/browser/extensions/activity_log/activity_database.h
index 8c3c42fae1bd993390c0f22c5accb03dbc11afb3..609eeb0f7d5f9a709bec873250b973933d21ea92 100644
--- a/chrome/browser/extensions/activity_log/activity_database.h
+++ b/chrome/browser/extensions/activity_log/activity_database.h
@@ -27,6 +27,29 @@ class FilePath;
 
 namespace extensions {
 
+// Interface defining callbacks that the ActivityDatabase can make into a
+// higher-level ActivityLogPolicy instance.  Callbacks are always invoked on
+// the database thread.  Classes other than ActivityDatabase should not call
+// these methods.
+class ActivityDatabaseCallbacks {

[Matt Perry, 2013/07/10 23:19:30]

I would not refer to this class or its methods as "callbacks". It's an
interface. Chrome commonly refers to a class like this as a Delegate or Client.
I'd say ActivityDatabaseDelegate is appropriate. Likewise, the methods would be
OnDatabaseInit, etc.

Also, it's common to make this an inner class of ActivityDatabase, named simply
Delegate. See e.g.
https://code.google.com/p/chromium/codesearch#chromium/src/net/url_request/ur...

[mvrable, 2013/07/11 17:30:33]

I've tried renaming this to be an inner class, ActivityDatabase::Delegate. 
Adrienne, does this look okay to you as well?

+ protected:
+  friend class ActivityDatabase;
+
+  // Initializes the database schema; this gives a policy a change to create or

[Matt Perry, 2013/07/10 23:19:30]

chance*

[mvrable, 2013/07/11 17:30:33]

Done.

+  // update database tables as needed.  Should return true on success.
+  virtual bool DatabaseInitCallback(sql::Connection* db) = 0;
+
+  // Called by ActivityDatabase just before the ActivityDatabase object is
+  // deleted.  The database will make no further callbacks after invoking this
+  // method, so it is an appropriate time for the policy to delete itself.
+  virtual void DatabaseCloseCallback() = 0;
+
+  // An ActivityDatabaseCallbacks instance should not be deleted directly; it
+  // is instead deleted by the ActivityDatabase which calls the Cleanup()
+  // method.
+  virtual ~ActivityDatabaseCallbacks() {}
+};
+
 // Encapsulates the SQL connection for the activity log database.
 // This class holds the database connection and has methods for writing.
 // All of the methods except constructor need to be
@@ -35,9 +58,10 @@ namespace extensions {
 class ActivityDatabase {
  public:
   // Need to call Init to actually use the ActivityDatabase.
-  ActivityDatabase();
+  explicit ActivityDatabase(ActivityDatabaseCallbacks* callbacks);
 
-  // Opens the DB and creates tables as necessary.
+  // Opens the DB.  Calls DatabaseInitCallback in the policy callbacks, which
+  // is responsible for creating or updating the database schema if needed.
   void Init(const base::FilePath& db_name);
 
   // The ActivityLog should call this to kill the ActivityDatabase.
@@ -100,6 +124,7 @@ class ActivityDatabase {
   // For unit testing only.
   void RecordBatchedActionsWhileTesting();
 
+  ActivityDatabaseCallbacks* policy_callbacks_;
   base::Clock* testing_clock_;
   sql::Connection db_;
   bool valid_db_;