Return an UPDATE_IN_PROGRESS error from CrosComponentManager

This disabmiguates the case where a component load fails due to no internet,
versus a component load being blocked behind a continuing update.

Bug: 1010796
Change-Id: I9cf6956a86f5f342f7b1bea61666575212e65712
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1930279
Commit-Queue: Nicholas Verne <nverne@chromium.org>
Reviewed-by: Fergus Dall <sidereal@google.com>
Reviewed-by: David Roger <droger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#719447}

chrome/browser/chromeos/crostini/crostini_manager.cc

@@ -838,7 +838,7 @@ void CrostiniManager::OnInstallTerminaComponent(
     CrostiniResultCallback callback,
     bool is_update_checked,
     component_updater::CrOSComponentManager::Error error,
-    const base::FilePath& result) {
+    const base::FilePath& path) {
   bool is_successful =
       error == component_updater::CrOSComponentManager::Error::NONE;
 
@@ -878,10 +878,17 @@ void CrostiniManager::OnInstallTerminaComponent(
     VLOG(1) << "cros-termina update check successful.";
     termina_update_check_needed_ = false;
   }
+  CrostiniResult result = CrostiniResult::SUCCESS;
+  if (!is_successful) {
+    if (error ==
+        component_updater::CrOSComponentManager::Error::UPDATE_IN_PROGRESS) {
+      result = CrostiniResult::LOAD_COMPONENT_UPDATE_IN_PROGRESS;
+    } else {
+      result = CrostiniResult::LOAD_COMPONENT_FAILED;
+    }
+  }
 
-  std::move(callback).Run(is_successful
-                              ? CrostiniResult::SUCCESS
-                              : CrostiniResult::LOAD_COMPONENT_FAILED);
+  std::move(callback).Run(result);
 }
 
 bool CrostiniManager::UninstallTerminaComponent() {

chrome/browser/chromeos/crostini/crostini_manager.h

@@ -630,7 +630,7 @@ class CrostiniManager : public KeyedService,
       CrostiniResultCallback callback,
       bool is_update_checked,
       component_updater::CrOSComponentManager::Error error,
-      const base::FilePath& result);
+      const base::FilePath& path);
 
   // Callback for CrostiniClient::StartConcierge. Called after the
   // DebugDaemon service method finishes.

chrome/browser/chromeos/crostini/crostini_simple_types.h

