[chromeos] Migrate AttestationCAClient to SimpleURLLoader

URLFetcher will stop working with advent of Network Service, and
SimpleURLLoader is the replacement API for most clients.
This CL migrates chromeos::chromeos::AttestationCAClient and the
respective unittests away from URLFetcher.

BUG=773295,879782

Change-Id: I9ccdad3a73108ad3e3e7ab4f148b93048433b3bd
Reviewed-on: https://chromium-review.googlesource.com/1202223
Commit-Queue: Antonio Gomes <tonikitoo@igalia.com>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#590089}

chrome/browser/chromeos/attestation/attestation_ca_client.cc

@@ -11,8 +11,9 @@
 #include "chromeos/chromeos_switches.h"
 #include "net/base/load_flags.h"
 #include "net/http/http_status_code.h"
-#include "net/url_request/url_fetcher.h"
 #include "net/url_request/url_request_status.h"
+#include "services/network/public/cpp/shared_url_loader_factory.h"
+#include "services/network/public/cpp/simple_url_loader.h"
 #include "url/gurl.h"
 
 namespace {
@@ -75,38 +76,36 @@ void AttestationCAClient::SendCertificateRequest(
            request, on_response);
 }
 
-void AttestationCAClient::OnURLFetchComplete(const net::URLFetcher* source) {
-  FetcherCallbackMap::iterator iter = pending_requests_.find(source);
-  if (iter == pending_requests_.end()) {
-    LOG(WARNING) << "Callback from unknown source.";
-    return;
-  }
+void AttestationCAClient::OnURLLoadComplete(
+    std::list<std::unique_ptr<network::SimpleURLLoader>>::iterator it,
+    const DataCallback& callback,
+    std::unique_ptr<std::string> response_body) {
+  // Move the loader out of the active loaders list.
+  std::unique_ptr<network::SimpleURLLoader> url_loader = std::move(*it);
+  url_loaders_.erase(it);
 
-  DataCallback callback = iter->second;
-  pending_requests_.erase(iter);
-  std::unique_ptr<const net::URLFetcher> scoped_source(source);
+  DCHECK(url_loader);
 
-  if (source->GetStatus().status() != net::URLRequestStatus::SUCCESS) {
-    LOG(ERROR) << "Attestation CA request failed, status: "
-               << source->GetStatus().status() << ", error: "
-               << source->GetStatus().error();
+  int response_code = -1;
+  if (url_loader->ResponseInfo() && url_loader->ResponseInfo()->headers) {
+    response_code = url_loader->ResponseInfo()->headers->response_code();
+  }
+
+  if (response_code < 200 || response_code > 299) {
+    LOG(ERROR) << "Attestation CA sent an error response: " << response_code;
     callback.Run(false, "");
     return;
   }
 
-  if (source->GetResponseCode() != net::HTTP_OK) {
-    LOG(ERROR) << "Attestation CA sent an error response: "
-               << source->GetResponseCode();
+  if (!response_body) {
+    LOG(ERROR) << "Attestation CA request failed, error: "
+               << url_loader->NetError();
     callback.Run(false, "");
     return;
   }
 
-  std::string response;
-  bool result = source->GetResponseAsString(&response);
-  DCHECK(result) << "Invalid fetcher setting.";
-
   // Run the callback last because it may delete |this|.
-  callback.Run(true, response);
+  callback.Run(true, *response_body);
 }
 
 void AttestationCAClient::FetchURL(const std::string& url,
@@ -140,18 +139,25 @@ void AttestationCAClient::FetchURL(const std::string& url,
             "cannot be controlled by a policy or through settings."
           policy_exception_justification: "Not implemented."
         })");
-  // The first argument allows the use of TestURLFetcherFactory in tests.
-  net::URLFetcher* fetcher =
-      net::URLFetcher::Create(0, GURL(url), net::URLFetcher::POST, this,
-                              traffic_annotation)
-          .release();
-  fetcher->SetRequestContext(g_browser_process->system_request_context());
-  fetcher->SetLoadFlags(net::LOAD_DO_NOT_SEND_COOKIES |
-                        net::LOAD_DO_NOT_SAVE_COOKIES |
-                        net::LOAD_DISABLE_CACHE);
-  fetcher->SetUploadData(kMimeContentType, request);
-  pending_requests_[fetcher] = on_response;
-  fetcher->Start();
+  auto resource_request = std::make_unique<network::ResourceRequest>();
+  resource_request->url = GURL(url);
+  resource_request->method = "POST";
+  resource_request->load_flags = net::LOAD_DO_NOT_SEND_COOKIES |
+                                 net::LOAD_DO_NOT_SAVE_COOKIES |
+                                 net::LOAD_DISABLE_CACHE;
+
+  auto url_loader = network::SimpleURLLoader::Create(
+      std::move(resource_request), traffic_annotation);
+  url_loader->AttachStringForUpload(request, kMimeContentType);
+
+  auto* raw_url_loader = url_loader.get();
+  url_loaders_.push_back(std::move(url_loader));
+
+  raw_url_loader->DownloadToStringOfUnboundedSizeUntilCrashAndDie(
+      g_browser_process->shared_url_loader_factory().get(),
+      base::BindOnce(&AttestationCAClient::OnURLLoadComplete,
+                     base::Unretained(this), std::move(--url_loaders_.end()),
+                     on_response));
 }
 
 PrivacyCAType AttestationCAClient::GetType() {

chrome/browser/chromeos/attestation/attestation_ca_client.h

@@ -5,21 +5,24 @@
 #ifndef CHROME_BROWSER_CHROMEOS_ATTESTATION_ATTESTATION_CA_CLIENT_H_
 #define CHROME_BROWSER_CHROMEOS_ATTESTATION_ATTESTATION_CA_CLIENT_H_
 
+#include <list>
 #include <map>
 #include <string>
 
 #include "base/macros.h"
 #include "chromeos/attestation/attestation_flow.h"
 #include "chromeos/dbus/attestation_constants.h"
-#include "net/url_request/url_fetcher_delegate.h"
+
+namespace network {
+class SimpleURLLoader;
+}
 
 namespace chromeos {
 namespace attestation {
 
 // This class is a ServerProxy implementation for the Chrome OS attestation
 // flow.  It sends all requests to an Attestation CA via HTTPS.
-class AttestationCAClient : public ServerProxy,
-                            public net::URLFetcherDelegate {
+class AttestationCAClient : public ServerProxy {
  public:
   AttestationCAClient();
   ~AttestationCAClient() override;
@@ -30,24 +33,24 @@ class AttestationCAClient : public ServerProxy,
   void SendCertificateRequest(const std::string& request,
                               const DataCallback& on_response) override;
 
-  // net::URLFetcherDelegate:
-  void OnURLFetchComplete(const net::URLFetcher* source) override;
+  void OnURLLoadComplete(
+      std::list<std::unique_ptr<network::SimpleURLLoader>>::iterator it,
+      const DataCallback& on_response,
+      std::unique_ptr<std::string> response_body);
 
   PrivacyCAType GetType() override;
 
  private:
   PrivacyCAType pca_type_;
 
-  typedef std::map<const net::URLFetcher*, DataCallback> FetcherCallbackMap;
-
   // POSTs |request| data to |url| and calls |on_response| with the response
   // data when the fetch is complete.
   void FetchURL(const std::string& url,
                 const std::string& request,
                 const DataCallback& on_response);
 
-  // Tracks all URL requests we have started.
-  FetcherCallbackMap pending_requests_;
+  // Loaders used for the processing the requests. Invalidated after completion.
+  std::list<std::unique_ptr<network::SimpleURLLoader>> url_loaders_;
 
   DISALLOW_COPY_AND_ASSIGN(AttestationCAClient);
 };

chrome/browser/chromeos/attestation/attestation_ca_client_unittest.cc

@@ -6,13 +6,16 @@
 
 #include "base/bind.h"
 #include "base/command_line.h"
+#include "base/test/bind_test_util.h"
+#include "chrome/test/base/testing_browser_process.h"
 #include "chromeos/chromeos_switches.h"
 #include "content/public/test/test_browser_thread_bundle.h"
 #include "net/base/net_errors.h"
 #include "net/http/http_status_code.h"
-#include "net/url_request/test_url_fetcher_factory.h"
-#include "net/url_request/url_fetcher.h"
 #include "net/url_request/url_request_status.h"
+#include "services/network/public/cpp/weak_wrapper_shared_url_loader_factory.h"
+#include "services/network/test/test_url_loader_factory.h"
+#include "services/network/test/test_utils.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace chromeos {
@@ -20,10 +23,25 @@ namespace attestation {
 
 class AttestationCAClientTest : public ::testing::Test {
  public:
-  AttestationCAClientTest() : num_invocations_(0), result_(false) {}
+  AttestationCAClientTest()
+      : test_shared_url_loader_factory_(
+            base::MakeRefCounted<network::WeakWrapperSharedURLLoaderFactory>(
+                &test_url_loader_factory_)),
+        num_invocations_(0),
+        result_(false) {}
 
   ~AttestationCAClientTest() override {}
 
+  void SetUp() override {
+    TestingBrowserProcess::GetGlobal()->SetSharedURLLoaderFactory(
+        test_shared_url_loader_factory_);
+
+    test_url_loader_factory_.SetInterceptor(base::BindLambdaForTesting(
+        [&](const network::ResourceRequest& request) {
+          last_resource_request_ = request;
+        }));
+  }
+
   void DataCallback(bool result, const std::string& data) {
     ++num_invocations_;
     result_ = result;
@@ -41,29 +59,36 @@ class AttestationCAClientTest : public ::testing::Test {
   void CheckURLAndSendResponse(GURL expected_url,
                                net::Error error,
                                int response_code) {
-    net::TestURLFetcher* fetcher = url_fetcher_factory_.GetFetcherByID(0);
-    CHECK(fetcher);
-    EXPECT_EQ(expected_url, fetcher->GetOriginalURL());
-    SendFetcherResponse(fetcher, error, response_code);
+    CHECK(test_url_loader_factory_.NumPending() == 1);
+    EXPECT_EQ(expected_url, last_resource_request_.url);
+    std::string response =
+        network::GetUploadData(last_resource_request_) + "_response";
+    test_url_loader_factory_.AddResponse(last_resource_request_.url.spec(),
+                                         response);
+    base::RunLoop().RunUntilIdle();
   }
 
-  void SendResponse(net::Error error, int response_code) {
-    net::TestURLFetcher* fetcher = url_fetcher_factory_.GetFetcherByID(0);
-    CHECK(fetcher);
-    SendFetcherResponse(fetcher, error, response_code);
-  }
-
-  void SendFetcherResponse(net::TestURLFetcher* fetcher,
-                           net::Error error,
-                           int response_code) {
-    fetcher->set_status(net::URLRequestStatus::FromError(error));
-    fetcher->set_response_code(response_code);
-    fetcher->SetResponseString(fetcher->upload_data() + "_response");
-    fetcher->delegate()->OnURLFetchComplete(fetcher);
+  void SendResponse(net::Error error, net::HttpStatusCode response_code) {
+    CHECK(test_url_loader_factory_.NumPending() == 1);
+    auto resource_response_head =
+        network::CreateResourceResponseHead(response_code);
+    network::URLLoaderCompletionStatus completion_status(error);
+    std::string response =
+        network::GetUploadData(last_resource_request_) + "_response";
+
+    test_url_loader_factory_.AddResponse(last_resource_request_.url,
+                                         resource_response_head, response,
+                                         completion_status);
+    base::RunLoop().RunUntilIdle();
   }
 
   content::TestBrowserThreadBundle test_browser_thread_bundle_;
-  net::TestURLFetcherFactory url_fetcher_factory_;
+
+  network::TestURLLoaderFactory test_url_loader_factory_;
+  scoped_refptr<network::SharedURLLoaderFactory>
+      test_shared_url_loader_factory_;
+
+  network::ResourceRequest last_resource_request_;
 
   // For use with DataCallback.
   int num_invocations_;