Adds some unittests for cros_component_installer_chromeos

The CL adds test coverage for CrOSComponentInstaller::Load method.
The tests set up fake components in a scoped test dir (containing
only manifest files) which is used to override component paths provided
by path service, and verify that Load requests load appropriate
component, issue on demand update requests as expected, and issue
requests to load the components to image loader service.

The focus in this cl is to add tests for behavior used for loading
demo-mode-resources component, but the set of tests can be expanded in
the future.

Change-Id: Ib4c2fab67492f1e62a3a1b3f1343139b9aa5baeb
Reviewed-on: https://chromium-review.googlesource.com/c/1278635Reviewed-by: Joshua Pawlicki <waffles@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>
Reviewed-by: Xiaochu Liu <xiaochu@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Toni Baržić <tbarzic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#601214}

chrome/browser/browser_process_platform_part_chromeos.cc

@@ -114,7 +114,8 @@ void BrowserProcessPlatformPart::InitializeCrosComponentManager() {
   cros_component_manager_ =
       std::make_unique<component_updater::CrOSComponentInstaller>(
           component_updater::MetadataTable::Create(
-              g_browser_process->local_state()));
+              g_browser_process->local_state()),
+          g_browser_process->component_updater());
 
   // Register all installed components for regular update.
   cros_component_manager_->RegisterInstalled();

chrome/browser/chromeos/dbus/component_updater_service_provider.cc

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

chrome/browser/component_updater/cros_component_installer_chromeos.cc

@@ -203,8 +203,10 @@ bool CrOSComponentInstallerPolicy::IsCompatible(
 }
 
 CrOSComponentInstaller::CrOSComponentInstaller(
-    std::unique_ptr<MetadataTable> metadata_table)
-    : metadata_table_(std::move(metadata_table)) {}
+    std::unique_ptr<MetadataTable> metadata_table,
+    ComponentUpdateService* component_updater)
+    : metadata_table_(std::move(metadata_table)),
+      component_updater_(component_updater) {}
 
 CrOSComponentInstaller::~CrOSComponentInstaller() {}
 
@@ -219,8 +221,7 @@ void CrOSComponentInstaller::Load(const std::string& name,
   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, update_policy, mount_policy, std::move(load_callback));
+    Install(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));
@@ -238,11 +239,10 @@ bool CrOSComponentInstaller::Unload(const std::string& name) {
     // Component |name| does not exist.
     return false;
   }
-  ComponentUpdateService* updater = g_browser_process->component_updater();
   const std::string id = GenerateId(config->sha2hash);
   metadata_table_->DeleteComponentForCurrentUser(name);
   return metadata_table_->HasComponentForAnyUser(name) ||
