Implement ComplexFeature to support permission features with multiple rules.

chrome/common/extensions/features/complex_feature_provider.h

Index: chrome/common/extensions/features/complex_feature_provider.h
diff --git a/chrome/common/extensions/features/complex_feature_provider.h b/chrome/common/extensions/features/complex_feature_provider.h
new file mode 100644
index 0000000000000000000000000000000000000000..f47f71ba9233f3af80bf24700ea04dce4913db0a
--- /dev/null
+++ b/chrome/common/extensions/features/complex_feature_provider.h
@@ -0,0 +1,45 @@
+// Copyright (c) 2012 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.
+
+#ifndef CHROME_COMMON_EXTENSIONS_FEATURES_COMPLEX_FEATURE_PROVIDER_H_
+#define CHROME_COMMON_EXTENSIONS_FEATURES_COMPLEX_FEATURE_PROVIDER_H_
+
+#include <map>
+#include <set>
+#include <string>
+
+#include "base/memory/linked_ptr.h"
+#include "base/values.h"
+#include "chrome/common/extensions/features/complex_feature.h"
+#include "chrome/common/extensions/features/feature_provider.h"
+
+namespace extensions {
+
+// ComplexFeatureProvider is an extension of SimpleFeatureProvider and provides
+// support for ComplexFeatures (i.e. specifying multiple rules for
+// PermissionFeatures) in addition to supporting the existing functionality from

[not at google - send to devlin, 2012/12/12 17:42:41]

why only PermissionFeatures?

+// SimpleFeatureProvider.

[not at google - send to devlin, 2012/12/12 17:42:41]

"is an extension of SimpleFeatureProvider" -> well it doesn't actually extend
that so it's a slightly misleading comment, also it's an implementation detail.

I'd say more like, ComplexFeatureProvider provides support for ComplexFeatures
(i.e. specifying multiple rules for Features). It can also parse SimpleFeatures.

+class ComplexFeatureProvider : public FeatureProvider {
+ public:
+  explicit ComplexFeatureProvider(DictionaryValue* root);

[not at google - send to devlin, 2012/12/12 17:42:41]

Does this take ownership? In which case pass scoped_ptr<DictionaryValue>.
Otherwise pass const DictionaryValue&

[justinlin, 2012/12/14 12:26:26]

Done.

+  virtual ~ComplexFeatureProvider();
+
+  // Gets an instance for the _permission_features.json file that is baked into
+  // Chrome as a resource.
+  static ComplexFeatureProvider* GetPermissionFeatures();

[not at google - send to devlin, 2012/12/12 17:42:41]

hm I find it a bit confusing that users need to know to get the permission
features from here, and the other features from SimpleFeatureProvider. I guess
that's where all the comments about only supporting PermissionFeature comes
from.

Consider moving this method to a new class PermissionFeatureProvider with a
method GetInstance, likewise for ManifestFeatureProvider. They can just return
FeatureProvider instances where internally they construct
ComplexFeatureProviders.

I note that manifest.h refers to a ManifestFeatureProvider, perhaps this is what
aa had in mind.

[justinlin, 2012/12/14 12:26:26]

Yea, it gets kind of complicated with a bunch of classes. I moved everything
into SimpleFeatureProvider. That should be the unified way to get
Manifest/Permission features. It should probably be renamed though.

+
+  // Returns all features described by this instance.
+  std::set<std::string> GetAllFeatureNames() const;
+
+  // Gets the feature |feature_name|, if it exists.
+  virtual Feature* GetFeature(const std::string& feature_name) OVERRIDE;
+
+ private:
+  typedef std::map<std::string, linked_ptr<ComplexFeature> > FeatureMap;
+  FeatureMap features_;
+};
+
+}  // namespace extensions
+
+#endif  // CHROME_COMMON_EXTENSIONS_FEATURES_COMPLEX_FEATURE_PROVIDER_H_