Reland "Store callback in DhcpPacFileFetcherChromeos."

Original CL landed at https://crrev.com/c/1073192.

Revert (with edits) landed at https://crrev.com/c/1230442.

This CL relands the original with the modification that |callback_| is
reset in DhcpPacFileFetcherChromeos::Cancel() and OnShutdown().  It is
believed that the crashes the original CL caused happened when Fetch()
was called after Cancel(), and this modification will prevent that from
happening.  Kudos to eroman@ for pointing this out at
https://crrev.com/c/1230442/3/chromeos/network/dhcp_pac_file_fetcher_chromeos.cc#63

Bug: 882996
Change-Id: I4747004c00fa3ac7a698d7a35c0b52999411adb5
Reviewed-on: https://chromium-review.googlesource.com/1232576Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Commit-Queue: Bence Béky <bnc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#592518}

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"
@@ -51,22 +50,27 @@ int DhcpPacFileFetcherChromeos::Fetch(
   if (!network_handler_task_runner_.get())
     return net::ERR_PAC_NOT_IN_DHCP;
   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;
 }
 
 void DhcpPacFileFetcherChromeos::Cancel() {
+  callback_.Reset();
   pac_file_fetcher_->Cancel();
   // Invalidate any pending callbacks (i.e. calls to ContinueFetch).
   weak_ptr_factory_.InvalidateWeakPtrs();
 }
 
 void DhcpPacFileFetcherChromeos::OnShutdown() {
+  callback_.Reset();
   pac_file_fetcher_->OnShutdown();
 }
 
@@ -80,22 +84,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

@@ -39,10 +39,6 @@ class CHROMEOS_EXPORT DhcpPacFileFetcherChromeos
   ~DhcpPacFileFetcherChromeos() override;
 
   // net::DhcpPacFileFetcher implementation
-
-  // TODO(https://crbug.com/882996)  Even though it is documented at
-  // DhcpPacFileFetcher::Fetch() that only one fetch is allowed to be
-  // outstanding at any given time, crashes show that this is not obeyed.
   int Fetch(base::string16* utf16_text,
             net::CompletionOnceCallback callback,
             const net::NetLogWithSource& net_log,
@@ -54,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_;