Make CookieStore no longer threadsafe.

net/cookies/cookie_monster.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.
 
 // 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 326 matching lines @@
                              int last_access_threshold_milliseconds)
     : initialized_(false),
       started_fetching_all_cookies_(false),
       finished_fetching_all_cookies_(false),
       fetch_strategy_(kUnknownFetch),
       store_(store),
       last_access_threshold_(base::TimeDelta::FromMilliseconds(
           last_access_threshold_milliseconds)),
       delegate_(delegate),
       last_statistic_record_time_(base::Time::Now()),
-      persist_session_cookies_(false) {
+      persist_session_cookies_(false),
+      weak_ptr_factory_(this) {
   InitializeHistograms();
   cookieable_schemes_.insert(
       cookieable_schemes_.begin(), kDefaultCookieableSchemes,
       kDefaultCookieableSchemes + kDefaultCookieableSchemesCount);
 }
 
 // Task classes for queueing the coming request.
 
 class CookieMonster::CookieMonsterTask
     : public base::RefCountedThreadSafe<CookieMonsterTask> {
  public:
   // Runs the task and invokes the client callback on the thread that
   // originally constructed the task.
   virtual void Run() = 0;
 
  protected:
   explicit CookieMonsterTask(CookieMonster* cookie_monster);
   virtual ~CookieMonsterTask();
 
-  // Invokes the callback immediately, if the current thread is the one
-  // that originated the task, or queues the callback for execution on the
-  // appropriate thread. Maintains a reference to this CookieMonsterTask
-  // instance until the callback completes.
-  void InvokeCallback(base::Closure callback);
-
   CookieMonster* cookie_monster() { return cookie_monster_; }
 
  private:
   friend class base::RefCountedThreadSafe<CookieMonsterTask>;
 
   CookieMonster* cookie_monster_;
-  scoped_refptr<base::SingleThreadTaskRunner> thread_;
 
   DISALLOW_COPY_AND_ASSIGN(CookieMonsterTask);
 };
 
 CookieMonster::CookieMonsterTask::CookieMonsterTask(
     CookieMonster* cookie_monster)
-    : cookie_monster_(cookie_monster),
-      thread_(base::ThreadTaskRunnerHandle::Get()) {
-}
+    : cookie_monster_(cookie_monster) {}
 
 CookieMonster::CookieMonsterTask::~CookieMonsterTask() {
 }
 
-// Unfortunately, one cannot re-bind a Callback with parameters into a closure.
-// Therefore, the closure passed to InvokeCallback is a clumsy binding of
-// Callback::Run on a wrapped Callback instance. Since Callback is not
-// reference counted, we bind to an instance that is a member of the
-// CookieMonsterTask subclass. Then, we cannot simply post the callback to a
-// message loop because the underlying instance may be destroyed (along with the
-// CookieMonsterTask instance) in the interim. Therefore, we post a callback
-// bound to the CookieMonsterTask, which *is* reference counted (thus preventing
-// destruction of the original callback), and which invokes the closure (which
-// invokes the original callback with the returned data).
-void CookieMonster::CookieMonsterTask::InvokeCallback(base::Closure callback) {
-  if (thread_->BelongsToCurrentThread()) {
-    callback.Run();
-  } else {
-    thread_->PostTask(FROM_HERE, base::Bind(&CookieMonsterTask::InvokeCallback,
-                                            this, callback));
-  }
-}
-
 // Task class for SetCookieWithDetails call.
 class CookieMonster::SetCookieWithDetailsTask : public CookieMonsterTask {
  public:
   SetCookieWithDetailsTask(CookieMonster* cookie_monster,
                            const GURL& url,
                            const std::string& name,
                            const std::string& value,
                            const std::string& domain,
                            const std::string& path,
                            base::Time creation_time,
@@ skipping 44 matching lines @@
   SetCookiesCallback callback_;
 
   DISALLOW_COPY_AND_ASSIGN(SetCookieWithDetailsTask);
 };
 
 void CookieMonster::SetCookieWithDetailsTask::Run() {
   bool success = this->cookie_monster()->SetCookieWithDetails(
       url_, name_, value_, domain_, path_, creation_time_, expiration_time_,
       last_access_time_, secure_, http_only_, same_site_,
       enforce_strict_secure_, priority_);
-  if (!callback_.is_null()) {
-    this->InvokeCallback(base::Bind(&SetCookiesCallback::Run,
-                                    base::Unretained(&callback_), success));
-  }
+  if (!callback_.is_null())
+    callback_.Run(success);
 }
 
 // Task class for GetAllCookies call.
 class CookieMonster::GetAllCookiesTask : public CookieMonsterTask {
  public:
   GetAllCookiesTask(CookieMonster* cookie_monster,
                     const GetCookieListCallback& callback)
       : CookieMonsterTask(cookie_monster), callback_(callback) {}
 
   // CookieMonsterTask
   void Run() override;
 
  protected:
   ~GetAllCookiesTask() override {}
 
  private:
   GetCookieListCallback callback_;
 
   DISALLOW_COPY_AND_ASSIGN(GetAllCookiesTask);
 };
 
 void CookieMonster::GetAllCookiesTask::Run() {
   if (!callback_.is_null()) {
     CookieList cookies = this->cookie_monster()->GetAllCookies();
-    this->InvokeCallback(base::Bind(&GetCookieListCallback::Run,
-                                    base::Unretained(&callback_), cookies));
+    callback_.Run(cookies);
   }
 }
 
 // Task class for GetCookieListWithOptionsAsync call.
 class CookieMonster::GetCookieListWithOptionsTask : public CookieMonsterTask {
  public:
   GetCookieListWithOptionsTask(CookieMonster* cookie_monster,
                                const GURL& url,
                                const CookieOptions& options,
                                const GetCookieListCallback& callback)
@@ skipping 13 matching lines @@
   CookieOptions options_;
   GetCookieListCallback callback_;
 
   DISALLOW_COPY_AND_ASSIGN(GetCookieListWithOptionsTask);
 };
 
 void CookieMonster::GetCookieListWithOptionsTask::Run() {
   if (!callback_.is_null()) {
     CookieList cookies =
         this->cookie_monster()->GetCookieListWithOptions(url_, options_);
-    this->InvokeCallback(base::Bind(&GetCookieListCallback::Run,
-                                    base::Unretained(&callback_), cookies));
+    callback_.Run(cookies);
   }
 }
 
 template <typename Result>
 struct CallbackType {
   typedef base::Callback<void(Result)> Type;
 };
 
 template <>
 struct CallbackType<void> {
@@ skipping 10 matching lines @@
 
   // CookieMonsterTask:
   void Run() override;
 
  protected:
   ~DeleteTask() override;
 
  private:
   // Runs the delete task and returns a result.
   virtual Result RunDeleteTask() = 0;
+  // Runs the delete task and then returns a callback to be called after
+  // flushing the persistent store.
+  // TODO(mmenke): This seems like a pretty ugly and needlessly confusing API.
+  // Simplify it?
   base::Closure RunDeleteTaskAndBindCallback();

[Mike West, 2016/03/01 13:59:28]

I'd suggest doing so, but in a separate CL. No need to refactor everything at
once.

-  void FlushDone(const base::Closure& callback);
 
   typename CallbackType<Result>::Type callback_;
 
   DISALLOW_COPY_AND_ASSIGN(DeleteTask);
 };
 
 template <typename Result>
 CookieMonster::DeleteTask<Result>::~DeleteTask() {
 }
 
 template <typename Result>
 base::Closure
 CookieMonster::DeleteTask<Result>::RunDeleteTaskAndBindCallback() {
   Result result = RunDeleteTask();
   if (callback_.is_null())
     return base::Closure();
   return base::Bind(callback_, result);
 }
 
 template <>
 base::Closure CookieMonster::DeleteTask<void>::RunDeleteTaskAndBindCallback() {
   RunDeleteTask();
   return callback_;
 }
 
 template <typename Result>
 void CookieMonster::DeleteTask<Result>::Run() {
-  this->cookie_monster()->FlushStore(base::Bind(
-      &DeleteTask<Result>::FlushDone, this, RunDeleteTaskAndBindCallback()));
-}
-
-template <typename Result>
-void CookieMonster::DeleteTask<Result>::FlushDone(
-    const base::Closure& callback) {
+  base::Closure callback = RunDeleteTaskAndBindCallback();
   if (!callback.is_null()) {
-    this->InvokeCallback(callback);
+    callback = base::Bind(
+        &CookieMonster::RunCallback,
+        this->cookie_monster()->weak_ptr_factory_.GetWeakPtr(), callback);
   }
+  this->cookie_monster()->FlushStore(callback);
 }
 
 // Task class for DeleteAllCreatedBetween call.
 class CookieMonster::DeleteAllCreatedBetweenTask : public DeleteTask<int> {
  public:
   DeleteAllCreatedBetweenTask(CookieMonster* cookie_monster,
                               const Time& delete_begin,
                               const Time& delete_end,
                               const DeleteCallback& callback)
       : DeleteTask<int>(cookie_monster, callback),
@@ skipping 100 matching lines @@
   std::string cookie_line_;
   CookieOptions options_;
   SetCookiesCallback callback_;
 
   DISALLOW_COPY_AND_ASSIGN(SetCookieWithOptionsTask);
 };
 
 void CookieMonster::SetCookieWithOptionsTask::Run() {
   bool result = this->cookie_monster()->SetCookieWithOptions(url_, cookie_line_,
                                                              options_);
-  if (!callback_.is_null()) {
-    this->InvokeCallback(base::Bind(&SetCookiesCallback::Run,
-                                    base::Unretained(&callback_), result));
-  }
+  if (!callback_.is_null())
+    callback_.Run(result);
 }
 
 // Task class for SetAllCookies call.
 class CookieMonster::SetAllCookiesTask : public CookieMonsterTask {
  public:
   SetAllCookiesTask(CookieMonster* cookie_monster,
                     const CookieList& list,
                     const SetCookiesCallback& callback)
       : CookieMonsterTask(cookie_monster), list_(list), callback_(callback) {}
 
@@ skipping 19 matching lines @@
 
   for (CookieList::const_iterator it = negative_diff.begin();
        it != negative_diff.end(); ++it) {
     this->cookie_monster()->DeleteCanonicalCookie(*it);
   }
 
   bool result = true;
   if (positive_diff.size() > 0)
     result = this->cookie_monster()->SetCanonicalCookies(list_);
 
-  if (!callback_.is_null()) {
-    this->InvokeCallback(base::Bind(&SetCookiesCallback::Run,
-                                    base::Unretained(&callback_), result));
-  }
+  if (!callback_.is_null())
+    callback_.Run(result);
 }
 
 // Task class for GetCookiesWithOptions call.
 class CookieMonster::GetCookiesWithOptionsTask : public CookieMonsterTask {
  public:
   GetCookiesWithOptionsTask(CookieMonster* cookie_monster,
                             const GURL& url,
                             const CookieOptions& options,
                             const GetCookiesCallback& callback)
       : CookieMonsterTask(cookie_monster),
@@ skipping 15 matching lines @@
   DISALLOW_COPY_AND_ASSIGN(GetCookiesWithOptionsTask);
 };
 
 void CookieMonster::GetCookiesWithOptionsTask::Run() {
   // TODO(mkwst): Remove ScopedTracker below once crbug.com/456373 is fixed.
   tracked_objects::ScopedTracker tracking_profile(
       FROM_HERE_WITH_EXPLICIT_FUNCTION(
           "456373 CookieMonster::GetCookiesWithOptionsTask::Run"));
   std::string cookie =
       this->cookie_monster()->GetCookiesWithOptions(url_, options_);
-  if (!callback_.is_null()) {
-    this->InvokeCallback(base::Bind(&GetCookiesCallback::Run,
-                                    base::Unretained(&callback_), cookie));
-  }
+  if (!callback_.is_null())
+    callback_.Run(cookie);
 }
 
 // Task class for DeleteCookie call.
 class CookieMonster::DeleteCookieTask : public DeleteTask<void> {
  public:
   DeleteCookieTask(CookieMonster* cookie_monster,
                    const GURL& url,
                    const std::string& cookie_name,
                    const base::Closure& callback)
       : DeleteTask<void>(cookie_monster, callback),
@@ skipping 56 matching lines @@
     CookiePriority priority,
     const SetCookiesCallback& callback) {
   scoped_refptr<SetCookieWithDetailsTask> task = new SetCookieWithDetailsTask(
       this, url, name, value, domain, path, creation_time, expiration_time,
       last_access_time, secure, http_only, same_site, enforce_strict_secure,
       priority, callback);
   DoCookieTaskForURL(task, url);
 }
 
 void CookieMonster::FlushStore(const base::Closure& callback) {
-  base::AutoLock autolock(lock_);
+  DCHECK(thread_checker_.CalledOnValidThread());
   if (initialized_ && store_.get())
     store_->Flush(callback);
   else if (!callback.is_null())
     base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE, callback);
 }
 
 void CookieMonster::SetForceKeepSessionState() {
+  DCHECK(thread_checker_.CalledOnValidThread());
   if (store_)
     store_->SetForceKeepSessionState();
 }
 
 void CookieMonster::SetAllCookiesAsync(const CookieList& list,
                                        const SetCookiesCallback& callback) {
   scoped_refptr<SetAllCookiesTask> task =
       new SetAllCookiesTask(this, list, callback);
   DoCookieTask(task);
 }
@@ skipping 77 matching lines @@
 void CookieMonster::DeleteSessionCookiesAsync(
     const CookieStore::DeleteCallback& callback) {
   scoped_refptr<DeleteSessionCookiesTask> task =
       new DeleteSessionCookiesTask(this, callback);
 
   DoCookieTask(task);
 }
 
 void CookieMonster::SetCookieableSchemes(
     const std::vector<std::string>& schemes) {
-  base::AutoLock autolock(lock_);
+  DCHECK(thread_checker_.CalledOnValidThread());
 
   // Calls to this method will have no effect if made after a WebView or
   // CookieManager instance has been created.
   if (initialized_)
     return;
 
   cookieable_schemes_ = schemes;
 }
 
 // This function must be called before the CookieMonster is used.
 void CookieMonster::SetPersistSessionCookies(bool persist_session_cookies) {
+  DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(!initialized_);
   persist_session_cookies_ = persist_session_cookies;
 }
 
 bool CookieMonster::IsCookieableScheme(const std::string& scheme) {
-  base::AutoLock autolock(lock_);
+  DCHECK(thread_checker_.CalledOnValidThread());
 
   return std::find(cookieable_schemes_.begin(), cookieable_schemes_.end(),
                    scheme) != cookieable_schemes_.end();
 }
 
 const char* const CookieMonster::kDefaultCookieableSchemes[] = {"http", "https",
                                                                 "ws", "wss"};
 const int CookieMonster::kDefaultCookieableSchemesCount =
     arraysize(kDefaultCookieableSchemes);
 
 scoped_ptr<CookieStore::CookieChangedSubscription>
 CookieMonster::AddCallbackForCookie(const GURL& gurl,
                                     const std::string& name,
                                     const CookieChangedCallback& callback) {
-  base::AutoLock autolock(lock_);
+  DCHECK(thread_checker_.CalledOnValidThread());
   std::pair<GURL, std::string> key(gurl, name);
   if (hook_map_.count(key) == 0)
     hook_map_[key] = make_linked_ptr(new CookieChangedCallbackList());
   return hook_map_[key]->Add(
       base::Bind(&RunAsync, base::ThreadTaskRunnerHandle::Get(), callback));
 }
 
 CookieMonster::~CookieMonster() {
-  // Clean up cookies
+  DCHECK(thread_checker_.CalledOnValidThread());
 
-  // InternalDeleteCookie expects the lock to be held, even though there can be
-  // no contention here.
-  base::AutoLock autolock(lock_);
   for (CookieMap::iterator cookie_it = cookies_.begin();
        cookie_it != cookies_.end();) {
     CookieMap::iterator current_cookie_it = cookie_it;
     ++cookie_it;
     InternalDeleteCookie(current_cookie_it, false /* sync_to_store */,
                          DELETE_COOKIE_DONT_RECORD);
   }
 }
 
 bool CookieMonster::SetCookieWithDetails(const GURL& url,
                                          const std::string& name,
                                          const std::string& value,
                                          const std::string& domain,
                                          const std::string& path,
                                          base::Time creation_time,
                                          base::Time expiration_time,
                                          base::Time last_access_time,
                                          bool secure,
                                          bool http_only,
                                          bool same_site,
                                          bool enforce_strict_secure,
                                          CookiePriority priority) {
-  base::AutoLock autolock(lock_);
+  DCHECK(thread_checker_.CalledOnValidThread());
 
   if (!HasCookieableScheme(url))
     return false;
 
   // TODO(mmenke): This class assumes each cookie to have a unique creation
   // time. Allowing the caller to set the creation time violates that
   // assumption. Worth fixing? Worth noting that time changes between browser
   // restarts can cause the same issue.
   base::Time actual_creation_time = creation_time;
   if (creation_time.is_null()) {
@@ skipping 13 matching lines @@
 
   CookieOptions options;
   options.set_include_httponly();
   options.set_include_same_site();
   if (enforce_strict_secure)
     options.set_enforce_strict_secure();
   return SetCanonicalCookie(std::move(cc), options);
 }
 
 CookieList CookieMonster::GetAllCookies() {
-  base::AutoLock autolock(lock_);
+  DCHECK(thread_checker_.CalledOnValidThread());
 
   // This function is being called to scrape the cookie list for management UI
   // or similar.  We shouldn't show expired cookies in this list since it will
   // just be confusing to users, and this function is called rarely enough (and
   // is already slow enough) that it's OK to take the time to garbage collect
   // the expired cookies now.
   //
   // Note that this does not prune cookies to be below our limits (if we've
   // exceeded them) the way that calling GarbageCollect() would.
   GarbageCollectExpired(
@@ skipping 12 matching lines @@
   for (std::vector<CanonicalCookie*>::const_iterator it = cookie_ptrs.begin();
        it != cookie_ptrs.end(); ++it)
     cookie_list.push_back(**it);
 
   return cookie_list;
 }
 
 CookieList CookieMonster::GetCookieListWithOptions(
     const GURL& url,
     const CookieOptions& options) {
-  base::AutoLock autolock(lock_);
+  DCHECK(thread_checker_.CalledOnValidThread());
 
   CookieList cookies;
   if (!HasCookieableScheme(url))
     return cookies;
 
   std::vector<CanonicalCookie*> cookie_ptrs;
   FindCookiesForHostAndDomain(url, options, &cookie_ptrs);
   std::sort(cookie_ptrs.begin(), cookie_ptrs.end(), CookieSorter);
 
   cookies.reserve(cookie_ptrs.size());
   for (std::vector<CanonicalCookie*>::const_iterator it = cookie_ptrs.begin();
        it != cookie_ptrs.end(); it++)
     cookies.push_back(**it);
 
   return cookies;
 }
 
 int CookieMonster::DeleteAllCreatedBetween(const Time& delete_begin,
                                            const Time& delete_end) {
-  base::AutoLock autolock(lock_);
+  DCHECK(thread_checker_.CalledOnValidThread());
 
   int num_deleted = 0;
   for (CookieMap::iterator it = cookies_.begin(); it != cookies_.end();) {
     CookieMap::iterator curit = it;
     CanonicalCookie* cc = curit->second;
     ++it;
 
     if (cc->CreationDate() >= delete_begin &&
         (delete_end.is_null() || cc->CreationDate() < delete_end)) {
       InternalDeleteCookie(curit, true, /*sync_to_store*/
                            DELETE_COOKIE_EXPLICIT);
       ++num_deleted;
     }
   }
 
   return num_deleted;
 }
 
 int CookieMonster::DeleteAllCreatedBetweenForHost(const Time delete_begin,
                                                   const Time delete_end,
                                                   const GURL& url) {
-  base::AutoLock autolock(lock_);
+  DCHECK(thread_checker_.CalledOnValidThread());
 
   if (!HasCookieableScheme(url))
     return 0;
 
   const std::string host(url.host());
 
   // We store host cookies in the store by their canonical host name;
   // domain cookies are stored with a leading ".".  So this is a pretty
   // simple lookup and per-cookie delete.
   int num_deleted = 0;
@@ skipping 15 matching lines @@
       InternalDeleteCookie(curit, true, DELETE_COOKIE_EXPLICIT);
     }
   }
   return num_deleted;
 }
 
 
 bool CookieMonster::SetCookieWithOptions(const GURL& url,
                                          const std::string& cookie_line,
                                          const CookieOptions& options) {
-  base::AutoLock autolock(lock_);
+  DCHECK(thread_checker_.CalledOnValidThread());
 
   if (!HasCookieableScheme(url)) {
     return false;
   }
 
   return SetCookieWithCreationTimeAndOptions(url, cookie_line, Time(), options);
 }
 
 std::string CookieMonster::GetCookiesWithOptions(const GURL& url,
                                                  const CookieOptions& options) {
-  base::AutoLock autolock(lock_);
+  DCHECK(thread_checker_.CalledOnValidThread());
 
   if (!HasCookieableScheme(url))
     return std::string();
 
   std::vector<CanonicalCookie*> cookies;
   FindCookiesForHostAndDomain(url, options, &cookies);
   std::sort(cookies.begin(), cookies.end(), CookieSorter);
 
   std::string cookie_line = BuildCookieLine(cookies);
 
   VLOG(kVlogGetCookies) << "GetCookies() result: " << cookie_line;
 
   return cookie_line;
 }
 
 void CookieMonster::DeleteCookie(const GURL& url,
                                  const std::string& cookie_name) {
-  base::AutoLock autolock(lock_);
+  DCHECK(thread_checker_.CalledOnValidThread());
 
   if (!HasCookieableScheme(url))
     return;
 
   CookieOptions options;
   options.set_include_httponly();
   options.set_include_same_site();
   // Get the cookies for this host and its domain(s).
   std::vector<CanonicalCookie*> cookies;
   FindCookiesForHostAndDomain(url, options, &cookies);
@@ skipping 11 matching lines @@
   for (CookieMap::iterator it = cookies_.begin(); it != cookies_.end();) {
     CookieMap::iterator curit = it;
     ++it;
     if (matching_cookies.find(curit->second) != matching_cookies.end()) {
       InternalDeleteCookie(curit, true, DELETE_COOKIE_EXPLICIT);
     }
   }
 }
 
 int CookieMonster::DeleteCanonicalCookie(const CanonicalCookie& cookie) {
-  base::AutoLock autolock(lock_);
+  DCHECK(thread_checker_.CalledOnValidThread());
 
   for (CookieMapItPair its = cookies_.equal_range(GetKey(cookie.Domain()));
        its.first != its.second; ++its.first) {
     // The creation date acts as the unique index...
     if (its.first->second->CreationDate() == cookie.CreationDate()) {
       InternalDeleteCookie(its.first, true, DELETE_COOKIE_EXPLICIT);
       return 1;
     }
   }
   return 0;
 }
 
 bool CookieMonster::SetCookieWithCreationTime(const GURL& url,
                                               const std::string& cookie_line,
                                               const base::Time& creation_time) {
+  DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(!store_.get()) << "This method is only to be used by unit-tests.";
-  base::AutoLock autolock(lock_);
 
   if (!HasCookieableScheme(url)) {
     return false;
   }
 
   MarkCookieStoreAsInitialized();
   if (ShouldFetchAllCookiesWhenFetchingAnyCookie())
     FetchAllCookiesIfNecessary();
 
   return SetCookieWithCreationTimeAndOptions(url, cookie_line, creation_time,
                                              CookieOptions());
 }
 
 int CookieMonster::DeleteSessionCookies() {
-  base::AutoLock autolock(lock_);
+  DCHECK(thread_checker_.CalledOnValidThread());
 
   int num_deleted = 0;
   for (CookieMap::iterator it = cookies_.begin(); it != cookies_.end();) {
     CookieMap::iterator curit = it;
     CanonicalCookie* cc = curit->second;
     ++it;
 
     if (!cc->IsPersistent()) {
       InternalDeleteCookie(curit, true, /*sync_to_store*/
                            DELETE_COOKIE_EXPIRED);
       ++num_deleted;
     }
   }
 
   return num_deleted;
 }
 
 void CookieMonster::MarkCookieStoreAsInitialized() {
+  DCHECK(thread_checker_.CalledOnValidThread());
   initialized_ = true;
 }
 
 void CookieMonster::FetchAllCookiesIfNecessary() {
+  DCHECK(thread_checker_.CalledOnValidThread());
   if (store_.get() && !started_fetching_all_cookies_) {
     started_fetching_all_cookies_ = true;
     FetchAllCookies();
   }
 }
 
 void CookieMonster::FetchAllCookies() {
+  DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(store_.get()) << "Store must exist to initialize";
   DCHECK(!finished_fetching_all_cookies_)
       << "All cookies have already been fetched.";
 
   // We bind in the current time so that we can report the wall-clock time for
   // loading cookies.
-  store_->Load(base::Bind(&CookieMonster::OnLoaded, this, TimeTicks::Now()));
+  store_->Load(base::Bind(&CookieMonster::OnLoaded,
+                          weak_ptr_factory_.GetWeakPtr(), TimeTicks::Now()));
 }
 
 bool CookieMonster::ShouldFetchAllCookiesWhenFetchingAnyCookie() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+
   if (fetch_strategy_ == kUnknownFetch) {
     const std::string group_name =
         base::FieldTrialList::FindFullName(kCookieMonsterFetchStrategyName);
     if (group_name == kFetchWhenNecessaryName) {
       fetch_strategy_ = kFetchWhenNecessary;
     } else if (group_name == kAlwaysFetchName) {
       fetch_strategy_ = kAlwaysFetch;
     } else {
       // The logic in the conditional is redundant, but it makes trials of
       // the Finch experiment more explicit.
       fetch_strategy_ = kAlwaysFetch;
     }
   }
 
   return fetch_strategy_ == kAlwaysFetch;
 }
 
 void CookieMonster::OnLoaded(TimeTicks beginning_time,
                              const std::vector<CanonicalCookie*>& cookies) {
+  DCHECK(thread_checker_.CalledOnValidThread());
   StoreLoadedCookies(cookies);
   histogram_time_blocked_on_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) {
-  // This function does its own separate locking.
+  DCHECK(thread_checker_.CalledOnValidThread());
+
   StoreLoadedCookies(cookies);
 
-  std::deque<scoped_refptr<CookieMonsterTask>> tasks_pending_for_key;
+  auto tasks_pending_for_key = tasks_pending_for_key_.find(key);
 
-  // We need to do this repeatedly until no more tasks were added to the queue
-  // during the period where we release the lock.
-  while (true) {
-    {
-      base::AutoLock autolock(lock_);
-      std::map<std::string,
-               std::deque<scoped_refptr<CookieMonsterTask>>>::iterator it =
-          tasks_pending_for_key_.find(key);
-      if (it == tasks_pending_for_key_.end()) {
-        keys_loaded_.insert(key);
-        return;
-      }
-      if (it->second.empty()) {
-        keys_loaded_.insert(key);
-        tasks_pending_for_key_.erase(it);
-        return;
-      }
-      it->second.swap(tasks_pending_for_key);
-    }
+  // TODO(mmenke): Can this be turned into a DCHECK?

[Mike West, 2016/03/01 13:59:28]

I think so. There shouldn't be any case in which we call `OnKeyLoaded` without
having asked for a key to be loaded at some point.

[mmenke, 2016/03/01 16:03:45]

My CL to re-order things may change that property (This comment pre-dates that
CL by over a month), but still may be able to do better.

+  if (tasks_pending_for_key == tasks_pending_for_key_.end())
+    return;
 
-    while (!tasks_pending_for_key.empty()) {
-      scoped_refptr<CookieMonsterTask> task = tasks_pending_for_key.front();
-      task->Run();
-      tasks_pending_for_key.pop_front();
-    }
+  // Run all tasks for the key. Note that running a task can result it multiple

[Mike West, 2016/03/01 13:59:28]

Nit: s/it multiple/in multiple/

[mmenke, 2016/03/01 16:03:45]

Done.

+  // tasks being added to the back of the deque.
+  while (!tasks_pending_for_key->second.empty()) {
+    // Removing a task from the deque before running it isn't strictly
+    // necessary, but just seems like a good idea.

[Mike West, 2016/03/01 13:59:28]

Nit: I'd drop the comment. I'd agree that it's a good idea, and doesn't seem
like it needs justification. :)

[mmenke, 2016/03/01 16:03:45]

Done.

+    scoped_refptr<CookieMonsterTask> task =
+        tasks_pending_for_key->second.front();
+    tasks_pending_for_key->second.pop_front();
+
+    task->Run();
   }
+
+  tasks_pending_for_key_.erase(tasks_pending_for_key);
+
+  // This has to be done last, in case running a task queues a new task for the
+  // key, to ensure tasks are run in the correct order.
+  keys_loaded_.insert(key);
 }
 
 void CookieMonster::StoreLoadedCookies(
     const std::vector<CanonicalCookie*>& cookies) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+
   // TODO(erikwright): Remove ScopedTracker below once crbug.com/457528 is
   // fixed.
   tracked_objects::ScopedTracker tracking_profile(
       FROM_HERE_WITH_EXPLICIT_FUNCTION(
           "457528 CookieMonster::StoreLoadedCookies"));
 
