Use OnceCallback in ChannelIDService.

Use OnceCallback instead of Callback, and CompletionOnceCallback instead
of CompletionCallback, in ChannelIDService.  Also restructure

ChannelIDService::JoinToInFlightRequest() to use early return if in
flight request in not found.

Bug: 807724
Change-Id: I45b55b0713ca9460fda8480d4f8ac14d5ead4435
Reviewed-on: https://chromium-review.googlesource.com/1117029Reviewed-by: Maks Orlovich <morlovich@chromium.org>
Commit-Queue: Bence Béky <bnc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#570844}

net/ssl/channel_id_service.cc

@@ -67,29 +67,30 @@ std::unique_ptr<ChannelIDStore::ChannelID> GenerateChannelID(
 // generation. Will take care of deleting itself once Start() is called.
 class ChannelIDServiceWorker {
  public:
-  typedef base::Callback<
+  typedef base::OnceCallback<
       void(const std::string&, int, std::unique_ptr<ChannelIDStore::ChannelID>)>
       WorkerDoneCallback;
 
   ChannelIDServiceWorker(const std::string& server_identifier,
-                         const WorkerDoneCallback& callback)
+                         WorkerDoneCallback callback)
       : server_identifier_(server_identifier),
         origin_task_runner_(base::ThreadTaskRunnerHandle::Get()),
-        callback_(callback) {}
+        callback_(std::move(callback)) {}
 
   // Starts the worker asynchronously.
   void Start(const scoped_refptr<base::TaskRunner>& task_runner) {
     DCHECK(origin_task_runner_->RunsTasksInCurrentSequence());
 
-    auto callback = base::Bind(&ChannelIDServiceWorker::Run, base::Owned(this));
+    auto callback =
+        base::BindOnce(&ChannelIDServiceWorker::Run, base::Owned(this));
 
     if (task_runner) {
-      task_runner->PostTask(FROM_HERE, callback);
+      task_runner->PostTask(FROM_HERE, std::move(callback));
     } else {
       base::PostTaskWithTraits(
           FROM_HERE,
           {base::MayBlock(), base::TaskShutdownBehavior::CONTINUE_ON_SHUTDOWN},
-          callback);
+          std::move(callback));
     }
   }
 
@@ -100,8 +101,8 @@ class ChannelIDServiceWorker {
     std::unique_ptr<ChannelIDStore::ChannelID> channel_id =
         GenerateChannelID(server_identifier_, &error);
     origin_task_runner_->PostTask(
-        FROM_HERE, base::Bind(callback_, server_identifier_, error,
-                              base::Passed(&channel_id)));
+        FROM_HERE, base::BindOnce(std::move(callback_), server_identifier_,
+                                  error, base::Passed(&channel_id)));
   }
 
   const std::string server_identifier_;
