Skip update if a component is found during registration

Update CrOSComponentManager to handle the case when an installed
compatible component is found during component registration started by
CrOSComponentManager::Load.

Previously, CrOSComponentManager would start OnDemandUpdate whenever no
compatible component path was registered when Load() was called - this
did not account for cases where Load was called early on, before
registration of installed components finished. This means that
CrOSComponentManager could end up starting foreground on demand update
even if a compatible version of component was already installed.

This CL moves decision whether an update should be requested later on -
after the component registration was done. Update will be run if it's
forced, or if there is no compatible component version after component
registration finishes (which should check for any installed components).

Note - this also moves ComponentInstaller::Register callback after
OnComponentReady is called, to ensure that any potential compatible
component path is registered before CrOSComponentManager::StartInstall
is called. (This is more in line with order OnComponentReady and the
callback are invoked during Install request, and no other callers seem
to depend on this ordering).

BUG=872871

Change-Id: I33ff01b5d127d17cd9016b012fc400562c03d9c1
Reviewed-on: https://chromium-review.googlesource.com/1170172Reviewed-by: Joshua Pawlicki <waffles@chromium.org>
Commit-Queue: Toni Baržić <tbarzic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582230}

chrome/browser/component_updater/cros_component_installer_chromeos.cc

@@ -217,18 +217,11 @@ void CrOSComponentManager::Load(const std::string& name,
                                 MountPolicy mount_policy,
                                 UpdatePolicy update_policy,
                                 LoadCallback load_callback) {
-  if (!IsCompatible(name)) {
-    // A compatible component is not installed.
-    // Start installation process.
+  if (!IsCompatible(name) || update_policy == UpdatePolicy::kForce) {
+    // A compatible component is not installed, or forced update is requested.
+    // Start registration and installation/update process.
     auto* const cus = g_browser_process->component_updater();
-    Install(cus, name, OnDemandUpdater::Priority::FOREGROUND, mount_policy,
-            std::move(load_callback));
-  } else if (update_policy == UpdatePolicy::kForce) {
-    // Caller forces update check.
-    // Start update process.
-    auto* const cus = g_browser_process->component_updater();
-    Install(cus, name, OnDemandUpdater::Priority::BACKGROUND, mount_policy,
-            std::move(load_callback));
+    Install(cus, name, update_policy, mount_policy, std::move(load_callback));
   } else if (mount_policy == MountPolicy::kMount) {
     // A compatible component is installed, load it.
     LoadInternal(name, std::move(load_callback));
@@ -297,7 +290,7 @@ void CrOSComponentManager::Register(ComponentUpdateService* cus,
 
 void CrOSComponentManager::Install(ComponentUpdateService* cus,
                                    const std::string& name,
-                                   OnDemandUpdater::Priority priority,
+                                   UpdatePolicy update_policy,
                                    MountPolicy mount_policy,
                                    LoadCallback load_callback) {
   const ComponentConfig* config = FindConfig(name);
@@ -308,10 +301,11 @@ void CrOSComponentManager::Install(ComponentUpdateService* cus,
                                   base::FilePath()));
     return;
   }
+
   Register(cus, *config,
            base::BindOnce(
                &CrOSComponentManager::StartInstall, base::Unretained(this), cus,
-               GenerateId(config->sha2hash), priority,
+               name, GenerateId(config->sha2hash), update_policy,
                base::BindOnce(&CrOSComponentManager::FinishInstall,
                               base::Unretained(this), name, mount_policy,
                               std::move(load_callback))));
@@ -319,9 +313,23 @@ void CrOSComponentManager::Install(ComponentUpdateService* cus,
 
 void CrOSComponentManager::StartInstall(
     ComponentUpdateService* cus,
+    const std::string& name,
     const std::string& id,
-    OnDemandUpdater::Priority priority,
+    UpdatePolicy update_policy,
     update_client::Callback install_callback) {
+  // Check whether an installed component was found during registration, and
+  // determine whether OnDemandUpdater should be started accordingly.
+  const bool is_compatible = IsCompatible(name);
+  if (is_compatible && update_policy != UpdatePolicy::kForce) {
+    base::ThreadTaskRunnerHandle::Get()->PostTask(
+        FROM_HERE, base::BindOnce(std::move(install_callback),
+                                  update_client::Error::NONE));
+    return;
+  }
+
+  const component_updater::OnDemandUpdater::Priority priority =
+      is_compatible ? component_updater::OnDemandUpdater::Priority::BACKGROUND
+                    : component_updater::OnDemandUpdater::Priority::FOREGROUND;
   cus->GetOnDemandUpdater().OnDemandUpdate(id, priority,
                                            std::move(install_callback));
 }
@@ -346,7 +354,6 @@ void CrOSComponentManager::FinishInstall(const std::string& name,
 
 void CrOSComponentManager::LoadInternal(const std::string& name,
                                         LoadCallback load_callback) {
-  DCHECK(IsCompatible(name));
   const base::FilePath path = GetCompatiblePath(name);
   // path is empty if no compatible component is available to load.
   if (!path.empty()) {

chrome/browser/component_updater/cros_component_installer_chromeos.h

@@ -164,15 +164,16 @@ class CrOSComponentManager {
   // Installs a component with a dedicated ComponentUpdateService instance.
   void Install(ComponentUpdateService* cus,
                const std::string& name,
-               OnDemandUpdater::Priority priority,
+               UpdatePolicy update_policy,
                MountPolicy mount_policy,
                LoadCallback load_callback);
 
   // Calls OnDemandUpdate to install the component right after being registered.
   // |id| is the component id generated from its sha2 hash.
   void StartInstall(ComponentUpdateService* cus,
+                    const std::string& name,
                     const std::string& id,
-                    OnDemandUpdater::Priority priority,
+                    UpdatePolicy update_policy,
                     update_client::Callback install_callback);
 
   // Calls LoadInternal to load the installed component.

components/component_updater/component_installer.cc

@@ -419,15 +419,14 @@ void ComponentInstaller::FinishRegistration(
     return;
   }
 
-  if (!callback.is_null())
-    std::move(callback).Run();
-
-  if (!registration_info->manifest) {
+  if (registration_info->manifest) {
+    ComponentReady(std::move(registration_info->manifest));
+  } else {
     DVLOG(1) << "No component found for " << installer_policy_->GetName();
-    return;
   }
 
-  ComponentReady(std::move(registration_info->manifest));
+  if (!callback.is_null())
+    std::move(callback).Run();
 }
 
 void ComponentInstaller::ComponentReady(