Split initial load of cookies by domains

chrome/browser/net/sqlite_persistent_cookie_store.cc

Index: chrome/browser/net/sqlite_persistent_cookie_store.cc
===================================================================
--- chrome/browser/net/sqlite_persistent_cookie_store.cc	(revision 100935)
+++ chrome/browser/net/sqlite_persistent_cookie_store.cc	(working copy)
@@ -4,8 +4,6 @@
 
 #include "chrome/browser/net/sqlite_persistent_cookie_store.h"
 
-#include <list>
-
 #include "base/basictypes.h"
 #include "base/bind.h"
 #include "base/file_path.h"
@@ -20,6 +18,7 @@
 #include "chrome/browser/diagnostics/sqlite_diagnostics.h"
 #include "content/browser/browser_thread.h"
 #include "googleurl/src/gurl.h"
+#include "net/base/registry_controlled_domain.h"
 #include "sql/meta_table.h"
 #include "sql/statement.h"
 #include "sql/transaction.h"
@@ -47,6 +46,10 @@
   // Creates or load the SQLite database.
   bool Load(const LoadedCallback& loaded_callback);
 
+  // Load cookies for the domain key (eTLD+1).
+  bool LoadCookiesForKey(const std::string& domain,
+      const LoadedCallback& loaded_callback);
+
   // Batch a cookie addition.
   void AddCookie(const net::CookieMonster::CanonicalCookie& cc);
 
@@ -100,16 +103,26 @@
  private:
   // Creates or load the SQLite database on DB thread.
   void LoadAndNotifyOnDBThread(const LoadedCallback& loaded_callback);
-  // Notify the CookieMonster when loading complete.
+
+  // Load cookies for the domain key (eTLD+1) on DB thread.
+  void LoadKeyAndNotifyOnDBThread(const std::string& domains,
+      const LoadedCallback& loaded_callback);
+
+  // Notify the CookieMonster when loading complete for a specific domain key
+  // or for all domain keys.
   void NotifyOnIOThread(
       const LoadedCallback& loaded_callback,
-      bool load_success,
-      const std::vector<net::CookieMonster::CanonicalCookie*>& cookies);
+      bool load_success);
   // Initialize the data base.
+
   bool InitializeDatabase();
-  // Load cookies to the data base, and read cookies.
-  bool LoadInternal(std::vector<net::CookieMonster::CanonicalCookie*>* cookies);
 
+  // Chain load cookies from DB by domain key.
+  void ChainLoadCookies(const LoadedCallback& loaded_callback);
+
+  // Load all cookies for a set of domains/hosts
+  bool LoadCookiesForDomains(const std::set<std::string>& key);
+
   // Batch a cookie operation (add or delete)
   void BatchOperation(PendingOperation::OperationType op,
                       const net::CookieMonster::CanonicalCookie& cc);
@@ -130,6 +143,15 @@
   // Guard |pending_|, |num_pending_| and |clear_local_state_on_exit_|.
   base::Lock lock_;
 
+  // Temporary buffer for cookies loaded from DB.
+  std::vector<net::CookieMonster::CanonicalCookie*> cookies_;
+
+  // List of domain keys(eTLD+1) to be loaded from DB.

[erikwright (departed), 2011/09/15 20:56:27]

Correct comment to indicate the meaning of the keys and values in this map.

+  std::map<std::string, std::set<std::string>> keys_to_load_;
+
+  // Indicates if DB has been initialized.
+  bool initialized_;
+
   DISALLOW_COPY_AND_ASSIGN(Backend);
 };
 
@@ -167,6 +189,9 @@
   // so we want those people to get it. Ignore errors, since it may exist.
   db->Execute("CREATE INDEX IF NOT EXISTS cookie_times ON cookies"
               " (creation_utc)");
+
+  db->Execute("CREATE INDEX IF NOT EXISTS domain ON cookies(host_key)");
+
   return true;
 }
 
@@ -183,27 +208,74 @@
   return true;

[erikwright (departed), 2011/09/15 20:56:27]

These 'return true' statements are now useless (lines 208, 218). Make the
methods void.

[guohui, 2011/09/16 18:21:09]

Done.

 }
 
+bool SQLitePersistentCookieStore::Backend::LoadCookiesForKey(
+    const std::string& key,
+    const LoadedCallback& loaded_callback) {
+  BrowserThread::PostTask(
+    BrowserThread::DB, FROM_HERE,
+    base::Bind(&Backend::LoadKeyAndNotifyOnDBThread, this,
+    key,
+    loaded_callback));
+  return true;
+}
+
 void SQLitePersistentCookieStore::Backend::LoadAndNotifyOnDBThread(
     const LoadedCallback& loaded_callback) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::DB));
