Shift cookie system callbacks to OnceCallback to impedance match mojo.

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 59b3dc72c503d679c1a47537a0579cb3b64d8106..3e4621b89bde32c74bb676df585c6b6c01c43c8f 100644
--- a/net/cookies/cookie_monster.cc
+++ b/net/cookies/cookie_monster.cc
@@ -431,7 +431,7 @@ class CookieMonster::SetCookieWithDetailsTask : public CookieMonsterTask {
                            bool http_only,
                            CookieSameSite same_site,
                            CookiePriority priority,
-                           const SetCookiesCallback& callback)
+                           SetCookiesCallback callback)
       : CookieMonsterTask(cookie_monster),
         url_(url),
         name_(name),
@@ -445,7 +445,7 @@ class CookieMonster::SetCookieWithDetailsTask : public CookieMonsterTask {
         http_only_(http_only),
         same_site_(same_site),
         priority_(priority),
-        callback_(callback) {}
+        callback_(std::move(callback)) {}
 
   // CookieMonsterTask:
   void Run() override;
@@ -476,15 +476,15 @@ void CookieMonster::SetCookieWithDetailsTask::Run() {
       url_, name_, value_, domain_, path_, creation_time_, expiration_time_,
       last_access_time_, secure_, http_only_, same_site_, priority_);
   if (!callback_.is_null())
-    callback_.Run(success);
+    std::move(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) {}
+                    GetCookieListCallback callback)
+      : CookieMonsterTask(cookie_monster), callback_(std::move(callback)) {}
 
   // CookieMonsterTask
   void Run() override;
