(reland) Migrate BrandcodeConfigFetcher to SimpleURLLoader

Relanding https://chromium-review.googlesource.com/c/chromium/src/+/1026290
with fix for build breakage (missing include file was not caught by
local build or any default trybot because it was behind a DEFINE) and
one other minor suggestion by leiz (use base::MakeRefCounted for
head.headers).

Bug: 773295
Change-Id: I25d9327a893debf35ccbc7d4f64b84b6d6e84267
Reviewed-on: https://chromium-review.googlesource.com/1031316Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Commit-Queue: Mark Pilgrim <pilgrim@chromium.org>
Cr-Commit-Position: refs/heads/master@{#554393}

chrome/browser/DEPS

@@ -61,8 +61,9 @@ include_rules = [
   "+services/metrics/public",
   "+services/network/ignore_errors_cert_verifier.h",
   "+services/network/network_service.h",
-  "+services/network/url_request_context_owner.h",
   "+services/network/public",
+  "+services/network/test",
+  "+services/network/url_request_context_owner.h",
   "+services/network/url_request_context_builder_mojo.h",
   "+services/preferences/public/cpp",
   "+services/preferences/public/mojom",

chrome/browser/profile_resetter/brandcode_config_fetcher.cc

@@ -16,8 +16,9 @@
 #include "net/base/load_flags.h"
 #include "net/http/http_response_headers.h"
 #include "net/traffic_annotation/network_traffic_annotation.h"
-#include "net/url_request/url_fetcher.h"
-#include "net/url_request/url_request_status.h"
+#include "services/network/public/cpp/resource_request.h"
+#include "services/network/public/cpp/simple_url_loader.h"
+#include "services/network/public/mojom/url_loader_factory.mojom.h"
 
 namespace {
 
@@ -139,9 +140,11 @@ bool XmlConfigParser::IsParsingData() const {
 
 }  // namespace
 
-BrandcodeConfigFetcher::BrandcodeConfigFetcher(const FetchCallback& callback,
-                                               const GURL& url,
-                                               const std::string& brandcode)
+BrandcodeConfigFetcher::BrandcodeConfigFetcher(
+    network::mojom::URLLoaderFactory* url_loader_factory,
+    const FetchCallback& callback,
+    const GURL& url,
+    const std::string& brandcode)
     : fetch_callback_(callback) {
   DCHECK(!brandcode.empty());
   net::NetworkTrafficAnnotationTag traffic_annotation =
@@ -166,17 +169,21 @@ BrandcodeConfigFetcher::BrandcodeConfigFetcher(const FetchCallback& callback,
             "Not implemented, considered not useful as enterprises don't need "
             "to install Chrome in a non-organic fashion."
         })");
-  config_fetcher_ =
-      net::URLFetcher::Create(0 /* ID used for testing */, url,
-                              net::URLFetcher::POST, this, traffic_annotation);
-  config_fetcher_->SetRequestContext(
-      g_browser_process->system_request_context());
-  config_fetcher_->SetUploadData("text/xml", GetUploadData(brandcode));
-  config_fetcher_->AddExtraRequestHeader("Accept: text/xml");
-  config_fetcher_->SetLoadFlags(net::LOAD_DO_NOT_SEND_COOKIES |
-                                net::LOAD_DO_NOT_SAVE_COOKIES |
-                                net::LOAD_DISABLE_CACHE);
-  config_fetcher_->Start();
+  auto resource_request = std::make_unique<network::ResourceRequest>();
+  resource_request->url = url;
+  resource_request->load_flags = net::LOAD_DO_NOT_SEND_COOKIES |
+                                 net::LOAD_DO_NOT_SAVE_COOKIES |
+                                 net::LOAD_DISABLE_CACHE;
+  resource_request->method = "POST";
+  resource_request->headers.SetHeader("Accept", "text/xml");
+  simple_url_loader_ = network::SimpleURLLoader::Create(
+      std::move(resource_request), traffic_annotation);
+  simple_url_loader_->AttachStringForUpload(GetUploadData(brandcode),
+                                            "text/xml");
+  simple_url_loader_->DownloadToStringOfUnboundedSizeUntilCrashAndDie(
+      url_loader_factory,
+      base::BindOnce(&BrandcodeConfigFetcher::OnSimpleLoaderComplete,
+                     base::Unretained(this)));
   // Abort the download attempt if it takes too long.
   download_timer_.Start(FROM_HERE,
                         base::TimeDelta::FromSeconds(kDownloadTimeoutSec),
@@ -190,31 +197,22 @@ void BrandcodeConfigFetcher::SetCallback(const FetchCallback& callback) {
   fetch_callback_ = callback;
 }
 
-void BrandcodeConfigFetcher::OnURLFetchComplete(const net::URLFetcher* source) {
-  if (source != config_fetcher_.get()) {
-    NOTREACHED() << "Callback from foreign URL fetcher";
-    return;
-  }
-  std::string response_string;
-  std::string mime_type;
-  if (config_fetcher_ &&
-      config_fetcher_->GetStatus().is_success() &&
-      config_fetcher_->GetResponseCode() == 200 &&
-      config_fetcher_->GetResponseHeaders()->GetMimeType(&mime_type) &&
-      mime_type == "text/xml" &&
-      config_fetcher_->GetResponseAsString(&response_string)) {
+void BrandcodeConfigFetcher::OnSimpleLoaderComplete(
+    std::unique_ptr<std::string> response_body) {
+  if (response_body && simple_url_loader_->ResponseInfo() &&
+      simple_url_loader_->ResponseInfo()->mime_type == "text/xml") {
     std::string master_prefs;
-    XmlConfigParser::Parse(response_string, &master_prefs);
+    XmlConfigParser::Parse(*response_body, &master_prefs);
     default_settings_.reset(new BrandcodedDefaultSettings(master_prefs));
   }
-  config_fetcher_.reset();
+  simple_url_loader_.reset();
   download_timer_.Stop();
   base::ResetAndReturn(&fetch_callback_).Run();
 }
 
 void BrandcodeConfigFetcher::OnDownloadTimeout() {
-  if (config_fetcher_) {
-    config_fetcher_.reset();
+  if (simple_url_loader_) {
+    simple_url_loader_.reset();
     base::ResetAndReturn(&fetch_callback_).Run();
   }
 }

chrome/browser/profile_resetter/brandcode_config_fetcher.h

@@ -12,23 +12,30 @@
 #include "base/callback.h"
 #include "base/macros.h"
 #include "base/timer/timer.h"
-#include "net/url_request/url_fetcher_delegate.h"
 
 class BrandcodedDefaultSettings;
 class GURL;
 
+namespace network {
+class SimpleURLLoader;
+namespace mojom {
+class URLLoaderFactory;
+}
+}  // namespace network
+
 // BrandcodeConfigFetcher fetches and parses the xml containing the brandcoded
 // default settings. Caller should provide a FetchCallback.
-class BrandcodeConfigFetcher : public net::URLFetcherDelegate {
+class BrandcodeConfigFetcher {
  public:
   typedef base::Callback<void ()> FetchCallback;
 
-  BrandcodeConfigFetcher(const FetchCallback& callback,
+  BrandcodeConfigFetcher(network::mojom::URLLoaderFactory* url_loader_factory,
+                         const FetchCallback& callback,
                          const GURL& url,
                          const std::string& brandcode);
-  ~BrandcodeConfigFetcher() override;
+  ~BrandcodeConfigFetcher();
 
-  bool IsActive() const { return !!config_fetcher_; }
+  bool IsActive() const { return !!simple_url_loader_; }
 
   std::unique_ptr<BrandcodedDefaultSettings> GetSettings() {
     return std::move(default_settings_);
@@ -38,8 +45,7 @@ class BrandcodeConfigFetcher : public net::URLFetcherDelegate {
   void SetCallback(const FetchCallback& callback);
 
  private:
-  // net::URLFetcherDelegate:
-  void OnURLFetchComplete(const net::URLFetcher* source) override;
+  void OnSimpleLoaderComplete(std::unique_ptr<std::string> response_body);
 
   void OnDownloadTimeout();
 
@@ -51,7 +57,7 @@ class BrandcodeConfigFetcher : public net::URLFetcherDelegate {
   FetchCallback fetch_callback_;
 
   // Helper to fetch the online config file.
-  std::unique_ptr<net::URLFetcher> config_fetcher_;
+  std::unique_ptr<network::SimpleURLLoader> simple_url_loader_;
 
   // Fetched settings.
   std::unique_ptr<BrandcodedDefaultSettings> default_settings_;

chrome/browser/profile_resetter/profile_resetter_unittest.cc

@@ -12,8 +12,10 @@
 #include <utility>
 
 #include "base/macros.h"
+#include "base/memory/scoped_refptr.h"
 #include "base/run_loop.h"
 #include "base/strings/utf_string_conversions.h"
+#include "base/test/bind_test_util.h"
 #include "base/test/scoped_path_override.h"
 #include "build/build_config.h"
 #include "chrome/browser/content_settings/host_content_settings_map_factory.h"
@@ -45,8 +47,9 @@
 #include "extensions/common/manifest_constants.h"
 #include "net/http/http_response_headers.h"
 #include "net/http/http_status_code.h"
-#include "net/url_request/test_url_fetcher_factory.h"
-#include "net/url_request/url_request_status.h"
+#include "net/http/http_util.h"
+#include "services/network/public/cpp/resource_request.h"
+#include "services/network/test/test_url_loader_factory.h"
 #include "url/gurl.h"
 
 #if defined(OS_WIN)
@@ -174,46 +177,31 @@ content::WebContents* PinnedTabsResetTest::CreateWebContents() {
 
 // ConfigParserTest -----------------------------------------------------------
 
-// URLFetcher delegate that simply records the upload data.
-struct URLFetcherRequestListener : net::URLFetcherDelegate {
-  URLFetcherRequestListener();
-  ~URLFetcherRequestListener() override;
-
-  void OnURLFetchComplete(const net::URLFetcher* source) override;
-
-  std::string upload_data;
-  net::URLFetcherDelegate* real_delegate;
-};
-
-URLFetcherRequestListener::URLFetcherRequestListener()
-    : real_delegate(NULL) {
-}
-
-URLFetcherRequestListener::~URLFetcherRequestListener() {
-}
-
-void URLFetcherRequestListener::OnURLFetchComplete(
-    const net::URLFetcher* source) {
-  const net::TestURLFetcher* test_fetcher =
-      static_cast<const net::TestURLFetcher*>(source);
-  upload_data = test_fetcher->upload_data();
-  DCHECK(real_delegate);
-  real_delegate->OnURLFetchComplete(source);
-}
-
 class ConfigParserTest : public testing::Test {
  protected:
   ConfigParserTest();
   ~ConfigParserTest() override;
 
+  std::string GetBodyFromRequest(const network::ResourceRequest& request) {
+    auto body = request.request_body;
+    if (!body)
+      return std::string();
+
+    CHECK_EQ(1u, body->elements()->size());
+    auto& element = body->elements()->at(0);
+    CHECK_EQ(network::DataElement::TYPE_BYTES, element.type());
+    return std::string(element.bytes(), element.length());
+  }
+
   std::unique_ptr<BrandcodeConfigFetcher> WaitForRequest(const GURL& url);
 
-  net::FakeURLFetcherFactory& factory() { return factory_; }
+  network::TestURLLoaderFactory& test_url_loader_factory() {
+    return test_url_loader_factory_;
+  }
 
  private:
-  std::unique_ptr<net::FakeURLFetcher> CreateFakeURLFetcher(
+  std::unique_ptr<network::SimpleURLLoader> CreateFakeURLLoader(
       const GURL& url,
-      net::URLFetcherDelegate* fetcher_delegate,
       const std::string& response_data,
       net::HttpStatusCode response_code,
       net::URLRequestStatus::Status status);
@@ -221,45 +209,31 @@ class ConfigParserTest : public testing::Test {
   MOCK_METHOD0(Callback, void(void));
 
   content::TestBrowserThreadBundle test_browser_thread_bundle_;
-  URLFetcherRequestListener request_listener_;
-  net::FakeURLFetcherFactory factory_;
+  network::TestURLLoaderFactory test_url_loader_factory_;
 };
 
 ConfigParserTest::ConfigParserTest()
     : test_browser_thread_bundle_(
-          content::TestBrowserThreadBundle::IO_MAINLOOP),
-      factory_(NULL,
-               base::Bind(&ConfigParserTest::CreateFakeURLFetcher,
-                          base::Unretained(this))) {}
+          content::TestBrowserThreadBundle::IO_MAINLOOP) {}
 
 ConfigParserTest::~ConfigParserTest() {}
 
 std::unique_ptr<BrandcodeConfigFetcher> ConfigParserTest::WaitForRequest(
     const GURL& url) {
   EXPECT_CALL(*this, Callback());
+  std::string upload_data;
+  test_url_loader_factory_.SetInterceptor(
+      base::BindLambdaForTesting([&](const network::ResourceRequest& request) {
+        upload_data = GetBodyFromRequest(request);
+      }));
   std::unique_ptr<BrandcodeConfigFetcher> fetcher(new BrandcodeConfigFetcher(
+      &test_url_loader_factory_,
       base::Bind(&ConfigParserTest::Callback, base::Unretained(this)), url,
       "ABCD"));
   base::RunLoop().RunUntilIdle();
   EXPECT_FALSE(fetcher->IsActive());
   // Look for the brand code in the request.
-  EXPECT_NE(std::string::npos, request_listener_.upload_data.find("ABCD"));
-  return fetcher;
-}
-
-std::unique_ptr<net::FakeURLFetcher> ConfigParserTest::CreateFakeURLFetcher(
-    const GURL& url,
-    net::URLFetcherDelegate* fetcher_delegate,
-    const std::string& response_data,
-    net::HttpStatusCode response_code,
-    net::URLRequestStatus::Status status) {
-  request_listener_.real_delegate = fetcher_delegate;
-  std::unique_ptr<net::FakeURLFetcher> fetcher(new net::FakeURLFetcher(
-      url, &request_listener_, response_data, response_code, status));
-  scoped_refptr<net::HttpResponseHeaders> download_headers =
-      new net::HttpResponseHeaders("");
-  download_headers->AddHeader("Content-Type: text/xml");
-  fetcher->set_response_headers(download_headers);
+  EXPECT_NE(std::string::npos, upload_data.find("ABCD"));
   return fetcher;
 }
 
@@ -771,8 +745,9 @@ TEST_F(ProfileResetterTest, ResetFewFlags) {
 // Tries to load unavailable config file.
 TEST_F(ConfigParserTest, NoConnectivity) {
   const GURL url("http://test");
-  factory().SetFakeResponse(url, "", net::HTTP_INTERNAL_SERVER_ERROR,
-                            net::URLRequestStatus::FAILED);
+  test_url_loader_factory().AddResponse(
+      url, network::ResourceResponseHead(), "",
+      network::URLLoaderCompletionStatus(net::HTTP_INTERNAL_SERVER_ERROR));
 
   std::unique_ptr<BrandcodeConfigFetcher> fetcher = WaitForRequest(GURL(url));
   EXPECT_FALSE(fetcher->GetSettings());
@@ -786,8 +761,14 @@ TEST_F(ConfigParserTest, ParseConfig) {
   ReplaceString(&xml_config,
                 "placeholder_for_id",
                 "abbaabbaabbaabbaabbaabbaabbaabba");
-  factory().SetFakeResponse(url, xml_config, net::HTTP_OK,
-                            net::URLRequestStatus::SUCCESS);
+  network::ResourceResponseHead head;
+  std::string headers("HTTP/1.1 200 OK\nContent-type: text/xml\n\n");
+  head.headers = base::MakeRefCounted<net::HttpResponseHeaders>(
+      net::HttpUtil::AssembleRawHeaders(headers.c_str(), headers.size()));
+  head.mime_type = "text/xml";
+  network::URLLoaderCompletionStatus status;
+  status.decoded_body_length = xml_config.size();
+  test_url_loader_factory().AddResponse(url, head, xml_config, status);
 
   std::unique_ptr<BrandcodeConfigFetcher> fetcher = WaitForRequest(GURL(url));
   std::unique_ptr<BrandcodedDefaultSettings> settings = fetcher->GetSettings();

chrome/browser/safe_browsing/settings_reset_prompt/default_settings_fetcher.cc

@@ -9,10 +9,13 @@
 
 #include "base/bind_helpers.h"
 #include "base/logging.h"
+#include "chrome/browser/browser_process.h"
 #include "chrome/browser/google/google_brand.h"
+#include "chrome/browser/net/system_network_context_manager.h"
 #include "chrome/browser/profile_resetter/brandcode_config_fetcher.h"
 #include "chrome/browser/profile_resetter/brandcoded_default_settings.h"
 #include "content/public/browser/browser_thread.h"
+#include "services/network/public/mojom/url_loader_factory.mojom.h"
 #include "url/gurl.h"
 
 namespace safe_browsing {
@@ -59,6 +62,8 @@ void DefaultSettingsFetcher::Start() {
   std::string brandcode;
   if (google_brand::GetBrand(&brandcode) && !brandcode.empty()) {
     config_fetcher_.reset(new BrandcodeConfigFetcher(
+        g_browser_process->system_network_context_manager()
+            ->GetURLLoaderFactory(),
         base::Bind(&DefaultSettingsFetcher::OnSettingsFetched,
                    base::Unretained(this)),
         GURL(kOmahaUrl), brandcode));

chrome/browser/ui/webui/settings/reset_settings_handler.cc

@@ -14,7 +14,9 @@
 #include "base/time/time.h"
 #include "base/values.h"
 #include "build/build_config.h"
+#include "chrome/browser/browser_process.h"
 #include "chrome/browser/google/google_brand.h"
+#include "chrome/browser/net/system_network_context_manager.h"
 #include "chrome/browser/prefs/chrome_pref_service_factory.h"
 #include "chrome/browser/profile_resetter/brandcode_config_fetcher.h"
 #include "chrome/browser/profile_resetter/brandcoded_default_settings.h"
@@ -26,6 +28,7 @@
 #include "components/prefs/pref_service.h"
 #include "content/public/browser/web_ui.h"
 #include "content/public/browser/web_ui_data_source.h"
+#include "services/network/public/mojom/url_loader_factory.mojom.h"
 #include "ui/base/l10n/l10n_util.h"
 
 #if defined(OS_CHROMEOS)
@@ -212,10 +215,11 @@ void ResetSettingsHandler::OnShowResetProfileDialog(
   if (brandcode_.empty())
     return;
   config_fetcher_.reset(new BrandcodeConfigFetcher(
+      g_browser_process->system_network_context_manager()
+          ->GetURLLoaderFactory(),
       base::Bind(&ResetSettingsHandler::OnSettingsFetched,
                  base::Unretained(this)),
-      GURL("https://tools.google.com/service/update2"),
-      brandcode_));
+      GURL("https://tools.google.com/service/update2"), brandcode_));
 }
 
 void ResetSettingsHandler::OnHideResetProfileDialog(