Activity Log: allow searching by day

Activity Log: allow searching by day

This allows the Watchdog app to look up history by day. It also gets rid of a redundant ReadData method (now wholly replaced by ReadFilteredData).

BUG=241672
R=mpcomplete@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=221831

[felt, 2013-09-05 01:19:08]

Hi Matt, can you please review this CL?

[Matt Perry, 2013-09-05 20:18:35]

lgtm

https://codereview.chromium.org/23980002/diff/1/chrome/common/extensions/api/...
File chrome/common/extensions/api/activity_log_private.json (right):

https://codereview.chromium.org/23980002/diff/1/chrome/common/extensions/api/...
chrome/common/extensions/api/activity_log_private.json:37: "daysAgo": {"type":
"integer", "optional": true, "description": "Exact match; today is 0"}
Does this fetch the entries added exactly <daysAgo>, or those added *since*
<daysAgo>? The description and comments in the code seem to indicate the former,
but you might want to make that clearer.

[felt, 2013-09-05 20:51:32]

On 2013/09/05 20:18:35, Matt Perry wrote:
> lgtm
> 
>
https://codereview.chromium.org/23980002/diff/1/chrome/common/extensions/api/...
> File chrome/common/extensions/api/activity_log_private.json (right):
> 
>
https://codereview.chromium.org/23980002/diff/1/chrome/common/extensions/api/...
> chrome/common/extensions/api/activity_log_private.json:37: "daysAgo": {"type":
> "integer", "optional": true, "description": "Exact match; today is 0"}
> Does this fetch the entries added exactly <daysAgo>, or those added *since*
> <daysAgo>? The description and comments in the code seem to indicate the
former,
> but you might want to make that clearer.

Thanks, updated the comment.

[commit-bot: I haz the power, 2013-09-06 01:38:04]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/felt@chromium.org/23980002/21001

[commit-bot: I haz the power, 2013-09-06 02:13:10]

Sorry for I got bad news for ya.
Compile failed with a clobber build on linux_rel.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-09-06 17:32:12]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/felt@chromium.org/23980002/42001

[commit-bot: I haz the power, 2013-09-06 17:32:21]

Failed to apply patch for
chrome/browser/extensions/activity_log/counting_policy.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file chrome/browser/extensions/activity_log/counting_policy.cc
  Hunk #2 FAILED at 409.
  Hunk #3 succeeded at 430 (offset 1 line).
  Hunk #4 succeeded at 474 (offset 1 line).
  Hunk #5 succeeded at 591 with fuzz 1 (offset 46 lines).
  Hunk #6 succeeded at 609 (offset 46 lines).
  1 out of 6 hunks FAILED -- saving rejects to file
chrome/browser/extensions/activity_log/counting_policy.cc.rej