-  // 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_);

[Mike West, 2016/03/01 13:59:28]

Appropos of nothing, I really like seeing all these AutoLock lines being
removed. :)

-
+  // Even if a key is expired, insert it so it can be garbage collected,
+  // removed, and sync'd.
   CookieItVector cookies_with_control_chars;
 
   for (std::vector<CanonicalCookie*>::const_iterator it = cookies.begin();
        it != cookies.end(); ++it) {
     int64_t cookie_creation_time = (*it)->CreationDate().ToInternalValue();
 
     if (creation_times_.insert(cookie_creation_time).second) {
       CookieMap::iterator inserted =
           InternalInsertCookie(GetKey((*it)->Domain()), *it, false);
       const Time cookie_access_time((*it)->LastAccessDate());
@@ skipping 32 matching lines @@
   // 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 (tasks_pending_.empty()) {
-        finished_fetching_all_cookies_ = true;
-        creation_times_.clear();
-        keys_loaded_.clear();
-        break;
-      }
-      request_task = tasks_pending_.front();
-      tasks_pending_.pop();
-    }
+  DCHECK(thread_checker_.CalledOnValidThread());
+
+  while (!tasks_pending_.empty()) {
+    scoped_refptr<CookieMonsterTask> request_task = tasks_pending_.front();
+    tasks_pending_.pop();
     request_task->Run();
   }
