Call CrOSComponentManager::Load callback on main thread

Most of the callers to CrOSComponentManager::Load expect the
callback to be called on the UI thread (the same as the thread from
which the method was called)- the only exception
is CrostiniManager, which does not assume the callback is called
on the UI thread, but still just relays the callback result back to
UI thread.

Currently, callbacks are run using base::PostTask, which post the
callback to the task runner. To make the behavior of the method match
the expectations, use ThreadTaskRunnerHandle::Get to get the current
task runner, and use it to asynchronously run the callback.

Bug: None
Change-Id: I9f365a11ba2634154488c580615ceede59e2a9d3
Reviewed-on: https://chromium-review.googlesource.com/1163071
Commit-Queue: Toni Barzic <tbarzic@chromium.org>
Reviewed-by: Nicholas Verne <nverne@chromium.org>
Reviewed-by: Joshua Pawlicki <waffles@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580903}

chrome/browser/chromeos/crostini/crostini_manager.cc

@@ -17,7 +17,6 @@
 #include "chrome/browser/chromeos/crostini/crostini_remover.h"
 #include "chrome/browser/chromeos/crostini/crostini_util.h"
 #include "chrome/browser/chromeos/profiles/profile_helper.h"
-#include "chrome/browser/component_updater/cros_component_installer_chromeos.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/ui/browser.h"
@@ -448,28 +447,6 @@ void CrostiniManager::MaybeUpgradeCrostiniAfterTerminaCheck(
   InstallTerminaComponent(base::DoNothing());
 }
 
-namespace {
-void InstallTerminaComponentLoaderCallback(
-    CrostiniManager::BoolCallback callback,
-    component_updater::CrOSComponentManager::Error error,
-    const base::FilePath& result) {
-  DCHECK(!content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
-
-  bool is_successful =
-      error == component_updater::CrOSComponentManager::Error::NONE;
-
-  if (!is_successful) {
-    LOG(ERROR)
-        << "Failed to install the cros-termina component with error code: "
-        << static_cast<int>(error);
-  }
-  // Hop to the UI thread to update state and run |callback|.
-  content::BrowserThread::PostTask(
-      content::BrowserThread::UI, FROM_HERE,
-      base::BindOnce(std::move(callback), is_successful));
-}
-}  // namespace
-
 void CrostiniManager::InstallTerminaComponent(BoolCallback callback) {
   if (chromeos::DBusThreadManager::Get()->IsUsingFakes()) {
     // Running in test. We still PostTask to prevent races.
@@ -477,7 +454,9 @@ void CrostiniManager::InstallTerminaComponent(BoolCallback callback) {
         content::BrowserThread::UI, FROM_HERE,
         base::BindOnce(&CrostiniManager::OnInstallTerminaComponent,
                        weak_ptr_factory_.GetWeakPtr(), std::move(callback),
-                       true, true));
+                       true,
+                       component_updater::CrOSComponentManager::Error::NONE,
+                       base::FilePath()));
     return;
   }
   auto* cros_component_manager =
@@ -517,20 +496,30 @@ void CrostiniManager::InstallTerminaComponent(BoolCallback callback) {
       imageloader::kTerminaComponentName,
       component_updater::CrOSComponentManager::MountPolicy::kMount,
       update_policy,
-      base::BindOnce(
-          InstallTerminaComponentLoaderCallback,
-          base::BindOnce(&CrostiniManager::OnInstallTerminaComponent,
-                         weak_ptr_factory_.GetWeakPtr(), std::move(callback),
-                         update_policy == UpdatePolicy::kForce)));
+      base::BindOnce(&CrostiniManager::OnInstallTerminaComponent,
+                     weak_ptr_factory_.GetWeakPtr(), std::move(callback),
+                     update_policy == UpdatePolicy::kForce));
 }
 
