Convert tabs, windows, and extension APIs to feature system.

chrome/common/extensions/api/extension_api.cc

Index: chrome/common/extensions/api/extension_api.cc
diff --git a/chrome/common/extensions/api/extension_api.cc b/chrome/common/extensions/api/extension_api.cc
index 6853761d4dd78e81b5e3a1153a498b35c73a6c6d..7ef84c41a6c0f8e98556c4be485d00bba1a23f55 100644
--- a/chrome/common/extensions/api/extension_api.cc
+++ b/chrome/common/extensions/api/extension_api.cc
@@ -16,6 +16,7 @@
 #include "base/string_split.h"
 #include "base/string_util.h"
 #include "base/values.h"
+#include "chrome/common/extensions/api/extension_api_feature.h"
 #include "chrome/common/extensions/api/generated_schemas.h"
 #include "chrome/common/extensions/extension.h"
 #include "chrome/common/extensions/extension_permission_set.h"
@@ -195,40 +196,18 @@ void ExtensionAPI::LoadSchema(const base::StringPiece& schema) {
     if (!uses_feature_system)
       continue;
 
-    Feature* feature = new Feature();
+    ExtensionAPIFeature* feature = new ExtensionAPIFeature();
     feature->set_name(schema_namespace);
     feature->Parse(schema);
 
-    FeatureMap* schema_features = new FeatureMap();
+    if (feature->contexts()->empty()) {
+      LOG(ERROR) << "API feature '" << schema_namespace << "' includes no "
+                 << "contexts and will never be available.";

[not at google - send to devlin, 2012/04/10 02:04:27]

Why not CHECK?

[Aaron Boodman, 2012/04/11 21:51:54]

Done.

+    }
+
     CHECK(features_.insert(
         std::make_pair(schema_namespace,
-                       make_linked_ptr(schema_features))).second);
-    CHECK(schema_features->insert(
-        std::make_pair("", make_linked_ptr(feature))).second);
-
-    for (size_t i = 0; i < arraysize(kChildKinds); ++i) {
-      ListValue* child_list = NULL;
-      schema->GetList(kChildKinds[i], &child_list);
-      if (!child_list)
-        continue;
-
-      for (size_t j = 0; j < child_list->GetSize(); ++j) {
-        DictionaryValue* child = NULL;
-        CHECK(child_list->GetDictionary(j, &child));
-
-        scoped_ptr<Feature> child_feature(new Feature(*feature));
-        child_feature->Parse(child);
-        if (child_feature->Equals(*feature))
-          continue;  // no need to store no-op features
-
-        std::string child_name;
-        CHECK(child->GetString("name", &child_name));
-        child_feature->set_name(schema_namespace + "." + child_name);
-        CHECK(schema_features->insert(
-            std::make_pair(child_name,
-                           make_linked_ptr(child_feature.release()))).second);
-      }
-    }
+                       make_linked_ptr(feature))).second);
   }
 }
 
