Add a partial index to the SQLitePersistentCookieStore.

content/browser/net/sqlite_persistent_cookie_store.cc

 // Copyright (c) 2012 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 "content/browser/net/sqlite_persistent_cookie_store.h"
 
 #include <list>
 #include <map>
 #include <set>
 #include <utility>
@@ skipping 288 matching lines @@
   // Not owned.
   net::CookieCryptoDelegate* crypto_;
 
   DISALLOW_COPY_AND_ASSIGN(Backend);
 };
 
 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
 // all new values written will be encrypted on selected operating systems.  New
 // records read by old clients will simply get an empty cookie value while old
 // records read by new clients will continue to operate with the unencrypted
 // version.  New and old clients alike will always write/update records with
 // what they support.
 //
 // Version 6 adds cookie priorities. This allows developers to influence the
 // order in which cookies are evicted in order to meet domain cookie limits.
 //
 // Version 5 adds the columns has_expires and is_persistent, so that the
 // database can store session cookies as well as persistent cookies. Databases
 // of version 5 are incompatible with older versions of code. If a database of
 // version 5 is read by older code, session cookies will be treated as normal
 // cookies. Currently, these fields are written, but not read anymore.
 //
 // In version 4, we migrated the time epoch.  If you open the DB with an older
 // version on Mac or Linux, the times will look wonky, but the file will likely
 // be usable. On Windows version 3 and 4 are the same.
 //
 // 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.
 enum DBCookiePriority {
   kCookiePriorityLow = 0,
   kCookiePriorityMedium = 1,
   kCookiePriorityHigh = 2,
 };
 
 DBCookiePriority CookiePriorityToDBCookiePriority(net::CookiePriority value) {
@@ skipping 40 matching lines @@
   }
 
  private:
   base::TimeDelta* delta_;
   base::TimeDelta original_value_;
   base::Time start_;
 
   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"))
     return false;
 
   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(
     const LoadedCallback& loaded_callback) {
   // This function should be called only once per instance.
   DCHECK(!db_.get());
   PostBackgroundTask(FROM_HERE, base::Bind(
       &Backend::LoadAndNotifyInBackground, this,
       loaded_callback, base::Time::Now()));
 }
@@ skipping 497 matching lines @@
     }
     ++cur_version;
     meta_table_.SetVersionNumber(cur_version);
     meta_table_.SetCompatibleVersionNumber(
         std::min(cur_version, kCompatibleVersionNumber));
     transaction.Commit();
     UMA_HISTOGRAM_TIMES("Cookie.TimeDatabaseMigrationToV8",
                         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) {
     UMA_HISTOGRAM_COUNTS_100("Cookie.CorruptMetaTable", 1);
 
     meta_table_.Reset();
     db_.reset(new sql::Connection);
     if (!sql::Connection::Delete(path_) ||
         !db_->Open(path_) ||
         !meta_table_.Init(
@@ skipping 429 matching lines @@
         (config.session_cookie_mode ==
          CookieStoreConfig::RESTORED_SESSION_COOKIES)) {
       cookie_monster->SetPersistSessionCookies(true);
     }
   }
 
   return cookie_monster;
 }
 
 }  // namespace content