Add cookie priority to the cookie database.

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 83119a38904c4868824a359cf5b01184f276e5fb..044cd3849f12ad187bf74f5b30cd7ac1173eb994 100644
--- a/content/browser/net/sqlite_persistent_cookie_store.cc
+++ b/content/browser/net/sqlite_persistent_cookie_store.cc
@@ -314,8 +314,13 @@ int SQLitePersistentCookieStore::Backend::KillDatabaseErrorDelegate::OnError(
   return error;
 }
 
+namespace {
+
 // Version number of the database.
 //
+// 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
@@ -329,10 +334,51 @@ int SQLitePersistentCookieStore::Backend::KillDatabaseErrorDelegate::OnError(
 // 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.
-static const int kCurrentVersionNumber = 5;
-static const int kCompatibleVersionNumber = 5;
+const int kCurrentVersionNumber = 6;
+const int kCompatibleVersionNumber = 5;
 
-namespace {
+// This anonymous enum defines the valid integer CookiePriority values.

[erikwright (departed), 2013/04/18 19:34:05]

Just say:

// Possible values for the 'priority' column.

[Roger McFarlane (Chromium), 2013/04/19 20:53:03]

Done.

+enum  {
+  kCookiePriorityLow = 0,
+  kCookiePriorityMedium = 1,
+  kCookiePriorityHigh = 2,
+
+  kCookiePriorityDefault = kCookiePriorityMedium

[erikwright (departed), 2013/04/18 19:34:05]

Not used.

[Roger McFarlane (Chromium), 2013/04/19 20:53:03]

Removed.

+};

[erikwright (departed), 2013/04/18 19:34:05]

Maybe name this enum "DBCookiePriority" and use that name instead of int below?

[Roger McFarlane (Chromium), 2013/04/19 20:53:03]

Done.

While I don't feel all that strongly about it, I'm not convinced that this
change improves the overall readability as compared to just using int.

+
+// Maps from the cookie priority values used elsewhere in the code to those
+// actually persisted in the cookie database. This decouples changes made in
+// the two value sets.
+int CookiePriorityToInt(net::CookiePriority priority) {

[erikwright (departed), 2013/04/18 19:34:05]

CookiePriorityToDBCookiePriority and the reverse below, then just remove the
comments.

[Roger McFarlane (Chromium), 2013/04/19 20:53:03]

Done.

+  switch (priority) {
+    case net::PRIORITY_LOW:
+      return kCookiePriorityLow;
+    case net::PRIORITY_MEDIUM:
+      return kCookiePriorityMedium;
+    case net::PRIORITY_HIGH:
+      return kCookiePriorityHigh;
+  }
+
+  NOTREACHED();
+  return kCookiePriorityMedium;
+}
+
+// Maps from the cookie priority values persisted in the cookie database to
+// those use elsewhere in the code.  This decouples changes made in the two
+// value sets.
+net::CookiePriority IntToCookiePriority(int value) {
+  switch (value) {
+    case kCookiePriorityLow:
+      return net::PRIORITY_LOW;
+    case kCookiePriorityMedium:
+      return net::PRIORITY_MEDIUM;
+    case kCookiePriorityHigh:
+      return net::PRIORITY_HIGH;
+  }
+
+  NOTREACHED();
+  return net::PRIORITY_MEDIUM;
+}
 
 // Increments a specified TimeDelta by the duration between this object's
 // constructor and destructor. Not thread safe. Multiple instances may be
@@ -360,20 +406,22 @@ class IncrementTimeDelta {
 // Initializes the cookies table, returning true on success.
 bool InitTable(sql::Connection* db) {
   if (!db->DoesTableExist("cookies")) {
-    if (!db->Execute("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)"))
-      // TODO(rogerm): Add priority.
+    if (!db->Execute(
+            "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 1)")) {  // Medium priority.

[erikwright (departed), 2013/04/18 19:34:05]

Would this compile if you replace

--
1)"
--

  with

--
" + base::IntToString(CookiePriorityToDBCookiePriority(net::PRIORITY_DEFAULT)) +
")"
--
?

[Roger McFarlane (Chromium), 2013/04/19 20:53:03]

Done.

Switched to using base::StringPrintf to get the statement text, here and in the
schema migration code.

       return false;
+    }
   }
 
   // Older code created an index on creation_utc, which is already
@@ -659,18 +707,16 @@ bool SQLitePersistentCookieStore::Backend::LoadCookiesForDomains(
   sql::Statement smt;
   if (restore_old_session_cookies_) {
     smt.Assign(db_->GetCachedStatement(
-      SQL_FROM_HERE,
-      "SELECT creation_utc, host_key, name, value, path, expires_utc, "
-      "secure, httponly, last_access_utc, has_expires, persistent "
-      "FROM cookies WHERE host_key = ?"));
-    // TODO(rogerm): Add priority.
+        SQL_FROM_HERE,
+        "SELECT creation_utc, host_key, name, value, path, expires_utc, "
+        "secure, httponly, last_access_utc, has_expires, persistent, priority "
+        "FROM cookies WHERE host_key = ?"));
   } else {
     smt.Assign(db_->GetCachedStatement(
-      SQL_FROM_HERE,
-      "SELECT creation_utc, host_key, name, value, path, expires_utc, "
-      "secure, httponly, last_access_utc, has_expires, persistent "
-      "FROM cookies WHERE host_key = ? AND persistent = 1"));
-    // TODO(rogerm): Add priority.
+        SQL_FROM_HERE,
+        "SELECT creation_utc, host_key, name, value, path, expires_utc, "
+        "secure, httponly, last_access_utc, has_expires, persistent, priority "
+        "FROM cookies WHERE host_key = ? AND persistent = 1"));
   }
   if (!smt.is_valid()) {
     smt.Clear();  // Disconnect smt_ref from db_.
@@ -697,9 +743,7 @@ bool SQLitePersistentCookieStore::Backend::LoadCookiesForDomains(
               Time::FromInternalValue(smt.ColumnInt64(8)),    // last_access_utc
               smt.ColumnInt(6) != 0,                          // secure
               smt.ColumnInt(7) != 0,                          // httponly
-              net::PRIORITY_DEFAULT));                        // priority
-      // TODO(rogerm): Change net::PRIORITY_DEFAULT above to
-      // net::StringToCookiePriority(smt.ColumnString(9))?
+              IntToCookiePriority(smt.ColumnInt(11))));       // priority
       DLOG_IF(WARNING,
               cc->CreationDate() > Time::Now()) << L"CreationDate too recent";
       cookies_per_origin_[CookieOrigin(cc->Domain(), cc->IsSecure())]++;