@@ -501,7 +501,7 @@ class CookieMonster::GetAllCookiesTask : public CookieMonsterTask {
 void CookieMonster::GetAllCookiesTask::Run() {
   if (!callback_.is_null()) {
     CookieList cookies = this->cookie_monster()->GetAllCookies();
-    callback_.Run(cookies);
+    std::move(callback_).Run(cookies);
   }
 }
 
@@ -511,11 +511,11 @@ class CookieMonster::GetCookieListWithOptionsTask : public CookieMonsterTask {
   GetCookieListWithOptionsTask(CookieMonster* cookie_monster,
                                const GURL& url,
                                const CookieOptions& options,
-                               const GetCookieListCallback& callback)
+                               GetCookieListCallback callback)
       : CookieMonsterTask(cookie_monster),
         url_(url),
         options_(options),
-        callback_(callback) {}
+        callback_(std::move(callback)) {}
 
   // CookieMonsterTask:
   void Run() override;
@@ -535,18 +535,18 @@ void CookieMonster::GetCookieListWithOptionsTask::Run() {
   if (!callback_.is_null()) {
     CookieList cookies =
         this->cookie_monster()->GetCookieListWithOptions(url_, options_);
-    callback_.Run(cookies);
+    std::move(callback_).Run(cookies);
   }
 }
 
 template <typename Result>
 struct CallbackType {
-  typedef base::Callback<void(Result)> Type;
+  typedef base::OnceCallback<void(Result)> Type;
 };
 
 template <>
 struct CallbackType<void> {
-  typedef base::Closure Type;
+  typedef base::OnceClosure Type;
 };
 
 // Base task class for Delete*Task.
@@ -554,8 +554,8 @@ template <typename Result>
 class CookieMonster::DeleteTask : public CookieMonsterTask {
  public:
   DeleteTask(CookieMonster* cookie_monster,
-             const typename CallbackType<Result>::Type& callback)
-      : CookieMonsterTask(cookie_monster), callback_(callback) {}
+             typename CallbackType<Result>::Type callback)
+      : CookieMonsterTask(cookie_monster), callback_(std::move(callback)) {}
 
   // CookieMonsterTask:
   void Run() override;
@@ -570,7 +570,7 @@ class CookieMonster::DeleteTask : public CookieMonsterTask {
   // flushing the persistent store.
   // TODO(mmenke): This seems like a pretty ugly and needlessly confusing API.
   // Simplify it?
-  base::Closure RunDeleteTaskAndBindCallback();
+  base::OnceClosure RunDeleteTaskAndBindCallback();
 
   typename CallbackType<Result>::Type callback_;
 
@@ -582,29 +582,31 @@ CookieMonster::DeleteTask<Result>::~DeleteTask() {
 }
 
 template <typename Result>
-base::Closure
+base::OnceClosure
 CookieMonster::DeleteTask<Result>::RunDeleteTaskAndBindCallback() {
   Result result = RunDeleteTask();
   if (callback_.is_null())
-    return base::Closure();
-  return base::Bind(callback_, result);
+    return base::OnceClosure();
+  return base::BindOnce(std::move(callback_), result);
 }
 
 template <>
-base::Closure CookieMonster::DeleteTask<void>::RunDeleteTaskAndBindCallback() {
+base::OnceClosure
+CookieMonster::DeleteTask<void>::RunDeleteTaskAndBindCallback() {
   RunDeleteTask();
-  return callback_;
+  return std::move(callback_);
 }
 
 template <typename Result>
 void CookieMonster::DeleteTask<Result>::Run() {
-  base::Closure callback = RunDeleteTaskAndBindCallback();
+  base::OnceClosure callback = RunDeleteTaskAndBindCallback();
   if (!callback.is_null()) {
-    callback = base::Bind(
-        &CookieMonster::RunCallback,
-        this->cookie_monster()->weak_ptr_factory_.GetWeakPtr(), callback);
+    callback =
+        base::BindOnce(&CookieMonster::RunCallback,
+                       this->cookie_monster()->weak_ptr_factory_.GetWeakPtr(),
+                       std::move(callback));
   }
-  this->cookie_monster()->FlushStore(callback);
+  this->cookie_monster()->FlushStore(std::move(callback));
 }
 
 // Task class for DeleteAllCreatedBetween call.
@@ -613,8 +615,8 @@ class CookieMonster::DeleteAllCreatedBetweenTask : public DeleteTask<int> {
   DeleteAllCreatedBetweenTask(CookieMonster* cookie_monster,
                               const Time& delete_begin,
                               const Time& delete_end,
-                              const DeleteCallback& callback)
-      : DeleteTask<int>(cookie_monster, callback),
+                              DeleteCallback callback)
+      : DeleteTask<int>(cookie_monster, std::move(callback)),
         delete_begin_(delete_begin),
         delete_end_(delete_end) {}
 
@@ -645,8 +647,8 @@ class CookieMonster::DeleteAllCreatedBetweenWithPredicateTask
       Time delete_begin,
       Time delete_end,
       base::Callback<bool(const CanonicalCookie&)> predicate,
-      const DeleteCallback& callback)
-      : DeleteTask<int>(cookie_monster, callback),
+      DeleteCallback callback)
+      : DeleteTask<int>(cookie_monster, std::move(callback)),
         delete_begin_(delete_begin),
         delete_end_(delete_end),
         predicate_(predicate) {}
@@ -675,8 +677,8 @@ class CookieMonster::DeleteCanonicalCookieTask : public DeleteTask<int> {
  public:
   DeleteCanonicalCookieTask(CookieMonster* cookie_monster,
                             const CanonicalCookie& cookie,
-                            const DeleteCallback& callback)
-      : DeleteTask<int>(cookie_monster, callback), cookie_(cookie) {}
+                            DeleteCallback callback)
+      : DeleteTask<int>(cookie_monster, std::move(callback)), cookie_(cookie) {}
 
   // DeleteTask:
   int RunDeleteTask() override;
@@ -701,12 +703,12 @@ class CookieMonster::SetCanonicalCookieTask : public CookieMonsterTask {
                          std::unique_ptr<CanonicalCookie> cookie,
                          bool secure_source,
                          bool modify_http_only,
-                         const SetCookiesCallback& callback)
+                         SetCookiesCallback callback)
       : CookieMonsterTask(cookie_monster),
         cookie_(std::move(cookie)),
         secure_source_(secure_source),
         modify_http_only_(modify_http_only),
-        callback_(callback) {}
+        callback_(std::move(callback)) {}
 
   // CookieMonsterTask:
   void Run() override;