@@ -176,12 +177,12 @@ void ChannelIDService::Request::Cancel() {
 
 void ChannelIDService::Request::RequestStarted(
     ChannelIDService* service,
-    const CompletionCallback& callback,
+    CompletionOnceCallback callback,
     std::unique_ptr<crypto::ECPrivateKey>* key,
     ChannelIDServiceJob* job) {
   DCHECK(service_ == NULL);
   service_ = service;
-  callback_ = callback;
+  callback_ = std::move(callback);
   key_ = key;
   job_ = job;
 }
@@ -225,7 +226,7 @@ std::string ChannelIDService::GetDomainForHost(const std::string& host) {
 int ChannelIDService::GetOrCreateChannelID(
     const std::string& host,
     std::unique_ptr<crypto::ECPrivateKey>* key,
-    const CompletionCallback& callback,
+    CompletionOnceCallback callback,
     Request* out_req) {
   DVLOG(1) << __func__ << " " << host;
   DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
@@ -243,19 +244,18 @@ int ChannelIDService::GetOrCreateChannelID(
 
   // See if a request for the same domain is currently in flight.
   bool create_if_missing = true;
-  if (JoinToInFlightRequest(domain, key, create_if_missing, callback,
+  if (JoinToInFlightRequest(domain, key, create_if_missing, &callback,
                             out_req)) {
     return ERR_IO_PENDING;
   }
 
-  int err = LookupChannelID(domain, key, create_if_missing, callback, out_req);
+  int err = LookupChannelID(domain, key, create_if_missing, &callback, out_req);
   if (err == ERR_FILE_NOT_FOUND) {
     // Sync lookup did not find a valid channel ID.  Start generating a new one.
     workers_created_++;
     ChannelIDServiceWorker* worker = new ChannelIDServiceWorker(
-        domain,
-        base::Bind(&ChannelIDService::GeneratedChannelID,
-                   weak_ptr_factory_.GetWeakPtr()));
+        domain, base::BindOnce(&ChannelIDService::GeneratedChannelID,
+                               weak_ptr_factory_.GetWeakPtr()));
     worker->Start(task_runner_);
 
     // We are waiting for key generation.  Create a job & request to track it.
@@ -263,7 +263,7 @@ int ChannelIDService::GetOrCreateChannelID(
     inflight_[domain] = base::WrapUnique(job);
 
     job->AddRequest(out_req);
-    out_req->RequestStarted(this, callback, key, job);
+    out_req->RequestStarted(this, std::move(callback), key, job);
     return ERR_IO_PENDING;
   }
 
@@ -272,7 +272,7 @@ int ChannelIDService::GetOrCreateChannelID(
 
 int ChannelIDService::GetChannelID(const std::string& host,
                                    std::unique_ptr<crypto::ECPrivateKey>* key,
-                                   const CompletionCallback& callback,
+                                   CompletionOnceCallback callback,
                                    Request* out_req) {
   DVLOG(1) << __func__ << " " << host;
   DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
@@ -290,12 +290,12 @@ int ChannelIDService::GetChannelID(const std::string& host,
 
   // See if a request for the same domain currently in flight.
   bool create_if_missing = false;
-  if (JoinToInFlightRequest(domain, key, create_if_missing, callback,
+  if (JoinToInFlightRequest(domain, key, create_if_missing, &callback,
                             out_req)) {
     return ERR_IO_PENDING;
   }
 
-  int err = LookupChannelID(domain, key, create_if_missing, callback, out_req);
+  int err = LookupChannelID(domain, key, create_if_missing, &callback, out_req);
   return err;
 }
 
@@ -328,9 +328,8 @@ void ChannelIDService::GotChannelID(int err,
   // one.
   workers_created_++;
   ChannelIDServiceWorker* worker = new ChannelIDServiceWorker(
-      server_identifier,
-      base::Bind(&ChannelIDService::GeneratedChannelID,
-                 weak_ptr_factory_.GetWeakPtr()));
+      server_identifier, base::BindOnce(&ChannelIDService::GeneratedChannelID,
+                                        weak_ptr_factory_.GetWeakPtr()));
   worker->Start(task_runner_);
 }
 
@@ -372,29 +371,27 @@ bool ChannelIDService::JoinToInFlightRequest(
     const std::string& domain,
     std::unique_ptr<crypto::ECPrivateKey>* key,
     bool create_if_missing,
-    const CompletionCallback& callback,
+    CompletionOnceCallback* callback,
     Request* out_req) {
-  ChannelIDServiceJob* job = NULL;
   auto j = inflight_.find(domain);
-  if (j != inflight_.end()) {
-    // A request for the same domain is in flight already. We'll attach our
-    // callback, but we'll also mark it as requiring a channel ID if one's
-    // mising.
-    job = j->second.get();
-    inflight_joins_++;
-
-    job->AddRequest(out_req, create_if_missing);
-    out_req->RequestStarted(this, callback, key, job);
-    return true;
-  }
-  return false;
+  if (j == inflight_.end())
+    return false;
+
+  // A request for the same domain is in flight already. We'll attach our
+  // callback, but we'll also mark it as requiring a channel ID if one's mising.
+  ChannelIDServiceJob* job = j->second.get();
+  inflight_joins_++;
+
+  job->AddRequest(out_req, create_if_missing);
+  out_req->RequestStarted(this, std::move(*callback), key, job);
+  return true;
 }
 
 int ChannelIDService::LookupChannelID(
     const std::string& domain,
     std::unique_ptr<crypto::ECPrivateKey>* key,
     bool create_if_missing,
-    const CompletionCallback& callback,
+    CompletionOnceCallback* callback,
     Request* out_req) {
   // Check if a channel ID key already exists for this domain.
   int err = channel_id_store_->GetChannelID(
@@ -414,7 +411,7 @@ int ChannelIDService::LookupChannelID(
     inflight_[domain] = base::WrapUnique(job);
 
     job->AddRequest(out_req);
-    out_req->RequestStarted(this, callback, key, job);
+    out_req->RequestStarted(this, std::move(*callback), key, job);
     return ERR_IO_PENDING;
   }
 

net/ssl/channel_id_service.h

@@ -18,7 +18,7 @@
 #include "base/memory/weak_ptr.h"
 #include "base/task_runner.h"
 #include "base/threading/thread_checker.h"
-#include "net/base/completion_callback.h"
+#include "net/base/completion_once_callback.h"
 #include "net/base/net_export.h"
 #include "net/ssl/channel_id_store.h"
 
@@ -49,14 +49,14 @@ class NET_EXPORT ChannelIDService {
     friend class ChannelIDServiceJob;
 
     void RequestStarted(ChannelIDService* service,
-                        const CompletionCallback& callback,
+                        CompletionOnceCallback callback,
                         std::unique_ptr<crypto::ECPrivateKey>* key,
                         ChannelIDServiceJob* job);
 
     void Post(int error, std::unique_ptr<crypto::ECPrivateKey> key);
 
     ChannelIDService* service_;
-    CompletionCallback callback_;
+    CompletionOnceCallback callback_;
     std::unique_ptr<crypto::ECPrivateKey>* key_;
     ChannelIDServiceJob* job_;
   };
@@ -91,7 +91,7 @@ class NET_EXPORT ChannelIDService {
   // |*out_req| will be initialized with a handle to the async request.
   int GetOrCreateChannelID(const std::string& host,
                            std::unique_ptr<crypto::ECPrivateKey>* key,
-                           const CompletionCallback& callback,
+                           CompletionOnceCallback callback,
                            Request* out_req);
 
   // Fetches the channel ID for the specified host if one exists.
@@ -111,7 +111,7 @@ class NET_EXPORT ChannelIDService {
   // |*out_req| will be initialized with a handle to the async request.
   int GetChannelID(const std::string& host,
                    std::unique_ptr<crypto::ECPrivateKey>* key,
-                   const CompletionCallback& callback,
+                   CompletionOnceCallback callback,
                    Request* out_req);
 
   // Returns the backing ChannelIDStore.
@@ -140,23 +140,24 @@ class NET_EXPORT ChannelIDService {
                     const std::string& server_identifier,
                     std::unique_ptr<crypto::ECPrivateKey> key);
 
-  // Searches for an in-flight request for the same domain. If found,
-  // attaches to the request and returns true. Returns false if no in-flight
-  // request is found.
+  // Searches for an in-flight request for the same domain. If found, attaches
+  // to the request, consumes |*callback|, and returns true.  Otherwise does not
+  // consume |*callback| and returns false.
   bool JoinToInFlightRequest(const std::string& domain,
                              std::unique_ptr<crypto::ECPrivateKey>* key,
                              bool create_if_missing,
-                             const CompletionCallback& callback,
+                             CompletionOnceCallback* callback,
                              Request* out_req);
 
-  // Looks for the channel ID for |domain| in this service's store.
-  // Returns OK if it can be found synchronously, ERR_IO_PENDING if the
-  // result cannot be obtained synchronously, or a network error code on
-  // failure (including failure to find a channel ID of |domain|).
+  // Looks for the channel ID for |domain| in this service's store.  Returns OK
+  // if it can be found synchronously, ERR_IO_PENDING if the result cannot be
+  // obtained synchronously, or a different network error code on failure
+  // (including failure to find a channel ID of |domain|).  Consumes |*callback|
+  // if and only if ERR_IO_PENDING is returned.
   int LookupChannelID(const std::string& domain,
                       std::unique_ptr<crypto::ECPrivateKey>* key,
                       bool create_if_missing,
-                      const CompletionCallback& callback,
+                      CompletionOnceCallback* callback,
                       Request* out_req);
 
   std::unique_ptr<ChannelIDStore> channel_id_store_;