FeatureProvider returns std::map instead of vector

To reduce binding time as much as possible, use std::map(O(1)) instead of std::vector(O(n)).

Also, use 'const_iterator' to fix improper usage of 'auto'.

BUG=592537
TEST=Every tests are green.

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

Cr-Commit-Position: refs/heads/master@{#379711}

4 files changed, 20 insertions, 25 deletions

diff --git a/extensions/common/features/base_feature_provider.cc b/extensions/common/features/base_feature_provider.cc
index 8bcec82..069de38 100644
--- a/extensions/common/features/base_feature_provider.cc
+++ b/extensions/common/features/base_feature_provider.cc
@@ -149,19 +149,12 @@ BaseFeatureProvider::BaseFeatureProvider(const base::DictionaryValue& root,
 BaseFeatureProvider::~BaseFeatureProvider() {
 }
 
-const std::vector<std::string>& BaseFeatureProvider::GetAllFeatureNames()
-    const {
-  if (feature_names_.empty()) {
-    for (const auto& feature : features_)
-      feature_names_.push_back(feature.first);
-    // A std::map is sorted by its keys, so we don't need to sort feature_names_
-    // now.
-  }
-  return feature_names_;
+const FeatureMap& BaseFeatureProvider::GetAllFeatures() const {
+  return features_;
 }
 
 Feature* BaseFeatureProvider::GetFeature(const std::string& name) const {
-  const auto iter = features_.find(name);
+  FeatureMap::const_iterator iter = features_.find(name);
   if (iter != features_.end())
     return iter->second.get();
   else
@@ -186,15 +179,17 @@ Feature* BaseFeatureProvider::GetParent(Feature* feature) const {
 std::vector<Feature*> BaseFeatureProvider::GetChildren(const Feature& parent)
     const {
   std::string prefix = parent.name() + ".";
-  const auto first_child = features_.lower_bound(prefix);
+  const FeatureMap::const_iterator first_child = features_.lower_bound(prefix);
 
   // All children have names before (parent.name() + ('.'+1)).
   ++prefix[prefix.size() - 1];
-  const auto after_children = features_.lower_bound(prefix);
+  const FeatureMap::const_iterator after_children =
+      features_.lower_bound(prefix);
 
   std::vector<Feature*> result;
   result.reserve(std::distance(first_child, after_children));
-  for (auto it = first_child; it != after_children; ++it) {
+  for (FeatureMap::const_iterator it = first_child; it != after_children;
+       ++it) {
     result.push_back(it->second.get());
   }
   return result;
diff --git a/extensions/common/features/base_feature_provider.h b/extensions/common/features/base_feature_provider.h
index 3b766c8..0d61292 100644
--- a/extensions/common/features/base_feature_provider.h
+++ b/extensions/common/features/base_feature_provider.h
@@ -35,7 +35,7 @@ class BaseFeatureProvider : public FeatureProvider {
   Feature* GetParent(Feature* feature) const override;
   std::vector<Feature*> GetChildren(const Feature& parent) const override;
 
-  const std::vector<std::string>& GetAllFeatureNames() const override;
+  const FeatureMap& GetAllFeatures() const override;
 
  private:
   std::map<std::string, scoped_ptr<Feature>> features_;
diff --git a/extensions/common/features/feature_provider.h b/extensions/common/features/feature_provider.h
index 40d8ab8..ae97513 100644
--- a/extensions/common/features/feature_provider.h
+++ b/extensions/common/features/feature_provider.h
@@ -5,13 +5,18 @@
 #ifndef EXTENSIONS_COMMON_FEATURES_FEATURE_PROVIDER_H_
 #define EXTENSIONS_COMMON_FEATURES_FEATURE_PROVIDER_H_
 
+#include <map>
 #include <string>
 #include <vector>
 
+#include "base/memory/scoped_ptr.h"
+
 namespace extensions {
 
 class Feature;
 
+using FeatureMap = std::map<std::string, scoped_ptr<Feature>>;
+
 // Implemented by classes that can vend features.
 class FeatureProvider {
  public:
@@ -52,8 +57,8 @@ class FeatureProvider {
   // Returns the features inside the |parent| namespace, recursively.
   virtual std::vector<Feature*> GetChildren(const Feature& parent) const = 0;
 
-  // Returns all features described by this instance, in asciibetical order.
-  virtual const std::vector<std::string>& GetAllFeatureNames() const = 0;
+  // Returns a map containing all features described by this instance.
+  virtual const FeatureMap& GetAllFeatures() const = 0;
 };
 
 }  // namespace extensions
diff --git a/extensions/common/manifest.cc b/extensions/common/manifest.cc
index 112628f..8bc2e28 100644
--- a/extensions/common/manifest.cc
+++ b/extensions/common/manifest.cc
@@ -146,20 +146,15 @@ bool Manifest::ValidateManifest(
 
   const FeatureProvider* manifest_feature_provider =
       FeatureProvider::GetManifestFeatures();
-  const std::vector<std::string>& feature_names =
-      manifest_feature_provider->GetAllFeatureNames();
-  for (std::vector<std::string>::const_iterator feature_name =
-           feature_names.begin();
-       feature_name != feature_names.end(); ++feature_name) {
+  for (const auto& map_entry : manifest_feature_provider->GetAllFeatures()) {
     // Use Get instead of HasKey because the former uses path expansion.
-    if (!value_->Get(*feature_name, NULL))
+    if (!value_->Get(map_entry.first, nullptr))
       continue;
 
-    Feature* feature = manifest_feature_provider->GetFeature(*feature_name);
-    Feature::Availability result = feature->IsAvailableToManifest(
+    Feature::Availability result = map_entry.second->IsAvailableToManifest(
         extension_id_, type_, location_, GetManifestVersion());
     if (!result.is_available())
-      warnings->push_back(InstallWarning(result.message(), *feature_name));
+      warnings->push_back(InstallWarning(result.message(), map_entry.first));
   }
 
   // Also generate warnings for keys that are not features.