Add minimum version to extension management

chrome/browser/extensions/extension_management_unittest.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include <algorithm>
 #include <string>
 #include <vector>
 
 #include "base/json/json_parser.h"
 #include "base/memory/scoped_ptr.h"
@@ skipping 25 matching lines @@
 const char kExampleUpdateUrl[] = "http://example.com/update_url";
 
 const char kNonExistingExtension[] = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa";
 const char kNonExistingUpdateUrl[] = "http://example.net/update.xml";
 
 const char kExampleDictPreference[] =
     "{"
     "  \"abcdefghijklmnopabcdefghijklmnop\": {"  // kTargetExtension
     "    \"installation_mode\": \"allowed\","
     "    \"blocked_permissions\": [\"fileSystem\", \"bookmarks\"],"
+    "    \"minimum_version_required\": \"1.1.0\","
     "  },"
     "  \"bcdefghijklmnopabcdefghijklmnopa\": {"  // kTargetExtension2
     "    \"installation_mode\": \"force_installed\","
     "    \"update_url\": \"http://example.com/update_url\","
     "    \"allowed_permissions\": [\"fileSystem\", \"bookmarks\"],"
     "  },"
     "  \"cdefghijklmnopabcdefghijklmnopab\": {"  // kTargetExtension3
     "    \"installation_mode\": \"normal_installed\","
     "    \"update_url\": \"http://example.com/update_url\","
     "    \"allowed_permissions\": [\"fileSystem\", \"downloads\"],"
@@ skipping 99 matching lines @@
         &error_msg));
     ASSERT_TRUE(parsed && parsed->IsType(base::Value::TYPE_DICTIONARY))
         << error_msg;
     SetPref(true, pref_names::kExtensionManagement, parsed.release());
   }
 
   ExtensionManagement::InstallationMode GetInstallationMode(
       const std::string& id,
       const std::string& update_url) {
     scoped_refptr<const Extension> extension =
-        CreateExtensionWithIdAndUpdateUrl(Manifest::UNPACKED, id, update_url);
+        CreateExtension(Manifest::UNPACKED, "0.1", id, update_url);
     return extension_management_->GetInstallationMode(extension.get());
   }
 
   APIPermissionSet GetBlockedAPIPermissions(const std::string& id,
                                             const std::string& update_url) {
     scoped_refptr<const Extension> extension =
-        CreateExtensionWithIdAndUpdateUrl(Manifest::UNPACKED, id, update_url);
+        CreateExtension(Manifest::UNPACKED, "0.1", id, update_url);
     return extension_management_->GetBlockedAPIPermissions(extension.get());
   }
 
+  bool CheckMinimumVersion(const std::string& id, const std::string& version) {

[Finnur, 2014/11/19 10:16:25]

s/CheckMinimumVersion/IsAboveMinimumVersion ?

It would be good to document the params here. At a glance it would seem you are
checking if extension with |id| has |version| as the minimum version, but that's
not the case.

[binjin, 2014/11/19 13:36:11]

Done. See comments in extension_management.h.

Also added comments stating these are just wrappers forfunctions from
ExtensionManagement

[Finnur, 2014/11/19 15:47:38]

I think you missed the point on the comment. You mention in the comment that
this function is a wrapper around CheckMinimumVersion in extension_management,
yet the wrapper has a different type for |version| and uses it in a different
way than the function it wraps. That is what should be clarified with a comment,
that |version| is the version the constructed extension will be

[binjin, 2014/11/19 17:21:41]

Oh, I see. Added comments for every wrapper.

+    scoped_refptr<const Extension> extension =
+        CreateExtension(Manifest::UNPACKED, version, id, kNonExistingUpdateUrl);
+    std::string minimum_version_required;
+    bool ret = extension_management_->CheckMinimumVersion(
+        extension.get(), &minimum_version_required);
+    EXPECT_EQ(ret, minimum_version_required.empty());
+    EXPECT_EQ(ret, extension_management_->CheckMinimumVersion(extension.get(),
+                                                              nullptr));
+    return ret;
+  }
+
  protected:
   scoped_ptr<TestingPrefServiceSimple> pref_service_;
   scoped_ptr<ExtensionManagement> extension_management_;
 
  private:
