Migrate ExtensionDownloader to SimpleURLLoader

In order to keep the extensive unittests set functional, a couple of
extensions were made, apart from the migration itself. Remarkably:

- Added a new method to ExtensionDownloaderDelegate class,
  ::OnExtensionDownloadRetryForTests, used exclusively for tests.
  Basically, *various* unit tests in ExtensionUpdaterTest (namely
  TestSingleExtensionDownloading* and ProtectedDownload*) perform
  intentional load failures/retries, where resource
  request parameters and load results are set up and verified differently
  for each load fail/retry sequence.
  Prior to this CL, with URLFetcher, tests could control when to call
  URLFetcherDelegate::OnURLFetchComplete.
  In SimpleURLLoader worlds, we can not really control when the "load
  complete" callback is called, since it is a mojo call. Hence, this
  testing-only API was added so that tests can more fine-grained control
  when loads are triggered.

BUG=773295

Cq-Include-Trybots: luci.chromium.try:linux_mojo
Change-Id: Ife6f51837648036c8b96dbedcb093113be2fc987
Reviewed-on: https://chromium-review.googlesource.com/1066370
Commit-Queue: Antonio Gomes <tonikitoo@igalia.com>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Minh Nguyen <mxnguyen@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: Alexander Alekseev <alemate@chromium.org>
Cr-Commit-Position: refs/heads/master@{#581322}

chrome/browser/chromeos/app_mode/kiosk_app_manager.cc

@@ -36,6 +36,7 @@
 #include "chrome/browser/extensions/external_loader.h"
 #include "chrome/browser/extensions/external_provider_impl.h"
 #include "chrome/browser/lifetime/application_lifetime.h"
+#include "chrome/browser/net/system_network_context_manager.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/extensions/extension_constants.h"
 #include "chromeos/chromeos_paths.h"
@@ -53,6 +54,7 @@
 #include "components/user_manager/user_manager.h"
 #include "extensions/common/extension_urls.h"
 #include "extensions/common/manifest_handlers/kiosk_mode_info.h"
+#include "services/network/public/cpp/shared_url_loader_factory.h"
 #include "third_party/cros_system_api/switches/chrome_switches.h"
 
 namespace chromeos {
@@ -165,9 +167,11 @@ std::unique_ptr<ExternalCache> CreateExternalCache(
   if (g_test_overrides)
     return g_test_overrides->CreateExternalCache(delegate, true);
 
+  scoped_refptr<network::SharedURLLoaderFactory> shared_url_loader_factory =
+      g_browser_process->shared_url_loader_factory();
   auto cache = std::make_unique<ExternalCacheImpl>(
-      GetCrxCacheDir(), g_browser_process->system_request_context(),
-      GetBackgroundTaskRunner(), delegate, true /* always_check_updates */,
+      GetCrxCacheDir(), shared_url_loader_factory, GetBackgroundTaskRunner(),
+      delegate, true /* always_check_updates */,
       false /* wait_for_cache_initialization */);
   cache->set_flush_on_put(true);
   return cache;

chrome/browser/chromeos/extensions/device_local_account_external_policy_loader.cc

@@ -12,10 +12,11 @@
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chromeos/extensions/external_cache_impl.h"
 #include "chrome/browser/extensions/policy_handlers.h"
+#include "chrome/browser/net/system_network_context_manager.h"
 #include "components/policy/core/common/policy_map.h"
 #include "components/prefs/pref_value_map.h"
 #include "extensions/browser/pref_names.h"
-#include "net/url_request/url_request_context_getter.h"
+#include "services/network/public/cpp/shared_url_loader_factory.h"
 
 namespace chromeos {
 
@@ -34,9 +35,12 @@ void DeviceLocalAccountExternalPolicyLoader::StartCache(
     const scoped_refptr<base::SequencedTaskRunner>& cache_task_runner) {
   DCHECK(!external_cache_);
   store_->AddObserver(this);
+
+  scoped_refptr<network::SharedURLLoaderFactory> shared_url_loader_factory =
+      g_browser_process->shared_url_loader_factory();
   external_cache_ = std::make_unique<ExternalCacheImpl>(
-      cache_dir_, g_browser_process->system_request_context(),
-      cache_task_runner, this, true /* always_check_updates */,
+      cache_dir_, shared_url_loader_factory, cache_task_runner, this,
+      true /* always_check_updates */,
       false /* wait_for_cache_initialization */);
 
   if (store_->is_initialized())

chrome/browser/chromeos/extensions/device_local_account_external_policy_loader_unittest.cc

@@ -37,10 +37,8 @@
 #include "extensions/common/extension.h"
 #include "extensions/common/extension_urls.h"
 #include "extensions/common/manifest.h"
-#include "net/url_request/test_url_fetcher_factory.h"
-#include "net/url_request/url_fetcher_delegate.h"
-#include "net/url_request/url_request_context_getter.h"
-#include "net/url_request/url_request_test_util.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"
 #include "url/gurl.h"
@@ -66,8 +64,6 @@ const char kCacheDir[] = "cache";
 const char kExtensionId[] = "ldnnhddmnhbkjipkidpdiheffobcpfmf";
 const char kExtensionUpdateManifest[] =
     "extensions/good_v1_update_manifest.xml";
-const char kExtensionCRXSourceDir[] = "extensions";
-const char kExtensionCRXFile[] = "good.crx";
 const char kExtensionCRXVersion[] = "1.0.0.0";
 
 class MockExternalPolicyProviderVisitor
@@ -111,11 +107,20 @@ class DeviceLocalAccountExternalPolicyLoaderTest : public testing::Test {
   void VerifyAndResetVisitorCallExpectations();
   void SetForceInstallListPolicy();
 
+  network::TestURLLoaderFactory::PendingRequest* GetPendingRequest(
+      size_t index = 0) {
+    if (index >= test_url_loader_factory_.pending_requests()->size())
+      return nullptr;
+    return &test_url_loader_factory_.pending_requests()->at(index);
+  }
+
   content::TestBrowserThreadBundle thread_bundle_;
   base::ScopedTempDir temp_dir_;
   base::FilePath cache_dir_;
   policy::MockCloudPolicyStore store_;
-  scoped_refptr<net::URLRequestContextGetter> request_context_getter_;
+  network::TestURLLoaderFactory test_url_loader_factory_;
+  scoped_refptr<network::WeakWrapperSharedURLLoaderFactory>
+      test_shared_loader_factory_;
   base::FilePath test_dir_;
 
   scoped_refptr<DeviceLocalAccountExternalPolicyLoader> loader_;
@@ -132,8 +137,10 @@ class DeviceLocalAccountExternalPolicyLoaderTest : public testing::Test {
 
 DeviceLocalAccountExternalPolicyLoaderTest::
     DeviceLocalAccountExternalPolicyLoaderTest()
-    : thread_bundle_(content::TestBrowserThreadBundle::IO_MAINLOOP) {
-}
+    : thread_bundle_(content::TestBrowserThreadBundle::IO_MAINLOOP),
+      test_shared_loader_factory_(
+          base::MakeRefCounted<network::WeakWrapperSharedURLLoaderFactory>(
+              &test_url_loader_factory_)) {}
 
 DeviceLocalAccountExternalPolicyLoaderTest::
     ~DeviceLocalAccountExternalPolicyLoaderTest() {
@@ -143,10 +150,8 @@ void DeviceLocalAccountExternalPolicyLoaderTest::SetUp() {
   ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
   cache_dir_ = temp_dir_.GetPath().Append(kCacheDir);
   ASSERT_TRUE(base::CreateDirectoryAndGetError(cache_dir_, NULL));
-  request_context_getter_ =
-      new net::TestURLRequestContextGetter(base::ThreadTaskRunnerHandle::Get());
-  TestingBrowserProcess::GetGlobal()->SetSystemRequestContext(
-      request_context_getter_.get());
+  TestingBrowserProcess::GetGlobal()->SetSharedURLLoaderFactory(
+      test_shared_loader_factory_);
   ASSERT_TRUE(base::PathService::Get(chrome::DIR_TEST_DATA, &test_dir_));
 
   loader_ = new DeviceLocalAccountExternalPolicyLoader(&store_, cache_dir_);
@@ -162,7 +167,7 @@ void DeviceLocalAccountExternalPolicyLoaderTest::SetUp() {
 }
 
 void DeviceLocalAccountExternalPolicyLoaderTest::TearDown() {
-  TestingBrowserProcess::GetGlobal()->SetSystemRequestContext(NULL);
+  TestingBrowserProcess::GetGlobal()->SetSharedURLLoaderFactory(nullptr);
 }
 
 void DeviceLocalAccountExternalPolicyLoaderTest::
@@ -240,25 +245,22 @@ TEST_F(DeviceLocalAccountExternalPolicyLoaderTest, ForceInstallListSet) {
 
   // Spin the loop, allowing the loader to process the force-install list.
   // Verify that the loader announces an empty extension list.
-  net::TestURLFetcherFactory factory;
   EXPECT_CALL(visitor_, OnExternalProviderReady(provider_.get()))
       .Times(1);
   base::RunLoop().RunUntilIdle();
 
   // Verify that a downloader has started and is attempting to download an
   // update manifest.
-  net::TestURLFetcher* fetcher = factory.GetFetcherByID(
-      extensions::ExtensionDownloader::kManifestFetcherId);
-  ASSERT_TRUE(fetcher);
-  ASSERT_TRUE(fetcher->delegate());
+  EXPECT_EQ(1, test_url_loader_factory_.NumPending());
 
   // Return a manifest to the downloader.
   std::string manifest;
   EXPECT_TRUE(base::ReadFileToString(test_dir_.Append(kExtensionUpdateManifest),
                                      &manifest));
-  fetcher->set_response_code(200);
-  fetcher->SetResponseString(manifest);
-  fetcher->delegate()->OnURLFetchComplete(fetcher);
+
+  auto* pending_request = GetPendingRequest();
+  test_url_loader_factory_.AddResponse(pending_request->request.url.spec(),
+                                       manifest);
 
   // Wait for the manifest to be parsed.
   content::WindowedNotificationObserver(
@@ -266,18 +268,12 @@ TEST_F(DeviceLocalAccountExternalPolicyLoaderTest, ForceInstallListSet) {
       content::NotificationService::AllSources()).Wait();
 
   // Verify that the downloader is attempting to download a CRX file.
-  fetcher = factory.GetFetcherByID(
-      extensions::ExtensionDownloader::kExtensionFetcherId);
-  ASSERT_TRUE(fetcher);
-  ASSERT_TRUE(fetcher->delegate());
-
-  // Create a temporary CRX file and return its path to the downloader.
-  EXPECT_TRUE(base::CopyFile(
-      test_dir_.Append(kExtensionCRXSourceDir).Append(kExtensionCRXFile),
-      temp_dir_.GetPath().Append(kExtensionCRXFile)));
-  fetcher->set_response_code(200);
-  fetcher->SetResponseFilePath(temp_dir_.GetPath().Append(kExtensionCRXFile));
-  fetcher->delegate()->OnURLFetchComplete(fetcher);
+  EXPECT_EQ(1, test_url_loader_factory_.NumPending());
+
+  // Trigger downloading of the temporary CRX file.
+  pending_request = GetPendingRequest();
+  test_url_loader_factory_.AddResponse(pending_request->request.url.spec(),
+                                       "Content is irrelevant.");
 
   // Spin the loop. Verify that the loader announces the presence of a new CRX
   // file, served from the cache directory.
@@ -298,12 +294,6 @@ TEST_F(DeviceLocalAccountExternalPolicyLoaderTest, ForceInstallListSet) {
   cache_run_loop.Run();
   VerifyAndResetVisitorCallExpectations();
 
-  // Verify that the CRX file actually exists in the cache directory and its
-  // contents matches the file returned to the downloader.
-  EXPECT_TRUE(base::ContentsEqual(
-      test_dir_.Append(kExtensionCRXSourceDir).Append(kExtensionCRXFile),
-      cached_crx_path));
-
   // Stop the cache. Verify that the loader announces an empty extension list.
   EXPECT_CALL(visitor_, OnExternalProviderReady(provider_.get()))
       .Times(1);

chrome/browser/chromeos/extensions/external_cache_impl.cc

@@ -31,7 +31,7 @@
 #include "extensions/common/extension.h"
 #include "extensions/common/extension_urls.h"
 #include "extensions/common/manifest.h"
-#include "net/url_request/url_request_context_getter.h"
+#include "services/network/public/cpp/shared_url_loader_factory.h"
 
 namespace chromeos {
 
@@ -67,13 +67,13 @@ void WrapOnceClosure(base::OnceClosure closure) {
 
 ExternalCacheImpl::ExternalCacheImpl(
     const base::FilePath& cache_dir,
-    net::URLRequestContextGetter* request_context,
+    scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
     const scoped_refptr<base::SequencedTaskRunner>& backend_task_runner,
     ExternalCacheDelegate* delegate,
     bool always_check_updates,
     bool wait_for_cache_initialization)
     : local_cache_(cache_dir, 0, base::TimeDelta(), backend_task_runner),
-      request_context_(request_context),
+      url_loader_factory_(std::move(url_loader_factory)),
       backend_task_runner_(backend_task_runner),
       delegate_(delegate),
       always_check_updates_(always_check_updates),
@@ -267,10 +267,10 @@ void ExternalCacheImpl::CheckCache() {
   if (local_cache_.is_shutdown())
     return;
 
-  // If request_context_ is missing we can't download anything.
-  if (request_context_.get()) {
-    downloader_ = ChromeExtensionDownloaderFactory::CreateForRequestContext(
-        request_context_.get(), this, GetConnector());
+  // If url_loader_factory_ is missing we can't download anything.
+  if (url_loader_factory_) {
+    downloader_ = ChromeExtensionDownloaderFactory::CreateForURLLoaderFactory(
+        url_loader_factory_, this, GetConnector());
   }
 
   cached_extensions_->Clear();

chrome/browser/chromeos/extensions/external_cache_impl.h

@@ -30,8 +30,8 @@ namespace extensions {
 class ExtensionDownloader;
 }
 
-namespace net {
-class URLRequestContextGetter;
+namespace network {
+class SharedURLLoaderFactory;
 }
 
 namespace service_manager {
@@ -47,15 +47,15 @@ class ExternalCacheImpl : public ExternalCache,
                           public content::NotificationObserver,
                           public extensions::ExtensionDownloaderDelegate {
  public:
-  // The |request_context| is used for update checks. All file I/O is done via
-  // the |backend_task_runner|. If |always_check_updates| is |false|, update
+  // The |url_loader_factory| is used for update checks. All file I/O is done
+  // via the |backend_task_runner|. If |always_check_updates| is |false|, update
   // checks are performed for extensions that have an |external_update_url|
   // only. If |wait_for_cache_initialization| is |true|, the cache contents will
   // not be read until a flag file appears in the cache directory, signaling
   // that the cache is ready.
   ExternalCacheImpl(
       const base::FilePath& cache_dir,
-      net::URLRequestContextGetter* request_context,
+      scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
       const scoped_refptr<base::SequencedTaskRunner>& backend_task_runner,
       ExternalCacheDelegate* delegate,
       bool always_check_updates,
@@ -129,8 +129,8 @@ class ExternalCacheImpl : public ExternalCache,
 
   extensions::LocalExtensionCache local_cache_;
 
-  // Request context used by the |downloader_|.
-  scoped_refptr<net::URLRequestContextGetter> request_context_;
+  // URL lader factory used by the |downloader_|.
+  scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory_;
 
   // Task runner for executing file I/O tasks.
   const scoped_refptr<base::SequencedTaskRunner> backend_task_runner_;

chrome/browser/chromeos/extensions/external_cache_impl_unittest.cc

@@ -24,9 +24,9 @@
 #include "content/public/test/test_browser_thread_bundle.h"
 #include "content/public/test/test_utils.h"
 #include "extensions/common/extension_urls.h"
-#include "net/url_request/test_url_fetcher_factory.h"
-#include "net/url_request/url_fetcher_impl.h"
-#include "net/url_request/url_request_test_util.h"
+#include "services/network/public/cpp/weak_wrapper_shared_url_loader_factory.h"
+#include "services/network/public/mojom/url_loader_factory.mojom.h"
+#include "services/network/test/test_url_loader_factory.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace {
@@ -45,23 +45,18 @@ class ExternalCacheImplTest : public testing::Test,
                               public ExternalCacheDelegate {
  public:
   ExternalCacheImplTest()
-      : thread_bundle_(content::TestBrowserThreadBundle::REAL_IO_THREAD) {}
+      : thread_bundle_(content::TestBrowserThreadBundle::REAL_IO_THREAD),
+        test_shared_loader_factory_(
+            base::MakeRefCounted<network::WeakWrapperSharedURLLoaderFactory>(
+                &test_url_loader_factory_)) {}
   ~ExternalCacheImplTest() override = default;
 
-  net::URLRequestContextGetter* request_context_getter() {
-    return request_context_getter_.get();
+  scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory() {
+    return test_shared_loader_factory_;
   }
 
   const base::DictionaryValue* provided_prefs() { return prefs_.get(); }
 
-  // testing::Test overrides:
-  void SetUp() override {
-    request_context_getter_ = new net::TestURLRequestContextGetter(
-        content::BrowserThread::GetTaskRunnerForThread(
-            content::BrowserThread::IO));
-    fetcher_factory_.reset(new net::TestURLFetcherFactory());
-  }
-
   // ExternalCacheDelegate:
   void OnExtensionListsUpdated(const base::DictionaryValue* prefs) override {
     prefs_.reset(prefs->DeepCopy());
@@ -125,8 +120,8 @@ class ExternalCacheImplTest : public testing::Test,
  private:
   content::TestBrowserThreadBundle thread_bundle_;
 
-  scoped_refptr<net::URLRequestContextGetter> request_context_getter_;
-  std::unique_ptr<net::TestURLFetcherFactory> fetcher_factory_;
+  network::TestURLLoaderFactory test_url_loader_factory_;
+  scoped_refptr<network::SharedURLLoaderFactory> test_shared_loader_factory_;
 
   base::ScopedTempDir cache_dir_;
   base::ScopedTempDir temp_dir_;
@@ -142,7 +137,7 @@ class ExternalCacheImplTest : public testing::Test,
 TEST_F(ExternalCacheImplTest, Basic) {
   base::FilePath cache_dir(CreateCacheDir(false));
   ExternalCacheImpl external_cache(
-      cache_dir, request_context_getter(),
+      cache_dir, url_loader_factory(),
       base::CreateSequencedTaskRunnerWithTraits({base::MayBlock()}), this, true,
       false);
   external_cache.use_null_connector_for_test();
@@ -261,7 +256,7 @@ TEST_F(ExternalCacheImplTest, Basic) {
 TEST_F(ExternalCacheImplTest, PreserveInstalled) {
   base::FilePath cache_dir(CreateCacheDir(false));
   ExternalCacheImpl external_cache(
-      cache_dir, request_context_getter(),
+      cache_dir, url_loader_factory(),
       base::CreateSequencedTaskRunnerWithTraits({base::MayBlock()}), this, true,
       false);
   external_cache.use_null_connector_for_test();

chrome/browser/extensions/updater/chrome_extension_downloader_factory.cc

@@ -16,20 +16,23 @@
 #include "components/signin/core/browser/profile_oauth2_token_service.h"
 #include "components/signin/core/browser/signin_manager.h"
 #include "components/update_client/update_query_params.h"
+#include "content/public/browser/storage_partition.h"
 #include "content/public/common/service_manager_connection.h"
 #include "extensions/browser/updater/extension_downloader.h"
+#include "services/network/public/cpp/shared_url_loader_factory.h"
 
 using extensions::ExtensionDownloader;
 using extensions::ExtensionDownloaderDelegate;
 using update_client::UpdateQueryParams;
 
 std::unique_ptr<ExtensionDownloader>
-ChromeExtensionDownloaderFactory::CreateForRequestContext(
-    net::URLRequestContextGetter* request_context,
+ChromeExtensionDownloaderFactory::CreateForURLLoaderFactory(
+    scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
     ExtensionDownloaderDelegate* delegate,
-    service_manager::Connector* connector) {
-  std::unique_ptr<ExtensionDownloader> downloader(
-      new ExtensionDownloader(delegate, request_context, connector));
+    service_manager::Connector* connector,
+    const base::FilePath& profile_path) {
+  std::unique_ptr<ExtensionDownloader> downloader(new ExtensionDownloader(
+      delegate, std::move(url_loader_factory), connector, profile_path));
 #if defined(GOOGLE_CHROME_BUILD)
   std::string brand;
   google_brand::GetBrand(&brand);
@@ -54,9 +57,10 @@ ChromeExtensionDownloaderFactory::CreateForProfile(
     ExtensionDownloaderDelegate* delegate) {
   service_manager::Connector* connector =
       content::ServiceManagerConnection::GetForProcess()->GetConnector();
-  std::unique_ptr<ExtensionDownloader> downloader =
-      CreateForRequestContext(profile->GetRequestContext(), delegate,
-        connector);
+  std::unique_ptr<ExtensionDownloader> downloader = CreateForURLLoaderFactory(
+      content::BrowserContext::GetDefaultStoragePartition(profile)
+          ->GetURLLoaderFactoryForBrowserProcess(),
+      delegate, connector, profile->GetPath());
 
   // NOTE: It is not obvious why it is OK to pass raw pointers to the token
   // service and signin manager here. The logic is as follows:

chrome/browser/extensions/updater/chrome_extension_downloader_factory.h

@@ -6,6 +6,8 @@
 #define CHROME_BROWSER_EXTENSIONS_UPDATER_CHROME_EXTENSION_DOWNLOADER_FACTORY_H_
 
 #include <memory>
+#include "base/files/file_path.h"
+#include "base/memory/scoped_refptr.h"
 
 class Profile;
 
@@ -14,9 +16,9 @@ class ExtensionDownloader;
 class ExtensionDownloaderDelegate;
 }
 
-namespace net {
-class URLRequestContextGetter;
-}
+namespace network {
+class SharedURLLoaderFactory;
+}  // namespace network
 
 namespace service_manager {
 class Connector;
@@ -26,12 +28,22 @@ class Connector;
 // ExtensionDownloader suitable for use from within Chrome.
 class ChromeExtensionDownloaderFactory {
  public:
-  // Creates a downloader with the given request context. No profile identity
-  // is associated with this downloader.
+  // Creates a downloader with a given "global" url loader factory instance.
+  // No profile identity is associated with this downloader, which means:
+  //
+  // - when this method is called directly, |file_path| is empty.
+  // - when this method is called through CreateForProfile, |profile_path| is
+  //   non-empty.
+  //
+  // |profile_path| is used exclusely to support download of extensions through
+  // the file:// protocol. In practice, it whitelists specific directories the
+  // the browser has access to.
   static std::unique_ptr<extensions::ExtensionDownloader>
-  CreateForRequestContext(net::URLRequestContextGetter* request_context,
-                          extensions::ExtensionDownloaderDelegate* delegate,
-                          service_manager::Connector* connector);
+  CreateForURLLoaderFactory(
+      scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
+      extensions::ExtensionDownloaderDelegate* delegate,
+      service_manager::Connector* connector,
+      const base::FilePath& profile_path = base::FilePath());
 
   // Creates a downloader for a given Profile. This downloader will be able
   // to authenticate as the signed-in user in the event that it's asked to

extensions/browser/updater/extension_downloader.h

@@ -22,15 +22,22 @@
 #include "extensions/browser/updater/safe_manifest_parser.h"
 #include "extensions/common/extension.h"
 #include "google_apis/gaia/oauth2_token_service.h"
-#include "net/url_request/url_fetcher_delegate.h"
+#include "net/http/http_request_headers.h"
 #include "url/gurl.h"
 
 namespace net {
-class URLFetcher;
-class URLRequestContextGetter;
 class URLRequestStatus;
 }
 
+namespace network {
+class SharedURLLoaderFactory;
+class SimpleURLLoader;
+namespace mojom {
+class URLLoaderFactory;
+}
+struct ResourceRequest;
+}  // namespace network
+
 namespace service_manager {
 class Connector;
 }
@@ -53,8 +60,7 @@ class ExtensionUpdaterTest;
 // the crx file when updates are found. It uses a |ExtensionDownloaderDelegate|
 // that takes ownership of the downloaded crx files, and handles events during
 // the update check.
-class ExtensionDownloader : public net::URLFetcherDelegate,
-                            public OAuth2TokenService::Consumer {
+class ExtensionDownloader : public OAuth2TokenService::Consumer {
  public:
   // A closure which constructs a new ExtensionDownloader to be owned by the
   // caller.
@@ -68,9 +74,11 @@ class ExtensionDownloader : public net::URLFetcherDelegate,
 
   // |delegate| is stored as a raw pointer and must outlive the
   // ExtensionDownloader.
-  ExtensionDownloader(ExtensionDownloaderDelegate* delegate,
-                      net::URLRequestContextGetter* request_context,
-                      service_manager::Connector* connector);
+  ExtensionDownloader(
+      ExtensionDownloaderDelegate* delegate,
+      scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
+      service_manager::Connector* connector,
+      const base::FilePath& profile_path = base::FilePath());
   ~ExtensionDownloader() override;
 
   // Adds |extension| to the list of extensions to check for updates.
@@ -239,18 +247,15 @@ class ExtensionDownloader : public net::URLFetcherDelegate,
   // Begins an update check.
   void StartUpdateCheck(std::unique_ptr<ManifestFetchData> fetch_data);
 
-  // Called by RequestQueue when a new manifest fetch request is started.
-  void CreateManifestFetcher();
+  // Returns the URLLoaderFactory instance to be used, depending on whether
+  // the URL being handled is file:// or not.
+  network::mojom::URLLoaderFactory* GetURLLoaderFactoryToUse(const GURL& url);
 
-  // net::URLFetcherDelegate implementation.
-  void OnURLFetchComplete(const net::URLFetcher* source) override;
+  // Called by RequestQueue when a new manifest load request is started.
+  void CreateManifestLoader();
 
   // Handles the result of a manifest fetch.
-  void OnManifestFetchComplete(const GURL& url,
-                               const net::URLRequestStatus& status,
-                               int response_code,
-                               const base::TimeDelta& backoff_delay,
-                               const std::string& data);
+  void OnManifestLoadComplete(std::unique_ptr<std::string> response_body);
 
   // Once a manifest is parsed, this starts fetches of any relevant crx files.
   // If |results| is null, it means something went wrong when parsing it.
@@ -275,15 +280,12 @@ class ExtensionDownloader : public net::URLFetcherDelegate,
   // Begins (or queues up) download of an updated extension.
   void FetchUpdatedExtension(std::unique_ptr<ExtensionFetch> fetch_data);
 
-  // Called by RequestQueue when a new extension fetch request is started.
-  void CreateExtensionFetcher();
+  // Called by RequestQueue when a new extension load request is started.
+  void CreateExtensionLoader();
+  void StartExtensionLoader();
 
   // Handles the result of a crx fetch.
-  void OnCRXFetchComplete(const net::URLFetcher* source,
-                          const GURL& url,
-                          const net::URLRequestStatus& status,
-                          int response_code,
-                          const base::TimeDelta& backoff_delay);
+  void OnExtensionLoadComplete(base::FilePath crx_path);
 
   // Invokes OnExtensionDownloadFailed() on the |delegate_| for each extension
   // in the set, with |error| as the reason for failure.
@@ -344,8 +346,14 @@ class ExtensionDownloader : public net::URLFetcherDelegate,
   // ExtensionDownloader, and that fills in optional ping and update url data.
   ExtensionDownloaderDelegate* delegate_;
 
-  // The request context to use for the URLFetchers.
-  scoped_refptr<net::URLRequestContextGetter> request_context_;
+  // The URL loader factory to use for the SimpleURLLoaders.
+  scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory_;
+
+  // The URL loader factory exclusively used to load file:// URLs.
+  std::unique_ptr<network::mojom::URLLoaderFactory> file_url_loader_factory_;
+
+  // The profile path used to load file:// URLs. It can be invalid.
+  base::FilePath profile_path_for_url_loader_factory_;
 
   // The connector to the ServiceManager.
   service_manager::Connector* connector_;
@@ -361,9 +369,10 @@ class ExtensionDownloader : public net::URLFetcherDelegate,
                             std::vector<std::unique_ptr<ManifestFetchData>>>;
   FetchMap fetches_preparing_;
 
-  // Outstanding url fetch requests for manifests and updates.
-  std::unique_ptr<net::URLFetcher> manifest_fetcher_;
-  std::unique_ptr<net::URLFetcher> extension_fetcher_;
+  // Outstanding url loader requests for manifests and updates.
+  std::unique_ptr<network::SimpleURLLoader> manifest_loader_;
+  std::unique_ptr<network::SimpleURLLoader> extension_loader_;
+  std::unique_ptr<network::ResourceRequest> extension_loader_resource_request_;
 
   // Pending manifests and extensions to be fetched when the appropriate fetcher
   // is available.
@@ -401,6 +410,10 @@ class ExtensionDownloader : public net::URLFetcherDelegate,
   // to update URLs which match this domain. Defaults to empty (no domain).
   std::string ping_enabled_domain_;
 
+  net::HttpRequestHeaders
+      last_extension_loader_resource_request_headers_for_testing_;
+  int last_extension_loader_load_flags_for_testing_ = 0;
+
   // Used to create WeakPtrs to |this|.
   base::WeakPtrFactory<ExtensionDownloader> weak_ptr_factory_;
 

extensions/browser/updater/extension_downloader_delegate.cc

@@ -25,6 +25,8 @@ void ExtensionDownloaderDelegate::OnExtensionDownloadFailed(
     const std::set<int>& request_id) {
 }
 
+void ExtensionDownloaderDelegate::OnExtensionDownloadRetryForTests() {}
+
 bool ExtensionDownloaderDelegate::GetPingDataForExtension(
     const std::string& id,
     ManifestFetchData::PingData* ping) {

extensions/browser/updater/extension_downloader_delegate.h

@@ -102,6 +102,11 @@ class ExtensionDownloaderDelegate {
                                            const std::set<int>& request_ids,
                                            const InstallCallback& callback) = 0;
 
+  // Invoked when an extension fails to load, but a retry is triggered.
+  // It allows unittests to easily set up and verify resourse request and
+  // load results between a failure / retry sequence.
+  virtual void OnExtensionDownloadRetryForTests();
+
   // The remaining methods are used by the ExtensionDownloader to retrieve
   // information about extensions from the delegate.