Port quirks::QuirksClient to network::SimpleURLLoader

Bug: 844953
Change-Id: I5a36a254228e1f5a13e1b24a11875f79be330464
Reviewed-on: https://chromium-review.googlesource.com/1123275Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
Reviewed-by: Maks Orlovich <morlovich@chromium.org>
Commit-Queue: Reilly Grant <reillyg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#573887}

chrome/browser/chromeos/chrome_browser_main_chromeos.cc

@@ -166,8 +166,6 @@
 #include "net/base/network_change_notifier.h"
 #include "net/cert/nss_cert_database.h"
 #include "net/cert/nss_cert_database_chromeos.h"
-#include "net/url_request/url_request.h"
-#include "net/url_request/url_request_context_getter.h"
 #include "printing/backend/print_backend.h"
 #include "rlz/buildflags/buildflags.h"
 #include "services/service_manager/public/cpp/connector.h"
@@ -661,7 +659,8 @@ void ChromeBrowserMainPartsChromeos::PreMainMessageLoopRun() {
       std::unique_ptr<quirks::QuirksManager::Delegate>(
           new quirks::QuirksManagerDelegateImpl()),
       g_browser_process->local_state(),
-      g_browser_process->system_request_context());
+      g_browser_process->system_network_context_manager()
+          ->GetSharedURLLoaderFactory());
 
   // Start loading machine statistics here. StatisticsProvider::Shutdown()
   // will ensure that loading is aborted on early exit.

chrome/browser/chromeos/display/quirks_browsertest.cc

@@ -5,10 +5,12 @@
 #include "base/files/file_util.h"
 #include "base/memory/ptr_util.h"
 #include "base/run_loop.h"
+#include "base/test/bind_test_util.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "components/quirks/quirks_manager.h"
 #include "content/public/test/test_utils.h"