-  // Create an extension with specified |location|, |id| and |update_url|.
-  scoped_refptr<const Extension> CreateExtensionWithIdAndUpdateUrl(
+  // Create an extension with specified |location|, |version|, |id| and
+  // |update_url|.
+  scoped_refptr<const Extension> CreateExtension(
       Manifest::Location location,
+      const std::string& version,
       const std::string& id,
       const std::string& update_url) {
     base::DictionaryValue manifest_dict;
     manifest_dict.SetString(manifest_keys::kName, "test");
-    manifest_dict.SetString(manifest_keys::kVersion, "0.1");
+    manifest_dict.SetString(manifest_keys::kVersion, version);
     manifest_dict.SetString(manifest_keys::kUpdateURL, update_url);
     std::string error;
     scoped_refptr<const Extension> extension =
         Extension::Create(base::FilePath(), location, manifest_dict,
                           Extension::NO_FLAGS, id, &error);
     CHECK(extension.get()) << error;
     return extension;
   }
 };
 
@@ skipping 243 matching lines @@
   api_permission_set.insert(APIPermission::kFileSystem);
   api_permission_set.insert(APIPermission::kHistory);
   EXPECT_EQ(api_permission_set,
             GetBlockedAPIPermissionsById(kTargetExtension3));
 
   api_permission_set.clear();
   api_permission_set.insert(APIPermission::kFileSystem);
   api_permission_set.insert(APIPermission::kBookmark);
   EXPECT_EQ(api_permission_set,
             GetBlockedAPIPermissionsByUpdateUrl(kExampleUpdateUrl));
+
+  // Verifies minimum version settings.
+  EXPECT_FALSE(CheckMinimumVersion(kTargetExtension, "1.0.99"));
+  EXPECT_TRUE(CheckMinimumVersion(kTargetExtension, "1.1"));
+  EXPECT_TRUE(CheckMinimumVersion(kTargetExtension, "1.1.0.1"));
 }
 
 // Tests the handling of installation mode in case it's specified in both
 // per-extension and per-update-url settings.
 TEST_F(ExtensionManagementServiceTest, InstallationModeConflictHandling) {
   SetExampleDictPref();
 
   // Per-extension installation mode settings should always override
   // per-update-url settings.
   EXPECT_EQ(GetInstallationMode(kTargetExtension, kExampleUpdateUrl),
@@ skipping 28 matching lines @@
   EXPECT_EQ(api_permission_set,
             GetBlockedAPIPermissions(kTargetExtension2, kExampleUpdateUrl));
 
   api_permission_set = blocked_permissions_for_update_url;
   api_permission_set.insert(APIPermission::kFileSystem);
   api_permission_set.insert(APIPermission::kHistory);
   EXPECT_EQ(api_permission_set,
             GetBlockedAPIPermissions(kTargetExtension3, kExampleUpdateUrl));
 }
 
+// Tests the 'minimum_version_required' settings of extension management.
+TEST_F(ExtensionManagementServiceTest, kMinimumVersionRequired) {
+  EXPECT_TRUE(CheckMinimumVersion(kTargetExtension, "0.0"));
+  EXPECT_TRUE(CheckMinimumVersion(kTargetExtension, "3.0.0"));
+  EXPECT_TRUE(CheckMinimumVersion(kTargetExtension, "9999.0"));
+
+  {
+    PrefUpdater pref(pref_service_.get());
+    pref.SetMinimumVersionRequired(kTargetExtension, "3.0");
+  }
+
+  EXPECT_FALSE(CheckMinimumVersion(kTargetExtension, "0.0"));
+  EXPECT_FALSE(CheckMinimumVersion(kTargetExtension, "2.99"));
+  EXPECT_TRUE(CheckMinimumVersion(kTargetExtension, "3.0.0"));
+  EXPECT_TRUE(CheckMinimumVersion(kTargetExtension, "3.0.1"));
+  EXPECT_TRUE(CheckMinimumVersion(kTargetExtension, "4.0"));
+}
+
 // Tests functionality of new preference as to deprecate legacy
 // ExtensionInstallSources policy.
 TEST_F(ExtensionManagementServiceTest, NewInstallSources) {
   // Set the legacy preference, and verifies that it works.
   base::ListValue allowed_sites_pref;
   allowed_sites_pref.AppendString("https://www.example.com/foo");
   SetPref(
       true, pref_names::kAllowedInstallSites, allowed_sites_pref.DeepCopy());
   EXPECT_TRUE(ReadGlobalSettings()->has_restricted_install_sources);
   EXPECT_TRUE(ReadGlobalSettings()->install_sources.MatchesURL(
@@ skipping 321 matching lines @@
   EXPECT_FALSE(error.empty());
 
   CreateExtension(Manifest::INTERNAL);
   error.clear();
   EXPECT_FALSE(MustRemainEnabled(extension_.get(), NULL));
   EXPECT_FALSE(MustRemainEnabled(extension_.get(), &error));
   EXPECT_TRUE(error.empty());
 }
 
 }  // namespace extensions