+
+  finished_fetching_all_cookies_ = true;
+  creation_times_.clear();
+  keys_loaded_.clear();
 }
 
 void CookieMonster::EnsureCookiesMapIsValid() {
-  lock_.AssertAcquired();
+  DCHECK(thread_checker_.CalledOnValidThread());
 
   // Iterate through all the of the cookies, grouped by host.
   CookieMap::iterator prev_range_end = cookies_.begin();
   while (prev_range_end != cookies_.end()) {
     CookieMap::iterator cur_range_begin = prev_range_end;
     const std::string key = cur_range_begin->first;  // Keep a copy.
     CookieMap::iterator cur_range_end = cookies_.upper_bound(key);
     prev_range_end = cur_range_end;
 
     // Ensure no equivalent cookies for this host.
     TrimDuplicateCookiesForKey(key, cur_range_begin, cur_range_end);
   }
 }
 
 void CookieMonster::TrimDuplicateCookiesForKey(const std::string& key,
                                                CookieMap::iterator begin,
                                                CookieMap::iterator end) {
-  lock_.AssertAcquired();
+  DCHECK(thread_checker_.CalledOnValidThread());
 
   // Set of cookies ordered by creation time.
   typedef std::set<CookieMap::iterator, OrderByCreationTimeDesc> CookieSet;
 
   // Helper map we populate to find the duplicates.
   typedef std::map<CookieSignature, CookieSet> EquivalenceMap;
   EquivalenceMap equivalent_cookies;
 
   // The number of duplicate cookies that have been found.
   int num_duplicates = 0;
@@ skipping 55 matching lines @@
                            DELETE_COOKIE_DUPLICATE_IN_BACKING_STORE);
     }
   }
   DCHECK_EQ(num_duplicates, num_duplicates_found);
 }
 
 void CookieMonster::FindCookiesForHostAndDomain(
     const GURL& url,
     const CookieOptions& options,
     std::vector<CanonicalCookie*>* cookies) {
-  lock_.AssertAcquired();
+  DCHECK(thread_checker_.CalledOnValidThread());
 
   const Time current_time(CurrentTime());
 
   // Probe to save statistics relatively frequently.  We do it here rather
   // than in the set path as many websites won't set cookies, and we
   // want to collect statistics whenever the browser's being used.
   RecordPeriodicStats(current_time);
 
   // Can just dispatch to FindCookiesForKey
   const std::string key(GetKey(url.host()));
   FindCookiesForKey(key, url, options, current_time, cookies);
 }
 
 void CookieMonster::FindCookiesForKey(const std::string& key,
                                       const GURL& url,
                                       const CookieOptions& options,
                                       const Time& current,
                                       std::vector<CanonicalCookie*>* cookies) {
-  lock_.AssertAcquired();
+  DCHECK(thread_checker_.CalledOnValidThread());
 
   for (CookieMapItPair its = cookies_.equal_range(key);
        its.first != its.second;) {
     CookieMap::iterator curit = its.first;
     CanonicalCookie* cc = curit->second;
     ++its.first;
 
     // If the cookie is expired, delete it.
     if (cc->IsExpired(current)) {
       InternalDeleteCookie(curit, true, DELETE_COOKIE_EXPIRED);
@@ skipping 13 matching lines @@
     }
     cookies->push_back(cc);
   }
 }
 
 bool CookieMonster::DeleteAnyEquivalentCookie(const std::string& key,
                                               const CanonicalCookie& ecc,
                                               bool skip_httponly,
                                               bool already_expired,
                                               bool enforce_strict_secure) {
-  lock_.AssertAcquired();
+  DCHECK(thread_checker_.CalledOnValidThread());
 
   bool found_equivalent_cookie = false;
   bool skipped_httponly = false;
   bool skipped_secure_cookie = false;
 
   histogram_cookie_delete_equivalent_->Add(COOKIE_DELETE_EQUIVALENT_ATTEMPT);
 
   for (CookieMapItPair its = cookies_.equal_range(key);
        its.first != its.second;) {
     CookieMap::iterator curit = its.first;
@@ skipping 42 matching lines @@
       found_equivalent_cookie = true;
     }
   }
   return skipped_httponly || skipped_secure_cookie;
 }
 
 CookieMonster::CookieMap::iterator CookieMonster::InternalInsertCookie(
     const std::string& key,
     CanonicalCookie* cc,
     bool sync_to_store) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+
   // TODO(mkwst): Remove ScopedTracker below once crbug.com/456373 is fixed.
   tracked_objects::ScopedTracker tracking_profile(
       FROM_HERE_WITH_EXPLICIT_FUNCTION(
           "456373 CookieMonster::InternalInsertCookie"));
