[Extensions] Force-installed extensions download with net::MEDIUM priority

Extensions manifest and crx would default to net::IDLE priority for download. In order to speed up "task runner" priority of downloads, change priority for manifest and crx resource requests to net::MEDIUM when the active request's fetch_priority is in the FOREGROUND.

Bug: 1095181
Change-Id: I2e81333afb968127126a13956a4d53b6e58cfbe0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2274477
Commit-Queue: Mohamadou Bella Bah <bellabah@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Nicolas Ouellet-Payeur <nicolaso@chromium.org>
Cr-Commit-Position: refs/heads/master@{#800235}

chrome/browser/extensions/updater/extension_updater_unittest.cc

@@ -1455,6 +1455,80 @@ class ExtensionUpdaterTest : public testing::Test {
     RunUntilIdle();
   }
 
+  // Checks that the manifest is fetched with a priority of |net::MEDIUM| (which
+  // maps to |TaskPriority::USER_BLOCKING| for the task runner) when the active
+  // request's |fetch_priority| is in the FOREGROUND.
+  void TestManifestFetchPriority(
+      ManifestFetchData::FetchPriority fetch_priority) {
+    ExtensionDownloaderTestHelper helper;
+    helper.downloader().manifests_queue_.set_backoff_policy(&kNoBackoffPolicy);
+    GURL test_url("http://localhost/manifest1");
+    std::unique_ptr<ManifestFetchData> fetch(
+        CreateManifestFetchData(test_url));
+    ManifestFetchData::PingData zero_days(0, 0, true, 0);
+
+    fetch->AddExtension("1111", "1.0", &zero_days, kEmptyUpdateUrlData,
+                        std::string(), Manifest::Location::INTERNAL,
+                        fetch_priority);
+
+    helper.downloader().StartUpdateCheck(std::move(fetch));
+    RunUntilIdle();
+
+    auto* request = helper.GetPendingRequest(0);
+    helper.test_url_loader_factory().SimulateResponseForPendingRequest(
+        request->request.url, network::URLLoaderCompletionStatus(net::OK),
+        network::CreateURLResponseHead(net::HTTP_INTERNAL_SERVER_ERROR), "");
+    RunUntilIdle();
+
+    if (fetch_priority == ManifestFetchData::FetchPriority::FOREGROUND) {
+      EXPECT_EQ(net::MEDIUM, request->request.priority);
+    } else {
+      EXPECT_EQ(net::IDLE, request->request.priority);
+    }
+  }
+
+  // Checks that the crx of a given extension is downloaded with a priority of
+  // |net::MEDIUM| (maps to |TaskPriority::USER_BLOCKING| for the task runner)
+  // when the active request's |fetch_priority| is in the FOREGROUND.
+  void TestSingleExtensionDownloadingPriority(
+      ManifestFetchData::FetchPriority fetch_priority) {
+    ExtensionUpdater::ScopedSkipScheduledCheckForTest skip_scheduled_checks;
+    ExtensionDownloaderTestHelper helper;
+    std::unique_ptr<ServiceForDownloadTests> service =
+        std::make_unique<ServiceForDownloadTests>(prefs_.get(),
+                                                  helper.url_loader_factory());
+    ExtensionUpdater updater(service.get(), service->extension_prefs(),
+                             service->pref_service(), service->profile(),
+                             kUpdateFrequencySecs, nullptr,
+                             service->GetDownloaderFactory());
+    MockExtensionDownloaderDelegate delegate;
+    delegate.DelegateTo(&updater);
+    service->OverrideDownloaderDelegate(&delegate);
+    updater.Start();
+    updater.EnsureDownloaderCreated();
+    updater.downloader_->extensions_queue_.set_backoff_policy(
+        &kNoBackoffPolicy);
+
+    GURL test_url("http://localhost/extension.crx");
+    const std::string id(32, 'a');
+    std::string hash;
+    CRXFileInfo crx_file_info;
+    base::Version version("0.0.1");
+    std::set<int> requests({0});
+    std::unique_ptr<ExtensionDownloader::ExtensionFetch> fetch =
+        std::make_unique<ExtensionDownloader::ExtensionFetch>(
+            id, test_url, hash, version.GetString(), requests, fetch_priority);
+
+    updater.downloader_->FetchUpdatedExtension(std::move(fetch), base::nullopt);
+
+    auto* request = helper.GetPendingRequest(0);
+    if (fetch_priority == ManifestFetchData::FetchPriority::FOREGROUND) {
+      EXPECT_EQ(net::MEDIUM, request->request.priority);
+    } else {
+      EXPECT_EQ(net::IDLE, request->request.priority);
+    }
+  }
+
   void TestSingleExtensionDownloading(bool pending, bool retry, bool fail) {
     ExtensionUpdater::ScopedSkipScheduledCheckForTest skip_scheduled_checks;
     ExtensionDownloaderTestHelper helper;
@@ -1486,7 +1560,8 @@ class ExtensionUpdaterTest : public testing::Test {
     requests.insert(0);
     std::unique_ptr<ExtensionDownloader::ExtensionFetch> fetch =
         std::make_unique<ExtensionDownloader::ExtensionFetch>(
-            id, test_url, hash, version.GetString(), requests);
+            id, test_url, hash, version.GetString(), requests,
+            ManifestFetchData::FetchPriority::BACKGROUND);
     updater.downloader_->FetchUpdatedExtension(std::move(fetch), base::nullopt);
 
     if (pending) {
@@ -1767,7 +1842,8 @@ class ExtensionUpdaterTest : public testing::Test {
     requests.insert(0);
     std::unique_ptr<ExtensionDownloader::ExtensionFetch> fetch =
         std::make_unique<ExtensionDownloader::ExtensionFetch>(
-            id, test_url, hash, version.GetString(), requests);
+            id, test_url, hash, version.GetString(), requests,
+            ManifestFetchData::FetchPriority::BACKGROUND);
     updater.downloader_->FetchUpdatedExtension(std::move(fetch), base::nullopt);
 
     EXPECT_EQ(
@@ -1981,15 +2057,18 @@ class ExtensionUpdaterTest : public testing::Test {
 
     std::string version1 = "0.1";
     std::string version2 = "0.1";
+
     std::set<int> requests;
     requests.insert(0);
     // Start two fetches
     std::unique_ptr<ExtensionDownloader::ExtensionFetch> fetch1 =
         std::make_unique<ExtensionDownloader::ExtensionFetch>(
-            id1, url1, hash1, version1, requests);
+            id1, url1, hash1, version1, requests,
+            ManifestFetchData::FetchPriority::BACKGROUND);
     std::unique_ptr<ExtensionDownloader::ExtensionFetch> fetch2 =
         std::make_unique<ExtensionDownloader::ExtensionFetch>(
-            id2, url2, hash2, version2, requests);
+            id2, url2, hash2, version2, requests,
+            ManifestFetchData::FetchPriority::BACKGROUND);
     updater.downloader_->FetchUpdatedExtension(std::move(fetch1),
                                                base::Optional<std::string>());
     updater.downloader_->FetchUpdatedExtension(std::move(fetch2),
@@ -2847,6 +2926,18 @@ TEST_F(ExtensionUpdaterTest, TestManifestFetchCredentials) {
   TestManifestCredentialsNonWebstore();
 }
 
+TEST_F(ExtensionUpdaterTest, TestManifestFetchPriority) {
+  TestManifestFetchPriority(ManifestFetchData::FetchPriority::BACKGROUND);
+  TestManifestFetchPriority(ManifestFetchData::FetchPriority::FOREGROUND);
+}
+
+TEST_F(ExtensionUpdaterTest, TestExtensionPriority) {
+  TestSingleExtensionDownloadingPriority(
+      ManifestFetchData::FetchPriority::BACKGROUND);
+  TestSingleExtensionDownloadingPriority(
+      ManifestFetchData::FetchPriority::FOREGROUND);
+}
+
 // TODO(asargent) - (http://crbug.com/12780) add tests for:
 // -prodversionmin (shouldn't update if browser version too old)
 // -manifests & updates arriving out of order / interleaved

extensions/browser/updater/extension_downloader.cc

@@ -196,12 +196,14 @@ ExtensionDownloader::ExtensionFetch::ExtensionFetch(
     const GURL& url,
     const std::string& package_hash,
     const std::string& version,
-    const std::set<int>& request_ids)
+    const std::set<int>& request_ids,
+    const ManifestFetchData::FetchPriority fetch_priority)
     : id(id),
       url(url),
       package_hash(package_hash),
       version(version),
       request_ids(request_ids),
+      fetch_priority(fetch_priority),
       credentials(CREDENTIALS_NONE),
       oauth2_attempt_count(0) {}
 
@@ -598,6 +600,11 @@ void ExtensionDownloader::CreateManifestLoader() {
   resource_request->url = active_request->full_url(),
   resource_request->load_flags = net::LOAD_DISABLE_CACHE;
 
+  if (active_request->fetch_priority() ==
+      ManifestFetchData::FetchPriority::FOREGROUND) {
+    resource_request->priority = net::MEDIUM;
+  }
+
   // Send traffic-management headers to the webstore, and omit credentials.
   // https://bugs.chromium.org/p/chromium/issues/detail?id=647516
   if (extension_urls::IsWebstoreUpdateUrl(active_request->full_url())) {
@@ -673,7 +680,8 @@ void ExtensionDownloader::TryFetchingExtensionsFromCache(
     // version if we have them.
     auto extension_fetch_data(std::make_unique<ExtensionFetch>(
         extension_id, fetch_data->base_url(), /*hash not fetched*/ "",
-        /*version not fetched*/ "", fetch_data->request_ids()));
+        /*version not fetched*/ "", fetch_data->request_ids(),
+        fetch_data->fetch_priority()));
     base::Optional<base::FilePath> cached_crx_path = GetCachedExtension(
         *extension_fetch_data, /*manifest_fetch_failed*/ true);
     if (cached_crx_path) {
@@ -849,10 +857,11 @@ void ExtensionDownloader::HandleManifestResults(
       DCHECK_EQ(fetch_data->fetch_priority(),
                 ManifestFetchData::FetchPriority::FOREGROUND);
     }
-    FetchUpdatedExtension(std::make_unique<ExtensionFetch>(
-                              extension_id, crx_url, update->package_hash,
-                              update->version, fetch_data->request_ids()),
-                          update->info);
+    FetchUpdatedExtension(
+        std::make_unique<ExtensionFetch>(
+            extension_id, crx_url, update->package_hash, update->version,
+            fetch_data->request_ids(), fetch_data->fetch_priority()),
+        update->info);
   }
 
   // If the manifest response included a <daystart> element, we want to save
@@ -1261,6 +1270,12 @@ void ExtensionDownloader::StartExtensionLoader() {
   last_extension_loader_load_flags_for_testing_ =
       extension_loader_resource_request_->load_flags;
 
+  const ExtensionFetch* active_request = extensions_queue_.active_request();
+  if (active_request->fetch_priority ==
+      ManifestFetchData::FetchPriority::FOREGROUND) {
+    extension_loader_resource_request_->priority = net::MEDIUM;
+  }
+
   network::mojom::URLLoaderFactory* url_loader_factory_to_use =
       GetURLLoaderFactoryToUse(extension_loader_resource_request_->url);
   extension_loader_ = network::SimpleURLLoader::Create(

extensions/browser/updater/extension_downloader.h

@@ -200,7 +200,8 @@ class ExtensionDownloader {
                    const GURL& url,
                    const std::string& package_hash,
                    const std::string& version,
-                   const std::set<int>& request_ids);
+                   const std::set<int>& request_ids,
+                   ManifestFetchData::FetchPriority fetch_priority);
     ~ExtensionFetch();
 
     std::string id;
@@ -208,6 +209,7 @@ class ExtensionDownloader {
     std::string package_hash;
     std::string version;
     std::set<int> request_ids;
+    ManifestFetchData::FetchPriority fetch_priority;
 
     enum CredentialsMode {
       CREDENTIALS_NONE = 0,