Migrate ChromeMetadataSource to SimpleURLLoader

TBR=sky@chromium.org

Bug: 844990
Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: I9f2caa470570053c539da83f9cac1d93f11a0b98
Reviewed-on: https://chromium-review.googlesource.com/1101562
Commit-Queue: Mark Pilgrim <pilgrim@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: Eugene But <eugenebut@chromium.org>
Reviewed-by: Mathieu Perreault <mathp@chromium.org>
Cr-Commit-Position: refs/heads/master@{#570464}

chrome/browser/autofill/address_normalizer_factory.cc

@@ -8,6 +8,8 @@
 
 #include "chrome/browser/autofill/validation_rules_storage_factory.h"
 #include "chrome/browser/browser_process.h"
+#include "chrome/browser/net/system_network_context_manager.h"
+#include "services/network/public/cpp/shared_url_loader_factory.h"
 #include "third_party/libaddressinput/chromium/chrome_metadata_source.h"
 #include "third_party/libaddressinput/chromium/chrome_storage_impl.h"
 
@@ -21,12 +23,16 @@ AddressNormalizer* AddressNormalizerFactory::GetInstance() {
 }
 
 AddressNormalizerFactory::AddressNormalizerFactory()
-    : address_normalizer_(std::unique_ptr<::i18n::addressinput::Source>(
-                              std::make_unique<ChromeMetadataSource>(
-                                  I18N_ADDRESS_VALIDATION_DATA_URL,
-                                  g_browser_process->system_request_context())),
-                          ValidationRulesStorageFactory::CreateStorage(),
-                          g_browser_process->GetApplicationLocale()) {}
+    : address_normalizer_(
+          std::unique_ptr<::i18n::addressinput::Source>(
+              std::make_unique<ChromeMetadataSource>(
+                  I18N_ADDRESS_VALIDATION_DATA_URL,
+                  g_browser_process->system_network_context_manager()
+                      ? g_browser_process->system_network_context_manager()
+                            ->GetSharedURLLoaderFactory()
+                      : nullptr)),
+          ValidationRulesStorageFactory::CreateStorage(),
+          g_browser_process->GetApplicationLocale()) {}
 
 AddressNormalizerFactory::~AddressNormalizerFactory() {}
 

chrome/browser/autofill/android/personal_data_manager_android.cc

@@ -20,6 +20,7 @@
 #include "chrome/browser/autofill/personal_data_manager_factory.h"
 #include "chrome/browser/autofill/validation_rules_storage_factory.h"
 #include "chrome/browser/browser_process.h"
+#include "chrome/browser/net/system_network_context_manager.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/common/pref_names.h"
@@ -41,6 +42,7 @@
 #include "components/prefs/pref_service.h"
 #include "content/public/browser/web_contents.h"
 #include "jni/PersonalDataManager_jni.h"
+#include "services/network/public/cpp/shared_url_loader_factory.h"
 #include "third_party/libaddressinput/chromium/chrome_metadata_source.h"
 #include "third_party/libaddressinput/chromium/chrome_storage_impl.h"
 #include "ui/base/l10n/l10n_util.h"
@@ -315,7 +317,8 @@ PersonalDataManagerAndroid::PersonalDataManagerAndroid(JNIEnv* env, jobject obj)
           ProfileManager::GetActiveUserProfile())),
       subkey_requester_(std::make_unique<ChromeMetadataSource>(
                             I18N_ADDRESS_VALIDATION_DATA_URL,
-                            g_browser_process->system_request_context()),
+                            g_browser_process->system_network_context_manager()
+                                ->GetSharedURLLoaderFactory()),
                         ValidationRulesStorageFactory::CreateStorage()) {
   personal_data_manager_->AddObserver(this);
 }

chrome/browser/payments/chrome_payment_request_delegate.cc

@@ -11,6 +11,7 @@
 #include "chrome/browser/autofill/personal_data_manager_factory.h"
 #include "chrome/browser/autofill/validation_rules_storage_factory.h"
 #include "chrome/browser/browser_process.h"
+#include "chrome/browser/net/system_network_context_manager.h"
 #include "chrome/browser/payments/payment_request_display_manager_factory.h"
 #include "chrome/browser/payments/ssl_validity_checker.h"
 #include "chrome/browser/profiles/profile.h"
@@ -32,6 +33,7 @@
 #include "components/signin/core/browser/signin_manager.h"
 #include "content/public/browser/web_contents.h"
 #include "services/metrics/public/cpp/ukm_recorder.h"
