Migrate ServicesCustomizationDocument to SimpleURLLoader

Bug: 773295
Change-Id: Id129cea8341c14bd8da4b5374034423223f03859
Reviewed-on: https://chromium-review.googlesource.com/884063
Commit-Queue: Mark Pilgrim <pilgrim@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#541884}

chrome/browser/chromeos/BUILD.gn

@@ -130,6 +130,7 @@ source_set("chromeos") {
     "//components/viz/host",
     "//services/identity/public/cpp",
     "//services/metrics/public/cpp:ukm_builders",
+    "//services/network:network_service",
     "//services/resource_coordinator/public/cpp:resource_coordinator_cpp",
     "//third_party/fontconfig",
     "//third_party/metrics_proto",

chrome/browser/chromeos/DEPS

@@ -27,6 +27,7 @@ include_rules = [
   "+mojo/edk/embedder",
   "+services/device/public",
   "+services/metrics/public",
+  "+services/network",
   "+services/tracing/public",
   "+services/viz/public/interfaces",
   # Chromeos should not use ozone directly, it must go through mojo as ozone

chrome/browser/chromeos/customization/customization_document.cc

@@ -34,6 +34,7 @@
 #include "chrome/browser/chromeos/net/delay_network_call.h"
 #include "chrome/browser/extensions/external_loader.h"
 #include "chrome/browser/extensions/external_provider_impl.h"
+#include "chrome/browser/net/system_network_context_manager.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/app_list/app_list_syncable_service.h"
 #include "chrome/browser/ui/app_list/app_list_syncable_service_factory.h"
@@ -44,11 +45,14 @@
 #include "components/prefs/pref_registry_simple.h"
 #include "components/prefs/pref_service.h"
 #include "content/public/browser/browser_thread.h"
+#include "content/public/browser/storage_partition.h"
 #include "extensions/common/extension_urls.h"
 #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/resource_request.h"
+#include "services/network/public/cpp/simple_url_loader.h"
+#include "services/network/url_loader_factory.h"
 
 namespace chromeos {
 namespace {
@@ -570,14 +574,40 @@ 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_DO_NOT_SEND_COOKIES |
-                             net::LOAD_DO_NOT_SAVE_COOKIES |
-                             net::LOAD_DISABLE_CACHE |
-                             net::LOAD_DO_NOT_SEND_AUTH_DATA);
-  url_fetcher_->Start();
+  net::NetworkTrafficAnnotationTag traffic_annotation =
+      net::DefineNetworkTrafficAnnotation("chromeos_customization_document", R"(
+          semantics {
+            sender: "Chrome OS Services Customization"
+            description:
+              "Chrome OS downloads the OEM services customization manifest."
+            trigger:
+              "When a public session starts on managed devices and an OEM "
+              "has uploaded a service customization document."
+            data:
+              "URL of the OEM service customization document. "
+              "No user information is sent."
+            destination: WEBSITE
+          }
+          policy {
+            cookies_allowed: NO
+            setting: "Unconditionally enabled on Chrome OS."
+          })");
+  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 | net::LOAD_DO_NOT_SEND_AUTH_DATA;
+  resource_request->headers.SetHeader("Accept", "application/json");
+  simple_loader_ = network::SimpleURLLoader::Create(std::move(resource_request),
+                                                    traffic_annotation);
+  network::mojom::URLLoaderFactory* loader_factory = url_loader_factory_;
+  if (!loader_factory) {
+    g_browser_process->system_network_context_manager()->GetURLLoaderFactory();
+  }
+  simple_loader_->DownloadToStringOfUnboundedSizeUntilCrashAndDie(
+      loader_factory,
+      base::BindOnce(&ServicesCustomizationDocument::OnSimpleLoaderComplete,
+                     base::Unretained(this)));
 }
 
 bool ServicesCustomizationDocument::LoadManifestFromString(
@@ -609,26 +639,30 @@ void ServicesCustomizationDocument::OnManifestLoaded() {
   }
 }
 
-void ServicesCustomizationDocument::OnURLFetchComplete(
-    const net::URLFetcher* source) {
-  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) {
-    LOG(ERROR) << "Customization manifest is missing on server: "
-               << source->GetURL().spec();
-    OnCustomizationNotFound();
-  } else {
+void ServicesCustomizationDocument::OnSimpleLoaderComplete(
+    std::unique_ptr<std::string> response_body) {
+  int response_code = 0;
+  bool retry = true;
+  if (simple_loader_->ResponseInfo() &&
+      simple_loader_->ResponseInfo()->headers) {
+    response_code = simple_loader_->ResponseInfo()->headers->response_code();
+    std::string mime_type;
+    simple_loader_->ResponseInfo()->headers->GetMimeType(&mime_type);
+    if (mime_type == "application/json" and response_body) {
+      LoadManifestFromString(*response_body);
+      retry = false;
+    } else if (response_code == net::HTTP_NOT_FOUND) {
+      LOG(ERROR) << "Customization manifest is missing on server: "
+                 << url_.spec();
+      OnCustomizationNotFound();
+      retry = false;
+    }
+  }
+  if (retry) {
     if (num_retries_ < kMaxFetchRetries) {
       num_retries_++;
       content::BrowserThread::PostDelayedTask(
-          content::BrowserThread::UI,
-          FROM_HERE,
+          content::BrowserThread::UI, FROM_HERE,
           base::Bind(&ServicesCustomizationDocument::StartFileFetch,
                      weak_ptr_factory_.GetWeakPtr()),
           base::TimeDelta::FromSeconds(kRetriesDelayInSec));
@@ -636,8 +670,8 @@ 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);
   }
@@ -777,8 +811,8 @@ void ServicesCustomizationDocument::SetOemFolderName(
 std::string ServicesCustomizationDocument::GetOemAppsFolderNameImpl(
     const std::string& locale,
     const base::DictionaryValue& root) const {
-  return GetLocaleSpecificStringImpl(
-      &root, locale, kLocalizedContent, kDefaultAppsFolderName);
+  return GetLocaleSpecificStringImpl(&root, locale, kLocalizedContent,
+                                     kDefaultAppsFolderName);
 }
 
 // static
@@ -851,10 +885,9 @@ void ServicesCustomizationDocument::CheckAndApplyWallpaper() {
 
   std::unique_ptr<bool> exists(new bool(false));
 
-  base::Closure check_file_exists =
-      base::Bind(&CheckWallpaperCacheExists,
-                 GetCustomizedWallpaperDownloadedFileName(),
-                 base::Unretained(exists.get()));
+  base::Closure check_file_exists = base::Bind(
+      &CheckWallpaperCacheExists, GetCustomizedWallpaperDownloadedFileName(),
+      base::Unretained(exists.get()));
   base::Closure on_checked_closure = base::Bind(
       &ServicesCustomizationDocument::OnCheckedWallpaperCacheExists,
       weak_ptr_factory_.GetWeakPtr(), base::Passed(std::move(exists)),

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,14 @@ namespace extensions {
 class ExternalLoader;
 }
 
-namespace net {
-class URLFetcher;
+namespace network {
+class SimpleURLLoader;
+}
+
+namespace network {
+namespace mojom {
+class URLLoaderFactory;
+}
 }
 
 namespace user_prefs {
@@ -141,8 +146,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();
 
@@ -200,6 +204,11 @@ class ServicesCustomizationDocument : public CustomizationDocument,
     return wallpaper_downloader_.get();
   }
 
+  void SetURLLoaderFactoryForTesting(
+      network::mojom::URLLoaderFactory* url_loader_factory) {
+    url_loader_factory_ = url_loader_factory;
+  }
+
  private:
   friend struct base::DefaultSingletonTraits<ServicesCustomizationDocument>;
   FRIEND_TEST_ALL_PREFIXES(CustomizationWallpaperDownloaderBrowserTest,
@@ -227,8 +236,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,9 +301,6 @@ class ServicesCustomizationDocument : public CustomizationDocument,
   // Services customization manifest URL.
   GURL url_;
 
-  // URLFetcher instance.
-  std::unique_ptr<net::URLFetcher> url_fetcher_;
-
   // How many times we already tried to fetch customization manifest file.
   int num_retries_;
 
@@ -309,6 +314,7 @@ class ServicesCustomizationDocument : public CustomizationDocument,
   ExternalLoaders external_loaders_;
 
   std::unique_ptr<CustomizationWallpaperDownloader> wallpaper_downloader_;
+  std::unique_ptr<network::SimpleURLLoader> simple_loader_;
 
   // This is barrier until customization is applied.
   // When number of finished tasks match number of started - customization is
@@ -321,6 +327,9 @@ class ServicesCustomizationDocument : public CustomizationDocument,
   // successfully.
   size_t apply_tasks_success_;
 
+  // Only used for unit tests. Not owned.
+  network::mojom::URLLoaderFactory* url_loader_factory_;
+
   // Weak factory for callbacks.
   base::WeakPtrFactory<ServicesCustomizationDocument> weak_ptr_factory_;
 

chrome/browser/chromeos/customization/customization_document_unittest.cc

@@ -32,13 +32,10 @@
 #include "extensions/common/manifest.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 "services/network/test/test_url_loader_factory.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
-using ::testing::Exactly;
-using ::testing::Invoke;
 using ::testing::Mock;
 using ::testing::_;
 using extensions::ExternalInstallInfoFile;
@@ -159,30 +156,6 @@ TEST(StartupCustomizationDocumentTest, BadManifest) {
   EXPECT_FALSE(customization.IsReady());
 }
 
-class TestURLFetcherCallback {
- 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;
-  }
-  MOCK_METHOD2(OnRequestCreate,
-               void(const GURL&, net::FakeURLFetcher*));
-};
-
-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,11 +176,6 @@ class MockExternalProviderVisitor
 
 class ServicesCustomizationDocumentTest : public testing::Test {
  protected:
-  ServicesCustomizationDocumentTest()
-      : factory_(nullptr,
-                 base::Bind(&TestURLFetcherCallback::CreateURLFetcher,
-                            base::Unretained(&url_callback_))) {}
-
   // testing::Test:
   void SetUp() override {
     ServicesCustomizationDocument::InitializeForTesting();
@@ -258,25 +226,24 @@ 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 head;
+    head.headers = base::MakeRefCounted<net::HttpResponseHeaders>("");
+    head.headers->AddHeader("Content-Type: application/json");
+    network::URLLoaderCompletionStatus status;
+    status.decoded_body_length = manifest.size();
+    factory_.AddResponse(url, head, manifest, status);
   }
 
   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 head;
+    head.headers = base::MakeRefCounted<net::HttpResponseHeaders>(
+        "HTTP/1.x 404 Not Found");
+    head.headers->AddHeader("Content-Type: application/json");
+    network::URLLoaderCompletionStatus status;
+    status.decoded_body_length = 0;
+    factory_.AddResponse(url, head, "", status);
   }
 
   std::unique_ptr<TestingProfile> CreateProfile() {
@@ -291,13 +258,13 @@ class ServicesCustomizationDocumentTest : public testing::Test {
     return profile_builder.Build();
   }
 
+  network::TestURLLoaderFactory factory_;
+
  private:
   content::TestBrowserThreadBundle thread_bundle_;
   system::ScopedFakeStatisticsProvider fake_statistics_provider_;
   ScopedCrosSettingsTestHelper scoped_cros_settings_test_helper_;
   TestingPrefServiceSimple local_state_;
-  TestURLFetcherCallback url_callback_;
-  net::FakeURLFetcherFactory factory_;
   NetworkPortalDetectorTestImpl network_portal_detector_;
 };
 
@@ -309,6 +276,7 @@ TEST_F(ServicesCustomizationDocumentTest, Basic) {
       ServicesCustomizationDocument::GetInstance();
   EXPECT_FALSE(doc->IsReady());
 
+  doc->SetURLLoaderFactoryForTesting(&factory_);
   doc->StartFetching();
   RunUntilIdle();
   EXPECT_TRUE(doc->IsReady());
@@ -346,6 +314,7 @@ TEST_F(ServicesCustomizationDocumentTest, NoCustomizationIdInVpd) {
       ServicesCustomizationDocument::GetInstance();
   EXPECT_FALSE(doc->IsReady());
 
+  doc->SetURLLoaderFactoryForTesting(&factory_);
   std::unique_ptr<TestingProfile> profile = CreateProfile();
   extensions::ExternalLoader* loader = doc->CreateExternalLoader(profile.get());
   EXPECT_TRUE(loader);
@@ -382,6 +351,7 @@ TEST_F(ServicesCustomizationDocumentTest, DefaultApps) {
       ServicesCustomizationDocument::GetInstance();
   EXPECT_FALSE(doc->IsReady());
 
+  doc->SetURLLoaderFactoryForTesting(&factory_);
   std::unique_ptr<TestingProfile> profile = CreateProfile();
   extensions::ExternalLoader* loader = doc->CreateExternalLoader(profile.get());
   EXPECT_TRUE(loader);
@@ -432,6 +402,7 @@ TEST_F(ServicesCustomizationDocumentTest, CustomizationManifestNotFound) {
       ServicesCustomizationDocument::GetInstance();
   EXPECT_FALSE(doc->IsReady());
 
+  doc->SetURLLoaderFactoryForTesting(&factory_);
   std::unique_ptr<TestingProfile> profile = CreateProfile();
   extensions::ExternalLoader* loader = doc->CreateExternalLoader(profile.get());
   EXPECT_TRUE(loader);