@@ -406,7 +385,7 @@ bool ExtensionAPI::IsAvailable(const std::string& full_name,
 
   for (std::set<std::string>::iterator iter = dependency_names.begin();
        iter != dependency_names.end(); ++iter) {
-    Feature* feature = GetFeatureDependency(full_name);
+    Feature* feature = GetFeatureDependency(*iter);
     CHECK(feature) << *iter;
 
     Feature::Availability availability =
@@ -423,7 +402,7 @@ bool ExtensionAPI::IsPrivileged(const std::string& full_name) {
   std::string api_name = GetAPINameFromFullName(full_name, &child_name);
 
   // First try to use the feature system.
-  Feature* feature(GetFeature(full_name));
+  Feature* feature = GetFeature(full_name);
   if (feature) {
     // An API is 'privileged' if it or any of its dependencies can only be run
     // in a blessed context.
@@ -433,14 +412,14 @@ bool ExtensionAPI::IsPrivileged(const std::string& full_name) {
     for (std::set<std::string>::iterator iter = resolved_dependencies.begin();
          iter != resolved_dependencies.end(); ++iter) {
       Feature* dependency = GetFeatureDependency(*iter);
-      for (std::set<Feature::Context>::iterator context =
-               dependency->contexts()->begin();
-           context != dependency->contexts()->end(); ++context) {
-        if (*context != Feature::BLESSED_EXTENSION_CONTEXT)
-          return false;
+      if (dependency->contexts()->size() == 1u) {
+        Feature::Context context =
+            *(dependency->contexts()->begin());
+        if (context == Feature::BLESSED_EXTENSION_CONTEXT)
+          return true;
       }
     }
-    return true;
+    return false;
   }
 
   // If this API hasn't been converted yet, fall back to the old system.
@@ -504,8 +483,8 @@ scoped_ptr<std::set<std::string> > ExtensionAPI::GetAPIsForContext(
 
   // First handle all the APIs that have been converted to the feature system.
   if (extension) {
-    for (APIFeatureMap::iterator iter = features_.begin();
-         iter != features_.end(); ++iter) {
+    for (FeatureMap::iterator iter = features_.begin(); iter != features_.end();
+         ++iter) {
       if (IsAvailable(iter->first, extension, context))
         temp_result.insert(iter->first);
     }
@@ -565,27 +544,19 @@ Feature* ExtensionAPI::GetFeature(const std::string& full_name) {
   std::string child_name;
   std::string api_namespace = GetAPINameFromFullName(full_name, &child_name);
 
-  APIFeatureMap::iterator feature_map = features_.find(api_namespace);
-  if (feature_map == features_.end())
+  FeatureMap::iterator parent = features_.find(api_namespace);
+  if (parent == features_.end())
     return NULL;
 
-  Feature* result = NULL;
-  FeatureMap::iterator child_feature = feature_map->second->find(child_name);
-  if (child_feature != feature_map->second->end()) {
-    result = child_feature->second.get();
+  Feature* child = parent->second->GetChild(child_name);
+  if (child) {
+    return child;
   } else {
-    FeatureMap::iterator parent_feature = feature_map->second->find("");
-    CHECK(parent_feature != feature_map->second->end());
-    result = parent_feature->second.get();
+    // TODO(aa): This is lame. We should generate a feature with the correct
+    // name on the fly. In order to do that, change the return type of this
+    // method to linked_ptr.
+    return parent->second.get();
   }
-
-  if (result->contexts()->empty()) {
-    LOG(ERROR) << "API feature '" << full_name
-               << "' must specify at least one context.";
-    return NULL;
-  }
-
-  return result;
 }
 
 Feature* ExtensionAPI::GetFeatureDependency(const std::string& full_name) {
@@ -674,6 +645,20 @@ void ExtensionAPI::GetMissingDependencies(
   if (feature_type != "api")
     return;
 
+  // If this API is controlled by the feature system, use it to get dependencies
+  // to avoid loading the schema.
+  ExtensionAPIFeature* feature =
+      static_cast<ExtensionAPIFeature*>(GetFeature(api_name));
+  if (feature) {
+    for (std::set<std::string>::iterator iter =
+             feature->dependencies()->begin();
+         iter != feature->dependencies()->end(); ++iter) {

[not at google - send to devlin, 2012/04/10 23:55:41]

nit: I think this would fit on 1 line. Certainly if you called it "it" instead
of "iter".

[Aaron Boodman, 2012/04/11 21:51:54]

Done.

+      out->insert(*iter);
+    }
+    return;
+  }
+
+  // Otherwise, fall back to the old system.
   const DictionaryValue* schema = GetSchema(feature_name);
   CHECK(schema) << "Schema for " << feature_name << " not found";
 
@@ -694,6 +679,11 @@ void ExtensionAPI::RemovePrivilegedAPIs(std::set<std::string>* apis) {
   std::set<std::string> privileged_apis;
   for (std::set<std::string>::iterator i = apis->begin(); i != apis->end();
       ++i) {
+    if (features_.find(*i) != features_.end()) {

[not at google - send to devlin, 2012/04/10 23:55:41]

This pattern is being used a lot. How about adding an IsControlledByFeatures()
method, then we also wouldn't need the comment that follows every check like
this.

[Aaron Boodman, 2012/04/11 21:51:54]

I like it. Done.

+      // This API is controlled by the feature system. Nothing to do here.
+      continue;
+    }
+
     if (!completely_unprivileged_apis_.count(*i) &&
         !partially_unprivileged_apis_.count(*i)) {
       privileged_apis.insert(*i);