optionally mount image loading a component

Add a bool parameter |mount| in Load API. It allows developer to control
whether to mount the image or not after component is installed.

I also add an alias for LoadCallback to increase readability.

BUG=chromium:797140
TEST=call Load API with true/false.

Change-Id: I2322eb95b59ea82cd8f51556cea14c424504fad1
Reviewed-on: https://chromium-review.googlesource.com/855097
Reviewed-by: Joshua Pawlicki <waffles@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>
Reviewed-by: Toni Barzic <tbarzic@chromium.org>
Commit-Queue: Xiaochu Liu <xiaochu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#530248}

chrome/browser/chromeos/dbus/chrome_component_updater_service_provider_delegate.cc

@@ -16,9 +16,13 @@ ChromeComponentUpdaterServiceProviderDelegate::
 
 void ChromeComponentUpdaterServiceProviderDelegate::LoadComponent(
     const std::string& name,
-    base::OnceCallback<void(const base::FilePath&)> load_callback) {
+    bool mount,
+    LoadCallback load_callback) {
   g_browser_process->platform_part()->cros_component_manager()->Load(
-      name, std::move(load_callback));
+      name,
+      mount ? component_updater::CrOSComponentManager::MountPolicy::kMount
+            : component_updater::CrOSComponentManager::MountPolicy::kDontMount,
+      std::move(load_callback));
 }
 
 bool ChromeComponentUpdaterServiceProviderDelegate::UnloadComponent(

chrome/browser/chromeos/dbus/chrome_component_updater_service_provider_delegate.h

@@ -18,9 +18,9 @@ class ChromeComponentUpdaterServiceProviderDelegate
   ~ChromeComponentUpdaterServiceProviderDelegate() override;
 
   // ComponentUpdaterServiceProvider::Delegate:
-  void LoadComponent(
-      const std::string& name,
-      base::OnceCallback<void(const base::FilePath&)> load_callback) override;
+  void LoadComponent(const std::string& name,
+                     bool mount,
+                     LoadCallback load_callback) override;
   bool UnloadComponent(const std::string& name) override;
 
  private:

chrome/browser/chromeos/printing/printer_configurer.cc

@@ -215,6 +215,7 @@ class PrinterConfigurerImpl : public PrinterConfigurer {
         auto& component_name = *components_requested.begin();
         g_browser_process->platform_part()->cros_component_manager()->Load(
             component_name,
+            component_updater::CrOSComponentManager::MountPolicy::kMount,
             base::BindOnce(&PrinterConfigurerImpl::OnComponentLoad,
                            weak_factory_.GetWeakPtr(), printer, ppd_contents,
                            std::move(cb)));

chrome/browser/component_updater/cros_component_installer.cc

@@ -179,16 +179,20 @@ CrOSComponentManager::CrOSComponentManager() {}
 
 CrOSComponentManager::~CrOSComponentManager() {}
 
-void CrOSComponentManager::Load(
-    const std::string& name,
-    base::OnceCallback<void(const base::FilePath&)> load_callback) {
+void CrOSComponentManager::Load(const std::string& name,
+                                MountPolicy mount_policy,
+                                LoadCallback load_callback) {
   if (!IsCompatible(name)) {
     // A compatible component is not installed, start installation process.
     auto* const cus = g_browser_process->component_updater();
-    Install(cus, name, std::move(load_callback));
-  } else {
-    // A compatible component is intalled, load it directly.
+    Install(cus, name, 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));
+  } else {
+    // A compatible component is installed, do not load it.
+    base::PostTask(FROM_HERE,
+                   base::BindOnce(std::move(load_callback), base::FilePath()));
   }
 }
 
@@ -241,10 +245,10 @@ void CrOSComponentManager::Register(ComponentUpdateService* cus,
   installer->Register(cus, std::move(register_callback));
 }
 
-void CrOSComponentManager::Install(
-    ComponentUpdateService* cus,
-    const std::string& name,
-    base::OnceCallback<void(const base::FilePath&)> load_callback) {
+void CrOSComponentManager::Install(ComponentUpdateService* cus,
+                                   const std::string& name,
+                                   MountPolicy mount_policy,
+                                   LoadCallback load_callback) {
   const ConfigMap components = CONFIG_MAP_CONTENT;
   const auto it = components.find(name);
   if (it == components.end()) {
@@ -255,12 +259,12 @@ void CrOSComponentManager::Install(
   ComponentConfig config(it->first, it->second.find("env_version")->second,
                          it->second.find("sha2hashstr")->second);
   Register(cus, config,
-           base::BindOnce(&CrOSComponentManager::StartInstall,
-                          base::Unretained(this), cus,
-                          GenerateId(it->second.find("sha2hashstr")->second),
-                          base::BindOnce(&CrOSComponentManager::FinishInstall,
-                                         base::Unretained(this), name,
-                                         std::move(load_callback))));
+           base::BindOnce(
+               &CrOSComponentManager::StartInstall, base::Unretained(this), cus,
+               GenerateId(it->second.find("sha2hashstr")->second),
+               base::BindOnce(&CrOSComponentManager::FinishInstall,
+                              base::Unretained(this), name, mount_policy,
+                              std::move(load_callback))));
 }
 
 void CrOSComponentManager::StartInstall(
@@ -270,16 +274,20 @@ void CrOSComponentManager::StartInstall(
   cus->GetOnDemandUpdater().OnDemandUpdate(id, std::move(install_callback));
 }
 
-void CrOSComponentManager::FinishInstall(
-    const std::string& name,
-    base::OnceCallback<void(const base::FilePath&)> load_callback,
-    update_client::Error error) {
-  LoadInternal(name, std::move(load_callback));
+void CrOSComponentManager::FinishInstall(const std::string& name,
+                                         MountPolicy mount_policy,
+                                         LoadCallback load_callback,
+                                         update_client::Error error) {
+  if (mount_policy == MountPolicy::kMount) {
+    LoadInternal(name, std::move(load_callback));
+  } else {
+    base::PostTask(FROM_HERE,
+                   base::BindOnce(std::move(load_callback), base::FilePath()));
+  }
 }
 
-void CrOSComponentManager::LoadInternal(
-    const std::string& name,
-    base::OnceCallback<void(const base::FilePath&)> load_callback) {
+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.
@@ -296,9 +304,8 @@ void CrOSComponentManager::LoadInternal(
   }
 }
 
-void CrOSComponentManager::FinishLoad(
-    base::OnceCallback<void(const base::FilePath&)> load_callback,
-    base::Optional<base::FilePath> result) {
+void CrOSComponentManager::FinishLoad(LoadCallback load_callback,
+                                      base::Optional<base::FilePath> result) {
   PostTask(FROM_HERE, base::BindOnce(std::move(load_callback),
                                      result.value_or(base::FilePath())));
 }

chrome/browser/component_updater/cros_component_installer.h

@@ -69,12 +69,19 @@ class CrOSComponentInstallerPolicy : public ComponentInstallerPolicy {
 // This class contains functions used to register and install a component.
 class CrOSComponentManager {
  public:
+  using LoadCallback = base::OnceCallback<void(const base::FilePath&)>;
+  enum class MountPolicy {
+    kMount,
+    kDontMount,
+  };
+
   CrOSComponentManager();
   ~CrOSComponentManager();
   // Installs a component and keeps it up-to-date. |load_callback| returns the
   // mount point path.
   void Load(const std::string& name,
-            base::OnceCallback<void(const base::FilePath&)> load_callback);
+            MountPolicy mount_policy,
+            LoadCallback load_callback);
 
   // Stops updating and removes a component.
   // Returns true if the component was successfully unloaded
@@ -110,7 +117,8 @@ class CrOSComponentManager {
   // Installs a component with a dedicated ComponentUpdateService instance.
   void Install(ComponentUpdateService* cus,
                const std::string& name,
-               base::OnceCallback<void(const base::FilePath&)> load_callback);
+               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.
@@ -119,19 +127,17 @@ class CrOSComponentManager {
                     update_client::Callback install_callback);
 
   // Calls LoadInternal to load the installed component.
-  void FinishInstall(
-      const std::string& name,
-      base::OnceCallback<void(const base::FilePath&)> load_callback,
-      update_client::Error error);
+  void FinishInstall(const std::string& name,
+                     MountPolicy mount_policy,
+                     LoadCallback load_callback,
+                     update_client::Error error);
 
   // Internal function to load a component.
-  void LoadInternal(
-      const std::string& name,
-      base::OnceCallback<void(const base::FilePath&)> load_callback);
+  void LoadInternal(const std::string& name, LoadCallback load_callback);
 
   // Calls load_callback and pass in the parameter |result| (component mount
   // point).
-  void FinishLoad(base::OnceCallback<void(const base::FilePath&)> load_callback,
+  void FinishLoad(LoadCallback load_callback,
                   base::Optional<base::FilePath> result);
 
   // Returns all installed components.

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

@@ -41,7 +41,9 @@ void MediaPerceptionAPIDelegateChromeOS::LoadCrOSComponent(
     const media_perception::ComponentType& type,
     LoadCrOSComponentCallback load_callback) {
   g_browser_process->platform_part()->cros_component_manager()->Load(
-      GetComponentNameForComponentType(type), std::move(load_callback));
+      GetComponentNameForComponentType(type),
+      component_updater::CrOSComponentManager::MountPolicy::kMount,
+      std::move(load_callback));
 }
 
 }  // namespace extensions

chromeos/dbus/services/component_updater_service_provider.cc

@@ -58,17 +58,20 @@ void ComponentUpdaterServiceProvider::LoadComponent(
     dbus::ExportedObject::ResponseSender response_sender) {
   dbus::MessageReader reader(method_call);
   std::string component_name;
+  // |mount| is an optional parameter, and by default is true.
+  bool mount = true;
   if (reader.PopString(&component_name)) {
+    reader.PopBool(&mount);
     delegate_->LoadComponent(
-        component_name,
+        component_name, mount,
         base::Bind(&ComponentUpdaterServiceProvider::OnLoadComponent,
                    weak_ptr_factory_.GetWeakPtr(), method_call,
                    response_sender));
   } else {
     std::unique_ptr<dbus::ErrorResponse> error_response =
         dbus::ErrorResponse::FromMethodCall(
             method_call, kErrorInvalidArgs,
-            "Missing component name string argument.");
+            "Need a string and a boolean parameter.");
     response_sender.Run(std::move(error_response));
   }
 }
@@ -77,18 +80,11 @@ void ComponentUpdaterServiceProvider::OnLoadComponent(
     dbus::MethodCall* method_call,
     dbus::ExportedObject::ResponseSender response_sender,
     const base::FilePath& result) {
-  if (!result.empty()) {
-    std::unique_ptr<dbus::Response> response =
-        dbus::Response::FromMethodCall(method_call);
-    dbus::MessageWriter writer(response.get());
-    writer.AppendString(result.value());
-    response_sender.Run(std::move(response));
-  } else {
-    std::unique_ptr<dbus::ErrorResponse> error_response =
-        dbus::ErrorResponse::FromMethodCall(method_call, kErrorInternalError,
-                                            "Failed to load component");
-    response_sender.Run(std::move(error_response));
-  }
+  std::unique_ptr<dbus::Response> response =
+      dbus::Response::FromMethodCall(method_call);
+  dbus::MessageWriter writer(response.get());
+  writer.AppendString(result.value());
+  response_sender.Run(std::move(response));
 }
 
 void ComponentUpdaterServiceProvider::UnloadComponent(

chromeos/dbus/services/component_updater_service_provider.h

@@ -30,7 +30,7 @@ namespace chromeos {
 //     --dest=org.chromium.ComponentUpdaterService
 //     /org/chromium/ComponentUpdaterService
 //     org.chromium.ComponentUpdaterService.LoadComponent
-//     "string:|component name|"
+//     "string:|component name|" "boolean:|mount|"
 //
 // % string "/run/imageloader/|component name|/|version|"
 //
@@ -49,12 +49,14 @@ class CHROMEOS_EXPORT ComponentUpdaterServiceProvider
   // ComponentUpdaterServiceProvider.
   class Delegate {
    public:
+    using LoadCallback = base::OnceCallback<void(const base::FilePath&)>;
+
     Delegate() {}
     virtual ~Delegate() {}
 
-    virtual void LoadComponent(
-        const std::string& name,
-        base::OnceCallback<void(const base::FilePath&)> load_callback) = 0;
+    virtual void LoadComponent(const std::string& name,
+                               bool mount,
+                               LoadCallback load_callback) = 0;
 
     virtual bool UnloadComponent(const std::string& name) = 0;