Adds PreviewsType version mechanism for clearing blacklist entries.

components/previews/core/previews_opt_out_store_sql.cc

Index: components/previews/core/previews_opt_out_store_sql.cc
diff --git a/components/previews/core/previews_opt_out_store_sql.cc b/components/previews/core/previews_opt_out_store_sql.cc
index a6fe9e7ea690cdba8274d72cbb8c90c75c69e98f..d448a4e2bb34de87649dc5d389b8ba1caafd60fa 100644
--- a/components/previews/core/previews_opt_out_store_sql.cc
+++ b/components/previews/core/previews_opt_out_store_sql.cc
@@ -4,7 +4,9 @@
 
 #include "components/previews/core/previews_opt_out_store_sql.h"
 
+#include <map>
 #include <string>
+#include <utility>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
@@ -55,18 +57,28 @@ int MaxRowsInOptOutDB() {
   return base::StringToInt(max_rows, &value) ? value : 3200;
 }
 
-// This is a macro instead of a const, so it can be used inline in other SQL
-// statements below.
+// Table names use a macro instead of a const, so the can be used inline in

[RyanSturm, 2017/01/25 23:17:53]

s/the/they/

[dougarnett, 2017/01/26 00:48:27]

Done.

+// other SQL statements below.
 #define PREVIEWS_TABLE_NAME "previews_v1"
+#define PREVIEWS_TYPE_VERSION_TABLE_NAME "previews_type_version"
 
 void CreateSchema(sql::Connection* db) {
-  const char kSql[] = "CREATE TABLE IF NOT EXISTS " PREVIEWS_TABLE_NAME
-                      " (host_name VARCHAR NOT NULL,"
-                      " time INTEGER NOT NULL,"
-                      " opt_out INTEGER NOT NULL,"
-                      " type INTEGER NOT NULL,"
-                      " PRIMARY KEY(host_name, time DESC, opt_out, type))";
-  if (!db->Execute(kSql))
+  const char kSqlCreatePreviewsTable[] =
+      "CREATE TABLE IF NOT EXISTS " PREVIEWS_TABLE_NAME
+      " (host_name VARCHAR NOT NULL,"
+      " time INTEGER NOT NULL,"
+      " opt_out INTEGER NOT NULL,"
+      " type INTEGER NOT NULL,"
+      " PRIMARY KEY(host_name, time DESC, opt_out, type))";
+  if (!db->Execute(kSqlCreatePreviewsTable))
+    return;
+
+  const char kSqlCreateTypeVersionTable[] =
+      "CREATE TABLE IF NOT EXISTS " PREVIEWS_TYPE_VERSION_TABLE_NAME
+      " (type INTEGER NOT NULL,"
+      " version INTEGER NOT NULL,"
+      " PRIMARY KEY(type))";
+  if (!db->Execute(kSqlCreateTypeVersionTable))
     return;
 }
 
@@ -168,10 +180,94 @@ void MaybeEvictHostEntryFromDataBase(sql::Connection* db,
   statement_delete_by_host.Run();
 }
 