@@ -727,7 +729,7 @@ void CookieMonster::SetCanonicalCookieTask::Run() {
   bool result = this->cookie_monster()->SetCanonicalCookie(
       std::move(cookie_), secure_source_, modify_http_only_);
   if (!callback_.is_null())
-    callback_.Run(result);
+    std::move(callback_).Run(result);
 }
 
 // Task class for SetCookieWithOptions call.
@@ -737,12 +739,12 @@ class CookieMonster::SetCookieWithOptionsTask : public CookieMonsterTask {
                            const GURL& url,
                            const std::string& cookie_line,
                            const CookieOptions& options,
-                           const SetCookiesCallback& callback)
+                           SetCookiesCallback callback)
       : CookieMonsterTask(cookie_monster),
         url_(url),
         cookie_line_(cookie_line),
         options_(options),
-        callback_(callback) {}
+        callback_(std::move(callback)) {}
 
   // CookieMonsterTask:
   void Run() override;
@@ -763,7 +765,7 @@ void CookieMonster::SetCookieWithOptionsTask::Run() {
   bool result = this->cookie_monster()->SetCookieWithOptions(url_, cookie_line_,
                                                              options_);
   if (!callback_.is_null())
-    callback_.Run(result);
+    std::move(callback_).Run(result);
 }
 
 // Task class for SetAllCookies call.
@@ -771,8 +773,10 @@ class CookieMonster::SetAllCookiesTask : public CookieMonsterTask {
  public:
   SetAllCookiesTask(CookieMonster* cookie_monster,
                     const CookieList& list,
-                    const SetCookiesCallback& callback)
-      : CookieMonsterTask(cookie_monster), list_(list), callback_(callback) {}
+                    SetCookiesCallback callback)
+      : CookieMonsterTask(cookie_monster),
+        list_(list),
+        callback_(std::move(callback)) {}
 
   // CookieMonsterTask:
   void Run() override;
@@ -804,7 +808,7 @@ void CookieMonster::SetAllCookiesTask::Run() {
     result = this->cookie_monster()->SetAllCookies(list_);
 
   if (!callback_.is_null())
-    callback_.Run(result);
+    std::move(callback_).Run(result);
 }
 
 // Task class for GetCookiesWithOptions call.
@@ -813,11 +817,11 @@ class CookieMonster::GetCookiesWithOptionsTask : public CookieMonsterTask {
   GetCookiesWithOptionsTask(CookieMonster* cookie_monster,
                             const GURL& url,
                             const CookieOptions& options,
-                            const GetCookiesCallback& callback)
+                            GetCookiesCallback callback)
       : CookieMonsterTask(cookie_monster),
         url_(url),
         options_(options),
-        callback_(callback) {}
+        callback_(std::move(callback)) {}
 
   // CookieMonsterTask:
   void Run() override;
@@ -837,7 +841,7 @@ void CookieMonster::GetCookiesWithOptionsTask::Run() {
   std::string cookie =
       this->cookie_monster()->GetCookiesWithOptions(url_, options_);
   if (!callback_.is_null())
-    callback_.Run(cookie);
+    std::move(callback_).Run(cookie);
 }
 
 // Task class for DeleteCookie call.
@@ -846,8 +850,8 @@ class CookieMonster::DeleteCookieTask : public DeleteTask<void> {
   DeleteCookieTask(CookieMonster* cookie_monster,
                    const GURL& url,
                    const std::string& cookie_name,
-                   const base::Closure& callback)
-      : DeleteTask<void>(cookie_monster, callback),
+                   base::OnceClosure callback)
+      : DeleteTask<void>(cookie_monster, std::move(callback)),
         url_(url),
         cookie_name_(cookie_name) {}
 