-  std::vector<net::CookieMonster::CanonicalCookie*> cookies;
 
-  bool load_success = LoadInternal(&cookies);
+  if (!InitializeDatabase()) {
+    BrowserThread::PostTask(
+      BrowserThread::IO, FROM_HERE,
+      base::Bind(&SQLitePersistentCookieStore::Backend::NotifyOnIOThread,
+                 this, loaded_callback, false));
+  } else {
+    ChainLoadCookies(loaded_callback);
+  }
+}
 
-  BrowserThread::PostTask(BrowserThread::IO, FROM_HERE, base::Bind(
-      &SQLitePersistentCookieStore::Backend::NotifyOnIOThread,
-      base::Unretained(this), loaded_callback, load_success, cookies));
+void SQLitePersistentCookieStore::Backend::LoadKeyAndNotifyOnDBThread(
+    const std::string& key,
+    const LoadedCallback& loaded_callback) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::DB));
+
+  bool success = false;
+  if (InitializeDatabase()) {
+    std::map<std::string, std::set<std::string> >::iterator
+      it = keys_to_load_.find(key);
+    if (it != keys_to_load_.end()) {
+      success = LoadCookiesForDomains(it->second);
+      keys_to_load_.erase(it);
+    } else {
+      success = true;
+    }
+  }
+
+  BrowserThread::PostTask(
+    BrowserThread::IO, FROM_HERE,
+    base::Bind(&SQLitePersistentCookieStore::Backend::NotifyOnIOThread,
+    this, loaded_callback, success));
 }
 
 void SQLitePersistentCookieStore::Backend::NotifyOnIOThread(
     const LoadedCallback& loaded_callback,
-    bool load_success,
-    const std::vector<net::CookieMonster::CanonicalCookie*>& cookies) {
+    bool load_success) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+
+  std::vector<net::CookieMonster::CanonicalCookie*> cookies;
+  {
+    base::AutoLock locked(lock_);
+    cookies = cookies_;
+    cookies_.clear();
+  }
+
   loaded_callback.Run(cookies);
 }
 
 bool SQLitePersistentCookieStore::Backend::InitializeDatabase() {
+  if (initialized_) {
+    return true;
+  }
+
   const FilePath dir = path_.DirName();
   if (!file_util::PathExists(dir) && !file_util::CreateDirectory(dir)) {
     return false;
@@ -225,45 +297,109 @@
   }
 
   db_->Preload();
+
+  // Retrieve all the domains
+  sql::Statement smt(db_->GetUniqueStatement(
+    "SELECT DISTINCT host_key FROM cookies"));
+
+  if (!smt) {
+    NOTREACHED() << "select statement prep failed";
+    db_.reset();
+    return false;
+  }
+
+  // Build a map of domain keys (eTLD+1) to domains.
+  while (smt.Step()) {
+    std::string domain = smt.ColumnString(0);
+    std::string key =
+      net::RegistryControlledDomainService::GetDomainAndRegistry(domain);
+
+    std::map<std::string, std::set<std::string> >::iterator it =
+      keys_to_load_.find(key);
+    if (it == keys_to_load_.end()) {
+      std::set<std::string> domains;
+      domains.insert(domain);
+      keys_to_load_.insert(
+        std::pair<std::string, std::set<std::string> >(key, domains));
+    } else {
+      it->second.insert(domain);
+    }
+  }
+
+  // Reserve space for the maximum amount of cookies a database should have.
+  // This prevents multiple vector growth / copies as we append cookies.
+  const size_t kMaxCookies = 3300;

[erikwright (departed), 2011/09/15 20:56:27]

This code (lines 329 to 332) was removed in my CL but you have accidentally
added it back. Please remove.

+  cookies_.reserve(kMaxCookies);
+
+  initialized_ = true;
   return true;
 }
 
