avoid copy in component registration

Skip imageloader RegisterComponent (component copy) in OnCustomInstall
and loads the copy in chrome.

Additions:
Add LoadComponentAtPath in ImageLoaderClient.
Save the (compatible) component path in browser_process.

BUG=chromium:780575
TEST=unittest; install/load component successfully.

Change-Id: Ic14f7d33ee6c8971dd6326f39fda546c94959e14
Reviewed-on: https://chromium-review.googlesource.com/801535Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>
Reviewed-by: Joshua Pawlicki <waffles@chromium.org>
Commit-Queue: Xiaochu Liu <xiaochu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521622}

chrome/browser/browser_process_platform_part_chromeos.cc

@@ -193,16 +193,24 @@ void BrowserProcessPlatformPart::DestroySystemClock() {
   system_clock_.reset();
 }
 
-void BrowserProcessPlatformPart::AddCompatibleCrOSComponent(
-    const std::string& name) {
-  compatible_cros_components_.insert(name);
+void BrowserProcessPlatformPart::SetCompatibleCrosComponentPath(
+    const std::string& name,
+    const base::FilePath& path) {
+  compatible_cros_components_[name] = path;
 }
 
-bool BrowserProcessPlatformPart::IsCompatibleCrOSComponent(
-    const std::string& name) {
+bool BrowserProcessPlatformPart::IsCompatibleCrosComponent(
+    const std::string& name) const {
   return compatible_cros_components_.count(name) > 0;
 }
 
+base::FilePath BrowserProcessPlatformPart::GetCompatibleCrosComponentPath(
+    const std::string& name) const {
+  const auto it = compatible_cros_components_.find(name);
+  return it == compatible_cros_components_.end() ? base::FilePath()
+                                                 : it->second;
+}
+
 ui::InputDeviceControllerClient*
 BrowserProcessPlatformPart::GetInputDeviceControllerClient() {
   if (!input_device_controller_client_) {

chrome/browser/browser_process_platform_part_chromeos.h

@@ -9,7 +9,7 @@
 #include <string>
 
 #include "base/compiler_specific.h"
-#include "base/containers/flat_set.h"
+#include "base/containers/flat_map.h"
 #include "base/macros.h"
 #include "base/sequence_checker.h"
 #include "chrome/browser/browser_process_platform_part_base.h"
@@ -104,9 +104,17 @@ class BrowserProcessPlatformPart : public BrowserProcessPlatformPartBase {
   chromeos::system::SystemClock* GetSystemClock();
   void DestroySystemClock();
 
-  void AddCompatibleCrOSComponent(const std::string& name);
+  // Saves the name and install path of a compatible component.
+  void SetCompatibleCrosComponentPath(const std::string& name,
+                                      const base::FilePath& path);
 
-  bool IsCompatibleCrOSComponent(const std::string& name);
+  // Checks if the current installed component is compatible given a component
+  // |name|. If compatible, sets |path| to be its installed path.
+  bool IsCompatibleCrosComponent(const std::string& name) const;
+
+  // Returns installed path of a compatible component given |name|. Returns an
+  // empty path if the component isn't compatible.
+  base::FilePath GetCompatibleCrosComponentPath(const std::string& name) const;
 
   ui::InputDeviceControllerClient* GetInputDeviceControllerClient();
 
@@ -136,7 +144,8 @@ class BrowserProcessPlatformPart : public BrowserProcessPlatformPartBase {
 
   std::unique_ptr<ScopedKeepAlive> keep_alive_;
 
-  base::flat_set<std::string> compatible_cros_components_;
+  // Maps from a compatible component name to its installed path.
+  base::flat_map<std::string, base::FilePath> compatible_cros_components_;
 
 #if defined(USE_OZONE)
   std::unique_ptr<ui::InputDeviceControllerClient>

chrome/browser/component_updater/cros_component_installer.cc

@@ -52,32 +52,6 @@ namespace component_updater {
 
 using ConfigMap = std::map<std::string, std::map<std::string, std::string>>;
 
-void LogRegistrationResult(base::Optional<bool> result) {
-  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-  if (!result.has_value()) {
-    DVLOG(1) << "Call to imageloader service failed.";
-    return;
-  }
-  if (!result.value()) {
-    DVLOG(1) << "Component registration failed";
-    return;
-  }
-}
-
-void ImageLoaderRegistration(const std::string& version,
-                             const base::FilePath& install_dir,
-                             const std::string& name) {
-  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-  chromeos::ImageLoaderClient* loader =
-      chromeos::DBusThreadManager::Get()->GetImageLoaderClient();
-  if (loader) {
-    loader->RegisterComponent(name, version, install_dir.value(),
-                              base::BindOnce(&LogRegistrationResult));
-  } else {
-    DVLOG(1) << "Failed to get ImageLoaderClient object.";
-  }
-}
-
 ComponentConfig::ComponentConfig(const std::string& name,
                                  const std::string& env_version,
                                  const std::string& sha2hashstr)
@@ -109,23 +83,10 @@ update_client::CrxInstaller::Result
 CrOSComponentInstallerPolicy::OnCustomInstall(
     const base::DictionaryValue& manifest,
     const base::FilePath& install_dir) {
-  std::string version;
-  if (!manifest.GetString("version", &version)) {
-    return ToInstallerResult(update_client::InstallError::GENERIC_ERROR);
-  }
-  BrowserThread::PostTask(
-      BrowserThread::UI, FROM_HERE,
-      base::BindOnce(&ImageLoaderRegistration, version, install_dir, name));
   return update_client::CrxInstaller::Result(update_client::InstallError::NONE);
 }
 
 void CrOSComponentInstallerPolicy::OnCustomUninstall() {
-  chromeos::ImageLoaderClient* loader =
-      chromeos::DBusThreadManager::Get()->GetImageLoaderClient();
-  if (loader) {
-    loader->RemoveComponent(
-        name, base::BindOnce(base::Callback<void(base::Optional<bool>)>()));
-  }
 }
 
 void CrOSComponentInstallerPolicy::ComponentReady(
@@ -135,7 +96,8 @@ void CrOSComponentInstallerPolicy::ComponentReady(
   std::string min_env_version;
   if (manifest && manifest->GetString("min_env_version", &min_env_version)) {
     if (IsCompatible(env_version, min_env_version)) {
-      g_browser_process->platform_part()->AddCompatibleCrOSComponent(GetName());
+      g_browser_process->platform_part()->SetCompatibleCrosComponentPath(
+          GetName(), path);
     }
   }
 }
@@ -194,15 +156,22 @@ static void LoadResult(
 static void LoadComponentInternal(
     const std::string& name,
     base::OnceCallback<void(const std::string&)> load_callback) {
-  DCHECK(g_browser_process->platform_part()->IsCompatibleCrOSComponent(name));
+  DCHECK(g_browser_process->platform_part()->IsCompatibleCrosComponent(name));
   chromeos::ImageLoaderClient* loader =
       chromeos::DBusThreadManager::Get()->GetImageLoaderClient();
   if (loader) {
-    loader->LoadComponent(
-        name, base::BindOnce(&LoadResult, std::move(load_callback)));
-  } else {
-    base::PostTask(FROM_HERE, base::BindOnce(std::move(load_callback), ""));
+    base::FilePath path;
+    path = g_browser_process->platform_part()->GetCompatibleCrosComponentPath(
+        name);
+    // path is empty if no compatible component is available to load.
+    if (!path.empty()) {
+      loader->LoadComponentAtPath(
+          name, path, base::BindOnce(&LoadResult, std::move(load_callback)));
+      return;
+    }
   }
+  base::PostTask(FROM_HERE,
+                 base::BindOnce(std::move(load_callback), std::string()));
 }
 
 // It calls LoadComponentInternal to load the installed component.
@@ -239,7 +208,8 @@ void CrOSComponent::InstallComponent(
   const ConfigMap components = CONFIG_MAP_CONTENT;
   const auto it = components.find(name);
   if (name.empty() || it == components.end()) {
-    base::PostTask(FROM_HERE, base::BindOnce(std::move(load_callback), ""));
+    base::PostTask(FROM_HERE,
+                   base::BindOnce(std::move(load_callback), std::string()));
     return;
   }
   ComponentConfig config(it->first, it->second.find("env_version")->second,
@@ -256,7 +226,7 @@ void CrOSComponent::InstallComponent(
 void CrOSComponent::LoadComponent(
     const std::string& name,
     base::OnceCallback<void(const std::string&)> load_callback) {
-  if (!g_browser_process->platform_part()->IsCompatibleCrOSComponent(name)) {
+  if (!g_browser_process->platform_part()->IsCompatibleCrosComponent(name)) {
     // A compatible component is not installed, start installation process.
     auto* const cus = g_browser_process->component_updater();
     InstallComponent(cus, name, std::move(load_callback));

chrome/browser/component_updater/cros_component_installer_unittest.cc

@@ -57,10 +57,16 @@ class MockCrOSComponentInstallerPolicy : public CrOSComponentInstallerPolicy {
 void load_callback(const std::string& result) {}
 
 TEST_F(CrOSComponentInstallerTest, BPPPCompatibleCrOSComponent) {
+  const std::string kComponent = "a";
   BrowserProcessPlatformPart bppp;
-  ASSERT_EQ(bppp.IsCompatibleCrOSComponent("a"), false);
-  bppp.AddCompatibleCrOSComponent("a");
-  ASSERT_EQ(bppp.IsCompatibleCrOSComponent("a"), true);
+  EXPECT_FALSE(bppp.IsCompatibleCrosComponent(kComponent));
+  EXPECT_EQ(bppp.GetCompatibleCrosComponentPath(kComponent).value(),
+            std::string());
+
+  const base::FilePath kPath("/component/path/v0");
+  bppp.SetCompatibleCrosComponentPath(kComponent, kPath);
+  EXPECT_TRUE(bppp.IsCompatibleCrosComponent(kComponent));
+  EXPECT_EQ(bppp.GetCompatibleCrosComponentPath("a"), kPath);
 }
 
 TEST_F(CrOSComponentInstallerTest, ComponentReadyCorrectManifest) {

chromeos/dbus/fake_image_loader_client.cc

@@ -23,6 +23,12 @@ void FakeImageLoaderClient::LoadComponent(
     DBusMethodCallback<std::string> callback) {
   std::move(callback).Run(base::nullopt);
 }
+void FakeImageLoaderClient::LoadComponentAtPath(
+    const std::string& name,
+    const base::FilePath& path,
+    DBusMethodCallback<std::string> callback) {
+  std::move(callback).Run(base::nullopt);
+}
 
 void FakeImageLoaderClient::RemoveComponent(const std::string& name,
                                             DBusMethodCallback<bool> callback) {

chromeos/dbus/fake_image_loader_client.h

@@ -28,6 +28,9 @@ class CHROMEOS_EXPORT FakeImageLoaderClient : public ImageLoaderClient {
                          DBusMethodCallback<bool> callback) override;
   void LoadComponent(const std::string& name,
                      DBusMethodCallback<std::string> callback) override;
+  void LoadComponentAtPath(const std::string& name,
+                           const base::FilePath& path,
+                           DBusMethodCallback<std::string> callback) override;
   void RemoveComponent(const std::string& name,
                        DBusMethodCallback<bool> callback) override;
   void RequestComponentVersion(

chromeos/dbus/image_loader_client.cc

@@ -51,6 +51,19 @@ class ImageLoaderClientImpl : public ImageLoaderClient {
                                       std::move(callback)));
   }
 
+  void LoadComponentAtPath(const std::string& name,
+                           const base::FilePath& path,
+                           DBusMethodCallback<std::string> callback) override {
+    dbus::MethodCall method_call(imageloader::kImageLoaderServiceInterface,
+                                 imageloader::kLoadComponentAtPath);
+    dbus::MessageWriter writer(&method_call);
+    writer.AppendString(name);
+    writer.AppendString(path.value());
+    proxy_->CallMethod(&method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
+                       base::BindOnce(&ImageLoaderClientImpl::OnStringMethod,
+                                      std::move(callback)));
+  }
+
   void RemoveComponent(const std::string& name,
                        DBusMethodCallback<bool> callback) override {
     dbus::MethodCall method_call(imageloader::kImageLoaderServiceInterface,

chromeos/dbus/image_loader_client.h

@@ -8,6 +8,7 @@
 #include <string>
 
 #include "base/callback.h"
+#include "base/files/file_path.h"
 #include "base/macros.h"
 #include "chromeos/chromeos_export.h"
 #include "chromeos/dbus/dbus_client.h"
@@ -33,6 +34,13 @@ class CHROMEOS_EXPORT ImageLoaderClient : public DBusClient {
   virtual void LoadComponent(const std::string& name,
                              DBusMethodCallback<std::string> callback) = 0;
 
+  // Mounts a component given the |name| and install path |path|, then returns
+  // the mount point (if call is successful).
+  virtual void LoadComponentAtPath(
+      const std::string& name,
+      const base::FilePath& path,
+      DBusMethodCallback<std::string> callback) = 0;
+
   // Requests the currently registered version of the given component |name|.
   virtual void RequestComponentVersion(
       const std::string& name,