@@ -872,8 +876,8 @@ void CookieMonster::DeleteCookieTask::RunDeleteTask() {
 class CookieMonster::DeleteSessionCookiesTask : public DeleteTask<int> {
  public:
   DeleteSessionCookiesTask(CookieMonster* cookie_monster,
-                           const DeleteCallback& callback)
-      : DeleteTask<int>(cookie_monster, callback) {}
+                           DeleteCallback callback)
+      : DeleteTask<int>(cookie_monster, std::move(callback)) {}
 
   // DeleteTask:
   int RunDeleteTask() override;
@@ -891,36 +895,37 @@ int CookieMonster::DeleteSessionCookiesTask::RunDeleteTask() {
 
 // Asynchronous CookieMonster API
 
-void CookieMonster::SetCookieWithDetailsAsync(
-    const GURL& url,
-    const std::string& name,
-    const std::string& value,
-    const std::string& domain,
-    const std::string& path,
-    Time creation_time,
-    Time expiration_time,
-    Time last_access_time,
-    bool secure,
-    bool http_only,
-    CookieSameSite same_site,
-    CookiePriority priority,
-    const SetCookiesCallback& callback) {
+void CookieMonster::SetCookieWithDetailsAsync(const GURL& url,
+                                              const std::string& name,
+                                              const std::string& value,
+                                              const std::string& domain,
+                                              const std::string& path,
+                                              Time creation_time,
+                                              Time expiration_time,
+                                              Time last_access_time,
+                                              bool secure,
+                                              bool http_only,
+                                              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, callback);
+      last_access_time, secure, http_only, same_site, priority,
+      std::move(callback));
   DoCookieTaskForURL(task, url);
 }
 
-void CookieMonster::FlushStore(const base::Closure& callback) {
+void CookieMonster::FlushStore(base::OnceClosure callback) {
   DCHECK(thread_checker_.CalledOnValidThread());
 
   if (initialized_ && store_.get()) {
     if (channel_id_service_) {
       channel_id_service_->GetChannelIDStore()->Flush();
     }
-    store_->Flush(callback);
+    store_->Flush(std::move(callback));
   } else if (!callback.is_null()) {
-    base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE, callback);
+    base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE,
+                                                  std::move(callback));
   }
 }
 
@@ -932,9 +937,9 @@ void CookieMonster::SetForceKeepSessionState() {
 }
 
 void CookieMonster::SetAllCookiesAsync(const CookieList& list,
-                                       const SetCookiesCallback& callback) {
+                                       SetCookiesCallback callback) {
   scoped_refptr<SetAllCookiesTask> task =
-      new SetAllCookiesTask(this, list, callback);
+      new SetAllCookiesTask(this, list, std::move(callback));

[kinuko, 2017/06/19 10:52:07]

I wonder if (eventually) these all CookieMonsterTasks could also be move-only /
retained by unique_ptr (looks like the common usage is to push back into
pending_tasks_ and eventually just run it, though I could be missing various
things)

[Randy Smith (Not in Mondays), 2017/06/21 17:54:53]

Yes, I think that would be a good design.  I'd also like to think about whether
they can all be abstracted away by just using callbacks; there's a metric tone
of boilerplate around the CookieMonsterTasks it would be nice to find a way to
get rid of.

   DoCookieTask(task);
 }
 
@@ -942,10 +947,11 @@ void CookieMonster::SetCanonicalCookieAsync(
     std::unique_ptr<CanonicalCookie> cookie,
     bool secure_source,
     bool modify_http_only,
-    const SetCookiesCallback& callback) {
+    SetCookiesCallback callback) {
   DCHECK(cookie->IsCanonical());
-  scoped_refptr<SetCanonicalCookieTask> task = new SetCanonicalCookieTask(
-      this, std::move(cookie), secure_source, modify_http_only, callback);
+  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).
   // This is tricky because we don't have the scheme in this routine
@@ -954,23 +960,21 @@ void CookieMonster::SetCanonicalCookieAsync(
   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);
+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);
 }
 