-bool SQLitePersistentCookieStore::Backend::LoadInternal(
-    std::vector<net::CookieMonster::CanonicalCookie*>* cookies) {
-  if (!InitializeDatabase()) {
-    return false;
+void SQLitePersistentCookieStore::Backend::ChainLoadCookies(
+    const LoadedCallback& loaded_callback) {
+  bool load_success = true;
+
+  if (keys_to_load_.size() > 0) {
+    // Load cookies for the first domain key.
+    std::map<std::string, std::set<std::string> >::iterator
+      it = keys_to_load_.begin();
+    load_success = LoadCookiesForDomains(it->second);
+    keys_to_load_.erase(it);
   }
 
-  // Slurp all the cookies into the out-vector.
-  sql::Statement smt(db_->GetUniqueStatement(
-      "SELECT creation_utc, host_key, name, value, path, expires_utc, secure, "
-      "httponly, last_access_utc FROM cookies"));
+  // If load is successful and there are more domain keys to be loaded,
+  // then post a DB task to continue chain-load;
+  // Otherwise notify on IO thread.
+  if (load_success && keys_to_load_.size() > 0) {
+    BrowserThread::PostTask(
+      BrowserThread::DB, FROM_HERE,
+      base::Bind(&Backend::ChainLoadCookies, this, loaded_callback));
+  } else {
+    BrowserThread::PostTask(
+      BrowserThread::IO, FROM_HERE,
+      base::Bind(&SQLitePersistentCookieStore::Backend::NotifyOnIOThread,
+                 this, loaded_callback, load_success));
+  }
+}
+
+bool SQLitePersistentCookieStore::Backend::LoadCookiesForDomains(
+  const std::set<std::string>& domains) {
+  base::AutoLock locked(lock_);
+
+  sql::Statement smt(db_->GetCachedStatement(SQL_FROM_HERE,
+    "SELECT creation_utc, host_key, name, value, path, expires_utc, secure, "
+    "httponly, last_access_utc FROM cookies WHERE host_key = ?"));
   if (!smt) {
     NOTREACHED() << "select statement prep failed";
     db_.reset();
     return false;
   }
 
-  while (smt.Step()) {
-    scoped_ptr<net::CookieMonster::CanonicalCookie> cc(
-        new net::CookieMonster::CanonicalCookie(
-            // The "source" URL is not used with persisted cookies.
-            GURL(),                                         // Source
-            smt.ColumnString(2),                            // name
-            smt.ColumnString(3),                            // value
-            smt.ColumnString(1),                            // domain
-            smt.ColumnString(4),                            // path
-            std::string(),  // TODO(abarth): Persist mac_key
-            std::string(),  // TODO(abarth): Persist mac_algorithm
-            Time::FromInternalValue(smt.ColumnInt64(0)),    // creation_utc
-            Time::FromInternalValue(smt.ColumnInt64(5)),    // expires_utc
-            Time::FromInternalValue(smt.ColumnInt64(8)),    // last_access_utc
-            smt.ColumnInt(6) != 0,                          // secure
-            smt.ColumnInt(7) != 0,                          // httponly
-            true));                                         // has_expires
-    DLOG_IF(WARNING,
-            cc->CreationDate() > Time::Now()) << L"CreationDate too recent";
-    cookies->push_back(cc.release());
+  std::set<std::string>::const_iterator it = domains.begin();
+  for (; it != domains.end(); ++it) {
+    smt.BindString(0, *it);
+    while (smt.Step()) {
+      scoped_ptr<net::CookieMonster::CanonicalCookie> cc(
+          new net::CookieMonster::CanonicalCookie(
+              // The "source" URL is not used with persisted cookies.
+              GURL(),                                         // Source
+              smt.ColumnString(2),                            // name
+              smt.ColumnString(3),                            // value
+              smt.ColumnString(1),                            // domain
+              smt.ColumnString(4),                            // path
+              std::string(),  // TODO(abarth): Persist mac_key
+              std::string(),  // TODO(abarth): Persist mac_algorithm
+              Time::FromInternalValue(smt.ColumnInt64(0)),    // creation_utc
+              Time::FromInternalValue(smt.ColumnInt64(5)),    // expires_utc
+              Time::FromInternalValue(smt.ColumnInt64(8)),    // last_access_utc
+              smt.ColumnInt(6) != 0,                          // secure
+              smt.ColumnInt(7) != 0,                          // httponly
+              true));                                         // has_expires
+      DLOG_IF(WARNING,
+              cc->CreationDate() > Time::Now()) << L"CreationDate too recent";
+      cookies_.push_back(cc.release());
+    }
+    smt.Reset();
   }
 
   return true;
@@ -538,6 +674,12 @@
   return backend_->Load(loaded_callback);
 }
 
+bool SQLitePersistentCookieStore::LoadCookiesForKey(
+    const std::string& key,
+    const LoadedCallback& loaded_callback) {
+  return backend_->LoadCookiesForKey(key, loaded_callback);
+}
+
 void SQLitePersistentCookieStore::AddCookie(
     const net::CookieMonster::CanonicalCookie& cc) {
   if (backend_.get())