Implement ComplexFeature to support permission features with multiple rules.

chrome/common/extensions/features/simple_feature_provider.cc

Index: chrome/common/extensions/features/simple_feature_provider.cc
diff --git a/chrome/common/extensions/features/simple_feature_provider.cc b/chrome/common/extensions/features/simple_feature_provider.cc
index 4edcfe11c19544ae346e296bf3014334dd142e34..362681da9fe545373c7bc4ed8cf81b1f780fa9d8 100644
--- a/chrome/common/extensions/features/simple_feature_provider.cc
+++ b/chrome/common/extensions/features/simple_feature_provider.cc
@@ -6,6 +6,7 @@
 
 #include "base/json/json_reader.h"
 #include "base/lazy_instance.h"
+#include "chrome/common/extensions/features/complex_feature.h"
 #include "chrome/common/extensions/features/manifest_feature.h"
 #include "chrome/common/extensions/features/permission_feature.h"
 #include "grit/common_resources.h"
@@ -16,7 +17,7 @@ namespace extensions {
 namespace {
 
 template<class FeatureClass>
-Feature* CreateFeature() {
+SimpleFeature* CreateFeature() {
   return new FeatureClass();
 }
 
@@ -54,40 +55,84 @@ struct Static {
     scoped_ptr<DictionaryValue> dictionary_value(
         static_cast<DictionaryValue*>(value));
     return scoped_ptr<SimpleFeatureProvider>(
-        new SimpleFeatureProvider(dictionary_value.get(), factory));
+        new SimpleFeatureProvider(*dictionary_value, factory));
   }
 };
 
-base::LazyInstance<Static> g_static = LAZY_INSTANCE_INITIALIZER;
-
-}  // namespace
-
-SimpleFeatureProvider::SimpleFeatureProvider(DictionaryValue* root,
-                                             FeatureFactory factory)
-    : factory_(factory ? factory :
-               static_cast<FeatureFactory>(&CreateFeature<Feature>)) {
-  for (DictionaryValue::Iterator iter(*root); iter.HasNext(); iter.Advance()) {
-    if (iter.value().GetType() != Value::TYPE_DICTIONARY) {
-      LOG(ERROR) << iter.key() << ": Feature description must be dictionary.";
-      continue;
-    }
-
-    linked_ptr<Feature> feature((*factory_)());
-    feature->set_name(iter.key());
-    feature->Parse(static_cast<const DictionaryValue*>(&iter.value()));
+bool ParseFeature(const DictionaryValue* value,
+                  const std::string& name,
+                  SimpleFeature* feature) {
+    feature->set_name(name);
+    feature->Parse(value);
 
     if (feature->extension_types()->empty()) {
-      LOG(ERROR) << iter.key() << ": Simple features must specify atleast one "
+      LOG(ERROR) << name << ": Simple features must specify at least one "
                  << "value for extension_types.";
-      continue;
+      return false;
     }
 
     if (!feature->contexts()->empty()) {
-      LOG(ERROR) << iter.key() << ": Simple features do not support contexts.";
-      continue;
+      LOG(ERROR) << name << ": Simple features do not support contexts.";
+      return false;

[not at google - send to devlin, 2012/12/14 19:10:42]

if these DCHECKed then this method can't fail, and logic gets simpler

[justinlin, 2012/12/14 21:07:52]

Done. Oops, had to undo. Tests do things like this.. so we have to skip.

     }
 
-    features_[iter.key()] = feature;
+    return true;
+}
+
+base::LazyInstance<Static> g_static = LAZY_INSTANCE_INITIALIZER;
+
+}  // namespace
+
+SimpleFeatureProvider::SimpleFeatureProvider(const DictionaryValue& root,
+                                             FeatureFactory factory)
+    : factory_(factory ? factory :
+               static_cast<FeatureFactory>(&CreateFeature<SimpleFeature>)) {
+  for (DictionaryValue::Iterator iter(root); iter.HasNext(); iter.Advance()) {
+    if (iter.value().GetType() == Value::TYPE_DICTIONARY) {
+      linked_ptr<SimpleFeature> feature((*factory_)());
+
+      if (!ParseFeature(static_cast<const DictionaryValue*>(&iter.value()),
+                        iter.key(),
+                        feature.get()))
+        continue;
+
+      features_[iter.key()] = feature;
+    } else if (iter.value().GetType() == Value::TYPE_LIST) {
+      // This is a complex feature.
+      const ListValue* list = static_cast<const ListValue*>(&iter.value());
+      if (list->GetSize() == 0) {
+        LOG(ERROR) << iter.key() << ": Feature has no rules.";
+        continue;
+      }
+
+      scoped_ptr<ComplexFeature::FeatureList> features(
+          new ComplexFeature::FeatureList());
+
+      // Parse and add all SimpleFeatures from the list.
+      for (ListValue::const_iterator list_iter = list->begin();
+           list_iter != list->end(); ++list_iter) {
+        if ((*list_iter)->GetType() != Value::TYPE_DICTIONARY) {
+          LOG(ERROR) << iter.key() << ": Feature rules must be dictionaries.";
+          continue;
+        }
+
+        scoped_ptr<SimpleFeature> feature((*factory_)());
+        if (!ParseFeature(static_cast<const DictionaryValue*>(*list_iter),
+                          iter.key(),
+                          feature.get()))
+          continue;
+
+        features->push_back(feature.release());
+      }
+
+      linked_ptr<ComplexFeature> feature(new ComplexFeature(features.Pass()));
+      feature->set_name(iter.key());
+
+      features_[iter.key()] = feature;
+    } else {
+      LOG(ERROR) << iter.key() << ": Feature description must be dictionary or"
+                 << " list of dictionaries.";
+    }
   }
 }