-  lock_.AssertAcquired();
 
   if ((cc->IsPersistent() || persist_session_cookies_) && store_.get() &&
       sync_to_store)
     store_->AddCookie(*cc);
   CookieMap::iterator inserted =
       cookies_.insert(CookieMap::value_type(key, cc));
   if (delegate_.get()) {
     delegate_->OnCookieChanged(*cc, false,
                                CookieMonsterDelegate::CHANGE_COOKIE_EXPLICIT);
   }
@@ skipping 17 matching lines @@
                          : COOKIE_SOURCE_NONSECURE_COOKIE_CRYPTOGRAPHIC_SCHEME;
     } else {
       cookie_source_sample =
           cc->IsSecure()
               ? COOKIE_SOURCE_SECURE_COOKIE_NONCRYPTOGRAPHIC_SCHEME
               : COOKIE_SOURCE_NONSECURE_COOKIE_NONCRYPTOGRAPHIC_SCHEME;
     }
     histogram_cookie_source_scheme_->Add(cookie_source_sample);
   }
 
-  RunCallbacks(*cc, false);
+  RunCookieChangedCallbacks(*cc, false);
 
   return inserted;
 }
 
 bool CookieMonster::SetCookieWithCreationTimeAndOptions(
     const GURL& url,
     const std::string& cookie_line,
     const Time& creation_time_or_null,
     const CookieOptions& options) {
-  lock_.AssertAcquired();
+  DCHECK(thread_checker_.CalledOnValidThread());
 
   VLOG(kVlogSetCookies) << "SetCookie() line: " << cookie_line;
 
   Time creation_time = creation_time_or_null;
   if (creation_time.is_null()) {
     creation_time = CurrentTime();
     last_time_seen_ = creation_time;
   }
 
   scoped_ptr<CanonicalCookie> cc(
       CanonicalCookie::Create(url, cookie_line, creation_time, options));
 
   if (!cc.get()) {
     VLOG(kVlogSetCookies) << "WARNING: Failed to allocate CanonicalCookie";
     return false;
   }
   return SetCanonicalCookie(std::move(cc), options);
 }
 
 bool CookieMonster::SetCanonicalCookie(scoped_ptr<CanonicalCookie> cc,
                                        const CookieOptions& options) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+
   Time creation_time = cc->CreationDate();
   const std::string key(GetKey(cc->Domain()));
   bool already_expired = cc->IsExpired(creation_time);
 
   if (DeleteAnyEquivalentCookie(key, *cc, options.exclude_httponly(),
                                 already_expired,
                                 options.enforce_strict_secure())) {
     std::string error;
     if (options.enforce_strict_secure()) {
       error =
@@ skipping 28 matching lines @@
   // querying a cookie.  Since setting a cookie can put us over our limits,
   // make sure that we garbage collect...  We can also make the assumption that
   // if a cookie was set, in the common case it will be used soon after,
   // and we will purge the expired cookies in GetCookies().
   GarbageCollect(creation_time, key, options.enforce_strict_secure());
 
   return true;
 }
 
 bool CookieMonster::SetCanonicalCookies(const CookieList& list) {
-  base::AutoLock autolock(lock_);
+  DCHECK(thread_checker_.CalledOnValidThread());
 
   CookieOptions options;
   options.set_include_httponly();
 
   for (const auto& cookie : list) {
     if (!SetCanonicalCookie(make_scoped_ptr(new CanonicalCookie(cookie)),
                             options)) {
       return false;
     }
   }
 
   return true;
 }
 
 void CookieMonster::InternalUpdateCookieAccessTime(CanonicalCookie* cc,
                                                    const Time& current) {
-  lock_.AssertAcquired();
+  DCHECK(thread_checker_.CalledOnValidThread());
 
   // Based off the Mozilla code.  When a cookie has been accessed recently,
   // don't bother updating its access time again.  This reduces the number of
   // updates we do during pageload, which in turn reduces the chance our storage
   // backend will hit its batch thresholds and be forced to update.
   if ((current - cc->LastAccessDate()) < last_access_threshold_)
     return;
 
   cc->SetLastAccessDate(current);
   if ((cc->IsPersistent() || persist_session_cookies_) && store_.get())
     store_->UpdateCookieAccessTime(*cc);
 }
 
 // InternalDeleteCookies must not invalidate iterators other than the one being
 // deleted.
 void CookieMonster::InternalDeleteCookie(CookieMap::iterator it,
                                          bool sync_to_store,
                                          DeletionCause deletion_cause) {
-  lock_.AssertAcquired();
+  DCHECK(thread_checker_.CalledOnValidThread());
 
   // Ideally, this would be asserted up where we define ChangeCauseMapping,
   // but DeletionCause's visibility (or lack thereof) forces us to make
   // this check here.
   static_assert(arraysize(ChangeCauseMapping) == DELETE_COOKIE_LAST_ENTRY + 1,
                 "ChangeCauseMapping size should match DeletionCause size");
 
   // See InitializeHistograms() for details.
   if (deletion_cause != DELETE_COOKIE_DONT_RECORD)
     histogram_cookie_deletion_cause_->Add(deletion_cause);
 
   CanonicalCookie* cc = it->second;
   VLOG(kVlogSetCookies) << "InternalDeleteCookie()"
                         << ", cause:" << deletion_cause
                         << ", cc: " << cc->DebugString();
 
   if ((cc->IsPersistent() || persist_session_cookies_) && store_.get() &&
       sync_to_store)
     store_->DeleteCookie(*cc);
   if (delegate_.get()) {
     ChangeCausePair mapping = ChangeCauseMapping[deletion_cause];
 
     if (mapping.notify)
       delegate_->OnCookieChanged(*cc, true, mapping.cause);
   }
-  RunCallbacks(*cc, true);
+  RunCookieChangedCallbacks(*cc, true);
   cookies_.erase(it);
   delete cc;
 }
 
 // Domain expiry behavior is unchanged by key/expiry scheme (the
 // meaning of the key is different, but that's not visible to this routine).
 size_t CookieMonster::GarbageCollect(const Time& current,
                                      const std::string& key,
                                      bool enforce_strict_secure) {
-  lock_.AssertAcquired();
+  DCHECK(thread_checker_.CalledOnValidThread());
 
   size_t num_deleted = 0;
   Time safe_date(Time::Now() - TimeDelta::FromDays(kSafeFromGlobalPurgeDays));
 
   // Collect garbage for this key, minding cookie priorities.
   if (cookies_.count(key) > kDomainMaxCookies) {
     VLOG(kVlogGarbageCollection) << "GarbageCollect() key: " << key;
 
     CookieItVector cookie_its;
 
@@ skipping 209 matching lines @@
         current, DELETE_COOKIE_EVICTED_DOMAIN_POST_SAFE, it_purge_middle,
         it_purge_end);
     it_purge_begin = it_purge_end;
   }
   return num_deleted;
 }
 
 size_t CookieMonster::GarbageCollectExpired(const Time& current,
                                             const CookieMapItPair& itpair,
                                             CookieItVector* cookie_its) {
-  lock_.AssertAcquired();
+  DCHECK(thread_checker_.CalledOnValidThread());
 
   int num_deleted = 0;
   for (CookieMap::iterator it = itpair.first, end = itpair.second; it != end;) {
     CookieMap::iterator curit = it;
     ++it;
 
     if (curit->second->IsExpired(current)) {
       InternalDeleteCookie(curit, true, DELETE_COOKIE_EXPIRED);
       ++num_deleted;
     } else if (cookie_its) {
       cookie_its->push_back(curit);
     }
   }
 
   return num_deleted;
 }
 
 size_t CookieMonster::GarbageCollectNonSecure(
     const CookieItVector& valid_cookies,
     CookieItVector* cookie_its) {
-  lock_.AssertAcquired();
+  DCHECK(thread_checker_.CalledOnValidThread());
 
   size_t num_deleted = 0;
   for (const auto& curr_cookie_it : valid_cookies) {
     if (!curr_cookie_it->second->IsSecure()) {
       InternalDeleteCookie(curr_cookie_it, true, DELETE_COOKIE_NON_SECURE);
       ++num_deleted;
     } else if (cookie_its) {
       cookie_its->push_back(curr_cookie_it);
     }
   }
 
   return num_deleted;
 }
 
 size_t CookieMonster::GarbageCollectDeleteRange(
     const Time& current,
     DeletionCause cause,
     CookieItVector::iterator it_begin,
     CookieItVector::iterator it_end) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+
   for (CookieItVector::iterator it = it_begin; it != it_end; it++) {
     histogram_evicted_last_access_minutes_->Add(
         (current - (*it)->second->LastAccessDate()).InMinutes());
     InternalDeleteCookie((*it), true, cause);
   }
   return it_end - it_begin;
 }
 
 size_t CookieMonster::GarbageCollectLeastRecentlyAccessed(
     const base::Time& current,
     const base::Time& safe_date,
     size_t purge_goal,
     CookieItVector cookie_its) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+
   // Sorts up to *and including* |cookie_its[purge_goal]|, so
   // |earliest_access_time| will be properly assigned even if
   // |global_purge_it| == |cookie_its.begin() + purge_goal|.
   SortLeastRecentlyAccessed(cookie_its.begin(), cookie_its.end(), purge_goal);
   // Find boundary to cookies older than safe_date.
   CookieItVector::iterator global_purge_it = LowerBoundAccessDate(
       cookie_its.begin(), cookie_its.begin() + purge_goal, safe_date);
   // Only delete the old cookies, and if strict secure is enabled, delete
   // non-secure ones first.
   size_t num_deleted =
