Save extension activity log to a file.

chrome/browser/extensions/activity_database.h

+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// User of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_EXTENSIONS_ACTIVITY_DATABASE_H_
+#define CHROME_BROWSER_EXTENSIONS_ACTIVITY_DATABASE_H_
+
+#include "base/basictypes.h"
+#include "base/file_path.h"
+#include "base/memory/ref_counted_memory.h"
+#include "chrome/browser/extensions/blocked_actions.h"
+#include "chrome/browser/extensions/manager_actions.h"
+#include "chrome/browser/extensions/url_actions.h"
+#include "sql/connection.h"
+#include "sql/init_status.h"
+
+class FilePath;
+
+namespace extensions {
+
+// Encapsulates the SQL connection for the activity log database.
+// This class holds the database connection and has methods for writing.
+class ActivityDatabase : public base::RefCountedThreadSafe<ActivityDatabase> {
+ public:
+  // Need to call Init to actually use the ActivityDatabase.
+  ActivityDatabase();
+
+  // Opens the DB and creates tables as necessary.
+  sql::InitStatus Init(const FilePath& db_name,
+                       sql::ErrorDelegate* error_delegate);
+
+  // Record a UrlAction in the database.
+  void RecordUrlAction(scoped_refptr<UrlAction> action);

[Matt Perry, 2013/01/03 20:53:35]

It seems to me this would be better handled by the Action subclasses. i.e. you
have a single ActivityDatabase::RecordAction(Action* action) method which calls
action->RecordAction(sql_connection);

Then the database structure is self contained in each action type's file. The
ActivityDatabase doesn't need to know about it.

[felt, 2013/01/07 23:44:22]

I agree that it would be neater to have a single
ActivityDatabase::RecordAction(Action), but I'm not sure how to do it well.  A
few options I thought about:

1) I don't want to move database logic out of ActivityDatabase and into the
action classes, since the db connection needs to be managed in a single place.
2) I can't overload ActivityDatabase::RecordAction with the 3 different types
because the templating in ScheduleAndForget fails if the method is overloaded.
3) I could have a single ActivityDatabase::RecordAction that checks what
subclass Action it's dealing with in some way -- e.g., by having a constant that
differs in value between the subclasses.  This seems like bad practice, but is
probably the best option I can think of.

I'm new to C++ so my style sense isn't the best.  Would I be best off with
option #3, is there a better way, or should I leave it as-is?

[Matt Perry, 2013/01/08 00:02:01]

I don't understand the reason you give here. Why can't ActivityAction have a
virtual method RecordAction which takes a pointer to the db_? Then just move the
code from RecordUrlAction to UrlAction::RecordAction, etc.

The way it is now, the database logic for each database is spread across 2
files. The table format is in foo_action.cc, and the table writing logic is in
this file.

[felt, 2013/01/08 06:38:51]

Done.

+
+  // Record a ManagerAction in the database.
+  void RecordManagerAction(scoped_refptr<ManagerAction> action);
+
+  // Record a BlockedAction in the database.
+  void RecordBlockedAction(scoped_refptr<BlockedAction> action);
+
+  void KillDatabase();
+
+  bool initialized() const { return initialized_; }
+
+  // Standard db operation wrappers.
+  void BeginTransaction();
+  void CommitTransaction();
+  void RollbackTransaction();
+  bool Raze();
+  void Close();
+
+ private:
+  friend class base::RefCountedThreadSafe<ActivityDatabase>;
+
+  virtual ~ActivityDatabase();
+
+  sql::InitStatus InitializeTable(const char* table_name,
+                                  const char* table_structure);
+
+  sql::Connection db_;
+  bool initialized_;
+
+  DISALLOW_COPY_AND_ASSIGN(ActivityDatabase);
+};
+
+}  // namespace extensions
+#endif  // CHROME_BROWSER_EXTENSIONS_ACTIVITY_DATABASE_H_
+