[chromeos] Migrate customization_document.cc to SimpleURLoader

It's currently using URLFetcher. It should use SimpleURLLoader
instead to make it eventually work with the network service.

Bug: 872880
Change-Id: Id192f3aee248334e3a40075b0adb190a799eef60
Reviewed-on: https://chromium-review.googlesource.com/1183229Reviewed-by: Dan Erat <derat@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Commit-Queue: Sergio Villar <svillar@igalia.com>
Cr-Commit-Position: refs/heads/master@{#584844}

chrome/browser/chromeos/customization/customization_document.cc

@@ -48,7 +48,8 @@
 #include "net/base/load_flags.h"
 #include "net/http/http_response_headers.h"
 #include "net/http/http_status_code.h"
-#include "net/url_request/url_fetcher.h"
+#include "services/network/public/cpp/shared_url_loader_factory.h"
+#include "services/network/public/cpp/simple_url_loader.h"
 
 namespace chromeos {
 namespace {
@@ -102,6 +103,10 @@ const char kEmptyServicesCustomizationManifest[] = "{ \"version\": \"1.0\" }";
 // Global overrider for ServicesCustomizationDocument for tests.
 ServicesCustomizationDocument* g_test_services_customization_document = NULL;
 
+// network::SharedURLLoaderFactory used by tests.
+scoped_refptr<network::SharedURLLoaderFactory>
+    g_shared_url_loader_factory_for_testing = nullptr;
+
 // Services customization document load results reported via the
 // "ServicesCustomization.LoadResult" histogram.
 // It is append-only enum due to use in a histogram!
@@ -417,14 +422,13 @@ void ServicesCustomizationDocument::ApplyingTask::Finished(bool success) {
 ServicesCustomizationDocument::ServicesCustomizationDocument()
     : CustomizationDocument(kAcceptedManifestVersion),
       num_retries_(0),
-      fetch_started_(false),
+      load_started_(false),
       network_delay_(
           base::TimeDelta::FromMilliseconds(kDefaultNetworkRetryDelayMS)),
       apply_tasks_started_(0),
       apply_tasks_finished_(0),
       apply_tasks_success_(0),
-      weak_ptr_factory_(this) {
-}
+      weak_ptr_factory_(this) {}
 
 ServicesCustomizationDocument::ServicesCustomizationDocument(
     const std::string& manifest)
@@ -522,7 +526,7 @@ ServicesCustomizationDocument::EnsureCustomizationAppliedClosure() {
 }
 
 void ServicesCustomizationDocument::StartFetching() {
-  if (IsReady() || fetch_started_)
+  if (IsReady() || load_started_)
     return;
 
   if (!url_.is_valid()) {
@@ -542,7 +546,7 @@ void ServicesCustomizationDocument::StartFetching() {
   }
 
   if (url_.is_valid()) {
-    fetch_started_ = true;
+    load_started_ = true;
     if (url_.SchemeIsFile()) {
       base::PostTaskWithTraitsAndReplyWithResult(
           FROM_HERE, {base::TaskPriority::BEST_EFFORT, base::MayBlock()},
@@ -560,7 +564,7 @@ void ServicesCustomizationDocument::OnManifestRead(
   if (!manifest.empty())
     LoadManifestFromString(manifest);
 
-  fetch_started_ = false;
+  load_started_ = false;
 }
 
 void ServicesCustomizationDocument::StartFileFetch() {
@@ -570,12 +574,21 @@ void ServicesCustomizationDocument::StartFileFetch() {
 }
 
 void ServicesCustomizationDocument::DoStartFileFetch() {
-  url_fetcher_ = net::URLFetcher::Create(url_, net::URLFetcher::GET, this);
-  url_fetcher_->SetRequestContext(g_browser_process->system_request_context());
-  url_fetcher_->AddExtraRequestHeader("Accept: application/json");
-  url_fetcher_->SetLoadFlags(net::LOAD_DISABLE_CACHE);
-  url_fetcher_->SetAllowCredentials(false);
-  url_fetcher_->Start();
+  auto request = std::make_unique<network::ResourceRequest>();
+  request->url = url_;
+  request->load_flags = net::LOAD_DISABLE_CACHE;
+  request->allow_credentials = false;
+  request->headers.SetHeader("Accept", "application/json");
+
+  url_loader_ = network::SimpleURLLoader::Create(std::move(request),
+                                                 NO_TRAFFIC_ANNOTATION_YET);
+
+  url_loader_->DownloadToStringOfUnboundedSizeUntilCrashAndDie(
+      g_shared_url_loader_factory_for_testing
+          ? g_shared_url_loader_factory_for_testing.get()
+          : g_browser_process->shared_url_loader_factory().get(),
+      base::BindOnce(&ServicesCustomizationDocument::OnSimpleLoaderComplete,
+                     base::Unretained(this)));
 }
 
 bool ServicesCustomizationDocument::LoadManifestFromString(
@@ -607,19 +620,20 @@ void ServicesCustomizationDocument::OnManifestLoaded() {
   }
 }
 
-void ServicesCustomizationDocument::OnURLFetchComplete(
-    const net::URLFetcher* source) {
+void ServicesCustomizationDocument::OnSimpleLoaderComplete(
+    std::unique_ptr<std::string> response_body) {
+  int response_code = -1;
   std::string mime_type;
-  std::string data;
-  if (source->GetStatus().is_success() &&
-      source->GetResponseCode() == net::HTTP_OK &&
-      source->GetResponseHeaders()->GetMimeType(&mime_type) &&
-      mime_type == "application/json" &&
-      source->GetResponseAsString(&data)) {
-    LoadManifestFromString(data);
-  } else if (source->GetResponseCode() == net::HTTP_NOT_FOUND) {
+  if (url_loader_->ResponseInfo() && url_loader_->ResponseInfo()->headers) {
+    response_code = url_loader_->ResponseInfo()->headers->response_code();
+    url_loader_->ResponseInfo()->headers->GetMimeType(&mime_type);
+  }
+
+  if (response_body && mime_type == "application/json") {
+    LoadManifestFromString(*response_body);
+  } else if (response_code == net::HTTP_NOT_FOUND) {
     LOG(ERROR) << "Customization manifest is missing on server: "
-               << source->GetURL().spec();
+               << url_.spec();
     OnCustomizationNotFound();
   } else {
     if (num_retries_ < kMaxFetchRetries) {
@@ -634,12 +648,12 @@ void ServicesCustomizationDocument::OnURLFetchComplete(
     }
     // This doesn't stop fetching manifest on next restart.
     LOG(ERROR) << "URL fetch for services customization failed:"
-               << " response code = " << source->GetResponseCode()
-               << " URL = " << source->GetURL().spec();
+               << " response code = " << response_code
+               << " URL = " << url_.spec();
 
     LogManifestLoadResult(HISTOGRAM_LOAD_RESULT_RETRIES_FAIL);
   }
-  fetch_started_ = false;
+  load_started_ = false;
 }
 
 bool ServicesCustomizationDocument::ApplyOOBECustomization() {
@@ -780,9 +794,11 @@ std::string ServicesCustomizationDocument::GetOemAppsFolderNameImpl(
 }
 
 // static
-void ServicesCustomizationDocument::InitializeForTesting() {
+void ServicesCustomizationDocument::InitializeForTesting(
+    scoped_refptr<network::SharedURLLoaderFactory> factory) {
   g_test_services_customization_document = new ServicesCustomizationDocument;
   g_test_services_customization_document->network_delay_ = base::TimeDelta();
+  g_shared_url_loader_factory_for_testing = std::move(factory);
 }
 
 // static

chrome/browser/chromeos/customization/customization_document.h

@@ -17,7 +17,6 @@
 #include "base/memory/singleton.h"
 #include "base/memory/weak_ptr.h"
 #include "base/values.h"
-#include "net/url_request/url_fetcher_delegate.h"
 #include "url/gurl.h"
 
 class PrefRegistrySimple;
@@ -32,8 +31,9 @@ namespace extensions {
 class ExternalLoader;
 }
 
-namespace net {
-class URLFetcher;
+namespace network {
+class SharedURLLoaderFactory;
+class SimpleURLLoader;
 }
 
 namespace user_prefs {
@@ -141,8 +141,7 @@ class StartupCustomizationDocument : public CustomizationDocument {
 // outside this class by calling StartFetching() or EnsureCustomizationApplied()
 // methods.
 // User of the file should check IsReady before use it.
-class ServicesCustomizationDocument : public CustomizationDocument,
-                                      private net::URLFetcherDelegate {
+class ServicesCustomizationDocument : public CustomizationDocument {
  public:
   static ServicesCustomizationDocument* GetInstance();
 
@@ -185,7 +184,8 @@ class ServicesCustomizationDocument : public CustomizationDocument,
 
   // Initialize instance of ServicesCustomizationDocument for tests that will
   // override singleton until ShutdownForTesting is called.
-  static void InitializeForTesting();
+  static void InitializeForTesting(
+      scoped_refptr<network::SharedURLLoaderFactory> factory);
 
   // Remove instance of ServicesCustomizationDocument for tests.
   static void ShutdownForTesting();
@@ -227,8 +227,7 @@ class ServicesCustomizationDocument : public CustomizationDocument,
   // Overriden from CustomizationDocument:
   bool LoadManifestFromString(const std::string& manifest) override;
 
-  // Overriden from net::URLFetcherDelegate:
-  void OnURLFetchComplete(const net::URLFetcher* source) override;
+  void OnSimpleLoaderComplete(std::unique_ptr<std::string> response_body);
 
   // Initiate file fetching. Wait for online status.
   void StartFileFetch();
@@ -293,14 +292,14 @@ class ServicesCustomizationDocument : public CustomizationDocument,
   // Services customization manifest URL.
   GURL url_;
 
-  // URLFetcher instance.
-  std::unique_ptr<net::URLFetcher> url_fetcher_;
+  // SimpleURLLoader instance.
+  std::unique_ptr<network::SimpleURLLoader> url_loader_;
 
   // How many times we already tried to fetch customization manifest file.
   int num_retries_;
 
   // Manifest fetch is already in progress.
-  bool fetch_started_;
+  bool load_started_;
 
   // Delay between checks for network online state.
   base::TimeDelta network_delay_;

chrome/browser/chromeos/customization/customization_document_unittest.cc

@@ -34,6 +34,8 @@
 #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 "services/network/public/cpp/weak_wrapper_shared_url_loader_factory.h"
+#include "services/network/test/test_url_loader_factory.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -159,30 +161,16 @@ TEST(StartupCustomizationDocumentTest, BadManifest) {
   EXPECT_FALSE(customization.IsReady());
 }
 
-class TestURLFetcherCallback {
+class TestURLLoaderFactoryInterceptor {
  public:
-  std::unique_ptr<net::FakeURLFetcher> CreateURLFetcher(
-      const GURL& url,
-      net::URLFetcherDelegate* d,
-      const std::string& response_data,
-      net::HttpStatusCode response_code,
-      net::URLRequestStatus::Status status) {
-    std::unique_ptr<net::FakeURLFetcher> fetcher(
-        new net::FakeURLFetcher(url, d, response_data, response_code, status));
-    OnRequestCreate(url, fetcher.get());
-    return fetcher;
+  explicit TestURLLoaderFactoryInterceptor(
+      network::TestURLLoaderFactory& factory) {
+    factory.SetInterceptor(base::BindRepeating(
+        &TestURLLoaderFactoryInterceptor::Intercept, base::Unretained(this)));
   }
-  MOCK_METHOD2(OnRequestCreate,
-               void(const GURL&, net::FakeURLFetcher*));
+  MOCK_METHOD1(Intercept, void(const network::ResourceRequest&));
 };
 
-void AddMimeHeader(const GURL& url, net::FakeURLFetcher* fetcher) {
-  scoped_refptr<net::HttpResponseHeaders> download_headers =
-      new net::HttpResponseHeaders("");
-  download_headers->AddHeader("Content-Type: application/json");
-  fetcher->set_response_headers(download_headers);
-}
-
 class MockExternalProviderVisitor
     : public extensions::ExternalProviderInterface::VisitorInterface {
  public:
@@ -203,14 +191,13 @@ class MockExternalProviderVisitor
 
 class ServicesCustomizationDocumentTest : public testing::Test {
  protected:
-  ServicesCustomizationDocumentTest()
-      : factory_(nullptr,
-                 base::Bind(&TestURLFetcherCallback::CreateURLFetcher,
-                            base::Unretained(&url_callback_))) {}
+  ServicesCustomizationDocumentTest() = default;
 
   // testing::Test:
   void SetUp() override {
-    ServicesCustomizationDocument::InitializeForTesting();
+    ServicesCustomizationDocument::InitializeForTesting(
+        base::MakeRefCounted<network::WeakWrapperSharedURLLoaderFactory>(
+            &loader_factory_));
 
     DBusThreadManager::Initialize();
     NetworkHandler::Initialize();
@@ -234,6 +221,9 @@ class ServicesCustomizationDocumentTest : public testing::Test {
 
     TestingBrowserProcess::GetGlobal()->SetLocalState(&local_state_);
     RegisterLocalState(local_state_.registry());
+
+    interceptor_ =
+        std::make_unique<TestURLLoaderFactoryInterceptor>(loader_factory_);
   }
 
   void TearDown() override {
@@ -241,6 +231,8 @@ class ServicesCustomizationDocumentTest : public testing::Test {
     NetworkHandler::Shutdown();
     DBusThreadManager::Shutdown();
     network_portal_detector::InitializeForTesting(nullptr);
+    loader_factory_.ClearResponses();
+    interceptor_.reset();
 
     ServicesCustomizationDocument::ShutdownForTesting();
   }
@@ -258,25 +250,26 @@ class ServicesCustomizationDocumentTest : public testing::Test {
                            const std::string& manifest) {
     GURL url(base::StringPrintf(ServicesCustomizationDocument::kManifestUrl,
                                 id.c_str()));
-    factory_.SetFakeResponse(url,
-                             manifest,
-                             net::HTTP_OK,
-                             net::URLRequestStatus::SUCCESS);
-    EXPECT_CALL(url_callback_, OnRequestCreate(url, _))
-      .Times(Exactly(1))
-      .WillRepeatedly(Invoke(AddMimeHeader));
+
+    network::ResourceResponseHead response_head;
+    response_head.headers = base::MakeRefCounted<net::HttpResponseHeaders>("");
+    response_head.headers->AddHeader("Content-Type: application/json");
+    loader_factory_.AddResponse(url, response_head, manifest,
+                                network::URLLoaderCompletionStatus(net::OK));
+    EXPECT_CALL(*interceptor_, Intercept).Times(Exactly(1));
   }
 
   void AddManifestNotFound(const std::string& id) {
     GURL url(base::StringPrintf(ServicesCustomizationDocument::kManifestUrl,
                                 id.c_str()));
-    factory_.SetFakeResponse(url,
-                             std::string(),
-                             net::HTTP_NOT_FOUND,
-                             net::URLRequestStatus::SUCCESS);
-    EXPECT_CALL(url_callback_, OnRequestCreate(url, _))
-      .Times(Exactly(1))
-      .WillRepeatedly(Invoke(AddMimeHeader));
+
+    network::ResourceResponseHead response_head;
+    response_head.headers = base::MakeRefCounted<net::HttpResponseHeaders>("");
+    response_head.headers->AddHeader("Content-Type: application/json");
+    response_head.headers->ReplaceStatusLine("HTTP/1.1 404 Not found");
+    loader_factory_.AddResponse(url, response_head, std::string(),
+                                network::URLLoaderCompletionStatus(net::OK));
+    EXPECT_CALL(*interceptor_, Intercept).Times(Exactly(1));
   }
 
   std::unique_ptr<TestingProfile> CreateProfile() {
@@ -300,8 +293,8 @@ class ServicesCustomizationDocumentTest : public testing::Test {
   system::ScopedFakeStatisticsProvider fake_statistics_provider_;
   ScopedCrosSettingsTestHelper scoped_cros_settings_test_helper_;
   TestingPrefServiceSimple local_state_;
-  TestURLFetcherCallback url_callback_;
-  net::FakeURLFetcherFactory factory_;
+  network::TestURLLoaderFactory loader_factory_;
+  std::unique_ptr<TestURLLoaderFactoryInterceptor> interceptor_;
   NetworkPortalDetectorTestImpl network_portal_detector_;
 };