Resolve feature dependencies within the Feature implementations

(SimpleFeature) rather than external to them (ExtensionAPI).
This fixes ComplexFeature dependency checking.

R=rockot@chromium.org

Review URL: https://codereview.chromium.org/308423002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@279224 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 112 insertions, 6 deletions

diff --git a/extensions/common/features/complex_feature_unittest.cc b/extensions/common/features/complex_feature_unittest.cc
index 0c3f2d3..bef056d 100644
--- a/extensions/common/features/complex_feature_unittest.cc
+++ b/extensions/common/features/complex_feature_unittest.cc
@@ -215,4 +215,56 @@ TEST_F(ExtensionComplexFeatureTest, NotBlockedInServiceWorker) {
   EXPECT_FALSE(ComplexFeature(features.Pass()).IsBlockedInServiceWorker());
 }
 
+// Tests that dependencies are correctly checked.
+TEST_F(ExtensionComplexFeatureTest, Dependencies) {
+  scoped_ptr<ComplexFeature::FeatureList> features(
+      new ComplexFeature::FeatureList());
+
+  // Rule which depends on an extension-only feature (omnibox).
+  scoped_ptr<SimpleFeature> simple_feature(CreateFeature());
+  scoped_ptr<base::DictionaryValue> rule =
+      DictionaryBuilder()
+          .Set("dependencies", ListBuilder().Append("manifest:omnibox"))
+          .Build();
+  simple_feature->Parse(rule.get());
+  features->push_back(simple_feature.release());
+
+  // Rule which depends on an platform-app-only feature (serial).
+  simple_feature.reset(CreateFeature());
+  rule = DictionaryBuilder()
+             .Set("dependencies", ListBuilder().Append("permission:serial"))
+             .Build();
+  simple_feature->Parse(rule.get());
+  features->push_back(simple_feature.release());
+
+  scoped_ptr<ComplexFeature> feature(new ComplexFeature(features.Pass()));
+
+  // Available to extensions because of the omnibox rule.
+  EXPECT_EQ(
+      Feature::IS_AVAILABLE,
+      feature->IsAvailableToManifest("extensionid",
+                                     Manifest::TYPE_EXTENSION,
+                                     Manifest::INVALID_LOCATION,
+                                     Feature::UNSPECIFIED_PLATFORM,
+                                     Feature::GetCurrentPlatform()).result());
+
+  // Available to platofrm apps because of the serial rule.
+  EXPECT_EQ(
+      Feature::IS_AVAILABLE,
+      feature->IsAvailableToManifest("platformappid",
+                                     Manifest::TYPE_PLATFORM_APP,
+                                     Manifest::INVALID_LOCATION,
+                                     Feature::UNSPECIFIED_PLATFORM,
+                                     Feature::GetCurrentPlatform()).result());
+
+  // Not available to hosted apps.
+  EXPECT_EQ(
+      Feature::INVALID_TYPE,
+      feature->IsAvailableToManifest("hostedappid",
+                                     Manifest::TYPE_HOSTED_APP,
+                                     Manifest::INVALID_LOCATION,
+                                     Feature::UNSPECIFIED_PLATFORM,
+                                     Feature::GetCurrentPlatform()).result());
+}
+
 }  // namespace