@@ skipping 21 matching lines @@
 // Arguably the right thing to do here is to make the key
 // algorithm dependent on the scheme, and make sure that the scheme is
 // available everywhere the key must be obtained (specfically at backing
 // store load time).  This would require either changing the backing store
 // database schema to include the scheme (far more trouble than it's worth), or
 // separating out file cookies into their own CookieMonster instance and
 // thus restricting each scheme to a single cookie monster (which might
 // be worth it, but is still too much trouble to solve what is currently a
 // non-problem).
 std::string CookieMonster::GetKey(const std::string& domain) const {
+  DCHECK(thread_checker_.CalledOnValidThread());
+
   std::string effective_domain(
       registry_controlled_domains::GetDomainAndRegistry(
           domain, registry_controlled_domains::INCLUDE_PRIVATE_REGISTRIES));
   if (effective_domain.empty())
     effective_domain = domain;
 
   if (!effective_domain.empty() && effective_domain[0] == '.')
     return effective_domain.substr(1);
   return effective_domain;
 }
 
 bool CookieMonster::HasCookieableScheme(const GURL& url) {
-  lock_.AssertAcquired();
+  DCHECK(thread_checker_.CalledOnValidThread());
 
   // Make sure the request is on a cookie-able url scheme.
   for (size_t i = 0; i < cookieable_schemes_.size(); ++i) {
     // We matched a scheme.
     if (url.SchemeIs(cookieable_schemes_[i].c_str())) {
       // We've matched a supported scheme.
       return true;
     }
   }
 
   // The scheme didn't match any in our whitelist.
   VLOG(kVlogPerCookieMonster)
       << "WARNING: Unsupported cookie scheme: " << url.scheme();
   return false;
 }
 
 // Test to see if stats should be recorded, and record them if so.
 // The goal here is to get sampling for the average browser-hour of
 // activity.  We won't take samples when the web isn't being surfed,
 // and when the web is being surfed, we'll take samples about every
 // kRecordStatisticsIntervalSeconds.
 // last_statistic_record_time_ is initialized to Now() rather than null
 // in the constructor so that we won't take statistics right after
 // startup, to avoid bias from browsers that are started but not used.
 void CookieMonster::RecordPeriodicStats(const base::Time& current_time) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+
   const base::TimeDelta kRecordStatisticsIntervalTime(
       base::TimeDelta::FromSeconds(kRecordStatisticsIntervalSeconds));
 
   // If we've taken statistics recently, return.
   if (current_time - last_statistic_record_time_ <=
       kRecordStatisticsIntervalTime) {
     return;
   }
 
   // See InitializeHistograms() for details.