+#include "services/network/public/cpp/shared_url_loader_factory.h"
 #include "third_party/libaddressinput/chromium/chrome_metadata_source.h"
 #include "third_party/libaddressinput/chromium/chrome_storage_impl.h"
 
@@ -39,11 +41,12 @@ namespace payments {
 
 namespace {
 
-std::unique_ptr<::i18n::addressinput::Source> GetAddressInputSource(
-    net::URLRequestContextGetter* url_context_getter) {
+std::unique_ptr<::i18n::addressinput::Source> GetAddressInputSource() {
   return std::unique_ptr<::i18n::addressinput::Source>(
-      new autofill::ChromeMetadataSource(I18N_ADDRESS_VALIDATION_DATA_URL,
-                                         url_context_getter));
+      new autofill::ChromeMetadataSource(
+          I18N_ADDRESS_VALIDATION_DATA_URL,
+          g_browser_process->system_network_context_manager()
+              ->GetSharedURLLoaderFactory()));
 }
 
 std::unique_ptr<::i18n::addressinput::Storage> GetAddressInputStorage() {
@@ -123,10 +126,9 @@ void ChromePaymentRequestDelegate::DoFullCardRequest(
 
 autofill::RegionDataLoader*
 ChromePaymentRequestDelegate::GetRegionDataLoader() {
-  return new autofill::RegionDataLoaderImpl(
-      GetAddressInputSource(g_browser_process->system_request_context())
-          .release(),
-      GetAddressInputStorage().release(), GetApplicationLocale());
+  return new autofill::RegionDataLoaderImpl(GetAddressInputSource().release(),
+                                            GetAddressInputStorage().release(),
+                                            GetApplicationLocale());
 }
 
 autofill::AddressNormalizer*

ios/chrome/browser/autofill/address_normalizer_factory.cc

@@ -6,6 +6,7 @@
 
 #include "ios/chrome/browser/application_context.h"
 #include "ios/chrome/browser/autofill/validation_rules_storage_factory.h"
+#include "services/network/public/cpp/shared_url_loader_factory.h"
 #include "third_party/libaddressinput/chromium/chrome_metadata_source.h"
 #include "third_party/libaddressinput/src/cpp/include/libaddressinput/source.h"
 #include "third_party/libaddressinput/src/cpp/include/libaddressinput/storage.h"
@@ -13,11 +14,11 @@
 namespace autofill {
 namespace {
 
-std::unique_ptr<::i18n::addressinput::Source> GetAddressInputSource(
-    net::URLRequestContextGetter* url_context_getter) {
+std::unique_ptr<::i18n::addressinput::Source> GetAddressInputSource() {
   return std::unique_ptr<::i18n::addressinput::Source>(
-      new autofill::ChromeMetadataSource(I18N_ADDRESS_VALIDATION_DATA_URL,
-                                         url_context_getter));
+      new autofill::ChromeMetadataSource(
+          I18N_ADDRESS_VALIDATION_DATA_URL,
+          GetApplicationContext()->GetSharedURLLoaderFactory()));
 }
 
 std::unique_ptr<::i18n::addressinput::Storage> GetAddressInputStorage() {
@@ -34,11 +35,9 @@ AddressNormalizer* AddressNormalizerFactory::GetInstance() {
 }
 
 AddressNormalizerFactory::AddressNormalizerFactory()
-    : address_normalizer_(
-          GetAddressInputSource(
-              GetApplicationContext()->GetSystemURLRequestContext()),
-          GetAddressInputStorage(),
-          GetApplicationContext()->GetApplicationLocale()) {}
+    : address_normalizer_(GetAddressInputSource(),
+                          GetAddressInputStorage(),
+                          GetApplicationContext()->GetApplicationLocale()) {}
 
 AddressNormalizerFactory::~AddressNormalizerFactory() {}
 

ios/chrome/browser/payments/payment_request.mm

@@ -47,11 +47,11 @@
 namespace payments {
 namespace {
 
-std::unique_ptr<::i18n::addressinput::Source> GetAddressInputSource(
-    net::URLRequestContextGetter* url_context_getter) {
+std::unique_ptr<::i18n::addressinput::Source> GetAddressInputSource() {
   return std::unique_ptr<::i18n::addressinput::Source>(
-      new autofill::ChromeMetadataSource(I18N_ADDRESS_VALIDATION_DATA_URL,
-                                         url_context_getter));
+      new autofill::ChromeMetadataSource(
+          I18N_ADDRESS_VALIDATION_DATA_URL,
+          GetApplicationContext()->GetSharedURLLoaderFactory()));
 }
 
 std::unique_ptr<::i18n::addressinput::Storage> GetAddressInputStorage() {
@@ -167,11 +167,9 @@ autofill::AddressNormalizer* PaymentRequest::GetAddressNormalizer() {
 }
 
 autofill::RegionDataLoader* PaymentRequest::GetRegionDataLoader() {
-  return new autofill::RegionDataLoaderImpl(
-      GetAddressInputSource(
-          GetApplicationContext()->GetSystemURLRequestContext())
-          .release(),
-      GetAddressInputStorage().release(), GetApplicationLocale());
+  return new autofill::RegionDataLoaderImpl(GetAddressInputSource().release(),
+                                            GetAddressInputStorage().release(),
+                                            GetApplicationLocale());
 }
 
 ukm::UkmRecorder* PaymentRequest::GetUkmRecorder() {

third_party/libaddressinput/BUILD.gn

@@ -132,6 +132,7 @@ static_library("libaddressinput") {
     "//base:i18n",
     "//components/prefs",
     "//net",
+    "//services/network/public/cpp",
     "//third_party/icu",
     "//third_party/re2",
   ]
@@ -243,7 +244,9 @@ test("libaddressinput_unittests") {
     ":test_support",
     "//base/test:run_all_unittests",
     "//components/prefs",
+    "//mojo/edk",
     "//net:test_support",
+    "//services/network:test_support",
     "//testing/gtest",
   ]
 }

third_party/libaddressinput/chromium/DEPS

@@ -2,7 +2,16 @@ include_rules = [
   '+base',
   "+components/prefs",
   '+net',
+  '+services/network/public/cpp',
+  '+services/network/test',
   '+testing',
   '+third_party/icu',
   '+url',
 ]
+
+specific_include_rules = {
+  "chrome_metadata_source_unittest.cc": [
+    "+mojo/edk",
+],
+}
+        

third_party/libaddressinput/chromium/chrome_metadata_source.cc

@@ -15,50 +15,18 @@
 #include "net/base/net_errors.h"
 #include "net/http/http_status_code.h"
 #include "net/traffic_annotation/network_traffic_annotation.h"
-#include "net/url_request/url_fetcher.h"
-#include "net/url_request/url_fetcher_response_writer.h"
+#include "services/network/public/cpp/resource_request.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 autofill {
 
-namespace {
-
-// A URLFetcherResponseWriter that writes into a provided buffer.
-class UnownedStringWriter : public net::URLFetcherResponseWriter {
- public:
-  UnownedStringWriter(std::string* data) : data_(data) {}
-  virtual ~UnownedStringWriter() {}
-
-  virtual int Initialize(const net::CompletionCallback& callback) override {
-    data_->clear();
-    return net::OK;
-  }
-
-  virtual int Write(net::IOBuffer* buffer,
-                    int num_bytes,
-                    const net::CompletionCallback& callback) override {
-    data_->append(buffer->data(), num_bytes);
-    return num_bytes;
-  }
-
-  virtual int Finish(int net_error,
-                     const net::CompletionCallback& callback) override {
-    return net::OK;
-  }
-
- private:
-  std::string* data_;  // weak reference.
-
-  DISALLOW_COPY_AND_ASSIGN(UnownedStringWriter);
-};
-
-}  // namespace
-
 ChromeMetadataSource::ChromeMetadataSource(
     const std::string& validation_data_url,
-    net::URLRequestContextGetter* getter)
+    scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory)
     : validation_data_url_(validation_data_url),
-      getter_(getter) {}
+      url_loader_factory_(std::move(url_loader_factory)) {}
 
 ChromeMetadataSource::~ChromeMetadataSource() {}
 
@@ -67,23 +35,24 @@ void ChromeMetadataSource::Get(const std::string& key,
   const_cast<ChromeMetadataSource*>(this)->Download(key, downloaded);
 }
 
-void ChromeMetadataSource::OnURLFetchComplete(const net::URLFetcher* source) {
-  auto request = requests_.find(source);
-  DCHECK(request != requests_.end());
-
-  bool ok = source->GetResponseCode() == net::HTTP_OK;
+void ChromeMetadataSource::OnSimpleLoaderComplete(
+    RequestList::iterator it,
+    std::unique_ptr<std::string> response_body) {
+  const Callback& callback = it->get()->callback;
+  const std::string& key = it->get()->key;
   std::unique_ptr<std::string> data(new std::string());
+  bool ok = !!response_body;
   if (ok)
-    data->swap(request->second->data);
-  request->second->callback(ok, request->second->key, data.release());
-
-  requests_.erase(request);
+    data->swap(*response_body);
+  callback(ok, key, data.release());
+  requests_.erase(it);
 }
 
-ChromeMetadataSource::Request::Request(const std::string& key,
-                                       std::unique_ptr<net::URLFetcher> fetcher,
-                                       const Callback& callback)
-    : key(key), fetcher(std::move(fetcher)), callback(callback) {}
+ChromeMetadataSource::Request::Request(
+    const std::string& key,
+    std::unique_ptr<network::SimpleURLLoader> loader,
+    const Callback& callback)
+    : key(key), loader(std::move(loader)), callback(callback) {}
 
 void ChromeMetadataSource::Download(const std::string& key,
                                     const Callback& downloaded) {
@@ -92,7 +61,7 @@ void ChromeMetadataSource::Download(const std::string& key,
     downloaded(false, key, NULL);
     return;
   }
-
+  DCHECK(url_loader_factory_);
   net::NetworkTrafficAnnotationTag traffic_annotation =
       net::DefineNetworkTrafficAnnotation("lib_address_input", R"(
         semantics {
@@ -118,18 +87,21 @@ void ChromeMetadataSource::Download(const std::string& key,
             "'Manage Autofill Settings' -> 'Addresses')."
           policy_exception_justification: "Not implemented."
         })");
-  std::unique_ptr<net::URLFetcher> fetcher = net::URLFetcher::Create(
-      resource, net::URLFetcher::GET, this, traffic_annotation);
-  fetcher->SetLoadFlags(
-      net::LOAD_DO_NOT_SEND_COOKIES | net::LOAD_DO_NOT_SAVE_COOKIES);
-  fetcher->SetRequestContext(getter_);
-
-  Request* request = new Request(key, std::move(fetcher), downloaded);
-  request->fetcher->SaveResponseWithWriter(
-      std::unique_ptr<net::URLFetcherResponseWriter>(
-          new UnownedStringWriter(&request->data)));
-  requests_[request->fetcher.get()] = base::WrapUnique(request);
-  request->fetcher->Start();
+  auto resource_request = std::make_unique<network::ResourceRequest>();
+  resource_request->url = resource;
+  resource_request->load_flags =
+      net::LOAD_DO_NOT_SEND_COOKIES | net::LOAD_DO_NOT_SAVE_COOKIES;
+  std::unique_ptr<network::SimpleURLLoader> loader =
+      network::SimpleURLLoader::Create(std::move(resource_request),
+                                       traffic_annotation);
+  auto it = requests_.insert(
+      requests_.begin(),
+      std::make_unique<Request>(key, std::move(loader), downloaded));
+  network::SimpleURLLoader* raw_loader = it->get()->loader.get();
+  raw_loader->DownloadToStringOfUnboundedSizeUntilCrashAndDie(
+      url_loader_factory_.get(),
+      base::BindOnce(&ChromeMetadataSource::OnSimpleLoaderComplete,
+                     base::Unretained(this), std::move(it)));
 }
 
 }  // namespace autofill

third_party/libaddressinput/chromium/chrome_metadata_source.h

@@ -5,59 +5,61 @@
 #ifndef THIRD_PARTY_LIBADDRESSINPUT_CHROMIUM_CHROME_METADATA_SOURCE_H_
 #define THIRD_PARTY_LIBADDRESSINPUT_CHROMIUM_CHROME_METADATA_SOURCE_H_
 
-#include <map>
+#include <list>
 #include <memory>
 #include <string>
 
 #include "base/macros.h"
-#include "net/url_request/url_fetcher_delegate.h"
+#include "base/memory/scoped_refptr.h"
 #include "third_party/libaddressinput/src/cpp/include/libaddressinput/source.h"
 
-namespace net {
-class URLFetcher;
-class URLRequestContextGetter;
+namespace network {
+class SharedURLLoaderFactory;
+class SimpleURLLoader;
 }
 
 namespace autofill {
 
 // A class for downloading rules to let libaddressinput validate international
 // addresses.
-class ChromeMetadataSource : public ::i18n::addressinput::Source,
-                             public net::URLFetcherDelegate {
+class ChromeMetadataSource : public ::i18n::addressinput::Source {
  public:
-  ChromeMetadataSource(const std::string& validation_data_url,
-                       net::URLRequestContextGetter* getter);
+  ChromeMetadataSource(
+      const std::string& validation_data_url,
+      scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory);
   virtual ~ChromeMetadataSource();
 
   // ::i18n::addressinput::Source:
   virtual void Get(const std::string& key,
                    const Callback& downloaded) const override;
 
-  // net::URLFetcherDelegate:
-  virtual void OnURLFetchComplete(const net::URLFetcher* source) override;
-
  private:
   struct Request {
     Request(const std::string& key,
-            std::unique_ptr<net::URLFetcher> fetcher,
+            std::unique_ptr<network::SimpleURLLoader> loader,
             const Callback& callback);
 
     std::string key;
     // The data that's received.
     std::string data;
     // The object that manages retrieving the data.
-    std::unique_ptr<net::URLFetcher> fetcher;
+    std::unique_ptr<network::SimpleURLLoader> loader;
     const Callback& callback;
   };
 
+  using RequestList = std::list<std::unique_ptr<Request>>;
+
   // Non-const method actually implementing Get().
   void Download(const std::string& key, const Callback& downloaded);
 
+  void OnSimpleLoaderComplete(RequestList::iterator it,
+                              std::unique_ptr<std::string> response_body);
+
   const std::string validation_data_url_;
-  net::URLRequestContextGetter* const getter_;  // weak
+  scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory_;
 
-  // Maps from active URL fetcher to request metadata.
-  std::map<const net::URLFetcher*, std::unique_ptr<Request>> requests_;
+  // Holds all pending requests and their URL loaders.
+  RequestList requests_;
 
   DISALLOW_COPY_AND_ASSIGN(ChromeMetadataSource);
 };

third_party/libaddressinput/chromium/chrome_metadata_source_unittest.cc

@@ -7,8 +7,10 @@
 #include "base/message_loop/message_loop.h"
 #include "base/run_loop.h"
 #include "base/threading/thread_task_runner_handle.h"
-#include "net/url_request/test_url_fetcher_factory.h"
-#include "net/url_request/url_request_test_util.h"
+#include "mojo/edk/embedder/embedder.h"
+#include "services/network/public/cpp/shared_url_loader_factory.h"
+#include "services/network/public/cpp/weak_wrapper_shared_url_loader_factory.h"
+#include "services/network/test/test_url_loader_factory.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace autofill {
@@ -19,25 +21,23 @@ static const char kFakeInsecureUrl[] = "http://example.com";
 class ChromeMetadataSourceTest : public testing::Test {
  public:
   ChromeMetadataSourceTest()
-      : fake_factory_(&factory_),
-        success_(false) {}
+      : test_shared_loader_factory_(
+            base::MakeRefCounted<network::WeakWrapperSharedURLLoaderFactory>(
+                &test_url_loader_factory_)),
+        success_(false) {
+    mojo::edk::Init();
+  }
   virtual ~ChromeMetadataSourceTest() {}
 
  protected:
   // Sets the response for the download.
   void SetFakeResponse(const std::string& payload, net::HttpStatusCode code) {
-    fake_factory_.SetFakeResponse(url_,
-                                  payload,
-                                  code,
-                                  net::URLRequestStatus::SUCCESS);
+    test_url_loader_factory_.AddResponse(url_.spec(), payload, code);
   }
 
   // Kicks off the download.
   void Get() {
-    scoped_refptr<net::TestURLRequestContextGetter> getter(
-        new net::TestURLRequestContextGetter(
-            base::ThreadTaskRunnerHandle::Get()));
-    ChromeMetadataSource impl(std::string(), getter.get());
+    ChromeMetadataSource impl(std::string(), test_shared_loader_factory_);
     std::unique_ptr<::i18n::addressinput::Source::Callback> callback(
         ::i18n::addressinput::BuildCallback(
             this, &ChromeMetadataSourceTest::OnDownloaded));
@@ -65,8 +65,8 @@ class ChromeMetadataSourceTest : public testing::Test {
   }
 
   base::MessageLoop loop_;
-  net::URLFetcherImplFactory factory_;
-  net::FakeURLFetcherFactory fake_factory_;
+  network::TestURLLoaderFactory test_url_loader_factory_;
+  scoped_refptr<network::SharedURLLoaderFactory> test_shared_loader_factory_;
   GURL url_;
   std::unique_ptr<std::string> data_;
   bool success_;