-         updater->UnregisterComponent(id);
+         component_updater_->UnregisterComponent(id);
 }
 
 void CrOSComponentInstaller::RegisterInstalled() {
@@ -281,16 +281,14 @@ bool CrOSComponentInstaller::IsRegistered(const std::string& name) const {
   return base::PathExists(root.Append(kComponentsRootPath).Append(name));
 }
 
-void CrOSComponentInstaller::Register(ComponentUpdateService* cus,
-                                      const ComponentConfig& config,
+void CrOSComponentInstaller::Register(const ComponentConfig& config,
                                       base::OnceClosure register_callback) {
   auto installer = base::MakeRefCounted<ComponentInstaller>(
       std::make_unique<CrOSComponentInstallerPolicy>(config, this));
-  installer->Register(cus, std::move(register_callback));
+  installer->Register(component_updater_, std::move(register_callback));
 }
 
-void CrOSComponentInstaller::Install(ComponentUpdateService* cus,
-                                     const std::string& name,
+void CrOSComponentInstaller::Install(const std::string& name,
                                      UpdatePolicy update_policy,
                                      MountPolicy mount_policy,
                                      LoadCallback load_callback) {
@@ -303,17 +301,16 @@ void CrOSComponentInstaller::Install(ComponentUpdateService* cus,
     return;
   }
 
-  Register(cus, *config,
+  Register(*config,
            base::BindOnce(
                &CrOSComponentInstaller::StartInstall, base::Unretained(this),
-               cus, name, GenerateId(config->sha2hash), update_policy,
+               name, GenerateId(config->sha2hash), update_policy,
                base::BindOnce(&CrOSComponentInstaller::FinishInstall,
                               base::Unretained(this), name, mount_policy,
-                              std::move(load_callback))));
+                              update_policy, std::move(load_callback))));
 }
 
 void CrOSComponentInstaller::StartInstall(
-    ComponentUpdateService* cus,
     const std::string& name,
     const std::string& id,
     UpdatePolicy update_policy,
@@ -332,12 +329,13 @@ void CrOSComponentInstaller::StartInstall(
   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));
+  component_updater_->GetOnDemandUpdater().OnDemandUpdate(
+      id, priority, std::move(install_callback));
 }
 
 void CrOSComponentInstaller::FinishInstall(const std::string& name,
                                            MountPolicy mount_policy,
+                                           UpdatePolicy update_policy,
                                            LoadCallback load_callback,
                                            update_client::Error error) {
   if (error != update_client::Error::NONE) {
@@ -345,6 +343,14 @@ void CrOSComponentInstaller::FinishInstall(const std::string& name,
         FROM_HERE,
         base::BindOnce(std::move(load_callback),
                        ReportError(Error::INSTALL_FAILURE), base::FilePath()));
+  } else if (!IsCompatible(name)) {
+    base::ThreadTaskRunnerHandle::Get()->PostTask(
+        FROM_HERE,
+        base::BindOnce(std::move(load_callback),
+                       ReportError(update_policy == UpdatePolicy::kSkip
+                                       ? Error::NOT_FOUND
+                                       : Error::COMPATIBILITY_CHECK_FAILED),
+                       base::FilePath()));
   } else if (mount_policy == MountPolicy::kMount) {
     LoadInternal(name, std::move(load_callback));
   } else {
@@ -357,8 +363,7 @@ void CrOSComponentInstaller::FinishInstall(const std::string& name,
 void CrOSComponentInstaller::LoadInternal(const std::string& name,
                                           LoadCallback load_callback) {
   const base::FilePath path = GetCompatiblePath(name);
-  // path is empty if no compatible component is available to load.
-  if (!path.empty()) {
+  DCHECK(!path.empty());
   chromeos::DBusThreadManager::Get()
       ->GetImageLoaderClient()
       ->LoadComponentAtPath(
@@ -366,13 +371,6 @@ void CrOSComponentInstaller::LoadInternal(const std::string& name,
           base::BindOnce(&CrOSComponentInstaller::FinishLoad,
                          base::Unretained(this), std::move(load_callback),
                          base::TimeTicks::Now(), name));
-  } else {
-    base::ThreadTaskRunnerHandle::Get()->PostTask(
-        FROM_HERE,
-        base::BindOnce(std::move(load_callback),
-                       ReportError(Error::COMPATIBILITY_CHECK_FAILED),
-                       base::FilePath()));
-  }
 }
 
 void CrOSComponentInstaller::FinishLoad(LoadCallback load_callback,
@@ -398,9 +396,8 @@ void CrOSComponentInstaller::FinishLoad(LoadCallback load_callback,
 void CrOSComponentInstaller::RegisterN(
     const std::vector<ComponentConfig>& configs) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-  ComponentUpdateService* updater = g_browser_process->component_updater();
   for (const auto& config : configs) {
-    Register(updater, config, base::OnceClosure());
+    Register(config, base::OnceClosure());
   }
 }
 

chrome/browser/component_updater/cros_component_installer_chromeos.h

@@ -76,8 +76,8 @@ class CrOSComponentInstallerPolicy : public ComponentInstallerPolicy {
 // This class contains functions used to register and install a component.
 class CrOSComponentInstaller : public CrOSComponentManager {
  public:
-  explicit CrOSComponentInstaller(
-      std::unique_ptr<MetadataTable> metadata_table);
+  CrOSComponentInstaller(std::unique_ptr<MetadataTable> metadata_table,
+                         ComponentUpdateService* component_updater);
   ~CrOSComponentInstaller() override;
 
   // CrOSComponentManager:
@@ -111,21 +111,18 @@ class CrOSComponentInstaller : public CrOSComponentManager {
   FRIEND_TEST_ALL_PREFIXES(CrOSComponentInstallerTest, CompatibleCrOSComponent);
 
   // Registers a component with a dedicated ComponentUpdateService instance.
-  void Register(ComponentUpdateService* cus,
-                const ComponentConfig& config,
+  void Register(const ComponentConfig& config,
                 base::OnceClosure register_callback);
 
   // Installs a component with a dedicated ComponentUpdateService instance.
-  void Install(ComponentUpdateService* cus,
-               const std::string& name,
+  void Install(const std::string& name,
                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,
+  void StartInstall(const std::string& name,
                     const std::string& id,
                     UpdatePolicy update_policy,
                     update_client::Callback install_callback);
@@ -133,6 +130,7 @@ class CrOSComponentInstaller : public CrOSComponentManager {
   // Calls LoadInternal to load the installed component.
   void FinishInstall(const std::string& name,
                      MountPolicy mount_policy,
+                     UpdatePolicy update_policy,
                      LoadCallback load_callback,
                      update_client::Error error);
 
@@ -162,6 +160,8 @@ class CrOSComponentInstaller : public CrOSComponentManager {
   // Table storing metadata (installs, usage, etc.).
   std::unique_ptr<MetadataTable> metadata_table_;
 
+  ComponentUpdateService* const component_updater_;
+
   DISALLOW_COPY_AND_ASSIGN(CrOSComponentInstaller);
 };
 

chrome/browser/component_updater/cros_component_manager.h

@@ -26,6 +26,8 @@ class CrOSComponentManager {
     INSTALL_FAILURE = 2,    // update_client fails to install component.
     MOUNT_FAILURE = 3,      // Component can not be mounted.
     COMPATIBILITY_CHECK_FAILED = 4,  // Compatibility check failed.
+    NOT_FOUND = 5,  // A component installation was not found - reported for
+                    // load requests with kSkip update policy.
     ERROR_MAX
   };
 

chrome/test/base/chrome_unit_test_suite.cc

@@ -121,7 +121,11 @@ void ChromeUnitTestSuite::InitializeProviders() {
   content::RegisterPathProvider();
   ui::RegisterPathProvider();
   component_updater::RegisterPathProvider(chrome::DIR_COMPONENTS,
+#if defined(OS_CHROMEOS)
+                                          chromeos::DIR_PREINSTALLED_COMPONENTS,
+#else
                                           chrome::DIR_INTERNAL_PLUGINS,
+#endif
                                           chrome::DIR_USER_DATA);
 
 #if defined(OS_CHROMEOS)

chromeos/dbus/fake_image_loader_client.cc

@@ -11,6 +11,7 @@
 #include "base/files/file_path.h"
 #include "base/location.h"
 #include "base/optional.h"
+#include "base/stl_util.h"
 #include "base/threading/thread_task_runner_handle.h"
 
 namespace chromeos {
@@ -25,12 +26,29 @@ void FakeImageLoaderClient::SetMountPathForComponent(
   mount_paths_[component_name] = mount_path;
 }
 
+bool FakeImageLoaderClient::IsLoaded(const std::string& name) const {
+  return base::ContainsKey(loaded_components_, name);
+}
+
+base::FilePath FakeImageLoaderClient::GetComponentInstallPath(
+    const std::string& name) const {
+  if (!IsLoaded(name))
+    return base::FilePath();
+
+  const auto it = component_install_paths_.find(name);
+  if (it == component_install_paths_.end())
+    return base::FilePath();
+  return it->second;
+}
+
 void FakeImageLoaderClient::RegisterComponent(
     const std::string& name,
     const std::string& version,
     const std::string& component_folder_abs_path,
     DBusMethodCallback<bool> callback) {
   registered_components_[name] = version;
+  component_install_paths_[name] =
+      base::FilePath(component_folder_abs_path).AppendASCII(version);
   base::ThreadTaskRunnerHandle::Get()->PostTask(
       FROM_HERE,
       base::BindOnce(std::move(callback), base::make_optional(true)));
@@ -53,6 +71,7 @@ void FakeImageLoaderClient::LoadComponent(
     return;
   }
 
+  loaded_components_.insert(name);
   base::ThreadTaskRunnerHandle::Get()->PostTask(
       FROM_HERE,
       base::BindOnce(
@@ -71,6 +90,10 @@ void FakeImageLoaderClient::LoadComponentAtPath(
         FROM_HERE, base::BindOnce(std::move(callback), base::nullopt));
     return;
   }
+
+  loaded_components_.insert(name);
+  component_install_paths_[name] = path;
+
   base::ThreadTaskRunnerHandle::Get()->PostTask(
       FROM_HERE, base::BindOnce(std::move(callback),
                                 base::make_optional(mount_path_it->second)));
@@ -79,6 +102,7 @@ void FakeImageLoaderClient::LoadComponentAtPath(
 void FakeImageLoaderClient::RemoveComponent(const std::string& name,
                                             DBusMethodCallback<bool> callback) {
   registered_components_.erase(name);
+  component_install_paths_.erase(name);
   UnmountComponent(name, std::move(callback));
 }
 
@@ -99,7 +123,8 @@ void FakeImageLoaderClient::RequestComponentVersion(
 void FakeImageLoaderClient::UnmountComponent(
     const std::string& name,
     DBusMethodCallback<bool> callback) {
-  mount_paths_.erase(name);
+  loaded_components_.erase(name);
+
   base::ThreadTaskRunnerHandle::Get()->PostTask(
       FROM_HERE,
       base::BindOnce(std::move(callback), base::make_optional(true)));

chromeos/dbus/fake_image_loader_client.h

@@ -6,6 +6,7 @@
 #define CHROMEOS_DBUS_FAKE_IMAGE_LOADER_CLIENT_H_
 
 #include <map>
+#include <set>
 #include <string>
 
 #include "base/macros.h"
@@ -27,6 +28,17 @@ class CHROMEOS_EXPORT FakeImageLoaderClient : public ImageLoaderClient {
   void SetMountPathForComponent(const std::string& component_name,
                                 const base::FilePath& mount_path);
 
+  // Returns whether the component with the specified name was loaded.
+  bool IsLoaded(const std::string& name) const;
+
+  // Returns the file path from which the specified component was loaded.
+  // For component loaded using LoadComponent this will be the component path
+  // registered for the component in RegisterComponent.
+  // For component loaded using LoadComponentAtPath, it will be the path passed
+  // into the load method.
+  // Returns empty file path if the component is not loaded at the time.
+  base::FilePath GetComponentInstallPath(const std::string& name) const;
+
   // DBusClient override.
   void Init(dbus::Bus* dbus) override {}
 
@@ -50,7 +62,7 @@ class CHROMEOS_EXPORT FakeImageLoaderClient : public ImageLoaderClient {
                         DBusMethodCallback<bool> callback) override;
 
  private:
-  // Maps registered component name to its registered varsion.
+  // Maps registered component name to its registered version.
   std::map<std::string, std::string> registered_components_;
 
   // Maps component names to paths to which they should be mounted.
@@ -58,6 +70,12 @@ class CHROMEOS_EXPORT FakeImageLoaderClient : public ImageLoaderClient {
   // called, and removed by a later call to UnmountComponent().
   std::map<std::string, base::FilePath> mount_paths_;
 
+  // Set of loaded components.
+  std::set<std::string> loaded_components_;
+
+  // Maps a loaded component to the path from which it was loaded.
+  std::map<std::string, base::FilePath> component_install_paths_;
+
   DISALLOW_COPY_AND_ASSIGN(FakeImageLoaderClient);
 };
 

tools/metrics/histograms/enums.xml

@@ -8941,6 +8941,7 @@ Called by update_net_error_codes.py.-->
   <int value="2" label="INSTALL_FAILURE"/>
   <int value="3" label="MOUNT_FAILURE"/>
   <int value="4" label="COMPATIBILITY_CHECK_FAILED"/>
+  <int value="5" label="NOT_FOUND"/>
 </enum>
 
 <enum name="CrosDictationToggleDictationMethod">