Split initial load of cookies by domains

net/base/cookie_monster.cc

 // Copyright (c) 2011 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.
 
 // Portions of this code based on Mozilla:
 //   (netwerk/cookie/src/nsCookieService.cpp)
 /* ***** BEGIN LICENSE BLOCK *****
  * Version: MPL 1.1/GPL 2.0/LGPL 2.1
  *
  * The contents of this file are subject to the Mozilla Public License Version
@@ skipping 31 matching lines @@
  *
  * ***** END LICENSE BLOCK ***** */
 
 #include "net/base/cookie_monster.h"
 
 #include <algorithm>
 #include <set>
 
 #include "base/basictypes.h"
 #include "base/bind.h"
+#include "base/callback.h"
 #include "base/format_macros.h"
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/message_loop.h"
 #include "base/message_loop_proxy.h"
 #include "base/metrics/histogram.h"
 #include "base/string_tokenizer.h"
 #include "base/string_util.h"
 #include "base/stringprintf.h"
 #include "googleurl/src/gurl.h"
 #include "googleurl/src/url_canon.h"
 #include "net/base/net_util.h"
 #include "net/base/registry_controlled_domain.h"
 
 using base::Time;
 using base::TimeDelta;
 using base::TimeTicks;
 
+// In steady state, most cookie requests can be satisfied by the in memory
+// cookie monster store.  However, if a request comes in during the initial
+// cookie load, it must be delayed until that load completes. That is done by
+// queueing it on CookieMonster::queue_ and running it when notification of
+// cookie load completion is received via CookieMonster::OnLoaded. This callback
+// is passed to the persistent store from CookieMonster::InitStore(), which is
+// called on the first operation invoked on the CookieMonster.
+//
+// On the browser critical paths (e.g. for loading initial web pages in a
+// session restore) it may take too long to wait for the full load. If a cookie
+// request is for a specific URL, DoCookieTaskForURL is called, which triggers a
+// priority load if the key is not loaded yet by calling PersistentCookieStore
+// :: LoadCookiesForKey. The request is queued in CookieMonster::tasks_queued
+// and executed upon receiving notification of key load completion via
+// CookieMonster::OnKeyLoaded(). If multiple requests for the same eTLD+1 are
+// received before key load completion, only the first request calls
+// PersistentCookieStore::LoadCookiesForKey, all subsequent requests are queued
+// in CookieMonster::tasks_queued and executed upon receiving notification of
+// key load completion triggered by the first request for the same eTLD+1.
+
 static const int kMinutesInTenYears = 10 * 365 * 24 * 60;
 
 namespace net {
 
 // See comments at declaration of these variables in cookie_monster.h
 // for details.
 const size_t CookieMonster::kDomainMaxCookies           = 180;
 const size_t CookieMonster::kDomainPurgeCookies         = 30;
 const size_t CookieMonster::kMaxCookies                 = 3300;
 const size_t CookieMonster::kPurgeCookies               = 300;
@@ skipping 906 matching lines @@
 void CookieMonster::SetCookieWithDetailsAsync(
     const GURL& url, const std::string& name, const std::string& value,
     const std::string& domain, const std::string& path,
     const base::Time& expiration_time, bool secure, bool http_only,
     const SetCookiesCallback& callback) {
   scoped_refptr<SetCookieWithDetailsTask> task =
       new SetCookieWithDetailsTask(this, url, name, value, domain, path,
                                    expiration_time, secure, http_only,
                                    callback);
 
-  DoCookieTask(task);
+  DoCookieTaskForURL(task, url);
 }
 
 void CookieMonster::GetAllCookiesAsync(const GetCookieListCallback& callback) {
   scoped_refptr<GetAllCookiesTask> task =
       new GetAllCookiesTask(this, callback);
 
   DoCookieTask(task);
 }
 
 
 void CookieMonster::GetAllCookiesForURLWithOptionsAsync(
     const GURL& url,
     const CookieOptions& options,
     const GetCookieListCallback& callback) {
   scoped_refptr<GetAllCookiesForURLWithOptionsTask> task =
       new GetAllCookiesForURLWithOptionsTask(this, url, options, callback);
 
-  DoCookieTask(task);
+  DoCookieTaskForURL(task, url);
 }
 
 void CookieMonster::GetAllCookiesForURLAsync(
     const GURL& url, const GetCookieListCallback& callback) {
   CookieOptions options;
   options.set_include_httponly();
   scoped_refptr<GetAllCookiesForURLWithOptionsTask> task =
       new GetAllCookiesForURLWithOptionsTask(this, url, options, callback);
 
-  DoCookieTask(task);
+  DoCookieTaskForURL(task, url);
 }
 
 void CookieMonster::DeleteAllAsync(const DeleteCallback& callback) {
   scoped_refptr<DeleteAllTask> task =
       new DeleteAllTask(this, callback);
 
   DoCookieTask(task);
 }
 
 void CookieMonster::DeleteAllCreatedBetweenAsync(
     const Time& delete_begin, const Time& delete_end,
     const DeleteCallback& callback) {
   scoped_refptr<DeleteAllCreatedBetweenTask> task =
       new DeleteAllCreatedBetweenTask(this, delete_begin, delete_end,
                                       callback);
 
   DoCookieTask(task);
 }
 
 void CookieMonster::DeleteAllForHostAsync(
     const GURL& url, const DeleteCallback& callback) {
   scoped_refptr<DeleteAllForHostTask> task =
       new DeleteAllForHostTask(this, url, callback);
 
-  DoCookieTask(task);
+  DoCookieTaskForURL(task, url);
 }
 
 void CookieMonster::DeleteCanonicalCookieAsync(
     const CanonicalCookie& cookie,
     const DeleteCookieCallback& callback) {
   scoped_refptr<DeleteCanonicalCookieTask> task =
       new DeleteCanonicalCookieTask(this, cookie, callback);
 
   DoCookieTask(task);
 }
 
 void CookieMonster::SetCookieWithOptionsAsync(
     const GURL& url,
     const std::string& cookie_line,
     const CookieOptions& options,
     const SetCookiesCallback& callback) {
   scoped_refptr<SetCookieWithOptionsTask> task =
       new SetCookieWithOptionsTask(this, url, cookie_line, options, callback);
 
-  DoCookieTask(task);
+  DoCookieTaskForURL(task, url);
 }
 
 void CookieMonster::GetCookiesWithOptionsAsync(
     const GURL& url,
     const CookieOptions& options,
     const GetCookiesCallback& callback) {
   scoped_refptr<GetCookiesWithOptionsTask> task =
       new GetCookiesWithOptionsTask(this, url, options, callback);
 
-  DoCookieTask(task);
+  DoCookieTaskForURL(task, url);
 }
 
 void CookieMonster::GetCookiesWithInfoAsync(
     const GURL& url,
     const CookieOptions& options,
     const GetCookieInfoCallback& callback) {
   scoped_refptr<GetCookiesWithInfoTask> task =
       new GetCookiesWithInfoTask(this, url, options, callback);
 
-  DoCookieTask(task);
+  DoCookieTaskForURL(task, url);
 }
 
 void CookieMonster::DeleteCookieAsync(const GURL& url,
                                       const std::string& cookie_name,
                                       const base::Closure& callback) {
   scoped_refptr<DeleteCookieTask> task =
       new DeleteCookieTask(this, url, cookie_name, callback);
 
-  DoCookieTask(task);
+  DoCookieTaskForURL(task, url);
 }
 
 void CookieMonster::DoCookieTask(
     const scoped_refptr<CookieMonsterTask>& task_item) {
-  InitIfNecessary();
-
   {
     base::AutoLock autolock(lock_);
+    InitIfNecessary();
     if (!loaded_) {
       queue_.push(task_item);
       return;
     }
   }
 
   task_item->Run();
 }
 
+void CookieMonster::DoCookieTaskForURL(
+    const scoped_refptr<CookieMonsterTask>& task_item,
+    const GURL& url) {
+  {
+    base::AutoLock autolock(lock_);
+    InitIfNecessary();
+    // If cookies for the requested domain key (eTLD+1) have been loaded from DB
+    // then run the task, otherwise load from DB.
+    if (!loaded_) {
+      // Checks if the domain key has been loaded.
+      std::string key(GetEffectiveDomain(url.scheme(), url.host()));
+      if (keys_loaded_.find(key) == keys_loaded_.end()) {
+        std::map<std::string, std::queue<scoped_refptr<CookieMonsterTask> > >
+          ::iterator it = tasks_queued_.find(key);
+        if (it == tasks_queued_.end()) {
+          store_->LoadCookiesForKey(key,
+            base::Bind(&CookieMonster::OnKeyLoaded, this, key));
+          it = tasks_queued_.insert(std::make_pair(key,
+            std::queue<scoped_refptr<CookieMonsterTask> >())).first;
+        }
+        it->second.push(task_item);
+        return;
+      }
+    }
+  }
+  task_item->Run();
+}
+
 bool CookieMonster::SetCookieWithDetails(
     const GURL& url, const std::string& name, const std::string& value,
     const std::string& domain, const std::string& path,
     const base::Time& expiration_time, bool secure, bool http_only) {
   base::AutoLock autolock(lock_);
 
   if (!HasCookieableScheme(url))
     return false;
 
   Time creation_time = CurrentTime();
@@ skipping 337 matching lines @@
 
 void CookieMonster::OnLoaded(TimeTicks beginning_time,
                              const std::vector<CanonicalCookie*>& cookies) {
   StoreLoadedCookies(cookies);
   histogram_time_load_->AddTime(TimeTicks::Now() - beginning_time);
 
   // Invoke the task queue of cookie request.
   InvokeQueue();
 }
 
+void CookieMonster::OnKeyLoaded(const std::string& key,
+                                const std::vector<CanonicalCookie*>& cookies) {
+  std::map<std::string, std::queue<scoped_refptr<CookieMonsterTask> > >
+    ::iterator it;
+  // This function does its own separate locking.
+  StoreLoadedCookies(cookies);
+
+  {
+    base::AutoLock autolock(lock_);
+    keys_loaded_.insert(key);
+    it = tasks_queued_.find(key);
+    if (it == tasks_queued_.end())
+      return;
+  }
+
+  // No lock is required here since 1) after the key is inserted into
+  // keys_loaded_, subsequent tasks for the same key will not be queued, so the
+  // queue pointed to by |it| is constant at this time point. 2) inserting a new
+  // element into a map or erasing an element from a map does not invalidate any
+  // iterator (except for iterators that point to the element that is being
+  // erased).

[Randy Smith (Not in Mondays), 2011/10/13 21:36:28]

I agree with this comment as stated, but my belief is that those guarantees by
the STL spec are all made in the context of single threaded access.  And STL
implementations are often really wonky, and rely on things right at the corners
of the C++ spec.  So I'd feel more comfortable if we were only working on a
local variable at this point.  

I think if you made the secondary structure inside of tasks_queued_ a deque
instead of a queue, you could use the "swap" method to pull the entire contents
of the specific keys queue into a stack based deque, and do all these operations
in about the same number of lines of code and performance without working inside
of a multi-threaded data structure.

Alternatively, I suspect it's ok if the variable that survived the autolock
block was a reference/pointer directly to the queue (i.e. to it->second rather
than it); I really can't see how that could be touched in a multi-threaded
context.  It still makes me twitch a bit, but I think it's safe, so with a
comment I'd be ok with it.

[erikwright (departed), 2011/10/14 01:15:50]

We hemmed and hawed over this one. I agree that it's a little ambiguous as to
whether this is OK.

I think the solution of switching to deque and swapping with a local variable is
a very reasonable and tidy one. Not only that, we will be able to do the erase
within the first autlock block :)

On 2011/10/13 21:36:28, rdsmith wrote:

[guohui, 2011/10/14 15:39:00]

Done.

+  while (!it->second.empty()) {
+    scoped_refptr<CookieMonsterTask> task = it->second.front();
+    task->Run();
+    it->second.pop();
+  }
+
+  {
+    base::AutoLock autolock(lock_);
+    tasks_queued_.erase(it);
+  }
+}
+
 void CookieMonster::StoreLoadedCookies(
     const std::vector<CanonicalCookie*>& cookies) {
   // Initialize the store and sync in any saved persistent cookies.  We don't
   // care if it's expired, insert it so it can be garbage collected, removed,
   // and sync'd.
   base::AutoLock autolock(lock_);
 
-  // Avoid ever letting cookies with duplicate creation times into the store;
-  // that way we don't have to worry about what sections of code are safe
-  // to call while it's in that state.
-  std::set<int64> creation_times;
-
-  // Presumably later than any access time in the store.
-  Time earliest_access_time;
-
   for (std::vector<CanonicalCookie*>::const_iterator it = cookies.begin();
        it != cookies.end(); ++it) {
     int64 cookie_creation_time = (*it)->CreationDate().ToInternalValue();
 
-    if (creation_times.insert(cookie_creation_time).second) {
+    if (creation_times_.insert(cookie_creation_time).second) {
       InternalInsertCookie(GetKey((*it)->Domain()), *it, false);
       const Time cookie_access_time((*it)->LastAccessDate());
-      if (earliest_access_time.is_null() ||
-          cookie_access_time < earliest_access_time)
-        earliest_access_time = cookie_access_time;
+      if (earliest_access_time_.is_null() ||
+          cookie_access_time < earliest_access_time_)
+        earliest_access_time_ = cookie_access_time;
     } else {
       LOG(ERROR) << base::StringPrintf("Found cookies with duplicate creation "
                                        "times in backing store: "
                                        "{name='%s', domain='%s', path='%s'}",
                                        (*it)->Name().c_str(),
                                        (*it)->Domain().c_str(),
                                        (*it)->Path().c_str());
       // We've been given ownership of the cookie and are throwing it
       // away; reclaim the space.
       delete (*it);
     }
   }
-  earliest_access_time_= earliest_access_time;
 
   // After importing cookies from the PersistentCookieStore, verify that
   // none of our other constraints are violated.
-  //
   // In particular, the backing store might have given us duplicate cookies.
+
+  // This method could be called multiple times due to priority loading, thus
+  // cookies loaded in previous runs will be validated again, but this is OK
+  // since they are expected to be much fewer than total DB.
   EnsureCookiesMapIsValid();
 }
 
 void CookieMonster::InvokeQueue() {
   while (true) {
     scoped_refptr<CookieMonsterTask> request_task;
     {
       base::AutoLock autolock(lock_);
       if (queue_.empty()) {
         loaded_ = true;
+        creation_times_.clear();
+        keys_loaded_.clear();
         break;
       }
       request_task = queue_.front();
       queue_.pop();
     }
     request_task->Run();
   }
 }
 
 void CookieMonster::EnsureCookiesMapIsValid() {
@@ skipping 1228 matching lines @@
 std::string CookieMonster::CanonicalCookie::DebugString() const {
   return base::StringPrintf(
       "name: %s value: %s domain: %s path: %s creation: %"
       PRId64,
       name_.c_str(), value_.c_str(),
       domain_.c_str(), path_.c_str(),
       static_cast<int64>(creation_date_.ToTimeT()));
 }
 
 }  // namespace