Replaced enum strings with ints in Activity Log database

chrome/browser/extensions/activity_log/api_actions.cc

Index: chrome/browser/extensions/activity_log/api_actions.cc
diff --git a/chrome/browser/extensions/activity_log/api_actions.cc b/chrome/browser/extensions/activity_log/api_actions.cc
index c894f38e4f034c65c66e1ac66c2500227ef048c3..88e966ac515e2a418427ed444b64946f5e7fb572 100644
--- a/chrome/browser/extensions/activity_log/api_actions.cc
+++ b/chrome/browser/extensions/activity_log/api_actions.cc
@@ -39,6 +39,8 @@ namespace extensions {
 const char* APIAction::kTableName = "activitylog_apis";
 const char* APIAction::kTableContentFields[] =
     {"api_type", "api_call", "args", "extra"};

[Matt Perry, 2013/05/17 19:08:55]

You could probably store the api_call as an integer too. We have a large enum of
them in extension_function_histogram_value.h.

[felt, 2013/05/18 00:27:18]

This is a good idea. I added code that does this. However the histogram was
missing events and a number of apps APIs so I added my own constants list.

On 2013/05/17 19:08:55, Matt Perry wrote:

+const char* APIAction::kTableFieldTypes[] =
+    {"INTEGER", "LONGVARCHAR", "LONGVARCHAR", "LONGVARCHAR"};
 
 // We should log the arguments to these API calls, even if argument logging is
 // disabled by default.
@@ -62,7 +64,7 @@ APIAction::APIAction(const std::string& extension_id,
 APIAction::APIAction(const sql::Statement& s)
     : Action(s.ColumnString(0),
           base::Time::FromInternalValue(s.ColumnInt64(1))),
-      type_(StringAsType(s.ColumnString(2))),
+      type_((Type)s.ColumnInt(2)),

[Matt Perry, 2013/05/17 19:08:55]

style guide forbids c-style casts. use static_cast<Type>()

[felt, 2013/05/18 00:27:18]

whoops, I didn't remember that. Done.

On 2013/05/17 19:08:55, Matt Perry wrote:

       api_call_(s.ColumnString(3)),
       args_(s.ColumnString(4)),
       extra_(s.ColumnString(5)) { }
@@ -82,10 +84,22 @@ bool APIAction::InitializeTable(sql::Connection* db) {
     if (!db->Execute(drop_table.c_str()))
       return false;
   }
+  // We also now use INTEGER instead of VARCHAR for api_type.
+  if (db->DoesColumnExist(kTableName, "api_type")) {
+    std::string select = base::StringPrintf(
+        "SELECT api_type FROM %s ORDER BY rowid LIMIT 1", kTableName);
+    sql::Statement statement(db->GetUniqueStatement(select.c_str()));
+    if (statement.DeclaredColumnType(0) != sql::COLUMN_TYPE_INTEGER) {
+      std::string drop_table = base::StringPrintf("DROP TABLE %s", kTableName);
+      if (!db->Execute(drop_table.c_str()))
+        return false;
+    }
+  }
   // Now initialize the table.
   return InitializeTableInternal(db,
                                  kTableName,
                                  kTableContentFields,
+                                 kTableFieldTypes,
                                  arraysize(kTableContentFields));
 }
 
@@ -97,7 +111,7 @@ void APIAction::Record(sql::Connection* db) {
       sql::StatementID(SQL_FROM_HERE), sql_str.c_str()));
   statement.BindString(0, extension_id());
   statement.BindInt64(1, time().ToInternalValue());
-  statement.BindString(2, TypeAsString());
+  statement.BindInt(2, (int)type_);

[Matt Perry, 2013/05/17 19:08:55]

cast style

[felt, 2013/05/18 00:27:18]

Done.

   statement.BindString(3, api_call_);
   statement.BindString(4, args_);
   statement.BindString(5, extra_);
@@ -163,16 +177,5 @@ std::string APIAction::TypeAsString() const {
   }
 }
 
-APIAction::Type APIAction::StringAsType(
-    const std::string& str) {
-  if (str == "CALL") {
-    return CALL;
-  } else if (str == "EVENT_CALLBACK") {
-    return EVENT_CALLBACK;
-  } else {
-    return UNKNOWN_TYPE;
-  }
-}
-
 }  // namespace extensions