-#include "net/url_request/test_url_fetcher_factory.h"
+#include "services/network/public/cpp/weak_wrapper_shared_url_loader_factory.h"
+#include "services/network/test/test_url_loader_factory.h"
 
 namespace quirks {
 
@@ -18,26 +20,6 @@ const char kFakeIccJson[] = "{\n  \"icc\": \"AAAIkCAgICACEAAA\"\n}";
 const char kFakeIccData[] = {0x00, 0x00, 0x08, 0x90, 0x20, 0x20,
                              0x20, 0x20, 0x02, 0x10, 0x00, 0x00};
 
-// Create FakeURLFetcher which returns fake icc file json.
-std::unique_ptr<net::URLFetcher> CreateFakeURLFetcherSuccess(
-    const GURL& url,
-    net::URLFetcherDelegate* delegate) {
-  net::URLFetcher* fetcher =
-      new net::FakeURLFetcher(url, delegate, kFakeIccJson, net::HTTP_OK,
-                              net::URLRequestStatus::SUCCESS);
-  return base::WrapUnique(fetcher);
-}
-
-// Create FakeURLFetcher which replies with HTTP_NOT_FOUND.
-std::unique_ptr<net::URLFetcher> CreateFakeURLFetcherFailure(
-    const GURL& url,
-    net::URLFetcherDelegate* delegate) {
-  net::URLFetcher* fetcher = new net::FakeURLFetcher(
-      url, delegate, "File not found", net::HTTP_NOT_FOUND,
-      net::URLRequestStatus::FAILED);
-  return base::WrapUnique(fetcher);
-}
-
 // Full path to fake icc file in <tmp test directory>/display_profiles/.
 base::FilePath GetPathForIccFile(int64_t product_id) {
   return QuirksManager::Get()->delegate()->GetDisplayProfileDirectory().Append(
@@ -48,7 +30,18 @@ base::FilePath GetPathForIccFile(int64_t product_id) {
 
 class QuirksBrowserTest : public InProcessBrowserTest {
  public:
-  QuirksBrowserTest() : file_existed_(false) {}
+  QuirksBrowserTest() {
+    test_url_loader_factory_.SetInterceptor(base::BindLambdaForTesting(
+        [this](const network::ResourceRequest& request) {
+          if (find_fake_file_) {
+            test_url_loader_factory_.AddResponse(request.url.spec(),
+                                                 kFakeIccJson);
+          } else {
+            test_url_loader_factory_.AddResponse(request.url.spec(), "",
+                                                 net::HTTP_NOT_FOUND);
+          }
+        }));
+  }
 
  protected:
   ~QuirksBrowserTest() override = default;
@@ -56,10 +49,7 @@ class QuirksBrowserTest : public InProcessBrowserTest {
   // Query QuirksManager for icc file, then run msg loop to wait for callback.
   // |find_fake_file| indicates that URLFetcher should respond with success.
   void TestQuirksClient(int64_t product_id, bool find_fake_file) {
-    // Set up fake url getter.
-    QuirksManager::Get()->SetFakeQuirksFetcherCreatorForTests(
-        base::Bind(find_fake_file ? &CreateFakeURLFetcherSuccess
-                                  : &CreateFakeURLFetcherFailure));
+    find_fake_file_ = find_fake_file;
 
     base::RunLoop run_loop;
     end_message_loop_ = run_loop.QuitClosure();
@@ -70,6 +60,8 @@ class QuirksBrowserTest : public InProcessBrowserTest {
                    base::Unretained(this)));
 
     run_loop.Run();
+
+    test_url_loader_factory_.ClearResponses();
   }
 
   // Callback from RequestIccProfilePath().
@@ -93,11 +85,20 @@ class QuirksBrowserTest : public InProcessBrowserTest {
 
     // Quirks clients can't run until after login.
     quirks::QuirksManager::Get()->OnLoginCompleted();
+
+    // Set up interception for URL requests.
+    QuirksManager::Get()->SetURLLoaderFactoryForTests(
+        base::MakeRefCounted<network::WeakWrapperSharedURLLoaderFactory>(
+            &test_url_loader_factory_));
   }
 
   base::Closure end_message_loop_;  // Callback to terminate message loop.
   base::FilePath icc_path_;         // Path to icc file if found or downloaded.
-  bool file_existed_;               // File was previously downloaded.
+  bool file_existed_ = false;       // File was previously downloaded.
+  bool find_fake_file_ = false;     // Return success from Quirks server
+                                    // request.
+
+  network::TestURLLoaderFactory test_url_loader_factory_;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(QuirksBrowserTest);

components/quirks/BUILD.gn

@@ -19,7 +19,7 @@ source_set("quirks") {
     "//base",
     "//components/prefs",
     "//components/version_info",
-    "//net",
+    "//services/network/public/cpp",
     "//url",
   ]
 }

components/quirks/DEPS

 include_rules = [
   "+components/prefs",
   "+components/version_info",
-  "+net",
+  "+net/base",
+  "+net/http",
+  "+net/traffic_annotation",
+  "+services/network/public/cpp",
 ]

components/quirks/quirks_client.cc

@@ -15,8 +15,7 @@
 #include "net/base/escape.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_context_getter.h"
+#include "services/network/public/cpp/simple_url_loader.h"
 
 namespace quirks {
 
@@ -86,31 +85,56 @@ void QuirksClient::StartDownload() {
 
   url += "key=" + manager_->delegate()->GetApiKey();
 
-  url_fetcher_ = manager_->CreateURLFetcher(GURL(url), this);
-  url_fetcher_->SetRequestContext(manager_->url_context_getter());
-  url_fetcher_->SetLoadFlags(net::LOAD_BYPASS_CACHE | net::LOAD_DISABLE_CACHE |
-                             net::LOAD_DO_NOT_SAVE_COOKIES |
-                             net::LOAD_DO_NOT_SEND_COOKIES |
-                             net::LOAD_DO_NOT_SEND_AUTH_DATA);
-  url_fetcher_->Start();
+  auto resource_request = std::make_unique<network::ResourceRequest>();
+  resource_request->url = GURL(url);
+  resource_request->load_flags =
+      net::LOAD_BYPASS_CACHE | net::LOAD_DISABLE_CACHE |
+      net::LOAD_DO_NOT_SAVE_COOKIES | net::LOAD_DO_NOT_SEND_COOKIES |
+      net::LOAD_DO_NOT_SEND_AUTH_DATA;
+
+  net::NetworkTrafficAnnotationTag traffic_annotation =
+      net::DefineNetworkTrafficAnnotation("quirks_display_fetcher", R"(
+          semantics {
+            sender: "Quirks"
+            description: "Download custom display calibration file."
+            trigger:
+                "Chrome OS attempts to download monitor calibration files on"
+                "first device login, and then once every 30 days."
+            data: "ICC files to calibrate and improve the quality of a display."
+            destination: GOOGLE_OWNED_SERVICE
+          }
+          policy {
+            cookies_allowed: NO
+            chrome_policy {
+              DeviceQuirksDownloadEnabled {
+                  DeviceQuirksDownloadEnabled: false
+              }
+            }
+          }
+        )");
+
+  url_loader_ = network::SimpleURLLoader::Create(std::move(resource_request),
+                                                 traffic_annotation);
+  url_loader_->DownloadToStringOfUnboundedSizeUntilCrashAndDie(
+      manager_->url_loader_factory(),
+      base::BindOnce(&QuirksClient::OnDownloadComplete,
+                     base::Unretained(this)));
 }
 
-void QuirksClient::OnURLFetchComplete(const net::URLFetcher* source) {
+void QuirksClient::OnDownloadComplete(
+    std::unique_ptr<std::string> response_body) {
   DCHECK(thread_checker_.CalledOnValidThread());
-  DCHECK_EQ(url_fetcher_.get(), source);
 
-  const int HTTP_INTERNAL_SERVER_ERROR_LAST =
-      net::HTTP_INTERNAL_SERVER_ERROR + 99;
-  const net::URLRequestStatus status = source->GetStatus();
-  const int response_code = source->GetResponseCode();
-  const bool server_error = !status.is_success() ||
-                            (response_code >= net::HTTP_INTERNAL_SERVER_ERROR &&
-                             response_code <= HTTP_INTERNAL_SERVER_ERROR_LAST);
+  // Take ownership of the loader in this scope.
+  std::unique_ptr<network::SimpleURLLoader> url_loader = std::move(url_loader_);
+
+  int response_code = 0;
+  if (url_loader->ResponseInfo() && url_loader->ResponseInfo()->headers)
+    response_code = url_loader->ResponseInfo()->headers->response_code();
 
   VLOG(2) << "QuirksClient::OnURLFetchComplete():"
-          << "  status=" << status.status()
-          << ",  response_code=" << response_code
-          << ",  server_error=" << server_error;
+          << "  net_error=" << url_loader->NetError()
+          << ", response_code=" << response_code;
 
   if (response_code == net::HTTP_NOT_FOUND) {
     VLOG(1) << IdToFileName(product_id_) << " not found on Quirks server.";
@@ -118,25 +142,23 @@ void QuirksClient::OnURLFetchComplete(const net::URLFetcher* source) {
     return;
   }
 
-  if (server_error) {
+  if (url_loader->NetError() != net::OK) {
     if (backoff_entry_.failure_count() >= kMaxServerFailures) {
       // After 10 retires (5+ hours), give up, and try again in a month.
       VLOG(1) << "Too many retries; Quirks Client shutting down.";
       Shutdown(false);
       return;
     }
-    url_fetcher_.reset();
     Retry();
     return;
   }
 
-  std::string response;
-  url_fetcher_->GetResponseAsString(&response);
-  VLOG(2) << "Quirks server response:\n" << response;
+  DCHECK(response_body);  // Guaranteed to be valid if NetError() is net::OK.
+  VLOG(2) << "Quirks server response:\n" << *response_body;
 
   // Parse response data and write to file on file thread.
   std::string data;
-  if (!ParseResult(response, &data)) {
+  if (!ParseResult(*response_body, &data)) {
     Shutdown(false);
     return;
   }

components/quirks/quirks_client.h

@@ -12,7 +12,10 @@
 #include "base/threading/thread_checker.h"
 #include "base/timer/timer.h"
 #include "net/base/backoff_entry.h"
-#include "net/url_request/url_fetcher_delegate.h"
+
+namespace network {
+class SimpleURLLoader;
+}
 
 namespace quirks {
 
@@ -23,21 +26,20 @@ using RequestFinishedCallback =
     base::Callback<void(const base::FilePath&, bool)>;
 
 // Handles downloading icc and other display data files from Quirks Server.
-class QuirksClient : public net::URLFetcherDelegate {
+class QuirksClient {
  public:
   QuirksClient(int64_t product_id,
                const std::string& display_name,
                const RequestFinishedCallback& on_request_finished,
                QuirksManager* manager);
-  ~QuirksClient() override;
+  ~QuirksClient();
 
   void StartDownload();
 
   int64_t product_id() const { return product_id_; }
 
  private:
-  // net::URLFetcherDelegate:
-  void OnURLFetchComplete(const net::URLFetcher* source) override;
+  void OnDownloadComplete(std::unique_ptr<std::string> response_body);
 
   // Send callback and tell manager to delete |this|.
   void Shutdown(bool success);
@@ -66,8 +68,8 @@ class QuirksClient : public net::URLFetcherDelegate {
   // The class is expected to run on UI thread.
   base::ThreadChecker thread_checker_;
 
-  // This fetcher is used to download icc file.
-  std::unique_ptr<net::URLFetcher> url_fetcher_;
+  // This loader is used to download icc file.
+  std::unique_ptr<network::SimpleURLLoader> url_loader_;
 
   // Pending retry.
   base::OneShotTimer request_scheduled_;

components/quirks/quirks_manager.cc

@@ -20,8 +20,6 @@
 #include "components/quirks/pref_names.h"
 #include "components/quirks/quirks_client.h"
 #include "net/traffic_annotation/network_traffic_annotation.h"
-#include "net/url_request/url_fetcher.h"
-#include "net/url_request/url_request_context_getter.h"
 #include "url/gurl.h"
 
 namespace quirks {
@@ -60,12 +58,12 @@ std::string IdToFileName(int64_t product_id) {
 QuirksManager::QuirksManager(
     std::unique_ptr<Delegate> delegate,
     PrefService* local_state,
-    scoped_refptr<net::URLRequestContextGetter> url_context_getter)
+    scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory)
     : waiting_for_login_(true),
       delegate_(std::move(delegate)),
       task_runner_(base::CreateTaskRunnerWithTraits({base::MayBlock()})),
       local_state_(local_state),
-      url_context_getter_(url_context_getter),
+      url_loader_factory_(std::move(url_loader_factory)),
       weak_ptr_factory_(this) {}
 
 QuirksManager::~QuirksManager() {
@@ -77,9 +75,9 @@ QuirksManager::~QuirksManager() {
 void QuirksManager::Initialize(
     std::unique_ptr<Delegate> delegate,
     PrefService* local_state,
-    scoped_refptr<net::URLRequestContextGetter> url_context_getter) {
-  manager_ =
-      new QuirksManager(std::move(delegate), local_state, url_context_getter);
+    scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory) {
+  manager_ = new QuirksManager(std::move(delegate), local_state,
+                               std::move(url_loader_factory));
 }
 
 // static
@@ -152,37 +150,6 @@ void QuirksManager::ClientFinished(QuirksClient* client) {
   clients_.erase(it);
 }
 
-std::unique_ptr<net::URLFetcher> QuirksManager::CreateURLFetcher(
-    const GURL& url,
-    net::URLFetcherDelegate* delegate) {
-  if (!fake_quirks_fetcher_creator_.is_null())
-    return fake_quirks_fetcher_creator_.Run(url, delegate);
-
-  net::NetworkTrafficAnnotationTag traffic_annotation =
-      net::DefineNetworkTrafficAnnotation("quirks_display_fetcher", R"(
-          semantics {
-            sender: "Quirks"
-            description: "Download custom display calibration file."
-            trigger:
-                "Chrome OS attempts to download monitor calibration files on"
-                "first device login, and then once every 30 days."
-            data: "ICC files to calibrate and improve the quality of a display."
-            destination: GOOGLE_OWNED_SERVICE
-          }
-          policy {
-            cookies_allowed: NO
-            chrome_policy {
-              DeviceQuirksDownloadEnabled {
-                  DeviceQuirksDownloadEnabled: false
-              }
-            }
-          }
-        )");
-
-  return net::URLFetcher::Create(url, net::URLFetcher::GET, delegate,
-                                 traffic_annotation);
-}
-
 void QuirksManager::OnIccFilePathRequestCompleted(
     int64_t product_id,
     const std::string& display_name,

components/quirks/quirks_manager.h

@@ -15,8 +15,8 @@
 #include "base/threading/thread_checker.h"
 #include "base/time/time.h"
 #include "components/quirks/quirks_export.h"
+#include "services/network/public/cpp/shared_url_loader_factory.h"
 
-class GURL;
 class PrefRegistrySimple;
 class PrefService;
 
@@ -24,12 +24,6 @@ namespace base {
 class TaskRunner;
 }
 
-namespace net {
-class URLFetcher;
-class URLFetcherDelegate;
-class URLRequestContextGetter;
-}
-
 namespace quirks {
 
 class QuirksClient;
@@ -52,11 +46,6 @@ QUIRKS_EXPORT std::string IdToFileName(int64_t product_id);
 // blocking pool, etc), and owns clients and manages their life cycles.
 class QUIRKS_EXPORT QuirksManager {
  public:
-  // Passed function to create a URLFetcher for tests.
-  // Same parameters as URLFetcher::Create().
-  using FakeQuirksFetcherCreator = base::Callback<
-      std::unique_ptr<net::URLFetcher>(const GURL&, net::URLFetcherDelegate*)>;
-
   // Delegate class, so implementation can access browser functionality.
   class Delegate {
    public:
@@ -79,7 +68,7 @@ class QUIRKS_EXPORT QuirksManager {
   static void Initialize(
       std::unique_ptr<Delegate> delegate,
       PrefService* local_state,
-      scoped_refptr<net::URLRequestContextGetter> url_context_getter);
+      scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory);
   static void Shutdown();
   static QuirksManager* Get();
 
@@ -96,29 +85,25 @@ class QUIRKS_EXPORT QuirksManager {
 
   void ClientFinished(QuirksClient* client);
 
-  // Creates a real URLFetcher for OS, and a fake one for tests.
-  std::unique_ptr<net::URLFetcher> CreateURLFetcher(
-      const GURL& url,
-      net::URLFetcherDelegate* delegate);
-
   Delegate* delegate() { return delegate_.get(); }
   base::TaskRunner* task_runner() { return task_runner_.get(); }
-  net::URLRequestContextGetter* url_context_getter() {
-    return url_context_getter_.get();
+  network::mojom::URLLoaderFactory* url_loader_factory() {
+    return url_loader_factory_.get();
   }
 
  protected:
   friend class QuirksBrowserTest;
 
-  void SetFakeQuirksFetcherCreatorForTests(
-      const FakeQuirksFetcherCreator& creator) {
-    fake_quirks_fetcher_creator_ = creator;
+  void SetURLLoaderFactoryForTests(
+      scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory) {
+    url_loader_factory_ = std::move(url_loader_factory);
   }
 
  private:
-  QuirksManager(std::unique_ptr<Delegate> delegate,
-                PrefService* local_state,
-                scoped_refptr<net::URLRequestContextGetter> url_context_getter);
+  QuirksManager(
+      std::unique_ptr<Delegate> delegate,
+      PrefService* local_state,
+      scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory);
   ~QuirksManager();
 
   // Callback after checking for existing icc file; proceed if not found.
@@ -147,9 +132,7 @@ class QUIRKS_EXPORT QuirksManager {
   std::unique_ptr<Delegate> delegate_;  // Impl runs from chrome/browser.
   scoped_refptr<base::TaskRunner> task_runner_;
   PrefService* local_state_;  // For local prefs.
-  scoped_refptr<net::URLRequestContextGetter> url_context_getter_;
-
-  FakeQuirksFetcherCreator fake_quirks_fetcher_creator_;  // For tests.
+  scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory_;
 
   // Factory for callbacks.
   base::WeakPtrFactory<QuirksManager> weak_ptr_factory_;