Record event activity to the extension activity log.

chrome/browser/extensions/api_actions.cc

Index: chrome/browser/extensions/api_actions.cc
diff --git a/chrome/browser/extensions/api_actions.cc b/chrome/browser/extensions/api_actions.cc
index b628afa8384be40c752c9ed2723574b9ba10c4c1..45f7d35451b217bd9ce02a3bca0b9ff3a6b6820e 100644
--- a/chrome/browser/extensions/api_actions.cc
+++ b/chrome/browser/extensions/api_actions.cc
@@ -4,6 +4,7 @@
 
 #include <string>
 #include "base/logging.h"
+#include "base/stringprintf.h"
 #include "chrome/browser/extensions/api_actions.h"
 #include "content/public/browser/browser_thread.h"
 
@@ -15,6 +16,7 @@ const char* APIAction::kTableName = "activitylog_apis";
 const char* APIAction::kTableStructure = "("
     "extension_id LONGVARCHAR NOT NULL, "
     "time INTEGER NOT NULL, "
+    "api_category LONGVARCHAR NOT NULL, "
     "api_action_type LONGVARCHAR NOT NULL, "
     "target_type LONGVARCHAR NOT NULL, "
     "api_call LONGVARCHAR NOT NULL, "
@@ -22,12 +24,14 @@ const char* APIAction::kTableStructure = "("
 
 APIAction::APIAction(const std::string& extension_id,
                      const base::Time& time,
+                     const APICategory category,
                      const APIActionType verb,
                      const APITargetType target,
                      const std::string& api_call,
                      const std::string& extra)
     : extension_id_(extension_id),
       time_(time),
+      category_(category),
       verb_(verb),
       target_(target),
       api_call_(api_call),
@@ -36,18 +40,42 @@ APIAction::APIAction(const std::string& extension_id,
 APIAction::~APIAction() {
 }
 
+// static
+bool APIAction::InitializeTable(sql::Connection* db) {
+  if (!db->DoesTableExist(kTableName)) {
+    std::string table_creator = base::StringPrintf(
+        "CREATE TABLE %s %s", kTableName, kTableStructure);
+    if (!db->Execute(table_creator.c_str()))
+      return false;
+  } else if (!db->DoesColumnExist(kTableName, "api_category")) {
+    // Old versions of the table lack the api_category column.  Add it if

[Eric Dingle, 2013/01/24 16:16:34]

This seems lousy to me. What if you want to add another column in the future? Is
it not possible to check the existing db's schema with the schema above, and
just blow away the table and start from scratch if the schemas are different?
Losing the data would be bad, but at least that would let you change the schema
over time without having to do checks like this.

Then again, maybe you can provide a default value for all columns so that you
can alter the table each time you add a new column instead of checking each
column one by one.

[mvrable, 2013/01/24 18:12:31]

This is similar to some code in chrome/browser/history/visit_database.cc (in
VisitDatabase::InitVisitTable).  I'm not sure if it's easy to compare the
schema; sql::Connection seems to only provide a DoesColumnExist method, not a
general way to get the schema (to get that I'd need to extend sql::Connection
myself).

If we wanted something generic, I could change kTableStructure from a string
into something structured, and write code to create the table or add columns as
needed.  Whether that is worthwhile depends on how much we expect the schema to
be changing--if there are few changes, one-off changes like this might be
simpler, but I don't mind writing the more generic code if you think that is
preferable.

[Eric Dingle, 2013/01/24 18:56:56]

If it's already done elsewhere, then I'm okay with it. It's just hard to know
when it's okay to remove it. Too bad it's not easier to detect schema
differences and adapt accordingly.

On 2013/01/24 18:12:31, mvrable wrote:

+    // needed, with values defaulting to "CALL".
+    //
+    // TODO(mvrable): Remove this update code once we're fairly certain that
+    // everyone will have converted to the new schema.
+    std::string table_updater = base::StringPrintf(
+        "ALTER TABLE %s ADD COLUMN api_category LONGVARCHAR; "
+        "UPDATE %s SET api_category = 'CALL'",
+        kTableName, kTableName);
+    if (!db->Execute(table_updater.c_str()))
+      return false;
+  }
+  return true;
+}
+
 void APIAction::Record(sql::Connection* db) {
   std::string sql_str = "INSERT INTO " + std::string(kTableName)
-      + " (extension_id, time, api_action_type, target_type, api_call, extra)"
-      " VALUES (?,?,?,?,?,?)";
+      + " (extension_id, time, api_category, api_action_type, target_type,"
+      " api_call, extra) VALUES (?,?,?,?,?,?,?)";
   sql::Statement statement(db->GetCachedStatement(
       sql::StatementID(SQL_FROM_HERE), sql_str.c_str()));
   statement.BindString(0, extension_id_);
   statement.BindInt64(1, time_.ToInternalValue());
-  statement.BindString(2, VerbAsString());
-  statement.BindString(3, TargetAsString());
-  statement.BindString(4, api_call_);
-  statement.BindString(5, extra_);
+  statement.BindString(2, CategoryAsString());
+  statement.BindString(3, VerbAsString());
+  statement.BindString(4, TargetAsString());
+  statement.BindString(5, api_call_);
+  statement.BindString(6, extra_);
 
   if (!statement.Run())
     LOG(ERROR) << "Activity log database I/O failed: " << sql_str;
@@ -60,8 +88,20 @@ std::string APIAction::PrettyPrintFori18n() {
 
 std::string APIAction::PrettyPrintForDebug() {
   // TODO(felt): implement this for real when the UI is redesigned.
-  return "ID: " + extension_id_ + ", VERB: " + VerbAsString() +
-      ", TARGET: " + TargetAsString() + ", API: " + api_call_;
+  return "ID: " + extension_id_ + + ", CATEGORY: " + CategoryAsString() +
+      ", VERB: " + VerbAsString() + ", TARGET: " + TargetAsString() +
+      ", API: " + api_call_;
+}
+
+std::string APIAction::CategoryAsString() const {
+  switch (category_) {
+    case CALL:
+      return "CALL";
+    case EVENT_CALLBACK:
+      return "EVENT_CALLBACK";
+    default:
+      return "UNKNOWN_CATEGORY";
+  }
 }
 
 std::string APIAction::VerbAsString() const {
@@ -106,6 +146,17 @@ std::string APIAction::TargetAsString() const {
   }
 }
 
+APIAction::APICategory APIAction::StringAsCategory(
+    const std::string& str) {
+  if (str == "CALL") {
+    return CALL;
+  } else if (str == "EVENT_CALLBACK") {
+    return EVENT_CALLBACK;
+  } else {
+    return UNKNOWN_CATEGORY;
+  }
+}
+
 APIAction::APIActionType APIAction::StringAsActionType(
     const std::string& str) {
   if (str == "READ") {