Replaced enum strings with ints in Activity Log database

chrome/browser/extensions/activity_log/api_actions.cc

 // Copyright (c) 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/logging.h"
 #include "base/stringprintf.h"
 #include "chrome/browser/extensions/activity_log/api_actions.h"
 #include "chrome/browser/extensions/extension_tab_util.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/web_contents.h"
@@ skipping 20 matching lines @@
     return std::string();
   }
 }
 
 }  // namespace
 
 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.
 const char* APIAction::kAlwaysLog[] =
     {"extension.connect", "extension.sendMessage",
      "tabs.executeScript", "tabs.insertCSS" };
 const int APIAction::kSizeAlwaysLog = arraysize(kAlwaysLog);
 
 APIAction::APIAction(const std::string& extension_id,
                      const base::Time& time,
                      const Type type,
                      const std::string& api_call,
                      const std::string& args,
                      const std::string& extra)
     : Action(extension_id, time),
       type_(type),
       api_call_(api_call),
       args_(args),
       extra_(extra) { }
 
 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)) { }
 
 APIAction::~APIAction() {
 }
 
 // static
 bool APIAction::InitializeTable(sql::Connection* db) {
   // The original table schema was different than the existing one.
   // We no longer want the api_action_type or target_type columns.
   // Sqlite doesn't let you delete or modify existing columns, so we drop it.
   // Any data loss incurred here doesn't matter since these fields existed
   // before we started using the AL for anything.
   if (db->DoesColumnExist(kTableName, "api_action_type")) {
     std::string drop_table = base::StringPrintf("DROP TABLE %s", kTableName);
     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));
 }
 
 void APIAction::Record(sql::Connection* db) {
   std::string sql_str = "INSERT INTO " + std::string(kTableName)
       + " (extension_id, time, api_type, api_call, args, 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, 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_);
   if (!statement.Run())
     LOG(ERROR) << "Activity log database I/O failed: " << sql_str;
 }
 
 // static
 void APIAction::LookupTabId(const std::string& api_call,
                             ListValue* args,
@@ skipping 45 matching lines @@
   switch (type_) {
     case CALL:
       return "CALL";
     case EVENT_CALLBACK:
       return "EVENT_CALLBACK";
     default:
       return "UNKNOWN_TYPE";
   }
 }
 
-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