@@ -71,7 +71,8 @@ enum class CrostiniResult {
   CANCEL_UPGRADE_CONTAINER_FAILED = 45,
   CONCIERGE_START_FAILED = 46,
   CONTAINER_CONFIGURATION_FAILED = 47,
-  kMaxValue = CONTAINER_CONFIGURATION_FAILED,
+  LOAD_COMPONENT_UPDATE_IN_PROGRESS = 48,
+  kMaxValue = LOAD_COMPONENT_UPDATE_IN_PROGRESS,
 };
 
 enum class InstallLinuxPackageProgressStatus {

chrome/browser/chromeos/dbus/component_updater_service_provider.cc

@@ -39,6 +39,8 @@ std::string ErrorToString(
       return "COMPATIBILITY_CHECK_FAILED";
     case component_updater::CrOSComponentManager::Error::NOT_FOUND:
       return "NOT_FOUND";
+    case component_updater::CrOSComponentManager::Error::UPDATE_IN_PROGRESS:
+      return "UPDATE_IN_PROGRESS";
     case component_updater::CrOSComponentManager::Error::ERROR_MAX:
       return "ERROR_MAX";
   }

chrome/browser/component_updater/cros_component_installer_chromeos.cc

@@ -336,10 +336,13 @@ void CrOSComponentInstaller::FinishInstall(const std::string& name,
                                            LoadCallback load_callback,
                                            update_client::Error error) {
   if (error != update_client::Error::NONE) {
+    Error err = Error::INSTALL_FAILURE;
+    if (error == update_client::Error::UPDATE_IN_PROGRESS) {
+      err = Error::UPDATE_IN_PROGRESS;
+    }
     base::ThreadTaskRunnerHandle::Get()->PostTask(
-        FROM_HERE,
-        base::BindOnce(std::move(load_callback),
-                       ReportError(Error::INSTALL_FAILURE), base::FilePath()));
+        FROM_HERE, base::BindOnce(std::move(load_callback), ReportError(err),
+                                  base::FilePath()));
   } else if (!IsCompatible(name)) {
     base::ThreadTaskRunnerHandle::Get()->PostTask(
         FROM_HERE,

chrome/browser/component_updater/cros_component_installer_chromeos_unittest.cc

@@ -118,9 +118,10 @@ class TestUpdater : public OnDemandUpdater {
                       Callback callback) override {
     const std::string& name = component_id_to_name_[id];
     ASSERT_FALSE(name.empty());
-    // Technically, this is a supported use case for background updates, but not
-    // needed for this tests.
-    ASSERT_FALSE(HasPendingUpdate(name));
+    if (HasPendingUpdate(name)) {
+      std::move(callback).Run(update_client::Error::UPDATE_IN_PROGRESS);
+      return;
+    }
 
     if (priority == OnDemandUpdater::Priority::BACKGROUND) {
       background_updates_.emplace(name, std::move(callback));
@@ -258,22 +259,33 @@ class CrOSComponentInstallerTest : public testing::Test {
   }
 
   // Creates a mock ComponentUpdateService. It sets the service up to expect a
-  // single registration request for the component |component_name|, and to
+  // |times| registration request for the component |component_name|, and to
   // redirect on-demand update requests to |updater|.
   std::unique_ptr<MockComponentUpdateService>
-  CreateUpdateServiceForSingleRegistration(const std::string& component_name,
-                                           TestUpdater* updater) {
+  CreateUpdateServiceForMultiRegistration(const std::string& component_name,
+                                          TestUpdater* updater,
+                                          int times) {
     auto service = std::make_unique<MockComponentUpdateService>();
     EXPECT_CALL(*service,
                 RegisterComponent(CrxComponentWithName(component_name)))
-        .Times(1)
-        .WillOnce(testing::Invoke(updater, &TestUpdater::RegisterComponent));
+        .Times(times)
+        .WillRepeatedly(
+            testing::Invoke(updater, &TestUpdater::RegisterComponent));
 
     EXPECT_CALL(*service, GetOnDemandUpdater())
         .WillRepeatedly(testing::ReturnRef(*updater));
     return service;
   }
 
+  // Creates a mock ComponentUpdateService. It sets the service up to expect a
+  // single registration request for the component |component_name|, and to
+  // redirect on-demand update requests to |updater|.
+  std::unique_ptr<MockComponentUpdateService>
+  CreateUpdateServiceForSingleRegistration(const std::string& component_name,
+                                           TestUpdater* updater) {
+    return CreateUpdateServiceForMultiRegistration(component_name, updater, 1);
+  }
+
   void RunUntilIdle() { task_environment_.RunUntilIdle(); }
 
   chromeos::FakeImageLoaderClient* image_loader_client() {
@@ -614,6 +626,64 @@ TEST_F(CrOSComponentInstallerTest, LoadNonInstalledComponent_DontForce_Mount) {
   EXPECT_EQ(base::FilePath(kTestComponentMountPath), mount_path);
 }
 
+TEST_F(CrOSComponentInstallerTest, LoadNonInstalledComponent_ForceTwice) {
+  image_loader_client()->SetMountPathForComponent(
+      kTestComponentName, base::FilePath(kTestComponentMountPath));
+
+  TestUpdater updater;
+  std::unique_ptr<MockComponentUpdateService> update_service =
+      CreateUpdateServiceForMultiRegistration(kTestComponentName, &updater, 2);
+  component_updater::CrOSComponentInstaller cros_component_manager(
+      nullptr, update_service.get());
+
+  base::Optional<CrOSComponentManager::Error> load_result1;
+  base::FilePath mount_path1;
+  cros_component_manager.Load(
+      kTestComponentName, CrOSComponentManager::MountPolicy::kMount,
+      CrOSComponentManager::UpdatePolicy::kForce,
+      base::BindOnce(&RecordLoadResult, &load_result1, &mount_path1));
+
+  base::Optional<CrOSComponentManager::Error> load_result2;
+  base::FilePath mount_path2;
+  cros_component_manager.Load(
+      kTestComponentName, CrOSComponentManager::MountPolicy::kMount,
+      CrOSComponentManager::UpdatePolicy::kForce,
+      base::BindOnce(&RecordLoadResult, &load_result2, &mount_path2));
+  RunUntilIdle();
+
+  base::Optional<base::FilePath> unpacked_path = CreateUnpackedComponent(
+      kTestComponentName, "2.0", kTestComponentValidMinEnvVersion);
+  ASSERT_TRUE(unpacked_path.has_value());
+  ASSERT_TRUE(updater.FinishForegroundUpdate(
+      kTestComponentName, update_client::Error::NONE, unpacked_path.value()));
+  RunUntilIdle();
+
+  EXPECT_FALSE(updater.HasPendingUpdate(kTestComponentName));
+
+  // Order of the load attempts is not deterministic, but one will have no error
+  // and a non-empty mount_path, the other will have error UPDATE_IN_PROGRESS
+  // and empty mount_path.
+  if (!mount_path1.empty()) {
+    VerifyComponentLoaded(cros_component_manager, kTestComponentName,
+                          load_result1,
+                          GetInstalledComponentPath(kTestComponentName, "2.0"));
+    EXPECT_EQ(base::FilePath(kTestComponentMountPath), mount_path1);
+    // Other load should have got a UPDATE_IN_PROGRESS error.
+    ASSERT_TRUE(load_result2.has_value());
+    EXPECT_EQ(load_result2.value(),
+              CrOSComponentManager::Error::UPDATE_IN_PROGRESS);
+  } else {
+    VerifyComponentLoaded(cros_component_manager, kTestComponentName,
+                          load_result2,
+                          GetInstalledComponentPath(kTestComponentName, "2.0"));
+    EXPECT_EQ(base::FilePath(kTestComponentMountPath), mount_path2);
+    // Other load should have got a UPDATE_IN_PROGRESS error.
+    ASSERT_TRUE(load_result1.has_value());
+    EXPECT_EQ(load_result1.value(),
+              CrOSComponentManager::Error::UPDATE_IN_PROGRESS);
+  }
+}
+
 TEST_F(CrOSComponentInstallerTest,
        LoadComponentWithInstallFail_DontForce_Mount) {
   image_loader_client()->SetMountPathForComponent(

chrome/browser/component_updater/cros_component_manager.h

@@ -28,6 +28,7 @@ class CrOSComponentManager {
     COMPATIBILITY_CHECK_FAILED = 4,  // Compatibility check failed.
     NOT_FOUND = 5,  // A component installation was not found - reported for
                     // load requests with kSkip update policy.
+    UPDATE_IN_PROGRESS = 6,  // Component update in progress.
     ERROR_MAX
   };
 

chrome/browser/extensions/api/media_perception_private/media_perception_api_delegate_chromeos.cc

@@ -51,6 +51,7 @@ GetComponentInstallationErrorForCrOSComponentManagerError(
       return api::media_perception_private::
           COMPONENT_INSTALLATION_ERROR_UNKNOWN_COMPONENT;
     case component_updater::CrOSComponentManager::Error::INSTALL_FAILURE:
+    case component_updater::CrOSComponentManager::Error::UPDATE_IN_PROGRESS:
       return api::media_perception_private::
           COMPONENT_INSTALLATION_ERROR_INSTALL_FAILURE;
     case component_updater::CrOSComponentManager::Error::MOUNT_FAILURE:

tools/metrics/histograms/enums.xml

@@ -11908,6 +11908,7 @@ Called by update_net_error_codes.py.-->
   <int value="3" label="MOUNT_FAILURE"/>
   <int value="4" label="COMPATIBILITY_CHECK_FAILED"/>
   <int value="5" label="NOT_FOUND"/>
+  <int value="6" label="UPDATE_IN_PROGRESS"/>
 </enum>
 
 <enum name="CrosDictationToggleDictationMethod">
@@ -12296,6 +12297,7 @@ Called by update_net_error_codes.py.-->
   <int value="45" label="CANCEL_UPGRADE_CONTAINER_FAILED"/>
   <int value="46" label="CONCIERGE_START_FAILED"/>
   <int value="47" label="CONTAINER_CONFIGURATION_FAILED"/>
+  <int value="48" label="LOAD_COMPONENT_UPDATE_IN_PROGRESS"/>
 </enum>
 
 <enum name="CrostiniSettingsEvent">