@@ skipping 20 matching lines @@
 // Note that these variables refer to the same underlying histogram,
 // so we still race (but safely) with other CookieMonster instances
 // for accumulation.
 //
 // To do this we've expanded out the individual histogram macros calls,
 // with declarations of the variables in the class decl, initialization here
 // (done from the class constructor) and direct calls to the accumulation
 // methods where needed.  The specific histogram macro calls on which the
 // initialization is based are included in comments below.
 void CookieMonster::InitializeHistograms() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+
   // From UMA_HISTOGRAM_CUSTOM_COUNTS
   histogram_expiration_duration_minutes_ = base::Histogram::FactoryGet(
       "Cookie.ExpirationDurationMinutes", 1, kMinutesInTenYears, 50,
       base::Histogram::kUmaTargetedHistogramFlag);
   histogram_evicted_last_access_minutes_ = base::Histogram::FactoryGet(
       "Cookie.EvictedLastAccessMinutes", 1, kMinutesInTenYears, 50,
       base::Histogram::kUmaTargetedHistogramFlag);
   histogram_count_ = base::Histogram::FactoryGet(
       "Cookie.Count", 1, 4000, 50, base::Histogram::kUmaTargetedHistogramFlag);
 
@@ skipping 23 matching lines @@
 // The system resolution is not high enough, so we can have multiple
 // set cookies that result in the same system time.  When this happens, we
 // increment by one Time unit.  Let's hope computers don't get too fast.
 Time CookieMonster::CurrentTime() {
   return std::max(Time::Now(), Time::FromInternalValue(
                                    last_time_seen_.ToInternalValue() + 1));
 }
 
 void CookieMonster::DoCookieTask(
     const scoped_refptr<CookieMonsterTask>& task_item) {
-  {
-    base::AutoLock autolock(lock_);
-    MarkCookieStoreAsInitialized();
-    FetchAllCookiesIfNecessary();
-    if (!finished_fetching_all_cookies_ && store_.get()) {
-      tasks_pending_.push(task_item);
-      return;
-    }
+  DCHECK(thread_checker_.CalledOnValidThread());
+
+  MarkCookieStoreAsInitialized();
+  FetchAllCookiesIfNecessary();
+
+  if (!finished_fetching_all_cookies_ && store_.get()) {
+    tasks_pending_.push(task_item);
+    return;
   }
 
   task_item->Run();
 }
 
 void CookieMonster::DoCookieTaskForURL(
     const scoped_refptr<CookieMonsterTask>& task_item,
     const GURL& url) {
-  {
-    base::AutoLock autolock(lock_);
-    MarkCookieStoreAsInitialized();
-    if (ShouldFetchAllCookiesWhenFetchingAnyCookie())
-      FetchAllCookiesIfNecessary();
-    // If cookies for the requested domain key (eTLD+1) have been loaded from DB
-    // then run the task, otherwise load from DB.
-    if (!finished_fetching_all_cookies_ && store_.get()) {
-      // Checks if the domain key has been loaded.
-      std::string key(
-          cookie_util::GetEffectiveDomain(url.scheme(), url.host()));
-      if (keys_loaded_.find(key) == keys_loaded_.end()) {
-        std::map<std::string,
-                 std::deque<scoped_refptr<CookieMonsterTask>>>::iterator it =
-            tasks_pending_for_key_.find(key);
-        if (it == tasks_pending_for_key_.end()) {
-          store_->LoadCookiesForKey(
-              key, base::Bind(&CookieMonster::OnKeyLoaded, this, key));
-          it = tasks_pending_for_key_
-                   .insert(std::make_pair(
-                       key, std::deque<scoped_refptr<CookieMonsterTask>>()))
-                   .first;
-        }
-        it->second.push_back(task_item);
-        return;
+  DCHECK(thread_checker_.CalledOnValidThread());
+
+  MarkCookieStoreAsInitialized();
+  if (ShouldFetchAllCookiesWhenFetchingAnyCookie())
+    FetchAllCookiesIfNecessary();
+
+  // If cookies for the requested domain key (eTLD+1) have been loaded from DB
+  // then run the task, otherwise load from DB.
+  if (!finished_fetching_all_cookies_ && store_.get()) {
+    // Checks if the domain key has been loaded.
+    std::string key(cookie_util::GetEffectiveDomain(url.scheme(), url.host()));
+    if (keys_loaded_.find(key) == keys_loaded_.end()) {
+      std::map<std::string,
+               std::deque<scoped_refptr<CookieMonsterTask>>>::iterator it =
+          tasks_pending_for_key_.find(key);
+      if (it == tasks_pending_for_key_.end()) {
+        store_->LoadCookiesForKey(
+            key, base::Bind(&CookieMonster::OnKeyLoaded,
+                            weak_ptr_factory_.GetWeakPtr(), key));
+        it = tasks_pending_for_key_
+                 .insert(std::make_pair(
+                     key, std::deque<scoped_refptr<CookieMonsterTask>>()))
+                 .first;
       }
+      it->second.push_back(task_item);
+      return;
     }
   }
+
   task_item->Run();
 }
 
 void CookieMonster::ComputeCookieDiff(CookieList* old_cookies,
                                       CookieList* new_cookies,
                                       CookieList* cookies_to_add,
                                       CookieList* cookies_to_delete) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+
   DCHECK(old_cookies);
   DCHECK(new_cookies);
   DCHECK(cookies_to_add);
   DCHECK(cookies_to_delete);
   DCHECK(cookies_to_add->empty());
   DCHECK(cookies_to_delete->empty());
 
   // Sort both lists.
   // A set ordered by FullDiffCookieSorter is also ordered by
   // PartialDiffCookieSorter.
   std::sort(old_cookies->begin(), old_cookies->end(), FullDiffCookieSorter);
   std::sort(new_cookies->begin(), new_cookies->end(), FullDiffCookieSorter);
 
   // Select any old cookie for deletion if no new cookie has the same name,
   // domain, and path.
   std::set_difference(
       old_cookies->begin(), old_cookies->end(), new_cookies->begin(),
       new_cookies->end(),
       std::inserter(*cookies_to_delete, cookies_to_delete->begin()),
       PartialDiffCookieSorter);
 
   // Select any new cookie for addition (or update) if no old cookie is exactly
   // equivalent.
   std::set_difference(new_cookies->begin(), new_cookies->end(),
                       old_cookies->begin(), old_cookies->end(),
                       std::inserter(*cookies_to_add, cookies_to_add->begin()),
                       FullDiffCookieSorter);
 }
 
-void CookieMonster::RunCallbacks(const CanonicalCookie& cookie, bool removed) {
-  lock_.AssertAcquired();
+void CookieMonster::RunCallback(const base::Closure& callback) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  callback.Run();
+}
+
+void CookieMonster::RunCookieChangedCallbacks(const CanonicalCookie& cookie,
+                                              bool removed) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+
   CookieOptions opts;
   opts.set_include_httponly();
   opts.set_include_same_site();
-  // Note that the callbacks in hook_map_ are wrapped with MakeAsync(), so they
+  // Note that the callbacks in hook_map_ are wrapped with RunAsync(), so they
   // are guaranteed to not take long - they just post a RunAsync task back to
-  // the appropriate thread's message loop and return. It is important that this
-  // method not run user-supplied callbacks directly, since the CookieMonster
-  // lock is held and it is easy to accidentally introduce deadlocks.
+  // the appropriate thread's message loop and return.
+  // TODO(mmenke): Consider running these synchronously?
   for (CookieChangedHookMap::iterator it = hook_map_.begin();
        it != hook_map_.end(); ++it) {
     std::pair<GURL, std::string> key = it->first;
     if (cookie.IncludeForRequestURL(key.first, opts) &&
         cookie.Name() == key.second) {
       it->second->Notify(cookie, removed);
     }
   }
 }
 
 }  // namespace net