Add a partial index to the SQLitePersistentCookieStore.

content/browser/net/sqlite_persistent_cookie_store.cc

Index: content/browser/net/sqlite_persistent_cookie_store.cc
diff --git a/content/browser/net/sqlite_persistent_cookie_store.cc b/content/browser/net/sqlite_persistent_cookie_store.cc
index c4c6a141bed7fa72ce1f187d36f93a7ec232577d..61735fb520bbbd7716623ea50998a8958eee9f35 100644
--- a/content/browser/net/sqlite_persistent_cookie_store.cc
+++ b/content/browser/net/sqlite_persistent_cookie_store.cc
@@ -306,6 +306,11 @@ namespace {
 
 // Version number of the database.
 //
+// Version 9 adds a partial index to track non-persistent cookies.
+// Non-persistent cookies sometimes need to be deleted on startup. There are
+// frequently few or no non-persistent cookies, so the partial index allows the
+// deletion to be sped up or skipped, without having to page in the DB.
+//
 // Version 8 adds "first-party only" cookies.
 //
 // Version 7 adds encrypted values.  Old values will continue to be used but
@@ -331,7 +336,7 @@ namespace {
 // Version 3 updated the database to include the last access time, so we can
 // expire them in decreasing order of use when we've reached the maximum
 // number of cookies.
-const int kCurrentVersionNumber = 8;
+const int kCurrentVersionNumber = 9;
 const int kCompatibleVersionNumber = 5;
 
 // Possible values for the 'priority' column.
@@ -392,30 +397,8 @@ class IncrementTimeDelta {
   DISALLOW_COPY_AND_ASSIGN(IncrementTimeDelta);
 };
 
-// Initializes the cookies table, returning true on success.
-bool InitTable(sql::Connection* db) {
-  if (!db->DoesTableExist("cookies")) {
-    std::string stmt(base::StringPrintf(
-        "CREATE TABLE cookies ("
-        "creation_utc INTEGER NOT NULL UNIQUE PRIMARY KEY,"
-        "host_key TEXT NOT NULL,"
-        "name TEXT NOT NULL,"
-        "value TEXT NOT NULL,"
-        "path TEXT NOT NULL,"
-        "expires_utc INTEGER NOT NULL,"
-        "secure INTEGER NOT NULL,"
-        "httponly INTEGER NOT NULL,"
-        "last_access_utc INTEGER NOT NULL, "
-        "has_expires INTEGER NOT NULL DEFAULT 1, "
-        "persistent INTEGER NOT NULL DEFAULT 1,"
-        "priority INTEGER NOT NULL DEFAULT %d,"
-        "encrypted_value BLOB DEFAULT '',"
-        "firstpartyonly INTEGER NOT NULL DEFAULT 0)",
-        CookiePriorityToDBCookiePriority(net::COOKIE_PRIORITY_DEFAULT)));
-    if (!db->Execute(stmt.c_str()))
-      return false;
-  }
-
+// Creates the indices on the cookie table, if they don't already exist.
+bool UpdateIndicesOnTable(sql::Connection* db) {
   // Older code created an index on creation_utc, which is already
   // primary key for the table.
   if (!db->Execute("DROP INDEX IF EXISTS cookie_times"))
@@ -424,9 +407,43 @@ bool InitTable(sql::Connection* db) {
   if (!db->Execute("CREATE INDEX IF NOT EXISTS domain ON cookies(host_key)"))
     return false;
 
+  if (!db->Execute(
+          "CREATE INDEX IF NOT EXISTS is_transient ON cookies(persistent) "
+          "where persistent != 1")) {
+    return false;
+  }
+
   return true;
 }
 
+// Initializes the cookies table, returning true on success.
+bool InitTable(sql::Connection* db) {
+  if (db->DoesTableExist("cookies"))
+    return true;
+
+  std::string stmt(base::StringPrintf(
+      "CREATE TABLE cookies ("
+      "creation_utc INTEGER NOT NULL UNIQUE PRIMARY KEY,"
+      "host_key TEXT NOT NULL,"
+      "name TEXT NOT NULL,"
+      "value TEXT NOT NULL,"
+      "path TEXT NOT NULL,"
+      "expires_utc INTEGER NOT NULL,"
+      "secure INTEGER NOT NULL,"
+      "httponly INTEGER NOT NULL,"
+      "last_access_utc INTEGER NOT NULL, "
+      "has_expires INTEGER NOT NULL DEFAULT 1, "
+      "persistent INTEGER NOT NULL DEFAULT 1,"
+      "priority INTEGER NOT NULL DEFAULT %d,"
+      "encrypted_value BLOB DEFAULT '',"
+      "firstpartyonly INTEGER NOT NULL DEFAULT 0)",
+      CookiePriorityToDBCookiePriority(net::COOKIE_PRIORITY_DEFAULT)));
+  if (!db->Execute(stmt.c_str()))
+    return false;
+
+  return UpdateIndicesOnTable(db);
+}
+
 }  // namespace
 
 void SQLitePersistentCookieStore::Backend::Load(
@@ -944,6 +961,25 @@ bool SQLitePersistentCookieStore::Backend::EnsureDatabaseVersion() {
                         base::TimeTicks::Now() - start_time);
   }
 
+  if (cur_version == 8) {
+    const base::TimeTicks start_time = base::TimeTicks::Now();
+    sql::Transaction transaction(db_.get());
+    if (!transaction.Begin())
+      return false;
+
+    if (!UpdateIndicesOnTable(db_.get())) {

[erikwright (departed), 2015/04/17 01:20:47]

I don't know that this pattern makes sense.

Imagine a column added in version 10, then an index added on it in version 11.

A user who has version 7 will migrate to version 8 and invoke
UpdateIndicesOnTable before the indexed column is created.

We already have some unsightly duplication with the table definition, it's just
less obvious (because the form of an ALTER TABLE is different from that of a
CREATE TABLE). I don't see any point in trying to get around it with cleverness
unless you want to rethink database definition and migration from scratch.

[erikchen, 2015/04/17 01:34:47]

Excellent point. I failed to notice that the pattern I used would make it very
easy to make a mistake in the future.

+      LOG(WARNING) << "Unable to update cookie database to version 9.";
+      return false;
+    }
+    ++cur_version;
+    meta_table_.SetVersionNumber(cur_version);
+    meta_table_.SetCompatibleVersionNumber(
+        std::min(cur_version, kCompatibleVersionNumber));
+    transaction.Commit();
+    UMA_HISTOGRAM_TIMES("Cookie.TimeDatabaseMigrationToV9",
+                        base::TimeTicks::Now() - start_time);
+  }
+
   // Put future migration cases here.
 
   if (cur_version < kCurrentVersionNumber) {