diff --git a/extensions/common/features/feature.h b/extensions/common/features/feature.h
index 0f0f35a..534a05f 100644
--- a/extensions/common/features/feature.h
+++ b/extensions/common/features/feature.h
@@ -103,7 +103,6 @@ class Feature {
 
   const std::string& name() const { return name_; }
   void set_name(const std::string& name) { name_ = name; }
-  const std::set<std::string>& dependencies() const { return dependencies_; }
   bool no_parent() const { return no_parent_; }
 
   // Gets the platform the code is currently running on.
@@ -157,7 +156,6 @@ class Feature {
 
  protected:
   std::string name_;
-  std::set<std::string> dependencies_;
   bool no_parent_;
 };
 
diff --git a/extensions/common/features/simple_feature.cc b/extensions/common/features/simple_feature.cc
index 21d4e22..762730b1f 100644
--- a/extensions/common/features/simple_feature.cc
+++ b/extensions/common/features/simple_feature.cc
@@ -7,6 +7,7 @@
 #include <map>
 #include <vector>
 
+#include "base/bind.h"
 #include "base/command_line.h"
 #include "base/debug/alias.h"
 #include "base/lazy_instance.h"
@@ -14,12 +15,33 @@
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
+#include "extensions/common/extension_api.h"
+#include "extensions/common/features/feature_provider.h"
 #include "extensions/common/switches.h"
 
 namespace extensions {
 
 namespace {
 
+Feature::Availability IsAvailableToManifestForBind(
+    const std::string& extension_id,
+    Manifest::Type type,
+    Manifest::Location location,
+    int manifest_version,
+    Feature::Platform platform,
+    const Feature* feature) {
+  return feature->IsAvailableToManifest(
+      extension_id, type, location, manifest_version, platform);
+}
+
+Feature::Availability IsAvailableToContextForBind(const Extension* extension,
+                                                  Feature::Context context,
+                                                  const GURL& url,
+                                                  Feature::Platform platform,
+                                                  const Feature* feature) {
+  return feature->IsAvailableToContext(extension, context, url, platform);
+}
+
 struct Mappings {
   Mappings() {
     extension_types["extension"] = Manifest::TYPE_EXTENSION;
@@ -234,6 +256,10 @@ SimpleFeature::SimpleFeature()
 
 SimpleFeature::~SimpleFeature() {}
 
+bool SimpleFeature::HasDependencies() {
+  return !dependencies_.empty();
+}
+
 void SimpleFeature::AddFilter(scoped_ptr<SimpleFeatureFilter> filter) {
   filters_.push_back(make_linked_ptr(filter.release()));
 }
@@ -343,7 +369,12 @@ Feature::Availability SimpleFeature::IsAvailableToManifest(
       return availability;
   }
 
-  return CreateAvailability(IS_AVAILABLE, type);
+  return CheckDependencies(base::Bind(&IsAvailableToManifestForBind,
+                                      extension_id,
+                                      type,
+                                      location,
+                                      manifest_version,
+                                      platform));
 }
 
 Feature::Availability SimpleFeature::IsAvailableToContext(
@@ -376,7 +407,8 @@ Feature::Availability SimpleFeature::IsAvailableToContext(
       return availability;
   }
 
-  return CreateAvailability(IS_AVAILABLE);
+  return CheckDependencies(base::Bind(
+      &IsAvailableToContextForBind, extension, context, url, platform));
 }
 
 std::string SimpleFeature::GetAvailabilityMessage(
@@ -522,4 +554,20 @@ bool SimpleFeature::MatchesManifestLocation(
   return false;
 }
 
+Feature::Availability SimpleFeature::CheckDependencies(
+    const base::Callback<Availability(const Feature*)>& checker) const {
+  for (std::set<std::string>::const_iterator it = dependencies_.begin();
+       it != dependencies_.end();
+       ++it) {
+    Feature* dependency =
+        ExtensionAPI::GetSharedInstance()->GetFeatureDependency(*it);
+    if (!dependency)
+      return CreateAvailability(NOT_PRESENT);
+    Availability dependency_availability = checker.Run(dependency);
+    if (!dependency_availability.is_available())
+      return dependency_availability;
+  }
+  return CreateAvailability(IS_AVAILABLE);
+}
+
 }  // namespace extensions
diff --git a/extensions/common/features/simple_feature.h b/extensions/common/features/simple_feature.h
index f81d62b4..34ae5b5 100644
--- a/extensions/common/features/simple_feature.h
+++ b/extensions/common/features/simple_feature.h
@@ -9,6 +9,7 @@
 #include <string>
 #include <vector>
 
+#include "base/callback_forward.h"
 #include "base/gtest_prod_util.h"
 #include "base/memory/linked_ptr.h"
 #include "base/memory/scoped_ptr.h"
@@ -20,8 +21,6 @@
 
 namespace extensions {
 
-class ComplexFeature;
-
 class SimpleFeature : public Feature {
  public:
   SimpleFeature();
@@ -57,6 +56,11 @@ class SimpleFeature : public Feature {
   std::set<std::string>* whitelist() { return &whitelist_; }
   std::set<Manifest::Type>* extension_types() { return &extension_types_; }
 
+  // Dependency resolution is a property of Features that is preferrably
+  // handled internally to avoid temptation, but FeatureFilters may need
+  // to know if there are any at all.
+  bool HasDependencies();
+
   // Adds a filter to this feature. The feature takes ownership of the filter.
   void AddFilter(scoped_ptr<SimpleFeatureFilter> filter);
 
@@ -122,12 +126,16 @@ class SimpleFeature : public Feature {
  private:
   bool MatchesManifestLocation(Manifest::Location manifest_location) const;
 
+  Availability CheckDependencies(
+      const base::Callback<Availability(const Feature*)>& checker) const;
+
   // For clarity and consistency, we handle the default value of each of these
   // members the same way: it matches everything. It is up to the higher level
   // code that reads Features out of static data to validate that data and set
   // sensible defaults.
   std::set<std::string> blacklist_;
   std::set<std::string> whitelist_;
+  std::set<std::string> dependencies_;
   std::set<Manifest::Type> extension_types_;
   std::set<Context> contexts_;
   URLPatternSet matches_;