Store callback in DhcpPacFileFetcherChromeos.

The //net idiom for async calls is that they take a callback, and either
* return ERR_IO_PENDING, and later call this callback (unless
  cancelled), or
* return something other than ERR_IO_PENDING and never call the callback.

DhcpPacFileFetcherChromeos has a Fetch() method which always returns
asynchronously.  This class creates a PacFileFetcher, and in its
ContinueFetch() method calls Fetch() on this PacFileFetcher.
PacFileFetcher::Fetch() can return synchronously or asynchronously.  In
the former case, DhcpPacFileFetcherChromeos must call the callback, in
the latter case, PacFileFetcher will.  Currently this is implemented by
calling AdaptCallbackForRepeating() in
DhcpPacFileFetcherChromeos::ContinueFetch(), which turns the
OnceCallback into a RepeatingCallback so that it can be passed in to
PacFileFetcher::Fetch() while also retained.  However,
AdaptCallbackForRepeating() is not meant to be used other than
temporarily for transitioning from Callback to OnceCallback (which is
exactly what we are doing).

This CL makes DhcpPacFileFetcherChromeos save the callback in a private
member, and passes in a bound method to PacFileFetcher::Fetch().  This
way there is no need to use AdaptCallbackForRepeating().

If DhcpPacFileFetcherChromeos::Fetch() is cancelled (Cancel() is called
or the class instance is destroyed), then PacFileFetcher is
appropriately notified and/or destroyed, so the (invalidated) callback
is never run.

This is a follow-up to
https://crrev.com/c/1066291/9/chromeos/network/dhcp_pac_file_fetcher_chromeos.cc#94.

Bug: 730593, 807724
Change-Id: Ic9597114bf54893a98990d56bdb1682cbf4aef5f
Reviewed-on: https://chromium-review.googlesource.com/1073192
Commit-Queue: Bence Béky <bnc@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#562083}

chromeos/network/dhcp_pac_file_fetcher_chromeos.cc

@@ -4,7 +4,6 @@
 
 #include "chromeos/network/dhcp_pac_file_fetcher_chromeos.h"
 
-#include "base/callback_helpers.h"
 #include "base/location.h"
 #include "base/task_runner_util.h"
 #include "chromeos/network/network_event_log.h"
@@ -50,13 +49,16 @@ int DhcpPacFileFetcherChromeos::Fetch(
     const net::NetworkTrafficAnnotationTag traffic_annotation) {
   if (!network_handler_task_runner_.get())
     return net::ERR_PAC_NOT_IN_DHCP;
-  CHECK(!callback.is_null());
+  CHECK(callback);
+  // DhcpPacFileFetcher only allows one Fetch in progress at a time.
+  CHECK(!callback_);
+  callback_ = std::move(callback);
   base::PostTaskAndReplyWithResult(
       network_handler_task_runner_.get(), FROM_HERE,
       base::BindOnce(&GetPacUrlFromDefaultNetwork),
       base::BindOnce(&DhcpPacFileFetcherChromeos::ContinueFetch,
                      weak_ptr_factory_.GetWeakPtr(), utf16_text,
-                     std::move(callback), traffic_annotation));
+                     traffic_annotation));
   return net::ERR_IO_PENDING;
 }
 
@@ -80,22 +82,26 @@ std::string DhcpPacFileFetcherChromeos::GetFetcherName() const {
 
 void DhcpPacFileFetcherChromeos::ContinueFetch(
     base::string16* utf16_text,
-    net::CompletionOnceCallback callback,
     const net::NetworkTrafficAnnotationTag traffic_annotation,
     std::string pac_url) {
   NET_LOG_EVENT("DhcpPacFileFetcher", pac_url);
   pac_url_ = GURL(pac_url);
   if (pac_url_.is_empty()) {
-    std::move(callback).Run(net::ERR_PAC_NOT_IN_DHCP);
+    std::move(callback_).Run(net::ERR_PAC_NOT_IN_DHCP);
     return;
   }
 
-  auto repeating_callback =
-      base::AdaptCallbackForRepeating(std::move(callback));
-  int res = pac_file_fetcher_->Fetch(pac_url_, utf16_text, repeating_callback,
-                                     traffic_annotation);
-  if (res != net::ERR_IO_PENDING)
-    repeating_callback.Run(res);
+  int result = pac_file_fetcher_->Fetch(
+      pac_url_, utf16_text,
+      base::BindOnce(&DhcpPacFileFetcherChromeos::OnFetchCompleted,
+                     weak_ptr_factory_.GetWeakPtr()),
+      traffic_annotation);
+  if (result != net::ERR_IO_PENDING)
+    std::move(callback_).Run(result);
+}
+
+void DhcpPacFileFetcherChromeos::OnFetchCompleted(int result) {
+  std::move(callback_).Run(result);
 }
 
 }  // namespace chromeos

chromeos/network/dhcp_pac_file_fetcher_chromeos.h

@@ -50,10 +50,12 @@ class CHROMEOS_EXPORT DhcpPacFileFetcherChromeos
 
  private:
   void ContinueFetch(base::string16* utf16_text,
-                     net::CompletionOnceCallback callback,
                      const net::NetworkTrafficAnnotationTag traffic_annotation,
                      std::string pac_url);
 
+  void OnFetchCompleted(int result);
+
+  net::CompletionOnceCallback callback_;
   std::unique_ptr<net::PacFileFetcher> pac_file_fetcher_;
   scoped_refptr<base::SingleThreadTaskRunner> network_handler_task_runner_;