@@ -800,6 +844,27 @@ bool SQLitePersistentCookieStore::Backend::EnsureDatabaseVersion() {
                         base::TimeTicks::Now() - start_time);
   }
 
+  if (cur_version == 5) {
+    const base::TimeTicks start_time = base::TimeTicks::Now();
+    sql::Transaction transaction(db_.get());
+    if (!transaction.Begin())
+      return false;
+    // Alter the table to add the priority column with a default value of
+    // 1 (Medium) for the value.
+    if (!db_->Execute("ALTER TABLE cookies "
+                      "ADD COLUMN priority INTEGER DEFAULT 1")) {
+      LOG(WARNING) << "Unable to update cookie database to version 6.";
+      return false;
+    }
+    ++cur_version;
+    meta_table_.SetVersionNumber(cur_version);
+    meta_table_.SetCompatibleVersionNumber(
+        std::min(cur_version, kCompatibleVersionNumber));
+    transaction.Commit();
+    UMA_HISTOGRAM_TIMES("Cookie.TimeDatabaseMigrationToV6",
+                        base::TimeTicks::Now() - start_time);
+  }
+
   // Put future migration cases here.
 
   if (cur_version < kCurrentVersionNumber) {
@@ -883,12 +948,11 @@ void SQLitePersistentCookieStore::Backend::Commit() {
   if (!db_.get() || ops.empty())
     return;
 
-  // TODO(rogerm): Add priority.
   sql::Statement add_smt(db_->GetCachedStatement(SQL_FROM_HERE,
       "INSERT INTO cookies (creation_utc, host_key, name, value, path, "
       "expires_utc, secure, httponly, last_access_utc, has_expires, "
-      "persistent) "
-      "VALUES (?,?,?,?,?,?,?,?,?,?,?)"));
+      "persistent, priority) "
+      "VALUES (?,?,?,?,?,?,?,?,?,?,?,?)"));
   if (!add_smt.is_valid())
     return;
 
@@ -926,7 +990,7 @@ void SQLitePersistentCookieStore::Backend::Commit() {
         add_smt.BindInt64(8, po->cc().LastAccessDate().ToInternalValue());
         add_smt.BindInt(9, po->cc().IsPersistent());
         add_smt.BindInt(10, po->cc().IsPersistent());
-        // TODO(rogerm): Add priority.
+        add_smt.BindInt(11, CookiePriorityToInt(po->cc().Priority()));
         if (!add_smt.Run())
           NOTREACHED() << "Could not add a cookie to the DB.";
         break;