+// Deletes every preview navigation entry for |type|.
+void ClearBlacklistForTypeInDataBase(
+    sql::Connection* db,
+    scoped_refptr<base::SingleThreadTaskRunner> runner,

[RyanSturm, 2017/01/25 23:17:53]

You don't use |runner| for a bunch of these.

[dougarnett, 2017/01/26 00:48:27]

thx

+    PreviewsType type) {
+  const char kSql[] = "DELETE FROM " PREVIEWS_TABLE_NAME " WHERE type == ?";
+  sql::Statement statement(db->GetUniqueStatement(kSql));
+  statement.BindInt(0, static_cast<int>(type));
+  statement.Run();
+}
+
+void InsertPreviewsVersionInDataBase(
+    sql::Connection* db,
+    scoped_refptr<base::SingleThreadTaskRunner> runner,
+    PreviewsType type,
+    int version) {
+  const char kSqlInsert[] = "INSERT INTO " PREVIEWS_TYPE_VERSION_TABLE_NAME
+                            " (type, version)"
+                            " VALUES "
+                            " (?, ?)";
+
+  sql::Statement statement_insert(
+      db->GetCachedStatement(SQL_FROM_HERE, kSqlInsert));

[RyanSturm, 2017/01/25 23:17:53]

GetUniqueStatement instead since this will only be called once per profile
start-up.

[dougarnett, 2017/01/26 00:48:27]

Done.

+  statement_insert.BindInt(0, static_cast<int>(type));
+  statement_insert.BindInt(1, version);
+  statement_insert.Run();
+}
+
+void UpdatePreviewsVersionInDataBase(
+    sql::Connection* db,
+    scoped_refptr<base::SingleThreadTaskRunner> runner,
+    PreviewsType type,
+    int version) {
+  const char kSqlUpdate[] = "UPDATE " PREVIEWS_TYPE_VERSION_TABLE_NAME
+                            " SET version = ?"
+                            " WHERE type = ?";
+
+  sql::Statement statement_update(
+      db->GetCachedStatement(SQL_FROM_HERE, kSqlUpdate));
+  statement_update.BindInt(0, version);
+  statement_update.BindInt(1, static_cast<int>(type));
+  statement_update.Run();
+}
+
+// Checks current PreviewsType versions with previously used versions, and
+// where different, cleans up any blacklist entries for the different version.
+void CheckAndReconcileEnabledPreviewsVersionWithDataBase(
+    sql::Connection* db,
+    scoped_refptr<base::SingleThreadTaskRunner> runner) {
+  const char kSqlLoadPreviewsVersions[] =
+      "SELECT type, version FROM " PREVIEWS_TYPE_VERSION_TABLE_NAME;
+
+  sql::Statement statement(db->GetUniqueStatement(kSqlLoadPreviewsVersions));
+
+  std::map<PreviewsType, int> stored_versions;
+  while (statement.Step()) {
+    PreviewsType type = static_cast<PreviewsType>(statement.ColumnInt(0));
+    int version = statement.ColumnInt(1);

[RyanSturm, 2017/01/25 23:17:53]

nit: You don't need the local variable for version or PreviewsType, but if you
think it  helps with readability, I won't complain.

[dougarnett, 2017/01/26 00:48:27]

Yeah, thought a bit more readable but don't feel strongly either way.

+    stored_versions.insert({type, version});
+  }
+
+  std::unique_ptr<std::vector<std::pair<PreviewsType, int>>> enabled_previews(
+      GetEnabledPreviews());
+
+  for (std::vector<std::pair<PreviewsType, int>>::const_iterator it =
+           enabled_previews->begin();
+       it != enabled_previews->end(); ++it) {
+    PreviewsType type = it->first;
+    int current_version = it->second;
+    std::map<PreviewsType, int>::const_iterator stored_it =
+        stored_versions.find(type);
+    if (stored_it == stored_versions.end()) {
+      InsertPreviewsVersionInDataBase(db, runner, type, current_version);
+    } else if (stored_it->second != current_version) {
+      DCHECK_GE(current_version, stored_it->second);
+      ClearBlacklistForTypeInDataBase(db, runner, type);
+      UpdatePreviewsVersionInDataBase(db, runner, type, current_version);
+    }
+  }
+}
+
 void LoadBlackListFromDataBase(
     sql::Connection* db,
     scoped_refptr<base::SingleThreadTaskRunner> runner,
     LoadBlackListCallback callback) {
+  // First handle any update needed wrt version of enabled previews.
+  CheckAndReconcileEnabledPreviewsVersionWithDataBase(db, runner);
+
   // Gets the table sorted by host and time. Limits the number of hosts using
   // most recent opt_out time as the limiting function. Sorting is free due to
   // the table structure, and it improves performance in the loop below.