-void CookieMonster::GetCookiesWithOptionsAsync(
-    const GURL& url,
-    const CookieOptions& options,
-    const GetCookiesCallback& callback) {
+void CookieMonster::GetCookiesWithOptionsAsync(const GURL& url,
+                                               const CookieOptions& options,
+                                               GetCookiesCallback callback) {
   scoped_refptr<GetCookiesWithOptionsTask> task =
-      new GetCookiesWithOptionsTask(this, url, options, callback);
+      new GetCookiesWithOptionsTask(this, url, options, std::move(callback));
 
   DoCookieTaskForURL(task, url);
 }
@@ -978,42 +982,43 @@ void CookieMonster::GetCookiesWithOptionsAsync(
 void CookieMonster::GetCookieListWithOptionsAsync(
     const GURL& url,
     const CookieOptions& options,
-    const GetCookieListCallback& callback) {
+    GetCookieListCallback callback) {
   scoped_refptr<GetCookieListWithOptionsTask> task =
-      new GetCookieListWithOptionsTask(this, url, options, callback);
+      new GetCookieListWithOptionsTask(this, url, options, std::move(callback));
 
   DoCookieTaskForURL(task, url);
 }
 
-void CookieMonster::GetAllCookiesAsync(const GetCookieListCallback& callback) {
-  scoped_refptr<GetAllCookiesTask> task = new GetAllCookiesTask(this, callback);
+void CookieMonster::GetAllCookiesAsync(GetCookieListCallback callback) {
+  scoped_refptr<GetAllCookiesTask> task =
+      new GetAllCookiesTask(this, std::move(callback));
 
   DoCookieTask(task);
 }
 
 void CookieMonster::DeleteCookieAsync(const GURL& url,
                                       const std::string& cookie_name,
-                                      const base::Closure& callback) {
+                                      base::OnceClosure callback) {
   scoped_refptr<DeleteCookieTask> task =
-      new DeleteCookieTask(this, url, cookie_name, callback);
+      new DeleteCookieTask(this, url, cookie_name, std::move(callback));
 
   DoCookieTaskForURL(task, url);
 }
 
 void CookieMonster::DeleteCanonicalCookieAsync(const CanonicalCookie& cookie,
-                                               const DeleteCallback& callback) {
+                                               DeleteCallback callback) {
   scoped_refptr<DeleteCanonicalCookieTask> task =
-      new DeleteCanonicalCookieTask(this, cookie, callback);
+      new DeleteCanonicalCookieTask(this, cookie, std::move(callback));
 
   DoCookieTask(task);
 }
 
-void CookieMonster::DeleteAllCreatedBetweenAsync(
-    const Time& delete_begin,
-    const Time& delete_end,
-    const DeleteCallback& 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, callback);
+      new DeleteAllCreatedBetweenTask(this, delete_begin, delete_end,
+                                      std::move(callback));
 
   DoCookieTask(task);
 }
@@ -1022,21 +1027,21 @@ void CookieMonster::DeleteAllCreatedBetweenWithPredicateAsync(
     const Time& delete_begin,
     const Time& delete_end,
     const base::Callback<bool(const CanonicalCookie&)>& predicate,
-    const DeleteCallback& callback) {
+    DeleteCallback callback) {
   if (predicate.is_null()) {
-    callback.Run(0);
+    std::move(callback).Run(0);
     return;
   }
   scoped_refptr<DeleteAllCreatedBetweenWithPredicateTask> task =
       new DeleteAllCreatedBetweenWithPredicateTask(
-          this, delete_begin, delete_end, predicate, callback);
+          this, delete_begin, delete_end, predicate, std::move(callback));
   DoCookieTask(task);
 }
 
 void CookieMonster::DeleteSessionCookiesAsync(
-    const CookieStore::DeleteCallback& callback) {
+    CookieStore::DeleteCallback callback) {
   scoped_refptr<DeleteSessionCookiesTask> task =
-      new DeleteSessionCookiesTask(this, callback);
+      new DeleteSessionCookiesTask(this, std::move(callback));
 
   DoCookieTask(task);
 }
@@ -2473,9 +2478,9 @@ void CookieMonster::ComputeCookieDiff(CookieList* old_cookies,
                       FullDiffCookieSorter);
 }
 
-void CookieMonster::RunCallback(const base::Closure& callback) {
+void CookieMonster::RunCallback(base::OnceClosure callback) {
   DCHECK(thread_checker_.CalledOnValidThread());
-  callback.Run();
+  std::move(callback).Run();
 }
 
 void CookieMonster::RunCookieChangedCallbacks(const CanonicalCookie& cookie,