Allow features to refer to subkeys in _manifest_features.json.

chrome/common/extensions/manifest.cc

Index: chrome/common/extensions/manifest.cc
diff --git a/chrome/common/extensions/manifest.cc b/chrome/common/extensions/manifest.cc
index 13f9e035442336f1bded25197123aad2a35d2cf6..29965d07cb3d14d6c36b2080119468515da3a280 100644
--- a/chrome/common/extensions/manifest.cc
+++ b/chrome/common/extensions/manifest.cc
@@ -25,32 +25,29 @@ Manifest::Manifest(Extension::Location location,
 Manifest::~Manifest() {
 }
 
-bool Manifest::ValidateManifest(string16* error) const {
-  for (DictionaryValue::key_iterator key = value_->begin_keys();
-       key != value_->end_keys(); ++key) {
-    Feature* feature =
-        SimpleFeatureProvider::GetManifestFeatures()->GetFeature(*key);
-    if (!feature) {
-      // When validating the extension manifests, we ignore keys that are not
-      // recognized for forward compatibility.
-      // TODO(aa): Consider having an error here in the case of strict error
-      // checking to let developers know when they screw up.
+void Manifest::ValidateManifest(std::vector<std::string>* warnings) const {
+  // Check every feature to see if its in the manifest. Note that this means
+  // we will ignore keys that are not features; we do this for forward
+  // compatibility.
+  // TODO(aa): Consider having an error here in the case of strict error
+  // checking to let developers know when they screw up.
+
+  std::set<std::string> feature_names =
+      SimpleFeatureProvider::GetManifestFeatures()->GetAllFeatureNames();
+  for (std::set<std::string>::iterator feature_name = feature_names.begin();
+       feature_name != feature_names.end(); ++feature_name) {
+    // Use Get instead of HasKey because the former uses path expansion.
+    if (!value_->Get(*feature_name, NULL))
       continue;
-    }
 
+    Feature* feature =
+        SimpleFeatureProvider::GetManifestFeatures()->GetFeature(*feature_name);
     Feature::Availability result = feature->IsAvailableToManifest(
         extension_id_, GetType(), Feature::ConvertLocation(location_),
         GetManifestVersion());
-    if (result != Feature::IS_AVAILABLE) {
-      *error = ExtensionErrorUtils::FormatErrorMessageUTF16(
-          errors::kFeatureNotAllowed,
-          *key,
-          feature->GetErrorMessage(result));
-      return false;
-    }
+    if (result != Feature::IS_AVAILABLE)
+      warnings->push_back(feature->GetErrorMessage(result));
   }
-
-  return true;
 }
 
 bool Manifest::HasKey(const std::string& key) const {
@@ -146,14 +143,21 @@ bool Manifest::IsHostedApp() const {
 bool Manifest::CanAccessPath(const std::string& path) const {
   std::vector<std::string> components;
   base::SplitString(path, '.', &components);
-  return CanAccessKey(components[0]);
+  std::string key;
+  for (size_t i = 0; i < components.size(); ++i) {
+    key += components[i];
+    if (!CanAccessKey(key))
+      return false;
+    key += '.';
+  }
+  return true;
 }
 
 bool Manifest::CanAccessKey(const std::string& key) const {
   Feature* feature =
       SimpleFeatureProvider::GetManifestFeatures()->GetFeature(key);
   if (!feature)
-    return false;
+    return true;

[Matt Perry, 2012/04/25 01:06:26]

This is technically a change in functionality (needed because not all subkeys
are features), but I think it shouldn't matter in practice, because we never
call Get() for non-features.

 
   return Feature::IS_AVAILABLE == feature->IsAvailableToManifest(
       extension_id_, GetType(), Feature::ConvertLocation(location_),