Check env-version upon component load

chrome/browser/component_updater/cros_component_installer.h

Index: chrome/browser/component_updater/cros_component_installer.h
diff --git a/chrome/browser/component_updater/cros_component_installer.h b/chrome/browser/component_updater/cros_component_installer.h
index 7673d744f2d468add48964847aa5d17111e3694d..c3b3516529b917a956757bb01beb0fdbecb59286 100644
--- a/chrome/browser/component_updater/cros_component_installer.h
+++ b/chrome/browser/component_updater/cros_component_installer.h
@@ -13,6 +13,12 @@
 #include "components/update_client/update_client.h"
 #include "crypto/sha2.h"
 
+#define CONFIG_MAP_CONTENT        \
+  {{"epson-inkjet-printer-escpr", \
+    {{"env_version", "2.0"},      \
+     {"sha2hashstr",              \
+      "1913a5e0a6cad30b6f03e176177e0d7ed62c5d6700a9c66da556d7c3f5d6a47e"}}}};

[waffles, 2017/05/26 17:28:26]

Define this in the .cc, since that's the only place you need it.

[xiaochu, 2017/05/26 19:44:02]

Done.

+
 namespace component_updater {
 
 #if defined(OS_CHROMEOS)
@@ -34,6 +40,9 @@ class CrOSComponentInstallerTraits : public ComponentInstallerTraits {
   ~CrOSComponentInstallerTraits() override {}
 
  private:
+  FRIEND_TEST_ALL_PREFIXES(CrOSComponentInstallerTest, IsCompatibleOrNot);
+  FRIEND_TEST_ALL_PREFIXES(CrOSComponentInstallerTest,
+                           ComponentReadyIsCompatible);
   // The following methods override ComponentInstallerTraits.
   bool SupportsGroupPolicyEnabledComponentUpdates() const override;
   bool RequiresNetworkEncryption() const override;
@@ -50,6 +59,9 @@ class CrOSComponentInstallerTraits : public ComponentInstallerTraits {
   std::string GetName() const override;
   update_client::InstallerAttributes GetInstallerAttributes() const override;
   std::vector<std::string> GetMimeTypes() const override;
+
+  static bool IsCompatible(const std::string& env_version,
+                           const std::string& min_env_version);
   std::string name;
   std::string env_version;
   uint8_t kSha2Hash_[crypto::kSHA256Length] = {};
@@ -74,41 +86,40 @@ class CrOSComponent {
   //    // [component mount point: result]
   //  }
   //  void install_callback(update_client::Error error){
-  //    // switch(error){
-  //    //   case update_client::Error::NONE:
-  //    //     // component is installed
-  //    //     break;
-  //    //   case update_client::Error::INVALID_ARGUMENT:
-  //    //     // your install failed due to your wrong parameters.
-  //    //     break;
-  //    //   default:
-  //    //     // your install failed due to system failure.
-  //    //     break;
-  //    // }
+  //    // error code: 1) update_client::Error::NONE, success 2) otherwise,
+  //    // fail.
   //    // Even when error code other than NONE returned (your install failed),
   //    // component might has already being installed previously.
   //    // Plus, if you want to know current version of component.
   //    component_updater:CrOSComponent::LoadCrOSComponent(name, load_callback);

[waffles, 2017/05/26 17:28:26]

This API is confusing to me; if you always want to call LoadCrOSComponent
anyways, why doesn't InstallCrOSComponent do it for you?

[xiaochu, 2017/05/26 19:44:02]

It was originally designed the way you are speaking. But component developers
have concern about network latency (update check, I know it's small) and
availability. So I de-coupled them and give developers more control over what
they are doing...

   //  }
   //  ...
-  //    component_updater::CrOSComponent::InstallCrOSComponent(
-  //              name,
-  //              base::Bind(&install_callback));
+  //  component_updater::CrOSComponent::InstallCrOSComponent(
+  //            name,
+  //            base::Bind(&install_callback));
   //

[waffles, 2017/05/26 17:28:26]

For some reason I didn't notice this comment before. Can we remove it or move it
out of the class definition? The top of this file is a decent place to put usage
information.

[xiaochu, 2017/05/26 19:44:02]

Done.

-  static bool InstallCrOSComponent(
+  static void InstallCrOSComponent(
       const std::string& name,
       const update_client::Callback& install_callback);
 
   static void LoadCrOSComponent(
       const std::string& name,
-      const base::Callback<void(const std::string&)>& mount_callback);
+      const base::Callback<void(const std::string&)>& load_callback);
 
  private:
+  FRIEND_TEST_ALL_PREFIXES(CrOSComponentInstallerTest,
+                           RegisterComponentSuccess);
+  FRIEND_TEST_ALL_PREFIXES(CrOSComponentInstallerTest, RegisterComponentFail);
   CrOSComponent() {}
-  // Register a component.
+  // Register a component (called by InstallCrOSComponent).
   static void RegisterCrOSComponentInternal(ComponentUpdateService* cus,
                                             const ComponentConfig& config,
                                             const base::Closure& callback);
+  // Install a component (called by InstallCrOSComponent).
+  static void InstallCrOSComponent(
+      ComponentUpdateService* cus,
+      const std::string& name,
+      const update_client::Callback& install_callback);
   // A helper function to pass into RegisterCrOSComonentInternal as a callback.
   // It calls OnDemandUpdate to install the component right after being
   // registered.