Add functions to clean URLs from the activity log

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 72e66bb0ba7f3cbf8809f4c478810cca65d0f96c..a2b24042e86ae88718f2481fbcd8b9c5a51c997d 100644
--- a/chrome/browser/extensions/activity_log/counting_policy.cc
+++ b/chrome/browser/extensions/activity_log/counting_policy.cc
@@ -567,6 +567,74 @@ scoped_ptr<Action::ActionVector> CountingPolicy::DoReadData(
   return actions.Pass();
 }
 
+void CountingPolicy::DoRemoveURLs(const std::vector<GURL>& restrict_urls) {
+  sql::Connection* db = GetDatabaseConnection();
+  if (!db) {
+    LOG(ERROR) << "Unable to connect to database";
+    return;
+  }
+
+  // Flush data first so the URL clearing affects queued-up data as well.
+  activity_database()->AdviseFlush(ActivityDatabase::kFlushImmediately);
+
+  // If no restrictions then then all URLs need to be removed.
+  if (restrict_urls.empty()) {
+    std::string sql_str = base::StringPrintf(
+      "UPDATE %s SET page_url_x=NULL,page_title_x=NULL,arg_url_x=NULL",
+      kTableName);
+
+    sql::Statement statement;
+    statement.Assign(db->GetCachedStatement(
+        sql::StatementID(SQL_FROM_HERE), sql_str.c_str()));
+
+    if (!statement.Run()) {
+      LOG(ERROR) << "Removing all URLs from database failed: "
+                 << statement.GetSQLStatement();

[felt, 2013/08/27 19:59:03]

I think you need to implement this method via the Delegate (like how the
flushing is done) so that the ActivityDatabase can handle failures for you.
That's why the other database-failure-sensitive methods return true/false --
false triggers error handling by the AD class.

[mvrable, 2013/08/27 21:16:34]

There are currently two styles--flushing goes via the Delegate, but reading is
done directly and does not return a status code.  Flushing is special because it
can be triggered asynchronously by a timer in ActivityDatabase, so it needs to
be part of the Delegate interface.

I've looked a bit at the code, and I think the database is still notified of
failures via DatabaseErrorCallback (which should cause SoftFailureClose to be
called).  We don't want to continue executing after an error, but perhaps just
returning early from DoRemoveURLs will be enough.

That is--any time there is an error, rather than just a LOG(ERROR) also include
a return.  Other cleanup code should still run as part of the
DatabaseErrorCallback, without more work needed here.

Adrienne, how does this seem?

[karenlees, 2013/08/27 22:28:55]

Done.

+    }
+  }
+
+  // If URLs are specified then restrict to only those URLs.
+  for (size_t i = 0; i < restrict_urls.size(); ++i) {
+    int64 url_id;
+    if (!restrict_urls[i].is_valid() ||
+        !url_table_.StringToInt(db, restrict_urls[i].spec(), &url_id)) {
+      continue;
+    }
+
+    // Remove any that match the page_url.
+    std::string sql_str = base::StringPrintf(
+      "UPDATE %s SET page_url_x=NULL,page_title_x=NULL WHERE page_url_x IS ?",
+      kTableName);
+
+    sql::Statement statement;
+    statement.Assign(db->GetCachedStatement(
+        sql::StatementID(SQL_FROM_HERE), sql_str.c_str()));
+    statement.BindInt64(0, url_id);
+
+    if (!statement.Run()) {
+      LOG(ERROR) << "Removing page URL from database failed: "
+                 << statement.GetSQLStatement();
+    }
+
+    // Remove any that match the arg_url.
+    sql_str = base::StringPrintf(
+      "UPDATE %s SET arg_url_x=NULL WHERE arg_url_x IS ?", kTableName);
+
+    statement.Assign(db->GetCachedStatement(
+        sql::StatementID(SQL_FROM_HERE), sql_str.c_str()));
+    statement.BindInt64(0, url_id);
+
+    if (!statement.Run()) {
+      LOG(ERROR) << "Removing arg URL from database failed: "
+                 << statement.GetSQLStatement();
+    }
+  }
+
+  // Clean up unused strings from the strings and urls table to really delete
+  // the urls and page titles.
+  CleanStringTables(db);
+}
+
 void CountingPolicy::ReadData(
     const std::string& extension_id,
     const int day,
@@ -602,6 +670,10 @@ void CountingPolicy::ReadFilteredData(
       callback);
 }
 
+void CountingPolicy::RemoveURLs(const std::vector<GURL>& restrict_urls) {
+  ScheduleAndForget(this, &CountingPolicy::DoRemoveURLs, restrict_urls);
+}
+
 void CountingPolicy::OnDatabaseFailure() {
   queued_actions_.clear();
 }