[Extensions] Use BEST_EFFORT task priority to install default apps

Since default apps' installation isn't urgent, it's fine if they
take a bit more time to install. This saves precious CPU cycles during
fresh profile startup, when Chrome is already busy doing a bunch of
other stuff.

There are 2 benefits:
  (1) Other extensions that install at the same time install faster
      (e.g. force-installed extensions)
  (2) It reduces jank during the first-run experience (FRE), especially
      for lower-end/older CPUs, and virtual machines. Since the FRE has
      fancy CSS animations, the jank is noticeable.

Before/after trace comparison: https://imgur.com/a/o9Y0Dqv

Bug: 1103447, 904486
Change-Id: I4b9f2eecf71c3414a36769d896296d4f089c81a4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2377888Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Commit-Queue: Nicolas Ouellet-Payeur <nicolaso@chromium.org>
Cr-Commit-Position: refs/heads/master@{#803180}

chrome/browser/extensions/crx_installer.cc

@@ -126,7 +126,6 @@ CrxInstaller::CrxInstaller(base::WeakPtr<ExtensionService> service_weak,
       did_handle_successfully_(true),
       error_on_unsupported_requirements_(false),
       shared_file_task_runner_(GetExtensionFileTaskRunner()),
-      unpacker_task_runner_(GetOneShotFileTaskRunner()),
       update_from_settings_page_(false),
       install_flags_(kInstallFlagNone) {
   if (!approval)
@@ -187,9 +186,9 @@ void CrxInstaller::InstallCrxFile(const CRXFileInfo& source_file) {
 
   auto unpacker = base::MakeRefCounted<SandboxedUnpacker>(
       install_source_, creation_flags_, install_directory_,
-      unpacker_task_runner_.get(), this);
+      GetUnpackerTaskRunner(), this);
 
-  if (!unpacker_task_runner_->PostTask(
+  if (!GetUnpackerTaskRunner()->PostTask(
           FROM_HERE, base::BindOnce(&SandboxedUnpacker::StartWithCrx, unpacker,
                                     source_file))) {
     NOTREACHED();
@@ -209,9 +208,9 @@ void CrxInstaller::InstallUnpackedCrx(const std::string& extension_id,
 
   auto unpacker = base::MakeRefCounted<SandboxedUnpacker>(
       install_source_, creation_flags_, install_directory_,
-      unpacker_task_runner_.get(), this);
+      GetUnpackerTaskRunner(), this);
 
-  if (!unpacker_task_runner_->PostTask(
+  if (!GetUnpackerTaskRunner()->PostTask(
           FROM_HERE,
           base::BindOnce(&SandboxedUnpacker::StartWithDirectory, unpacker,
                          extension_id, public_key, unpacked_dir))) {
@@ -496,14 +495,14 @@ void CrxInstaller::ShouldComputeHashesOnUI(
       extensions::ExtensionSystem::Get(profile_)->content_verifier();
   bool result = content_verifier &&
                 content_verifier->ShouldComputeHashesOnInstall(*extension);
-  unpacker_task_runner_->PostTask(FROM_HERE,
-                                  base::BindOnce(std::move(callback), result));
+  GetUnpackerTaskRunner()->PostTask(
+      FROM_HERE, base::BindOnce(std::move(callback), result));
 }
 
 void CrxInstaller::ShouldComputeHashesForOffWebstoreExtension(
     scoped_refptr<const Extension> extension,
     base::OnceCallback<void(bool)> callback) {
-  DCHECK(unpacker_task_runner_->RunsTasksInCurrentSequence());
+  DCHECK(GetUnpackerTaskRunner()->RunsTasksInCurrentSequence());
   if (!content::GetUIThreadTaskRunner({})->PostTask(
           FROM_HERE,
           base::BindOnce(&CrxInstaller::ShouldComputeHashesOnUI, this,
@@ -513,7 +512,7 @@ void CrxInstaller::ShouldComputeHashesForOffWebstoreExtension(
 }
 
 void CrxInstaller::OnUnpackFailure(const CrxInstallError& error) {
-  DCHECK(unpacker_task_runner_->RunsTasksInCurrentSequence());
+  DCHECK(GetUnpackerTaskRunner()->RunsTasksInCurrentSequence());
   if (!content::GetUIThreadTaskRunner({})->PostTask(
           FROM_HERE, base::BindOnce(&CrxInstaller::ReportFailureFromUIThread,
                                     this, error))) {
@@ -528,7 +527,7 @@ void CrxInstaller::OnUnpackSuccess(
     const Extension* extension,
     const SkBitmap& install_icon,
     declarative_net_request::RulesetInstallPrefs ruleset_install_prefs) {
-  DCHECK(unpacker_task_runner_->RunsTasksInCurrentSequence());
+  DCHECK(GetUnpackerTaskRunner()->RunsTasksInCurrentSequence());
   shared_file_task_runner_->PostTask(
       FROM_HERE,
       base::BindOnce(&CrxInstaller::OnUnpackSuccessOnSharedFileThread, this,
@@ -1046,7 +1045,7 @@ void CrxInstaller::ReportSuccessFromUIThread() {
 
 void CrxInstaller::ReportInstallationStage(InstallationStage stage) {
   if (!BrowserThread::CurrentlyOn(BrowserThread::UI)) {
-    DCHECK(unpacker_task_runner_->RunsTasksInCurrentSequence() ||
+    DCHECK(GetUnpackerTaskRunner()->RunsTasksInCurrentSequence() ||
            shared_file_task_runner_->RunsTasksInCurrentSequence());
     if (!content::GetUIThreadTaskRunner({})->PostTask(
             FROM_HERE, base::BindOnce(&CrxInstaller::ReportInstallationStage,
@@ -1203,6 +1202,18 @@ void CrxInstaller::ConfirmReEnable() {
   }
 }
 
+base::SequencedTaskRunner* CrxInstaller::GetUnpackerTaskRunner() {
+  if (!unpacker_task_runner_) {
+    bool low_priority =
+        (creation_flags_ & Extension::WAS_INSTALLED_BY_DEFAULT) &&
+        !(creation_flags_ & Extension::WAS_INSTALLED_BY_OEM);
+    unpacker_task_runner_ = GetOneShotFileTaskRunner(
+        low_priority ? base::TaskPriority::BEST_EFFORT
+                     : base::TaskPriority::USER_VISIBLE);
+  }
+  return unpacker_task_runner_.get();
+}
+
 void CrxInstaller::set_installer_callback(InstallerResultCallback callback) {
   installer_callback_ = std::move(callback);
 }

chrome/browser/extensions/crx_installer.h

@@ -361,6 +361,9 @@ class CrxInstaller : public SandboxedUnpackerClient {
       install_flags_ &= ~flag;
   }
 
+  // Returns |unpacker_task_runner_|. Initializes it if it's still nullptr.
+  base::SequencedTaskRunner* GetUnpackerTaskRunner();
+
   // The Profile the extension is being installed in.
   Profile* profile_;
 
@@ -506,10 +509,8 @@ class CrxInstaller : public SandboxedUnpackerClient {
   // unpacker uses its own temp dir, it won't hit race conditions, and can use a
   // separate task runner per instance (for better performance).
   //
-  // TODO(nicolaso): Adjust this task runner's priority based on the install
-  // location. e.g. default apps shouldn't be USER_VISIBLE, to avoid wasting CPU
-  // time.
-  scoped_refptr<base::SequencedTaskRunner> unpacker_task_runner_;
+  // Lazily initialized by GetUnpackerTaskRunner().
+  scoped_refptr<base::SequencedTaskRunner> unpacker_task_runner_ = nullptr;
 
   // Used to show the install dialog.
   ExtensionInstallPrompt::ShowDialogCallback show_dialog_callback_;

chrome/browser/extensions/external_pref_loader.h

@@ -122,6 +122,9 @@ class ExternalPrefLoader : public ExternalLoader {
   // tests may not be set.
   const std::string user_type_;
 
+  // Task runner for tasks that touch file.
+  scoped_refptr<base::SequencedTaskRunner> file_task_runner_;
+
 #if defined(OS_CHROMEOS)
   std::vector<std::unique_ptr<PrioritySyncReadyWaiter>> pending_waiter_list_;
 #endif

chrome/browser/extensions/updater/extension_updater.cc

@@ -263,9 +263,12 @@ void ExtensionUpdater::SetBackoffPolicyForTesting(
 }
 
 void ExtensionUpdater::DoCheckSoon() {
-  DCHECK(will_check_soon_);
+  if (!will_check_soon_) {
+    // Another caller called CheckNow() between CheckSoon() and now. Skip this
+    // check.
+    return;
+  }
   CheckNow(CheckParams());
-  will_check_soon_ = false;
 }
 
 void ExtensionUpdater::AddToDownloader(
@@ -299,6 +302,12 @@ void ExtensionUpdater::AddToDownloader(
 }
 
 void ExtensionUpdater::CheckNow(CheckParams params) {
+  if (params.ids.empty()) {
+    // Checking all extensions. Cancel pending DoCheckSoon() call if there's
+    // one, as it would be redundant.
+    will_check_soon_ = false;
+  }
+
   int request_id = next_request_id_++;
 
   VLOG(2) << "Starting update check " << request_id;

extensions/browser/extension_file_task_runner.cc

@@ -30,10 +30,11 @@ scoped_refptr<base::SequencedTaskRunner> GetExtensionFileTaskRunner() {
   return g_task_runner.Get();
 }
 
-scoped_refptr<base::SequencedTaskRunner> GetOneShotFileTaskRunner() {
+scoped_refptr<base::SequencedTaskRunner> GetOneShotFileTaskRunner(
+    base::TaskPriority priority) {
   return base::ThreadPool::CreateSequencedTaskRunner(
       {base::MayBlock(), base::TaskShutdownBehavior::SKIP_ON_SHUTDOWN,
-       base::TaskPriority::USER_VISIBLE});
+       priority});
 }
 
 }  // namespace extensions

extensions/browser/extension_file_task_runner.h

@@ -6,6 +6,7 @@
 #define EXTENSIONS_BROWSER_EXTENSION_FILE_TASK_RUNNER_H_
 
 #include "base/memory/ref_counted.h"
+#include "base/task/task_traits.h"
 
 namespace base {
 class SequencedTaskRunner;
@@ -23,7 +24,8 @@ scoped_refptr<base::SequencedTaskRunner> GetExtensionFileTaskRunner();
 // race with each other. Currently, this is used to unpack multiple extensions
 // in parallel. They each touch a different set of files, which avoids potential
 // race conditions.
-scoped_refptr<base::SequencedTaskRunner> GetOneShotFileTaskRunner();
+scoped_refptr<base::SequencedTaskRunner> GetOneShotFileTaskRunner(
+    base::TaskPriority priority);
 
 }  // namespace extensions