Switch cookie async mechanism over to using callbacks.

net/cookies/cookie_monster.cc

Index: net/cookies/cookie_monster.cc
diff --git a/net/cookies/cookie_monster.cc b/net/cookies/cookie_monster.cc
index 7b3830158a748fbc95151aa8665905395d0c4c88..176b36e30176e97788d496057cecd4268d99d5da 100644
--- a/net/cookies/cookie_monster.cc
+++ b/net/cookies/cookie_monster.cc
@@ -82,9 +82,9 @@ using base::TimeTicks;
 //
 // 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
+// request is for a specific URL, DoCookieCallbackForURL 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_pending_for_key_ 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
@@ -102,6 +102,64 @@ const char kFetchWhenNecessaryName[] = "FetchWhenNecessary";
 const char kAlwaysFetchName[] = "AlwaysFetch";
 const char kCookieMonsterFetchStrategyName[] = "CookieMonsterFetchStrategy";
 
+void WrapGetCookieListCallback(
+    base::OnceCallback<net::CookieList()> function,
+    base::OnceCallback<void(const net::CookieList&)> response) {
+  if (response.is_null())
+    return;
+  std::move(response).Run(std::move(function).Run());
+}
+
+void WrapSetCookiesCallback(base::OnceCallback<bool()> function,
+                            base::OnceCallback<void(bool)> response) {
+  bool result = std::move(function).Run();
+  if (response.is_null())
+    return;
+  std::move(response).Run(result);
+}
+
+void WrapGetCookiesCallback(
+    base::OnceCallback<std::string()> function,
+    base::OnceCallback<void(const std::string&)> response) {
+  std::string result = std::move(function).Run();
+  if (response.is_null())
+    return;
+  std::move(response).Run(result);
+}
+
+void ConditionalDeleteCallback(base::WeakPtr<net::CookieMonster> cookie_monster,
+                               base::OnceClosure callback) {
+  if (cookie_monster)
+    std::move(callback).Run();
+}
+
+void WrapDeleteCallback(base::WeakPtr<net::CookieMonster> cookie_monster,
+                        base::OnceCallback<uint32_t()> function,
+                        base::OnceCallback<void(uint32_t)> response) {
+  uint32_t num_deleted = std::move(function).Run();
+
+  // This function is always called directly from a CookieMonster routine,
+  // so unconditional use of the weak pointer here is safe.
+  cookie_monster->FlushStore(
+      response.is_null()
+          ? base::OnceClosure()
+          : base::BindOnce(&ConditionalDeleteCallback, cookie_monster,
+                           base::BindOnce(std::move(response), num_deleted)));
+}
+
+void WrapDeleteCookieCallback(base::WeakPtr<net::CookieMonster> cookie_monster,
+                              base::OnceCallback<void()> function,
+                              base::OnceCallback<void()> response) {
+  std::move(function).Run();
+
+  // This function is always called directly from a CookieMonster routine,
+  // so unconditional use of the weak pointer here is safe.
+  cookie_monster->FlushStore(
+      response.is_null() ? base::OnceClosure()
+                         : base::BindOnce(&ConditionalDeleteCallback,
+                                          cookie_monster, std::move(response)));
+}
+
 }  // namespace
 
 namespace net {
@@ -385,516 +443,6 @@ CookieMonster::CookieMonster(PersistentCookieStore* store,
       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();
-
-  CookieMonster* cookie_monster() { return cookie_monster_; }
-
- private:
-  friend class base::RefCountedThreadSafe<CookieMonsterTask>;
-
-  CookieMonster* cookie_monster_;
-
-  DISALLOW_COPY_AND_ASSIGN(CookieMonsterTask);
-};
-
-CookieMonster::CookieMonsterTask::CookieMonsterTask(
-    CookieMonster* cookie_monster)
-    : cookie_monster_(cookie_monster) {}
-
-CookieMonster::CookieMonsterTask::~CookieMonsterTask() {
-}
-
-// 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,
-                           base::Time expiration_time,
-                           base::Time last_access_time,
-                           bool secure,
-                           bool http_only,
-                           CookieSameSite same_site,
-                           CookiePriority priority,
-                           SetCookiesCallback callback)
-      : CookieMonsterTask(cookie_monster),
-        url_(url),
-        name_(name),
-        value_(value),
-        domain_(domain),
-        path_(path),
-        creation_time_(creation_time),
-        expiration_time_(expiration_time),
-        last_access_time_(last_access_time),
-        secure_(secure),
-        http_only_(http_only),
-        same_site_(same_site),
-        priority_(priority),
-        callback_(std::move(callback)) {}
-
-  // CookieMonsterTask:
-  void Run() override;
-
- protected:
-  ~SetCookieWithDetailsTask() override {}
-
- private:
-  GURL url_;
-  std::string name_;
-  std::string value_;
-  std::string domain_;
-  std::string path_;
-  base::Time creation_time_;
-  base::Time expiration_time_;
-  base::Time last_access_time_;
-  bool secure_;
-  bool http_only_;
-  CookieSameSite same_site_;
-  CookiePriority priority_;
-  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_, priority_);
-  if (!callback_.is_null())
-    std::move(callback_).Run(success);
-}
-
-// Task class for GetAllCookies call.
-class CookieMonster::GetAllCookiesTask : public CookieMonsterTask {
- public:
-  GetAllCookiesTask(CookieMonster* cookie_monster,
-                    GetCookieListCallback callback)
-      : CookieMonsterTask(cookie_monster), callback_(std::move(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();
-    std::move(callback_).Run(cookies);
-  }
-}
-
-// Task class for GetCookieListWithOptionsAsync call.
-class CookieMonster::GetCookieListWithOptionsTask : public CookieMonsterTask {
- public:
-  GetCookieListWithOptionsTask(CookieMonster* cookie_monster,
-                               const GURL& url,
-                               const CookieOptions& options,
-                               GetCookieListCallback callback)
-      : CookieMonsterTask(cookie_monster),
-        url_(url),
-        options_(options),
-        callback_(std::move(callback)) {}
-
-  // CookieMonsterTask:
-  void Run() override;
-
- protected:
-  ~GetCookieListWithOptionsTask() override {}
-
- private:
-  GURL url_;
-  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_);
-    std::move(callback_).Run(cookies);
-  }
-}
-
-template <typename Result>
-struct CallbackType {
-  typedef base::OnceCallback<void(Result)> Type;
-};
-
-template <>
-struct CallbackType<void> {
-  typedef base::OnceClosure Type;
-};
-
-// Base task class for Delete*Task.
-template <typename Result>
-class CookieMonster::DeleteTask : public CookieMonsterTask {
- public:
-  DeleteTask(CookieMonster* cookie_monster,
-             typename CallbackType<Result>::Type callback)
-      : CookieMonsterTask(cookie_monster), callback_(std::move(callback)) {}
-
-  // 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::OnceClosure RunDeleteTaskAndBindCallback();
-
-  typename CallbackType<Result>::Type callback_;
-
-  DISALLOW_COPY_AND_ASSIGN(DeleteTask);
-};
-
-template <typename Result>
-CookieMonster::DeleteTask<Result>::~DeleteTask() {
-}
-
-template <typename Result>
-base::OnceClosure
-CookieMonster::DeleteTask<Result>::RunDeleteTaskAndBindCallback() {
-  Result result = RunDeleteTask();
-  if (callback_.is_null())
-    return base::OnceClosure();
-  return base::BindOnce(std::move(callback_), result);
-}
-
-template <>
-base::OnceClosure
-CookieMonster::DeleteTask<void>::RunDeleteTaskAndBindCallback() {
-  RunDeleteTask();
-  return std::move(callback_);
-}
-
-template <typename Result>
-void CookieMonster::DeleteTask<Result>::Run() {
-  base::OnceClosure callback = RunDeleteTaskAndBindCallback();
-  if (!callback.is_null()) {
-    callback =
-        base::BindOnce(&CookieMonster::RunCallback,
-                       this->cookie_monster()->weak_ptr_factory_.GetWeakPtr(),
-                       std::move(callback));
-  }
-  this->cookie_monster()->FlushStore(std::move(callback));
-}
-
-// Task class for DeleteAllCreatedBetween call.
-class CookieMonster::DeleteAllCreatedBetweenTask : public DeleteTask<uint32_t> {
- public:
-  DeleteAllCreatedBetweenTask(CookieMonster* cookie_monster,
-                              const Time& delete_begin,
-                              const Time& delete_end,
-                              DeleteCallback callback)
-      : DeleteTask<uint32_t>(cookie_monster, std::move(callback)),
-        delete_begin_(delete_begin),
-        delete_end_(delete_end) {}
-
-  // DeleteTask:
-  uint32_t RunDeleteTask() override;
-
- protected:
-  ~DeleteAllCreatedBetweenTask() override {}
-
- private:
-  Time delete_begin_;
-  Time delete_end_;
-
-  DISALLOW_COPY_AND_ASSIGN(DeleteAllCreatedBetweenTask);
-};
-
-uint32_t CookieMonster::DeleteAllCreatedBetweenTask::RunDeleteTask() {
-  return this->cookie_monster()->DeleteAllCreatedBetween(delete_begin_,
-                                                         delete_end_);
-}
-
-// Task class for DeleteAllCreatedBetweenWithPredicate call.
-class CookieMonster::DeleteAllCreatedBetweenWithPredicateTask
-    : public DeleteTask<uint32_t> {
- public:
-  DeleteAllCreatedBetweenWithPredicateTask(
-      CookieMonster* cookie_monster,
-      Time delete_begin,
-      Time delete_end,
-      base::Callback<bool(const CanonicalCookie&)> predicate,
-      DeleteCallback callback)
-      : DeleteTask<uint32_t>(cookie_monster, std::move(callback)),
-        delete_begin_(delete_begin),
-        delete_end_(delete_end),
-        predicate_(predicate) {}
-
-  // DeleteTask:
-  uint32_t RunDeleteTask() override;
-
- protected:
-  ~DeleteAllCreatedBetweenWithPredicateTask() override {}
-
- private:
-  Time delete_begin_;
-  Time delete_end_;
-  base::Callback<bool(const CanonicalCookie&)> predicate_;
-
-  DISALLOW_COPY_AND_ASSIGN(DeleteAllCreatedBetweenWithPredicateTask);
-};
-
-uint32_t
-CookieMonster::DeleteAllCreatedBetweenWithPredicateTask::RunDeleteTask() {
-  return this->cookie_monster()->DeleteAllCreatedBetweenWithPredicate(
-      delete_begin_, delete_end_, predicate_);
-}
-
-// Task class for DeleteCanonicalCookie call.
-class CookieMonster::DeleteCanonicalCookieTask : public DeleteTask<uint32_t> {
- public:
-  DeleteCanonicalCookieTask(CookieMonster* cookie_monster,
-                            const CanonicalCookie& cookie,
-                            DeleteCallback callback)
-      : DeleteTask<uint32_t>(cookie_monster, std::move(callback)),
-        cookie_(cookie) {}
-
-  // DeleteTask:
-  uint32_t RunDeleteTask() override;
-
- protected:
-  ~DeleteCanonicalCookieTask() override {}
-
- private:
-  CanonicalCookie cookie_;
-
-  DISALLOW_COPY_AND_ASSIGN(DeleteCanonicalCookieTask);
-};
-
-uint32_t CookieMonster::DeleteCanonicalCookieTask::RunDeleteTask() {
-  return this->cookie_monster()->DeleteCanonicalCookie(cookie_);
-}
-
-// Task class for SetCanonicalCookie call.
-class CookieMonster::SetCanonicalCookieTask : public CookieMonsterTask {
- public:
-  SetCanonicalCookieTask(CookieMonster* cookie_monster,
-                         std::unique_ptr<CanonicalCookie> cookie,
-                         bool secure_source,
-                         bool modify_http_only,
-                         SetCookiesCallback callback)
-      : CookieMonsterTask(cookie_monster),
-        cookie_(std::move(cookie)),
-        secure_source_(secure_source),
-        modify_http_only_(modify_http_only),
-        callback_(std::move(callback)) {}
-
-  // CookieMonsterTask:
-  void Run() override;
-
- protected:
-  ~SetCanonicalCookieTask() override {}
-
- private:
-  std::unique_ptr<CanonicalCookie> cookie_;
-  bool secure_source_;
-  bool modify_http_only_;
-  SetCookiesCallback callback_;
-
-  DISALLOW_COPY_AND_ASSIGN(SetCanonicalCookieTask);
-};
-
-void CookieMonster::SetCanonicalCookieTask::Run() {
-  bool result = this->cookie_monster()->SetCanonicalCookie(
-      std::move(cookie_), secure_source_, modify_http_only_);
-  if (!callback_.is_null())
-    std::move(callback_).Run(result);
-}
-
-// Task class for SetCookieWithOptions call.
-class CookieMonster::SetCookieWithOptionsTask : public CookieMonsterTask {
- public:
-  SetCookieWithOptionsTask(CookieMonster* cookie_monster,
-                           const GURL& url,
-                           const std::string& cookie_line,
-                           const CookieOptions& options,
-                           SetCookiesCallback callback)
-      : CookieMonsterTask(cookie_monster),
-        url_(url),
-        cookie_line_(cookie_line),
-        options_(options),
-        callback_(std::move(callback)) {}
-
-  // CookieMonsterTask:
-  void Run() override;
-
- protected:
-  ~SetCookieWithOptionsTask() override {}
-
- private:
-  GURL url_;
-  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())
-    std::move(callback_).Run(result);
-}
-
-// Task class for SetAllCookies call.
-class CookieMonster::SetAllCookiesTask : public CookieMonsterTask {
- public:
-  SetAllCookiesTask(CookieMonster* cookie_monster,
-                    const CookieList& list,
-                    SetCookiesCallback callback)
-      : CookieMonsterTask(cookie_monster),
-        list_(list),
-        callback_(std::move(callback)) {}
-
-  // CookieMonsterTask:
-  void Run() override;
-
- protected:
-  ~SetAllCookiesTask() override {}
-
- private:
-  CookieList list_;
-  SetCookiesCallback callback_;
-
-  DISALLOW_COPY_AND_ASSIGN(SetAllCookiesTask);
-};
-
-void CookieMonster::SetAllCookiesTask::Run() {
-  CookieList positive_diff;
-  CookieList negative_diff;
-  CookieList old_cookies = this->cookie_monster()->GetAllCookies();
-  this->cookie_monster()->ComputeCookieDiff(&old_cookies, &list_,
-                                            &positive_diff, &negative_diff);
-
-  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()->SetAllCookies(list_);
-
-  if (!callback_.is_null())
-    std::move(callback_).Run(result);
-}
-
-// Task class for GetCookiesWithOptions call.
-class CookieMonster::GetCookiesWithOptionsTask : public CookieMonsterTask {
- public:
-  GetCookiesWithOptionsTask(CookieMonster* cookie_monster,
-                            const GURL& url,
-                            const CookieOptions& options,
-                            GetCookiesCallback callback)
-      : CookieMonsterTask(cookie_monster),
-        url_(url),
-        options_(options),
-        callback_(std::move(callback)) {}
-
-  // CookieMonsterTask:
-  void Run() override;
-
- protected:
-  ~GetCookiesWithOptionsTask() override {}
-
- private:
-  GURL url_;
-  CookieOptions options_;
-  GetCookiesCallback callback_;
-
-  DISALLOW_COPY_AND_ASSIGN(GetCookiesWithOptionsTask);
-};
-
-void CookieMonster::GetCookiesWithOptionsTask::Run() {
-  std::string cookie =
-      this->cookie_monster()->GetCookiesWithOptions(url_, options_);
-  if (!callback_.is_null())
-    std::move(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,
-                   base::OnceClosure callback)
-      : DeleteTask<void>(cookie_monster, std::move(callback)),
-        url_(url),
-        cookie_name_(cookie_name) {}
-
-  // DeleteTask:
-  void RunDeleteTask() override;
-
- protected:
-  ~DeleteCookieTask() override {}
-
- private:
-  GURL url_;
-  std::string cookie_name_;
-
-  DISALLOW_COPY_AND_ASSIGN(DeleteCookieTask);
-};
-
-void CookieMonster::DeleteCookieTask::RunDeleteTask() {
-  this->cookie_monster()->DeleteCookie(url_, cookie_name_);
-}
-
-// Task class for DeleteSessionCookies call.
-class CookieMonster::DeleteSessionCookiesTask : public DeleteTask<uint32_t> {
- public:
-  DeleteSessionCookiesTask(CookieMonster* cookie_monster,
-                           DeleteCallback callback)
-      : DeleteTask<uint32_t>(cookie_monster, std::move(callback)) {}
-
-  // DeleteTask:
-  uint32_t RunDeleteTask() override;
-
- protected:
-  ~DeleteSessionCookiesTask() override {}
-
- private:
-  DISALLOW_COPY_AND_ASSIGN(DeleteSessionCookiesTask);
-};
-
-uint32_t CookieMonster::DeleteSessionCookiesTask::RunDeleteTask() {
-  return this->cookie_monster()->DeleteSessionCookies();
-}
-
 // Asynchronous CookieMonster API
 
 void CookieMonster::SetCookieWithDetailsAsync(const GURL& url,
@@ -910,11 +458,18 @@ void CookieMonster::SetCookieWithDetailsAsync(const GURL& url,
                                               CookieSameSite same_site,
                                               CookiePriority priority,
                                               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, priority,
-      std::move(callback));
-  DoCookieTaskForURL(task, url);
+  DoCookieCallbackForURL(
+      base::BindOnce(
+          &WrapSetCookiesCallback,
+          base::BindOnce(
+              // base::Unretained is safe as DoCookieCallbackForURL stores
+              // the callback on |*this|, so the callback will not outlive
+              // the object.
+              &CookieMonster::SetCookieWithDetails, base::Unretained(this), url,
+              name, value, domain, path, creation_time, expiration_time,
+              last_access_time, secure, http_only, same_site, priority),
+          std::move(callback)),
+      url);
 }
 
 void CookieMonster::FlushStore(base::OnceClosure callback) {
@@ -940,9 +495,14 @@ void CookieMonster::SetForceKeepSessionState() {
 
 void CookieMonster::SetAllCookiesAsync(const CookieList& list,
                                        SetCookiesCallback callback) {
-  scoped_refptr<SetAllCookiesTask> task =
-      new SetAllCookiesTask(this, list, std::move(callback));
-  DoCookieTask(task);
+  DoCookieCallback(base::BindOnce(
+      &WrapSetCookiesCallback,
+      base::BindOnce(
+          // base::Unretained is safe as DoCookieCallbackForURL stores
+          // the callback on |*this|, so the callback will not outlive
+          // the object.
+          &CookieMonster::SetAllCookies, base::Unretained(this), list),
+      std::move(callback)));
 }
 
 void CookieMonster::SetCanonicalCookieAsync(
@@ -951,78 +511,125 @@ void CookieMonster::SetCanonicalCookieAsync(
     bool modify_http_only,
     SetCookiesCallback callback) {
   DCHECK(cookie->IsCanonical());
-  scoped_refptr<SetCanonicalCookieTask> task =
-      new SetCanonicalCookieTask(this, std::move(cookie), secure_source,
-                                 modify_http_only, std::move(callback));
 
-  // TODO(rdsmith): Switch to DoCookieTaskForURL (or the equivalent).
+  // TODO(rdsmith): Switch to DoCookieCallbackForURL (or the equivalent).
   // This is tricky because we don't have the scheme in this routine
-  // and DoCookieTaskForURL uses cookie_util::GetEffectiveDomain(scheme, host)
+  // and DoCookieCallbackForURL uses
+  // cookie_util::GetEffectiveDomain(scheme, host)
   // to generate the database key to block behind.
-  DoCookieTask(task);
+  DoCookieCallback(base::BindOnce(
+      &WrapSetCookiesCallback,
+      base::BindOnce(
+          // base::Unretained is safe as DoCookieCallbackForURL stores
+          // the callback on |*this|, so the callback will not outlive
+          // the object.
+          &CookieMonster::SetCanonicalCookie, base::Unretained(this),
+          std::move(cookie), secure_source, modify_http_only),
+      std::move(callback)));
 }
 
 void CookieMonster::SetCookieWithOptionsAsync(const GURL& url,
                                               const std::string& cookie_line,
                                               const CookieOptions& options,
                                               SetCookiesCallback callback) {
-  scoped_refptr<SetCookieWithOptionsTask> task = new SetCookieWithOptionsTask(
-      this, url, cookie_line, options, std::move(callback));
-
-  DoCookieTaskForURL(task, url);
+  DoCookieCallbackForURL(
+      base::BindOnce(
+          &WrapSetCookiesCallback,
+          base::BindOnce(
+              // base::Unretained is safe as DoCookieCallbackForURL stores
+              // the callback on |*this|, so the callback will not outlive
+              // the object.
+              &CookieMonster::SetCookieWithOptions, base::Unretained(this), url,
+              cookie_line, options),
+          std::move(callback)),
+      url);
 }
 
 void CookieMonster::GetCookiesWithOptionsAsync(const GURL& url,
                                                const CookieOptions& options,
                                                GetCookiesCallback callback) {
-  scoped_refptr<GetCookiesWithOptionsTask> task =
-      new GetCookiesWithOptionsTask(this, url, options, std::move(callback));
-
-  DoCookieTaskForURL(task, url);
+  DoCookieCallbackForURL(
+      base::BindOnce(
+          &WrapGetCookiesCallback,
+          base::BindOnce(
+              // base::Unretained is safe as DoCookieCallbackForURL stores
+              // the callback on |*this|, so the callback will not outlive
+              // the object.
+              &CookieMonster::GetCookiesWithOptions, base::Unretained(this),
+              url, options),
+          std::move(callback)),
+      url);
 }
 
 void CookieMonster::GetCookieListWithOptionsAsync(
     const GURL& url,
     const CookieOptions& options,
     GetCookieListCallback callback) {
-  scoped_refptr<GetCookieListWithOptionsTask> task =
-      new GetCookieListWithOptionsTask(this, url, options, std::move(callback));
-
-  DoCookieTaskForURL(task, url);
+  DoCookieCallbackForURL(
+      base::BindOnce(
+          &WrapGetCookieListCallback,
+          base::BindOnce(
+              // base::Unretained is safe as DoCookieCallbackForURL stores
+              // the callback on |*this|, so the callback will not outlive
+              // the object.
+              &CookieMonster::GetCookieListWithOptions, base::Unretained(this),
+              url, options),
+          std::move(callback)),
+      url);
 }
 
 void CookieMonster::GetAllCookiesAsync(GetCookieListCallback callback) {
-  scoped_refptr<GetAllCookiesTask> task =
-      new GetAllCookiesTask(this, std::move(callback));
-
-  DoCookieTask(task);
+  DoCookieCallback(base::BindOnce(
+      &WrapGetCookieListCallback,
+      base::BindOnce(
+          // base::Unretained is safe as DoCookieCallbackForURL stores
+          // the callback on |*this|, so the callback will not outlive
+          // the object.
+          &CookieMonster::GetAllCookies, base::Unretained(this)),
+      std::move(callback)));
 }
 
 void CookieMonster::DeleteCookieAsync(const GURL& url,
                                       const std::string& cookie_name,
                                       base::OnceClosure callback) {
-  scoped_refptr<DeleteCookieTask> task =
-      new DeleteCookieTask(this, url, cookie_name, std::move(callback));
-
-  DoCookieTaskForURL(task, url);
+  DoCookieCallbackForURL(
+      base::BindOnce(
+          &WrapDeleteCookieCallback, weak_ptr_factory_.GetWeakPtr(),
+          base::BindOnce(
+              // base::Unretained is safe as DoCookieCallbackForURL stores
+              // the callback on |*this|, so the callback will not outlive
+              // the object.
+              &CookieMonster::DeleteCookie, base::Unretained(this), url,
+              cookie_name),
+          std::move(callback)),
+      url);
 }
 
 void CookieMonster::DeleteCanonicalCookieAsync(const CanonicalCookie& cookie,
                                                DeleteCallback callback) {
-  scoped_refptr<DeleteCanonicalCookieTask> task =
-      new DeleteCanonicalCookieTask(this, cookie, std::move(callback));
-
-  DoCookieTask(task);
+  DoCookieCallback(base::BindOnce(
+      &WrapDeleteCallback, weak_ptr_factory_.GetWeakPtr(),
+      base::BindOnce(
+          // base::Unretained is safe as DoCookieCallbackForURL stores
+          // the callback on |*this|, so the callback will not outlive
+          // the object.
+          &CookieMonster::DeleteCanonicalCookie, base::Unretained(this),
+          cookie),
+      std::move(callback)));
 }
 
 void CookieMonster::DeleteAllCreatedBetweenAsync(const Time& delete_begin,
                                                  const Time& delete_end,
                                                  DeleteCallback callback) {
-  scoped_refptr<DeleteAllCreatedBetweenTask> task =
-      new DeleteAllCreatedBetweenTask(this, delete_begin, delete_end,
-                                      std::move(callback));
-
-  DoCookieTask(task);
+  DoCookieCallback(base::BindOnce(
+      &WrapDeleteCallback, weak_ptr_factory_.GetWeakPtr(),
+      base::BindOnce(
+          // base::Unretained is safe as DoCookieCallbackForURL stores
+          // the callback on |*this|, so the callback will not outlive
+          // the object.
+          &CookieMonster::DeleteAllCreatedBetween, base::Unretained(this),
+          delete_begin, delete_end),
+      std::move(callback)));
 }
 
 void CookieMonster::DeleteAllCreatedBetweenWithPredicateAsync(
@@ -1034,18 +641,28 @@ void CookieMonster::DeleteAllCreatedBetweenWithPredicateAsync(
     std::move(callback).Run(0);
     return;
   }
-  scoped_refptr<DeleteAllCreatedBetweenWithPredicateTask> task =
-      new DeleteAllCreatedBetweenWithPredicateTask(
-          this, delete_begin, delete_end, predicate, std::move(callback));
-  DoCookieTask(task);
+
+  DoCookieCallback(base::BindOnce(
+      &WrapDeleteCallback, weak_ptr_factory_.GetWeakPtr(),
+      base::BindOnce(
+          // base::Unretained is safe as DoCookieCallbackForURL stores
+          // the callback on |*this|, so the callback will not outlive
+          // the object.
+          &CookieMonster::DeleteAllCreatedBetweenWithPredicate,
+          base::Unretained(this), delete_begin, delete_end, predicate),
+      std::move(callback)));
 }
 
 void CookieMonster::DeleteSessionCookiesAsync(
     CookieStore::DeleteCallback callback) {
-  scoped_refptr<DeleteSessionCookiesTask> task =
-      new DeleteSessionCookiesTask(this, std::move(callback));
-
-  DoCookieTask(task);
+  DoCookieCallback(base::BindOnce(
+      &WrapDeleteCallback, weak_ptr_factory_.GetWeakPtr(),
+      base::BindOnce(
+          // base::Unretained is safe as DoCookieCallbackForURL stores
+          // the callback on |*this|, so the callback will not outlive
+          // the object.
+          &CookieMonster::DeleteSessionCookies, base::Unretained(this)),
+      std::move(callback)));
 }
 
 void CookieMonster::SetCookieableSchemes(
@@ -1440,11 +1057,9 @@ void CookieMonster::OnKeyLoaded(
   // Run all tasks for the key. Note that running a task can result in multiple
   // tasks being added to the back of the deque.
   while (!tasks_pending_for_key->second.empty()) {
-    scoped_refptr<CookieMonsterTask> task =
-        tasks_pending_for_key->second.front();
+    base::OnceClosure task = std::move(tasks_pending_for_key->second.front());
     tasks_pending_for_key->second.pop_front();
-
-    task->Run();
+    std::move(task).Run();
   }
 
   tasks_pending_for_key_.erase(tasks_pending_for_key);
@@ -1524,16 +1139,17 @@ void CookieMonster::InvokeQueue() {
   // Needed to prevent any recursively queued tasks from going back into the
   // per-key queues.
   seen_global_task_ = true;
-  for (const auto& tasks_for_key : tasks_pending_for_key_) {
-    tasks_pending_.insert(tasks_pending_.begin(), tasks_for_key.second.begin(),
-                          tasks_for_key.second.end());
+  for (auto& tasks_for_key : tasks_pending_for_key_) {
+    tasks_pending_.insert(tasks_pending_.begin(),
+                          std::make_move_iterator(tasks_for_key.second.begin()),
+                          std::make_move_iterator(tasks_for_key.second.end()));
   }
   tasks_pending_for_key_.clear();
 
   while (!tasks_pending_.empty()) {
-    scoped_refptr<CookieMonsterTask> request_task = tasks_pending_.front();
+    base::OnceClosure request_task = std::move(tasks_pending_.front());
     tasks_pending_.pop_front();
-    request_task->Run();
+    std::move(request_task).Run();
   }
 
   DCHECK(tasks_pending_for_key_.empty());
@@ -1880,8 +1496,21 @@ bool CookieMonster::SetCanonicalCookie(std::unique_ptr<CanonicalCookie> cc,
   return true;
 }
 
-bool CookieMonster::SetAllCookies(const CookieList& list) {
+bool CookieMonster::SetAllCookies(CookieList list) {
   DCHECK(thread_checker_.CalledOnValidThread());
+  CookieList positive_diff;
+  CookieList negative_diff;
+  CookieList old_cookies = GetAllCookies();
+  ComputeCookieDiff(&old_cookies, &list, &positive_diff, &negative_diff);
+
+  for (CookieList::const_iterator it = negative_diff.begin();
+       it != negative_diff.end(); ++it) {
+    DeleteCanonicalCookie(*it);
+  }
+
+  if (positive_diff.size() == 0)
+    return true;
+
   for (const auto& cookie : list) {

[Randy Smith (Not in Mondays), 2017/07/08 15:05:22]

I'll note that using |list| rather than |positive_diff| here makes me twitch,
though I believe it's a faithful refactor.  If you'd like me to try and tweak
this code to make more sense, I'm willing, though as noted in a previous CL,
getting rid of it would be a better (and much more time consuming) choice.

     const std::string key(GetKey(cookie.Domain()));
     Time creation_time = cookie.CreationDate();
@@ -2387,8 +2016,7 @@ Time CookieMonster::CurrentTime() {
                                    last_time_seen_.ToInternalValue() + 1));
 }
 
-void CookieMonster::DoCookieTask(
-    const scoped_refptr<CookieMonsterTask>& task_item) {
+void CookieMonster::DoCookieCallback(base::OnceClosure callback) {
   DCHECK(thread_checker_.CalledOnValidThread());
 
   MarkCookieStoreAsInitialized();
@@ -2396,16 +2024,15 @@ void CookieMonster::DoCookieTask(
   seen_global_task_ = true;
 
   if (!finished_fetching_all_cookies_ && store_.get()) {
-    tasks_pending_.push_back(task_item);
+    tasks_pending_.push_back(std::move(callback));
     return;
   }
 
-  task_item->Run();
+  std::move(callback).Run();
 }
 
-void CookieMonster::DoCookieTaskForURL(
-    const scoped_refptr<CookieMonsterTask>& task_item,
-    const GURL& url) {
+void CookieMonster::DoCookieCallbackForURL(base::OnceClosure callback,
+                                           const GURL& url) {
   MarkCookieStoreAsInitialized();
   if (ShouldFetchAllCookiesWhenFetchingAnyCookie())
     FetchAllCookiesIfNecessary();
@@ -2418,31 +2045,29 @@ void CookieMonster::DoCookieTaskForURL(
     // the global queue, |tasks_pending_| may be empty, which is why another
     // bool is needed.
     if (seen_global_task_) {
-      tasks_pending_.push_back(task_item);
+      tasks_pending_.push_back(std::move(callback));
       return;
     }
 
     // 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 =
+      std::map<std::string, std::deque<base::OnceClosure>>::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>>()))
+                 .insert(std::make_pair(key, std::deque<base::OnceClosure>()))
                  .first;
       }
-      it->second.push_back(task_item);
+      it->second.push_back(std::move(callback));
       return;
     }
   }
 
-  task_item->Run();
+  std::move(callback).Run();
 }
 
 void CookieMonster::ComputeCookieDiff(CookieList* old_cookies,
@@ -2480,11 +2105,6 @@ void CookieMonster::ComputeCookieDiff(CookieList* old_cookies,
                       FullDiffCookieSorter);
 }
 
-void CookieMonster::RunCallback(base::OnceClosure callback) {
-  DCHECK(thread_checker_.CalledOnValidThread());
-  std::move(callback).Run();
-}
-
 void CookieMonster::RunCookieChangedCallbacks(const CanonicalCookie& cookie,
                                               ChangeCause cause) {
   DCHECK(thread_checker_.CalledOnValidThread());