Compressed activity log database storage

chrome/browser/extensions/activity_log/fullstream_ui_policy.cc

Index: chrome/browser/extensions/activity_log/fullstream_ui_policy.cc
diff --git a/chrome/browser/extensions/activity_log/fullstream_ui_policy.cc b/chrome/browser/extensions/activity_log/fullstream_ui_policy.cc
index 24f54947e58b82dad5f53433cb8643ad58f715ed..01a0a92804fd9e1f70ba67afce1713757faa3790 100644
--- a/chrome/browser/extensions/activity_log/fullstream_ui_policy.cc
+++ b/chrome/browser/extensions/activity_log/fullstream_ui_policy.cc
@@ -33,17 +33,6 @@ namespace constants = activity_log_constants;
 
 namespace {
 
-// Key strings for passing parameters to the ProcessAction member function.
-const char kKeyReason[] =         "fsuip.reason";
-const char kKeyDomainAction[] =   "fsuip.domact";
-const char kKeyURLTitle[] =       "fsuip.urltitle";
-const char kKeyDetailsString[] =  "fsuip.details";
-
-// Obsolete database tables: these should be dropped from the database if
-// found.
-const char* kObsoleteTables[] = {"activitylog_apis", "activitylog_blocked",
-                                 "activitylog_urls"};
-
 std::string Serialize(const base::Value* value) {
   std::string value_as_text;
   if (!value) {
@@ -68,7 +57,8 @@ const char* FullStreamUIPolicy::kTableFieldTypes[] = {
   "LONGVARCHAR NOT NULL", "INTEGER", "INTEGER", "LONGVARCHAR", "LONGVARCHAR",
   "LONGVARCHAR", "LONGVARCHAR", "LONGVARCHAR", "LONGVARCHAR"
 };
-const int FullStreamUIPolicy::kTableFieldCount = arraysize(kTableContentFields);
+const int FullStreamUIPolicy::kTableFieldCount =
+    arraysize(FullStreamUIPolicy::kTableContentFields);
 
 FullStreamUIPolicy::FullStreamUIPolicy(Profile* profile)
     : ActivityLogDatabasePolicy(
@@ -78,17 +68,8 @@ FullStreamUIPolicy::FullStreamUIPolicy(Profile* profile)
 FullStreamUIPolicy::~FullStreamUIPolicy() {}
 
 bool FullStreamUIPolicy::InitDatabase(sql::Connection* db) {
-  // Drop old database tables.
-  for (size_t i = 0; i < arraysize(kObsoleteTables); i++) {
-    const char* table_name = kObsoleteTables[i];
-    if (db->DoesTableExist(table_name)) {
-      std::string drop_statement =
-          base::StringPrintf("DROP TABLE %s", table_name);
-      if (!db->Execute(drop_statement.c_str())) {
-        return false;
-      }
-    }
-  }
+  if (!Util::DropObsoleteTables(db))
+    return false;
 
   // Create the unified activity log entry table.
   return ActivityDatabase::InitializeTable(db,
@@ -113,13 +94,6 @@ bool FullStreamUIPolicy::FlushDatabase(sql::Connection* db) {
   sql::Statement statement(db->GetCachedStatement(
       sql::StatementID(SQL_FROM_HERE), sql_str.c_str()));
 
-  url_canon::Replacements<char> url_sanitizer;
-  if (!CommandLine::ForCurrentProcess()->HasSwitch(
-          switches::kEnableExtensionActivityLogTesting)) {
-    url_sanitizer.ClearQuery();
-    url_sanitizer.ClearRef();
-  }
-
   Action::ActionVector::size_type i;
   for (i = 0; i != queued_actions_.size(); ++i) {
     const Action& action = *queued_actions_[i];
@@ -135,8 +109,7 @@ bool FullStreamUIPolicy::FlushDatabase(sql::Connection* db) {
       if (action.page_incognito()) {
         statement.BindString(5, constants::kIncognitoUrl);
       } else {
-        statement.BindString(
-            5, action.page_url().ReplaceComponents(url_sanitizer).spec());
+        statement.BindString(5, action.page_url().spec());
       }
     }
     if (!action.page_title().empty() && !action.page_incognito()) {
@@ -146,8 +119,7 @@ bool FullStreamUIPolicy::FlushDatabase(sql::Connection* db) {
       if (action.arg_incognito()) {
         statement.BindString(7, constants::kIncognitoUrl);

[felt, 2013/08/07 01:09:42]

Probably should have caught this before. Why are we doing the incognito URL for
full stream? We explicitly don't care about privacy in the full stream version.
Same question above.

[mvrable, 2013/08/07 17:01:19]

Looks like I forgot to update this code.  The intended new behavior is (unless
you think I should change it): visiting http://google.com/ in an incognito tab
should log the string "<incognito>http://google.com/" to the database.  That way
we have the full URL, but we also know that the access was to an incognito tab.

From the counting policy, we'll log simply "<incognito>" without including the
URL.

[felt, 2013/08/08 02:08:50]

Hmm. It seems like doing "<incognito>url" is overloading the field and it would
be nicer to have that info in the extras, but OTOH then we'd have "<incognito>"
in two places. So although it sits a little uncomfortably with me I do think
what you have here is the better option, but maybe we could revisit this in the
future (particularly when we start doing more complicated lookups with URL
regexes).

       } else {
-        statement.BindString(
-            7, action.arg_url().ReplaceComponents(url_sanitizer).spec());
+        statement.BindString(7, action.arg_url().spec());
       }
     }
     if (action.other()) {
@@ -170,6 +142,10 @@ bool FullStreamUIPolicy::FlushDatabase(sql::Connection* db) {
 scoped_ptr<Action::ActionVector> FullStreamUIPolicy::DoReadData(
     const std::string& extension_id,
     const int days_ago) {
+  // Ensure data is flushed to the database first so that we query over all
+  // data.
+  activity_database()->AdviseFlush(ActivityDatabase::kFlushImmediately);
+
   DCHECK_GE(days_ago, 0);
   scoped_ptr<Action::ActionVector> actions(new Action::ActionVector());
 
@@ -178,21 +154,9 @@ scoped_ptr<Action::ActionVector> FullStreamUIPolicy::DoReadData(
     return actions.Pass();
   }
 
-  // Compute the time bounds for that day.
-  base::Time morning_midnight = Now().LocalMidnight();
-  int64 early_bound = 0;
-  int64 late_bound = 0;
-  if (days_ago == 0) {
-      early_bound = morning_midnight.ToInternalValue();
-      late_bound = base::Time::Max().ToInternalValue();
-  } else {
-      base::Time early_time = morning_midnight -
-          base::TimeDelta::FromDays(days_ago);
-      base::Time late_time = morning_midnight -
-          base::TimeDelta::FromDays(days_ago-1);
-      early_bound = early_time.ToInternalValue();
-      late_bound = late_time.ToInternalValue();
-  }
+  int64 early_bound;
+  int64 late_bound;
+  Util::ComputeDatabaseTimeBounds(Now(), days_ago, &early_bound, &late_bound);
   std::string query_str = base::StringPrintf(
       "SELECT time, action_type, api_name, args, page_url, page_title, "
       "arg_url, other "
@@ -277,22 +241,6 @@ void FullStreamUIPolicy::ReadData(
       callback);
 }
 
-std::string FullStreamUIPolicy::GetKey(ActivityLogPolicy::KeyType key_ty) const

[felt, 2013/08/07 01:09:42]

where did this go?

[mvrable, 2013/08/07 17:01:19]

This code isn't being used any longer.  The constants have been moved to
activity_action_constants.cc, and there is no longer a need for a function to
look them up since they don't depend on which policy is installed.

[felt, 2013/08/08 02:08:50]

Ahhh OK, right, I couldn't find them in this CL because that had been done
earlier.

-{
-  switch (key_ty) {
-    case PARAM_KEY_REASON:
-      return std::string(kKeyReason);
-    case PARAM_KEY_DOM_ACTION:
-      return std::string(kKeyDomainAction);
-    case PARAM_KEY_URL_TITLE:
-      return std::string(kKeyURLTitle);
-    case PARAM_KEY_DETAILS_STRING:
-      return std::string(kKeyDetailsString);
-    default:
-      return std::string();
-  }
-}
-
 scoped_refptr<Action> FullStreamUIPolicy::ProcessArguments(
     scoped_refptr<Action> action) const {
   return action;
@@ -310,7 +258,7 @@ void FullStreamUIPolicy::ProcessAction(scoped_refptr<Action> action) {
 void FullStreamUIPolicy::QueueAction(scoped_refptr<Action> action) {
   if (activity_database()->is_db_valid()) {
     queued_actions_.push_back(action);
-    activity_database()->NotifyAction();
+    activity_database()->AdviseFlush(queued_actions_.size());
   }
 }