Patch:       chrome/browser/extensions/activity_log/counting_policy.cc
Index: chrome/browser/extensions/activity_log/counting_policy.cc
diff --git a/chrome/browser/extensions/activity_log/counting_policy.cc
b/chrome/browser/extensions/activity_log/counting_policy.cc
index
b854be56126ea4cc4a07e1d1d9056d9710d00cd9..d1ba1626f8d8c4fb67941a3a500ec00d556a0d7b
100644
--- a/chrome/browser/extensions/activity_log/counting_policy.cc
+++ b/chrome/browser/extensions/activity_log/counting_policy.cc
@@ -379,7 +379,8 @@ scoped_ptr<Action::ActionVector>
CountingPolicy::DoReadFilteredData(
     const Action::ActionType type,
     const std::string& api_name,
     const std::string& page_url,
-    const std::string& arg_url) {
+    const std::string& arg_url,
+    const int days_ago) {
   // Ensure data is flushed to the database first so that we query over all
   // data.
   activity_database()->AdviseFlush(ActivityDatabase::kFlushImmediately);
@@ -408,12 +409,17 @@ scoped_ptr<Action::ActionVector>
CountingPolicy::DoReadFilteredData(
     where_str += where_next + "page_url LIKE ?";
     where_next = " AND ";
   }
-  if (!arg_url.empty())
+  if (!arg_url.empty()) {
     where_str += where_next + "arg_url LIKE ?";
+    where_next = " AND ";
+  }
+  if (days_ago >= 0)
+    where_str += where_next + "time BETWEEN ? AND ?";
+
   std::string query_str = base::StringPrintf(
       "SELECT extension_id,time, action_type, api_name, args, page_url,"
-      "page_title, arg_url, other, count FROM %s WHERE %s ORDER BY count DESC "
-      "LIMIT 300",
+      "page_title, arg_url, other, count FROM %s WHERE %s ORDER BY count DESC,"
+      " time DESC LIMIT 300",
       kReadViewName,
       where_str.c_str());
   sql::Statement query(db->GetUniqueStatement(query_str.c_str()));
@@ -428,6 +434,13 @@ scoped_ptr<Action::ActionVector>
CountingPolicy::DoReadFilteredData(
     query.BindString(++i, page_url + "%");
   if (!arg_url.empty())
     query.BindString(++i, arg_url + "%");
+  if (days_ago >= 0) {
+    int64 early_bound;
+    int64 late_bound;
+    Util::ComputeDatabaseTimeBounds(Now(), days_ago, &early_bound,
&late_bound);
+    query.BindInt64(++i, early_bound);
+    query.BindInt64(++i, late_bound);
+  }
 
   // Execute the query and get results.
   while (query.is_valid() && query.Step()) {
@@ -465,79 +478,6 @@ scoped_ptr<Action::ActionVector>
CountingPolicy::DoReadFilteredData(
   return actions.Pass();
 }
 
-scoped_ptr<Action::ActionVector> CountingPolicy::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());
-
-  sql::Connection* db = GetDatabaseConnection();
-  if (!db) {
-    return actions.Pass();
-  }
-
-  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, count "
-      "FROM %s WHERE extension_id=? AND time>? AND time<=? "
-      "ORDER BY time DESC",
-      kReadViewName);
-  sql::Statement query(db->GetCachedStatement(SQL_FROM_HERE,
-                                              query_str.c_str()));
-  query.BindString(0, extension_id);
-  query.BindInt64(1, early_bound);
-  query.BindInt64(2, late_bound);
-
-  while (query.is_valid() && query.Step()) {
-    scoped_refptr<Action> action =
-        new Action(extension_id,
-                   base::Time::FromInternalValue(query.ColumnInt64(0)),
-                   static_cast<Action::ActionType>(query.ColumnInt(1)),
-                   query.ColumnString(2));
-
-    if (query.ColumnType(3) != sql::COLUMN_TYPE_NULL) {
-      scoped_ptr<Value> parsed_value(
-          base::JSONReader::Read(query.ColumnString(3)));
-      if (parsed_value && parsed_value->IsType(Value::TYPE_LIST)) {
-        action->set_args(
-            make_scoped_ptr(static_cast<ListValue*>(parsed_value.release())));
-      } else {
-        LOG(WARNING) << "Unable to parse args: '" << query.ColumnString(3)
-                     << "'";
-      }
-    }
-
-    action->ParsePageUrl(query.ColumnString(4));
-    action->set_page_title(query.ColumnString(5));
-    action->ParseArgUrl(query.ColumnString(6));
-
-    if (query.ColumnType(7) != sql::COLUMN_TYPE_NULL) {
-      scoped_ptr<Value> parsed_value(
-          base::JSONReader::Read(query.ColumnString(7)));
-      if (parsed_value && parsed_value->IsType(Value::TYPE_DICTIONARY)) {
-        action->set_other(make_scoped_ptr(
-            static_cast<DictionaryValue*>(parsed_value.release())));
-      } else {
-        LOG(WARNING) << "Unable to parse other: '" << query.ColumnString(7)
-                     << "'";
-      }
-    }
-
-    action->set_count(query.ColumnInt(8));
-
-    actions->push_back(action);
-  }
-
-  return actions.Pass();
-}
-
 void CountingPolicy::DoRemoveURLs(const std::vector<GURL>& restrict_urls) {
   sql::Connection* db = GetDatabaseConnection();
   if (!db) {
@@ -610,26 +550,13 @@ void CountingPolicy::DoRemoveURLs(const std::vector<GURL>&
restrict_urls) {
   CleanStringTables(db);
 }
 
-void CountingPolicy::ReadData(
-    const std::string& extension_id,
-    const int day,
-    const base::Callback<void(scoped_ptr<Action::ActionVector>)>& callback) {
-  BrowserThread::PostTaskAndReplyWithResult(
-      BrowserThread::DB,
-      FROM_HERE,
-      base::Bind(&CountingPolicy::DoReadData,
-                 base::Unretained(this),
-                 extension_id,
-                 day),
-      callback);
-}
-
 void CountingPolicy::ReadFilteredData(
     const std::string& extension_id,
     const Action::ActionType type,
     const std::string& api_name,
     const std::string& page_url,
     const std::string& arg_url,
+    const int days_ago,
     const base::Callback
         <void(scoped_ptr<Action::ActionVector>)>& callback) {
   BrowserThread::PostTaskAndReplyWithResult(
@@ -641,7 +568,8 @@ void CountingPolicy::ReadFilteredData(
                  type,
                  api_name,
                  page_url,
-                 arg_url),
+                 arg_url,
+                 days_ago),
       callback);
 }

[commit-bot: I haz the power, 2013-09-06 17:51:42]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/felt@chromium.org/23980002/46001

[commit-bot: I haz the power, 2013-09-06 18:19:00]

Sorry for I got bad news for ya.
Compile failed with a clobber build on android_clang_dbg.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_cl...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-09-06 20:02:18]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/felt@chromium.org/23980002/63001

[commit-bot: I haz the power, 2013-09-07 00:21:58]

Change committed as 221831