Check env-version upon component load

chrome/browser/component_updater/cros_component_installer.cc

Index: chrome/browser/component_updater/cros_component_installer.cc
diff --git a/chrome/browser/component_updater/cros_component_installer.cc b/chrome/browser/component_updater/cros_component_installer.cc
index f2ed8cf6ca8871d65a050bcc410508145f14cb9b..c035fd4ce892efa93887f26330873903538dcad1 100644
--- a/chrome/browser/component_updater/cros_component_installer.cc
+++ b/chrome/browser/component_updater/cros_component_installer.cc
@@ -16,6 +16,12 @@
 #include "chromeos/dbus/image_loader_client.h"
 #endif  // defined(OS_CHROMEOS)
 
+#define CONFIG_MAP_CONTENT        \
+  {{"epson-inkjet-printer-escpr", \
+    {{"env_version", "2.1"},      \
+     {"sha2hashstr",              \
+      "1913a5e0a6cad30b6f03e176177e0d7ed62c5d6700a9c66da556d7c3f5d6a47e"}}}};
+
 using content::BrowserThread;
 
 namespace component_updater {
@@ -81,7 +87,6 @@ update_client::CrxInstaller::Result
 CrOSComponentInstallerTraits::OnCustomInstall(
     const base::DictionaryValue& manifest,
     const base::FilePath& install_dir) {
-  DVLOG(1) << "[CrOSComponentInstallerTraits::OnCustomInstall]";
   std::string version;
   if (!manifest.GetString("version", &version)) {
     return ToInstallerResult(update_client::InstallError::GENERIC_ERROR);
@@ -96,7 +101,12 @@ void CrOSComponentInstallerTraits::ComponentReady(
     const base::Version& version,
     const base::FilePath& path,
     std::unique_ptr<base::DictionaryValue> manifest) {
-  g_browser_process->platform_part()->AddCompatibleCrOSComponent(GetName());
+  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());
+    }
+  }
 }
 
 bool CrOSComponentInstallerTraits::VerifyInstallation(
@@ -129,49 +139,39 @@ std::vector<std::string> CrOSComponentInstallerTraits::GetMimeTypes() const {
   return mime_types;
 }
 
-void CrOSComponent::RegisterCrOSComponentInternal(
-    ComponentUpdateService* cus,
-    const ComponentConfig& config,
-    const base::Closure& installcallback) {
-  std::unique_ptr<ComponentInstallerTraits> traits(
-      new CrOSComponentInstallerTraits(config));
-  // |cus| will take ownership of |installer| during
-  // installer->Register(cus).
-  DefaultComponentInstaller* installer =
-      new DefaultComponentInstaller(std::move(traits));
-  installer->Register(cus, installcallback);
+bool CrOSComponentInstallerTraits::IsCompatible(
+    const std::string& env_version_str,
+    const std::string& min_env_version_str) {
+  // env_version format: [0-9]+.[0-9]+
+  base::Version env_version(env_version_str);
+  base::Version min_env_version(min_env_version_str);
+  if (env_version.components().size() == 2 &&
+      min_env_version.components().size() == 2 &&
+      env_version.components()[0] == min_env_version.components()[0] &&
+      env_version.components()[1] >= min_env_version.components()[1]) {

[waffles, 2017/05/30 22:12:17]

You can replace the == 2 check with .IsValid and the env[1] >= min_env[1] check
with env > min_env.

[xiaochu, 2017/05/31 15:40:11]

Done.

+    return true;
+  }
+  return false;

[waffles, 2017/05/30 22:12:17]

IMHO it's better to just return the value of the expression, even though it's a
multi-line expression.

[xiaochu, 2017/05/31 15:40:11]

Done.

 }
 
-void CrOSComponent::InstallChromeOSComponent(
-    ComponentUpdateService* cus,
-    const std::string& id,
+void CrOSComponent::InstallCrOSComponent(
+    const std::string& name,
     const update_client::Callback& install_callback) {
-  cus->GetOnDemandUpdater().OnDemandUpdate(id, install_callback);
+  auto* const cus = g_browser_process->component_updater();
+  InstallCrOSComponent(cus, name, install_callback);
 }
 
-bool CrOSComponent::InstallCrOSComponent(
+void CrOSComponent::InstallCrOSComponent(
+    ComponentUpdateService* cus,
     const std::string& name,
     const update_client::Callback& install_callback) {
-  auto* const cus = g_browser_process->component_updater();
-  const ConfigMap components = {
-      {"epson-inkjet-printer-escpr",
-       {{"env_version", "0.0"},
-        {"sha2hashstr",
-         "1913a5e0a6cad30b6f03e176177e0d7ed62c5d6700a9c66da556d7c3f5d6a47e"}}}};
-  if (name.empty()) {
-    base::PostTask(
-        FROM_HERE,
-        base::Bind(install_callback, update_client::Error::INVALID_ARGUMENT));
-    return false;
-  }
+  const ConfigMap components = CONFIG_MAP_CONTENT;
   const auto it = components.find(name);
-  if (it == components.end()) {
-    DVLOG(1) << "[RegisterCrOSComponents] component " << name
-             << " is not in configuration.";
+  if (name.empty() || it == components.end()) {
     base::PostTask(
         FROM_HERE,
         base::Bind(install_callback, update_client::Error::INVALID_ARGUMENT));
-    return false;
+    return;
   }
   ComponentConfig config(it->first, it->second.find("env_version")->second,
                          it->second.find("sha2hashstr")->second);
@@ -182,36 +182,52 @@ bool CrOSComponent::InstallCrOSComponent(
                      it->second.find("sha2hashstr")->second)
                      .substr(0, 32),
                  install_callback));
-  return true;
 }
 
-void MountResult(const base::Callback<void(const std::string&)>& mount_callback,
-                 chromeos::DBusMethodCallStatus call_status,
-                 const std::string& result) {
-  if (call_status != chromeos::DBUS_METHOD_CALL_SUCCESS) {
-    DVLOG(1) << "Call to imageloader service failed.";
-    base::PostTask(FROM_HERE, base::Bind(mount_callback, ""));
-    return;
-  }
-  if (result.empty()) {
-    DVLOG(1) << "Component load failed";
-    base::PostTask(FROM_HERE, base::Bind(mount_callback, ""));
-    return;
-  }
-  base::PostTask(FROM_HERE, base::Bind(mount_callback, result));
+void CrOSComponent::RegisterCrOSComponentInternal(
+    ComponentUpdateService* cus,
+    const ComponentConfig& config,
+    const base::Closure& installcallback) {
+  std::unique_ptr<ComponentInstallerTraits> traits(
+      new CrOSComponentInstallerTraits(config));
+  // |cus| will take ownership of |installer| during
+  // installer->Register(cus).
+  DefaultComponentInstaller* installer =
+      new DefaultComponentInstaller(std::move(traits));
+  installer->Register(cus, installcallback);
+}
+
+void CrOSComponent::InstallChromeOSComponent(
+    ComponentUpdateService* cus,
+    const std::string& id,
+    const update_client::Callback& install_callback) {
+  cus->GetOnDemandUpdater().OnDemandUpdate(id, install_callback);
+}
+
+void LoadResult(const base::Callback<void(const std::string&)>& load_callback,
+                chromeos::DBusMethodCallStatus call_status,
+                const std::string& result) {
+  PostTask(
+      FROM_HERE,
+      base::Bind(
+          load_callback,
+          call_status != chromeos::DBUS_METHOD_CALL_SUCCESS ? "" : result));
 }
 
 void CrOSComponent::LoadCrOSComponent(
     const std::string& name,
-    const base::Callback<void(const std::string&)>& mount_callback) {
+    const base::Callback<void(const std::string&)>& load_callback) {
+  if (!g_browser_process->platform_part()->IsCompatibleCrOSComponent(name))
+    return;
   chromeos::ImageLoaderClient* loader =
       chromeos::DBusThreadManager::Get()->GetImageLoaderClient();
   if (loader) {
-    loader->LoadComponent(name, base::Bind(&MountResult, mount_callback));
-  } else {
-    DVLOG(1) << "Failed to get ImageLoaderClient object.";
+    loader->LoadComponent(name, base::Bind(&LoadResult, load_callback));
+    return;
   }
+  base::PostTask(FROM_HERE, base::Bind(load_callback, ""));
 }
+
 #endif  // defined(OS_CHROMEOS
 
 }  // namespace component_updater