-void CrostiniManager::OnInstallTerminaComponent(BoolCallback callback,
-                                                bool is_update_checked,
-                                                bool is_successful) {
+void CrostiniManager::OnInstallTerminaComponent(
+    CrostiniManager::BoolCallback callback,
+    bool is_update_checked,
+    component_updater::CrOSComponentManager::Error error,
+    const base::FilePath& result) {
+  bool is_successful =
+      error == component_updater::CrOSComponentManager::Error::NONE;
+
+  if (!is_successful) {
+    LOG(ERROR)
+        << "Failed to install the cros-termina component with error code: "
+        << static_cast<int>(error);
+  }
+
   if (is_successful && is_update_checked) {
     VLOG(1) << "cros-termina update check successful.";
     termina_update_check_needed_ = false;
   }
+
   std::move(callback).Run(is_successful);
 }
 

chrome/browser/chromeos/crostini/crostini_manager.h

@@ -16,6 +16,7 @@
 #include "base/memory/singleton.h"
 #include "base/memory/weak_ptr.h"
 #include "base/optional.h"
+#include "chrome/browser/component_updater/cros_component_installer_chromeos.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/extensions/app_launch_params.h"
 #include "chromeos/dbus/cicerone/cicerone_service.pb.h"
@@ -383,9 +384,11 @@ class CrostiniManager : public chromeos::ConciergeClient::Observer,
 
   // Callback for CrostiniManager::InstallCrostiniComponent. Must be called on
   // the UI thread.
-  void OnInstallTerminaComponent(BoolCallback callback,
-                                 bool is_update_checked,
-                                 bool is_successful);
+  void OnInstallTerminaComponent(
+      BoolCallback callback,
+      bool is_update_checked,
+      component_updater::CrOSComponentManager::Error error,
+      const base::FilePath& result);
 
   // Callback for CrostiniClient::StartConcierge. Called after the
   // DebugDaemon service method finishes.

chrome/browser/component_updater/cros_component_installer_chromeos.cc

@@ -12,6 +12,7 @@
 #include "base/path_service.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/task/post_task.h"
+#include "base/threading/thread_task_runner_handle.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/component_updater/component_installer_errors.h"
 #include "chrome/browser/component_updater/metadata_table_chromeos.h"
@@ -230,8 +231,8 @@ void CrOSComponentManager::Load(const std::string& name,
     LoadInternal(name, std::move(load_callback));
   } else {
     // A compatible component is installed, do not load it.
-    base::PostTask(FROM_HERE,
-                   base::BindOnce(std::move(load_callback),
+    base::ThreadTaskRunnerHandle::Get()->PostTask(
+        FROM_HERE, base::BindOnce(std::move(load_callback),
                                   ReportError(Error::NONE), base::FilePath()));
   }
 }
@@ -298,8 +299,8 @@ void CrOSComponentManager::Install(ComponentUpdateService* cus,
                                    LoadCallback load_callback) {
   const ComponentConfig* config = FindConfig(name);
   if (!config) {
-    base::PostTask(FROM_HERE,
-                   base::BindOnce(std::move(load_callback),
+    base::ThreadTaskRunnerHandle::Get()->PostTask(
+        FROM_HERE, base::BindOnce(std::move(load_callback),
                                   ReportError(Error::UNKNOWN_COMPONENT),
                                   base::FilePath()));
     return;
@@ -327,15 +328,15 @@ void CrOSComponentManager::FinishInstall(const std::string& name,
                                          LoadCallback load_callback,
                                          update_client::Error error) {
   if (error != update_client::Error::NONE) {
-    base::PostTask(
+    base::ThreadTaskRunnerHandle::Get()->PostTask(
         FROM_HERE,
         base::BindOnce(std::move(load_callback),
                        ReportError(Error::INSTALL_FAILURE), base::FilePath()));
   } else if (mount_policy == MountPolicy::kMount) {
     LoadInternal(name, std::move(load_callback));
   } else {
-    base::PostTask(FROM_HERE,
-                   base::BindOnce(std::move(load_callback),
+    base::ThreadTaskRunnerHandle::Get()->PostTask(
+        FROM_HERE, base::BindOnce(std::move(load_callback),
                                   ReportError(Error::NONE), base::FilePath()));
   }
 }
@@ -354,7 +355,7 @@ void CrOSComponentManager::LoadInternal(const std::string& name,
                            base::Unretained(this), std::move(load_callback),
                            base::TimeTicks::Now(), name));
   } else {
-    base::PostTask(
+    base::ThreadTaskRunnerHandle::Get()->PostTask(
         FROM_HERE,
         base::BindOnce(std::move(load_callback),
                        ReportError(Error::COMPATIBILITY_CHECK_FAILED),
@@ -370,13 +371,14 @@ void CrOSComponentManager::FinishLoad(LoadCallback load_callback,
   UMA_HISTOGRAM_LONG_TIMES("ComponentUpdater.ChromeOS.MountTime",
                            base::TimeTicks::Now() - start_time);
   if (!result.has_value()) {
-    base::PostTask(FROM_HERE, base::BindOnce(std::move(load_callback),
-                                             ReportError(Error::MOUNT_FAILURE),
-                                             base::FilePath()));
+    base::ThreadTaskRunnerHandle::Get()->PostTask(
+        FROM_HERE,
+        base::BindOnce(std::move(load_callback),
+                       ReportError(Error::MOUNT_FAILURE), base::FilePath()));
   } else {
     metadata_table_->AddComponentForCurrentUser(name);
-    base::PostTask(FROM_HERE,
-                   base::BindOnce(std::move(load_callback),
+    base::ThreadTaskRunnerHandle::Get()->PostTask(
+        FROM_HERE, base::BindOnce(std::move(load_callback),
                                   ReportError(Error::NONE), result.value()));
   }
 }

chrome/browser/component_updater/cros_component_installer_chromeos.h

@@ -113,6 +113,7 @@ class CrOSComponentManager {
   void SetDelegate(Delegate* delegate);
 
   // Installs a component and keeps it up-to-date.
+  // The |load_callback| is run on the calling thread.
   void Load(const std::string& name,
             MountPolicy mount_policy,
             UpdatePolicy update_policy,