Support origin-based deletion for password store statistics

components/password_manager/core/browser/statistics_table.cc

 // Copyright 2015 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 "components/password_manager/core/browser/statistics_table.h"
 
 #include <stdint.h>
 
 #include <algorithm>
 #include <limits>
+#include <set>
 
 #include "base/memory/ptr_util.h"
 #include "sql/connection.h"
 #include "sql/statement.h"
 
 namespace password_manager {
 namespace {
 
 // Convenience enum for interacting with SQL queries that use all the columns.
 enum LoginTableColumns {
@@ skipping 98 matching lines @@
     result.push_back(base::WrapUnique(new InteractionsStats));
     result.back()->origin_domain = GURL(s.ColumnString(COLUMN_ORIGIN_DOMAIN));
     result.back()->username_value = s.ColumnString16(COLUMN_USERNAME);
     result.back()->dismissal_count = s.ColumnInt(COLUMN_DISMISSALS);
     result.back()->update_time =
         base::Time::FromInternalValue(s.ColumnInt64(COLUMN_DATE));
   }
   return result;
 }
 
-bool StatisticsTable::RemoveStatsBetween(base::Time delete_begin,
-                                         base::Time delete_end) {
-  sql::Statement s(db_->GetCachedStatement(
-      SQL_FROM_HERE,
-      "DELETE FROM stats WHERE update_time >= ? AND update_time < ?"));
-  s.BindInt64(0, delete_begin.ToInternalValue());
-  s.BindInt64(1, delete_end.is_null() ? std::numeric_limits<int64_t>::max()
-                                      : delete_end.ToInternalValue());
-  return s.Run();
+bool StatisticsTable::RemoveStatsByOriginAndTime(
+    const base::Callback<bool(const GURL&)>& origin_filter,
+    base::Time delete_begin,
+    base::Time delete_end) {
+  if (delete_end.is_null())
+    delete_end = base::Time::Max();
+
+  // All origins.
+  if (origin_filter.is_null()) {
+    sql::Statement delete_statement(db_->GetCachedStatement(
+        SQL_FROM_HERE,
+        "DELETE FROM stats WHERE update_time >= ? AND update_time < ?"));
+    delete_statement.BindInt64(0, delete_begin.ToInternalValue());
+    delete_statement.BindInt64(1, delete_end.ToInternalValue());
+
+    return delete_statement.Run();
+  }
+
+  // Origin filtering.
+  sql::Statement select_statement(
+      db_->GetCachedStatement(SQL_FROM_HERE,
+                              "SELECT origin_domain FROM stats "
+                              "WHERE update_time >= ? AND update_time < ?"));
+  select_statement.BindInt64(0, delete_begin.ToInternalValue());
+  select_statement.BindInt64(1, delete_end.ToInternalValue());
+
+  std::set<std::string> origins;
+  while (select_statement.Step()) {
+    if (!origin_filter.Run(GURL(select_statement.ColumnString(0))))
+      continue;
+
+    origins.insert(select_statement.ColumnString(0));
+  }
+
+  bool success = true;
+
+  for (const std::string& origin : origins) {
+    sql::Statement origin_delete_statement(db_->GetCachedStatement(
+        SQL_FROM_HERE,
+        "DELETE FROM stats "
+        "WHERE origin_domain = ? AND update_time >= ? AND update_time < ?"));
+    origin_delete_statement.BindString(0, origin);
+    origin_delete_statement.BindInt64(1, delete_begin.ToInternalValue());
+    origin_delete_statement.BindInt64(2, delete_end.ToInternalValue());
+    success &= origin_delete_statement.Run();

[vabr (Chromium), 2016/09/08 09:04:36]

I'm feeling uncomfortable about mixing bit-wise & and bool values. In this case
it should be OK, because Run() returns bool, an hopefully any non-zero value
will be identical to |true|. However, if somebody changed the signature to a
numerical type, it might change. What about the bit more verbose:
success = success && origin_delete_statement.Run();
?

[msramek, 2016/09/08 10:45:29]

Done. Makes sense.

+  }
+
+  return success;
 }
 
 }  // namespace password_manager