Extension activity log database refactoring (step 2)

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/json/json_string_value_serializer.h"
 #include "base/logging.h"
 #include "base/memory/singleton.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/stringprintf.h"
 #include "chrome/browser/extensions/activity_log/api_actions.h"
 #include "chrome/browser/extensions/activity_log/api_name_constants.h"
+#include "chrome/browser/extensions/activity_log/fullstream_ui_policy.h"
 #include "chrome/browser/extensions/extension_tab_util.h"
 #include "chrome/browser/ui/browser.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/web_contents.h"
 #include "url/gurl.h"
 
 using content::BrowserThread;
 
 namespace {
 
@@ skipping 72 matching lines @@
       return shortname;
     else
       return it->second;
   }
 
  private:
   std::map<std::string, std::string> names_to_nums_;  // <name, number label>
   std::map<std::string, std::string> nums_to_names_;  // <number label, name>
 };
 
+std::string Serialize(const base::Value& value) {

[felt, 2013/07/18 17:09:59]

I notice there is also a new Serialize in activity_actions.cc. Is there a reason
for two separate ones? Also, the other definition has a null check that seems to
be missing in this one.

[mvrable, 2013/07/18 18:20:34]

I wanted the function in activity_actions.cc to be local to the file.  This one
is temporary and is going away in the next patch, so it wasn't worth creating a
common version.

In the case of APIAction the list is always non-null so a check wasn't needed.

+  std::string value_as_text;
+  JSONStringValueSerializer serializer(&value_as_text);
+  serializer.SerializeAndOmitBinaryValues(value);
+  return value_as_text;
+}
+
 }  // namespace
 
 namespace extensions {
 
 using api::activity_log_private::ExtensionActivity;
 using api::activity_log_private::DomActivityDetail;
 using api::activity_log_private::ChromeActivityDetail;
 using api::activity_log_private::BlockedChromeActivityDetail;
 
-const char* APIAction::kTableName = "activitylog_apis";
-const char* APIAction::kTableContentFields[] =
-    {"api_type", "api_call", "args", "extra"};
-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);
 
 // A string used in place of the real URL when the URL is hidden because it is
 // in an incognito window.  Extension activity logs mentioning kIncognitoUrl
 // let the user know that an extension is manipulating incognito tabs without
 // recording specific data about the pages.
 const char* APIAction::kIncognitoUrl = "http://incognito/";
 
 APIAction::APIAction(const std::string& extension_id,
                      const base::Time& time,
                      const Type type,
                      const std::string& api_call,
                      const std::string& args,
+                     const base::ListValue& args_list,
                      const std::string& extra)
     : Action(extension_id, time, ExtensionActivity::ACTIVITY_TYPE_CHROME),
       type_(type),
       api_call_(api_call),
       args_(args),
+      args_list_(args_list.DeepCopy()),
       extra_(extra) { }
 
-APIAction::APIAction(const sql::Statement& s)
-    : Action(s.ColumnString(0),
-             base::Time::FromInternalValue(s.ColumnInt64(1)),
-             ExtensionActivity::ACTIVITY_TYPE_CHROME),
-      type_(static_cast<Type>(s.ColumnInt(2))),
-      api_call_(APINameMap::GetInstance()->ShortnameToApi(s.ColumnString(3))),
-      args_(s.ColumnString(4)),
-      extra_(s.ColumnString(5)) { }
-
 APIAction::~APIAction() {
 }
 
 scoped_ptr<ExtensionActivity> APIAction::ConvertToExtensionActivity() {
   scoped_ptr<ExtensionActivity> formatted_activity;
   formatted_activity.reset(new ExtensionActivity);
   formatted_activity->extension_id.reset(
       new std::string(extension_id()));
   formatted_activity->activity_type = activity_type();
   formatted_activity->time.reset(new double(time().ToJsTime()));
   ChromeActivityDetail* details = new ChromeActivityDetail;
   details->api_activity_type = ChromeActivityDetail::ParseApiActivityType(
       TypeAsString());
   details->api_call.reset(new std::string(api_call_));
   details->args.reset(new std::string(args_));
   details->extra.reset(new std::string(extra_));
   formatted_activity->chrome_activity_detail.reset(details);
   return formatted_activity.Pass();
 }
 
-// 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));
-}
-
 bool APIAction::Record(sql::Connection* db) {
-  std::string sql_str = "INSERT INTO " + std::string(kTableName)
-      + " (extension_id, time, api_type, api_call, args, extra) VALUES"
-      " (?,?,?,?,?,?)";
+  std::string sql_str =
+      "INSERT INTO " + std::string(FullStreamUIPolicy::kTableName) +
+      " (extension_id, time, action_type, api_name, args) 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.BindInt(2, static_cast<int>(type_));
-  statement.BindString(3, APINameMap::GetInstance()->ApiToShortname(api_call_));
-  statement.BindString(4, args_);
-  statement.BindString(5, extra_);
+  switch (type_) {
+    case CALL:
+      statement.BindInt(2, static_cast<int>(Action::ACTION_API_CALL));
+      break;
+    case EVENT_CALLBACK:
+      statement.BindInt(2, static_cast<int>(Action::ACTION_API_EVENT));
+      break;
+    default:
+      LOG(ERROR) << "Invalid action type: " << type_;
+      return false;
+  }
+  statement.BindString(3, api_call_);
+  statement.BindString(4, Serialize(*args_list_));
   if (!statement.Run()) {
     LOG(ERROR) << "Activity log database I/O failed: " << sql_str;
     statement.Clear();
     return false;
   }
   return true;
 }
 
 // static
 void APIAction::LookupTabId(const std::string& api_call,
@@ skipping 48 matching lines @@
       return "call";
     case EVENT_CALLBACK:
       return "event_callback";
     default:
       return "unknown_type";
   }
 }
 
 }  // namespace extensions