Remove GetContexts() from the public interface of extensions::Feature. It was

being misused in such a way that makes it impossible to add other trusted
extension context types. It's also a nice cleanup, though requires rewriting
the ExtensionAPI::IsPrivileged function with sightly different semantics.

BUG=391944
R=rockot@chromium.org

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

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

chrome/common/extensions/api/extension_api_unittest.cc

@@ -85,43 +85,73 @@ TEST(ExtensionAPITest, SplitDependencyName) {
   }
 }
 
-TEST(ExtensionAPITest, IsPrivileged) {
+TEST(ExtensionAPITest, IsAvailableInUntrustedContext) {
   scoped_ptr<ExtensionAPI> extension_api(
       ExtensionAPI::CreateWithDefaultConfiguration());
-
-  EXPECT_FALSE(extension_api->IsPrivileged("runtime.connect"));
-  EXPECT_FALSE(extension_api->IsPrivileged("runtime.onConnect"));
-  EXPECT_FALSE(extension_api->IsPrivileged("runtime.lastError"));
+  scoped_refptr<const Extension> extension =
+      ExtensionBuilder()
+          .SetManifest(DictionaryBuilder()
+                           .Set("name", "extension")
+                           .Set("version", "1")
+                           .Set("permissions", ListBuilder().Append("storage"))
+                           .Set("manifest_version", 2))
+          .Build();
+
+  EXPECT_TRUE(extension_api->IsAvailableInUntrustedContext("runtime.connect",
+                                                           extension.get()));
+  EXPECT_TRUE(extension_api->IsAvailableInUntrustedContext("runtime.onConnect",
+                                                           extension.get()));
+  EXPECT_TRUE(extension_api->IsAvailableInUntrustedContext("runtime.lastError",
+                                                           extension.get()));
 
   // Exists, but privileged.
-  EXPECT_TRUE(extension_api->IsPrivileged("extension.getViews"));
-  EXPECT_TRUE(extension_api->IsPrivileged("history.search"));
+  EXPECT_FALSE(extension_api->IsAvailableInUntrustedContext(
+      "extension.getViews", extension.get()));
+  EXPECT_FALSE(extension_api->IsAvailableInUntrustedContext("history.search",
+                                                            extension.get()));
 
   // Whole APIs that are unprivileged.
-  EXPECT_FALSE(extension_api->IsPrivileged("app.getDetails"));
-  EXPECT_FALSE(extension_api->IsPrivileged("app.isInstalled"));
-  EXPECT_FALSE(extension_api->IsPrivileged("storage.local"));
-  EXPECT_FALSE(extension_api->IsPrivileged("storage.local.onChanged"));
-  EXPECT_FALSE(extension_api->IsPrivileged("storage.local.set"));
-  EXPECT_FALSE(extension_api->IsPrivileged("storage.local.MAX_ITEMS"));
-  EXPECT_FALSE(extension_api->IsPrivileged("storage.set"));
+  EXPECT_TRUE(
+      extension_api->IsAvailableInUntrustedContext("app", extension.get()));
+  EXPECT_TRUE(extension_api->IsAvailableInUntrustedContext("app.getDetails",
+                                                           extension.get()));
+  // There is no feature "app.isInstalled" (it's "app.getIsInstalled") but
+  // this should be available nonetheless.
+  EXPECT_TRUE(extension_api->IsAvailableInUntrustedContext("app.isInstalled",
+                                                           extension.get()));
+  EXPECT_TRUE(extension_api->IsAvailableInUntrustedContext("storage.local",
+                                                           extension.get()));
+  EXPECT_TRUE(extension_api->IsAvailableInUntrustedContext(
+      "storage.local.onChanged", extension.get()));
+  EXPECT_TRUE(extension_api->IsAvailableInUntrustedContext("storage.local.set",
+                                                           extension.get()));
+  EXPECT_TRUE(extension_api->IsAvailableInUntrustedContext(
+      "storage.local.MAX_ITEMS", extension.get()));
+  EXPECT_TRUE(extension_api->IsAvailableInUntrustedContext("storage.set",
+                                                           extension.get()));
+
+  // APIs which override unprivileged APIs.
+  EXPECT_FALSE(extension_api->IsAvailableInUntrustedContext("app.runtime",
+                                                            extension.get()));
+  EXPECT_FALSE(extension_api->IsAvailableInUntrustedContext("app.window",
+                                                            extension.get()));
 }
 
-TEST(ExtensionAPITest, IsPrivilegedFeatures) {
+TEST(ExtensionAPITest, IsAvailableInUntrustedContextFeatures) {
   struct {
     std::string api_full_name;
-    bool expect_is_privilged;
-  } test_data[] = {
-    { "test1", false },
-    { "test1.foo", true },
-    { "test2", true },
-    { "test2.foo", false },
-    { "test2.bar", false },
-    { "test2.baz", true },
-    { "test3", false },
-    { "test3.foo", true },
-    { "test4", false }
-  };
+    bool expect_is_available;
+  } test_data[] = {{"test1", true},
+                   {"test1.foo", false},
+                   {"test2", false},
+                   {"test2.foo", true},
+                   {"test2.bar", true},
+                   {"test2.baz", false},
+                   {"test2.qux", false},
+                   {"test3", true},
+                   {"test3.foo", false},
+                   {"test4", true},
+                   {"test5", true}};
 
   base::FilePath api_features_path;
   PathService::Get(chrome::DIR_TEST_DATA, &api_features_path);
@@ -137,11 +167,16 @@ TEST(ExtensionAPITest, IsPrivilegedFeatures) {
       base::JSONReader::Read(api_features_str)));
   BaseFeatureProvider api_feature_provider(*value, CreateAPIFeature);
 
+  scoped_refptr<Extension> extension =
+      BuildExtension(ExtensionBuilder().Pass()).Build();
+
   for (size_t i = 0; i < ARRAYSIZE_UNSAFE(test_data); ++i) {
     ExtensionAPI api;
     api.RegisterDependencyProvider("api", &api_feature_provider);
-    EXPECT_EQ(test_data[i].expect_is_privilged,
-              api.IsPrivileged(test_data[i].api_full_name)) << i;
+    EXPECT_EQ(test_data[i].expect_is_available,
+              api.IsAvailableInUntrustedContext(test_data[i].api_full_name,
+                                                extension.get()))
+        << i;
   }
 }
 
@@ -708,10 +743,6 @@ TEST(ExtensionAPITest, DefaultConfigurationFeatures) {
     EXPECT_TRUE(feature->whitelist()->empty());
     EXPECT_TRUE(feature->extension_types()->empty());
 
-    EXPECT_EQ(1u, feature->GetContexts()->size());
-    EXPECT_TRUE(feature->GetContexts()->count(
-        Feature::BLESSED_EXTENSION_CONTEXT));
-
     EXPECT_EQ(SimpleFeature::UNSPECIFIED_LOCATION, feature->location());
     EXPECT_TRUE(feature->platforms()->empty());
     EXPECT_EQ(0, feature->min_manifest_version());

chrome/test/data/extensions/extension_api_unittest/privileged_api_features.json

@@ -8,8 +8,8 @@
     "contexts": ["blessed_extension"]
   },
   "test2": {
-    "contexts": ["blessed_extension"],
-    "dependencies": ["api:test1"]
+    "channel": "trunk",
+    "contexts": ["blessed_extension"]
   },
   "test2.foo": {
     "channel": "trunk",
@@ -20,8 +20,8 @@
     "contexts": ["content_script", "blessed_extension"]
   },
   "test2.baz": {
-    "contexts": ["blessed_extension"],
-    "dependencies": ["api:test2.foo"]
+    "channel": "trunk",
+    "contexts": ["blessed_extension"]
   },
   "test3": {
     "channel": "trunk",
@@ -32,7 +32,12 @@
     "contexts": ["blessed_extension"]
   },
   "test4": {
-    "contexts": ["content_script"],
-    "dependencies": ["api:test1.foo"]
+    "channel": "trunk",
+    "contexts": ["web_page"],
+    "matches": ["<all_urls>"]
+  },
+  "test5": {
+    "channel": "trunk",
+    "contexts": ["blessed_web_page"]
   }
 }

extensions/browser/event_router.cc

@@ -525,8 +525,9 @@ void EventRouter::DispatchEventToProcess(const std::string& extension_id,
   ProcessMap* process_map = ProcessMap::Get(listener_context);
   // If the event is privileged, only send to extension processes. Otherwise,
   // it's OK to send to normal renderers (e.g., for content scripts).
-  if (ExtensionAPI::GetSharedInstance()->IsPrivileged(event->event_name) &&
-      !process_map->Contains(extension->id(), process->GetID())) {
+  if (!process_map->Contains(extension->id(), process->GetID()) &&
+      !ExtensionAPI::GetSharedInstance()->IsAvailableInUntrustedContext(
+          event->event_name, extension)) {
     return;
   }
 

extensions/browser/extension_function_dispatcher.cc

@@ -474,7 +474,7 @@ ExtensionFunction* ExtensionFunctionDispatcher::CreateExtensionFunction(
 
   // Privileged APIs can only be called from the process the extension
   // is running in.
-  if (allowed && api->IsPrivileged(params.name))
+  if (allowed && !api->IsAvailableInUntrustedContext(params.name, extension))
     allowed = process_map.Contains(extension->id(), requesting_process_id);
 
   if (!allowed) {

extensions/common/extension_api.cc

@@ -262,6 +262,7 @@ bool ExtensionAPI::IsAnyFeatureAvailableToContext(const Feature& api,
                                                   const GURL& url) {
   FeatureProviderMap::iterator provider = dependency_providers_.find("api");
   CHECK(provider != dependency_providers_.end());
+
   if (api.IsAvailableToContext(extension, context, url).is_available())
     return true;
 
@@ -289,13 +290,17 @@ Feature::Availability ExtensionAPI::IsAvailable(const std::string& full_name,
   return feature->IsAvailableToContext(extension, context, url);
 }
 
-bool ExtensionAPI::IsPrivileged(const std::string& full_name) {
-  Feature* feature = GetFeatureDependency(full_name);
-  CHECK(feature) << full_name;
-  DCHECK(!feature->GetContexts()->empty()) << full_name;
-  // An API is 'privileged' if it can only be run in a blessed context.
-  return feature->GetContexts()->size() ==
-      feature->GetContexts()->count(Feature::BLESSED_EXTENSION_CONTEXT);
+bool ExtensionAPI::IsAvailableInUntrustedContext(const std::string& name,
+                                                 const Extension* extension) {
+  return IsAvailable(name, extension, Feature::CONTENT_SCRIPT_CONTEXT, GURL())
+             .is_available() ||
+         IsAvailable(
+             name, extension, Feature::UNBLESSED_EXTENSION_CONTEXT, GURL())
+             .is_available() ||
+         IsAvailable(name, extension, Feature::BLESSED_WEB_PAGE_CONTEXT, GURL())
+             .is_available() ||
+         IsAvailable(name, extension, Feature::WEB_PAGE_CONTEXT, GURL())
+             .is_available();
 }
 
 const base::DictionaryValue* ExtensionAPI::GetSchema(

extensions/common/extension_api.h

@@ -104,11 +104,10 @@ class ExtensionAPI {
                                       Feature::Context context,
                                       const GURL& url);
 
-  // Returns true if |name| is a privileged API path. Privileged paths can only
-  // be called from extension code which is running in its own designated
-  // extension process. They cannot be called from extension code running in
-  // content scripts, or other low-privileged contexts.
-  bool IsPrivileged(const std::string& name);
+  // Returns true if |name| is available to |extension| in any untrusted
+  // extension context, such as content scripts, iframes, or web pages.
+  bool IsAvailableInUntrustedContext(const std::string& name,
+                                     const Extension* extension);
 
   // Gets the schema for the extension API with namespace |full_name|.
   // Ownership remains with this object.

extensions/common/extension_api_stub.cc

@@ -36,7 +36,8 @@ bool ExtensionAPI::IsAnyFeatureAvailableToContext(const Feature& api,
   return false;
 }
 
-bool ExtensionAPI::IsPrivileged(const std::string& full_name) {
+bool ExtensionAPI::IsAvailableInUntrustedContext(const std::string& full_name,
+                                                 const Extension* extension) {
   return false;
 }
 

extensions/common/features/api_feature.cc

@@ -28,7 +28,7 @@ std::string APIFeature::Parse(const base::DictionaryValue* value) {
   value->GetBoolean("internal", &internal_);
   value->GetBoolean("blocked_in_service_worker", &blocked_in_service_worker_);
 
-  if (GetContexts()->empty())
+  if (contexts()->empty())
     return name() + ": API features must specify at least one context.";
 
   return std::string();

extensions/common/features/complex_feature.cc

@@ -12,18 +12,14 @@ ComplexFeature::ComplexFeature(scoped_ptr<FeatureList> features) {
   no_parent_ = features_[0]->no_parent();
 
 #if !defined(NDEBUG) || defined(DCHECK_ALWAYS_ON)
-  // Verify GetContexts, IsInternal, & IsBlockedInServiceWorker are consistent
-  // across all features.
-  std::set<Feature::Context>* first_contexts = features_[0]->GetContexts();
+  // Verify IsInternal and IsBlockedInServiceWorker are consistent across all
+  // features.
   bool first_is_internal = features_[0]->IsInternal();
   bool first_blocked_in_service_worker =
       features_[0]->IsBlockedInServiceWorker();
   for (FeatureList::const_iterator it = features_.begin() + 1;
        it != features_.end();
        ++it) {
-    DCHECK(*first_contexts == *(*it)->GetContexts())
-        << "Complex feature must have consistent values of "
-           "contexts across all sub features.";
     DCHECK(first_is_internal == (*it)->IsInternal())
         << "Complex feature must have consistent values of "
            "internal across all sub features.";
@@ -112,16 +108,9 @@ bool ComplexFeature::IsBlockedInServiceWorker() const {
   return features_[0]->IsBlockedInServiceWorker();
 }
 
-std::set<Feature::Context>* ComplexFeature::GetContexts() {
-  // TODO(justinlin): Current use cases for ComplexFeatures are simple (e.g.
-  // allow API in dev channel for everyone but stable channel for a whitelist),
-  // but if they get more complicated, we need to return some meaningful context
-  // set. Either that or remove this method from the Feature interface.
-  return features_[0]->GetContexts();
-}
-
 bool ComplexFeature::IsInternal() const {
-  // TODO(justinlin): Same as the above TODO.
+  // Constructor verifies that composed features are consistent, thus we can
+  // return just the first feature's value.
   return features_[0]->IsInternal();
 }
 

extensions/common/features/complex_feature.h

@@ -49,8 +49,6 @@ class ComplexFeature : public Feature {
       const GURL& url,
       Context context) const OVERRIDE;
 
-  virtual std::set<Context>* GetContexts() OVERRIDE;
-
   virtual bool IsInternal() const OVERRIDE;
 
  private:

extensions/common/features/feature.h

@@ -108,8 +108,6 @@ class Feature {
   // Gets the platform the code is currently running on.
   static Platform GetCurrentPlatform();
 
-  virtual std::set<Context>* GetContexts() = 0;
-
   // Tests whether this is an internal API or not.
   virtual bool IsInternal() const = 0;
 

extensions/common/features/manifest_feature.cc

@@ -44,7 +44,7 @@ std::string ManifestFeature::Parse(const base::DictionaryValue* value) {
         "value for extension_types.";
   }
 
-  if (!GetContexts()->empty())
+  if (value->HasKey("contexts"))
     return name() + ": Manifest features do not support contexts.";
 
   return std::string();

extensions/common/features/permission_feature.cc

@@ -43,7 +43,7 @@ std::string PermissionFeature::Parse(const base::DictionaryValue* value) {
         "value for extension_types.";
   }
 
-  if (!GetContexts()->empty())
+  if (value->HasKey("contexts"))
     return name() + ": Permission features do not support contexts.";
 
   return std::string();

extensions/common/features/simple_feature.cc

@@ -287,10 +287,14 @@ std::string SimpleFeature::Parse(const base::DictionaryValue* value) {
   value->GetBoolean("component_extensions_auto_granted",
                     &component_extensions_auto_granted_);
 
-  if (matches_.is_empty() && contexts_.count(WEB_PAGE_CONTEXT) != 0) {
-    return name() + ": Allowing web_page contexts requires supplying a value " +
-        "for matches.";
-  }
+  // NOTE: ideally we'd sanity check that "matches" can be specified if and
+  // only if there's a "web_page" context, but without (Simple)Features being
+  // aware of their own heirarchy this is impossible.
+  //
+  // For example, we might have feature "foo" available to "web_page" context
+  // and "matches" google.com/*. Then a sub-feature "foo.bar" might override
+  // "matches" to be chromium.org/*. That sub-feature doesn't need to specify
+  // "web_page" context because it's inherited, but we don't know that here.
 
   for (FilterList::iterator filter_iter = filters_.begin();
        filter_iter != filters_.end();
@@ -395,7 +399,7 @@ Feature::Availability SimpleFeature::IsAvailableToContext(
   if (!contexts_.empty() && contexts_.find(context) == contexts_.end())
     return CreateAvailability(INVALID_CONTEXT, context);
 
-  if (!matches_.is_empty() && !matches_.MatchesURL(url))
+  if (context == WEB_PAGE_CONTEXT && !matches_.MatchesURL(url))
     return CreateAvailability(INVALID_URL, url);
 
   for (FilterList::const_iterator filter_iter = filters_.begin();
@@ -502,10 +506,6 @@ Feature::Availability SimpleFeature::CreateAvailability(
                                      context));
 }
 
-std::set<Feature::Context>* SimpleFeature::GetContexts() {
-  return &contexts_;
-}
-
 bool SimpleFeature::IsInternal() const {
   return false;
 }

extensions/common/features/simple_feature.h

@@ -55,6 +55,7 @@ class SimpleFeature : public Feature {
   std::set<std::string>* blacklist() { return &blacklist_; }
   std::set<std::string>* whitelist() { return &whitelist_; }
   std::set<Manifest::Type>* extension_types() { return &extension_types_; }
+  std::set<Context>* contexts() { return &contexts_; }
 
   // Dependency resolution is a property of Features that is preferrably
   // handled internally to avoid temptation, but FeatureFilters may need
@@ -104,8 +105,6 @@ class SimpleFeature : public Feature {
                                              const GURL& url,
                                              Context context) const OVERRIDE;
 
-  virtual std::set<Context>* GetContexts() OVERRIDE;
-
   virtual bool IsInternal() const OVERRIDE;
   virtual bool IsBlockedInServiceWorker() const OVERRIDE;
 

extensions/common/features/simple_feature_unittest.cc

@@ -289,7 +289,7 @@ TEST_F(ExtensionSimpleFeatureTest, PackageType) {
 TEST_F(ExtensionSimpleFeatureTest, Context) {
   SimpleFeature feature;
   feature.set_name("somefeature");
-  feature.GetContexts()->insert(Feature::BLESSED_EXTENSION_CONTEXT);
+  feature.contexts()->insert(Feature::BLESSED_EXTENSION_CONTEXT);
   feature.extension_types()->insert(Manifest::TYPE_LEGACY_PACKAGED_APP);
   feature.platforms()->insert(Feature::CHROMEOS_PLATFORM);
   feature.set_min_manifest_version(21);
@@ -328,9 +328,9 @@ TEST_F(ExtensionSimpleFeatureTest, Context) {
 
   feature.extension_types()->clear();
   feature.extension_types()->insert(Manifest::TYPE_LEGACY_PACKAGED_APP);
-  feature.GetContexts()->clear();
-  feature.GetContexts()->insert(Feature::UNBLESSED_EXTENSION_CONTEXT);
-  feature.GetContexts()->insert(Feature::CONTENT_SCRIPT_CONTEXT);
+  feature.contexts()->clear();
+  feature.contexts()->insert(Feature::UNBLESSED_EXTENSION_CONTEXT);
+  feature.contexts()->insert(Feature::CONTENT_SCRIPT_CONTEXT);
   {
     Feature::Availability availability = feature.IsAvailableToContext(
         extension.get(), Feature::BLESSED_EXTENSION_CONTEXT,
@@ -341,7 +341,7 @@ TEST_F(ExtensionSimpleFeatureTest, Context) {
               availability.message());
   }
 
-  feature.GetContexts()->insert(Feature::WEB_PAGE_CONTEXT);
+  feature.contexts()->insert(Feature::WEB_PAGE_CONTEXT);
   {
     Feature::Availability availability = feature.IsAvailableToContext(
         extension.get(), Feature::BLESSED_EXTENSION_CONTEXT,
@@ -352,8 +352,8 @@ TEST_F(ExtensionSimpleFeatureTest, Context) {
               availability.message());
   }
 
-  feature.GetContexts()->clear();
-  feature.GetContexts()->insert(Feature::BLESSED_EXTENSION_CONTEXT);
+  feature.contexts()->clear();
+  feature.contexts()->insert(Feature::BLESSED_EXTENSION_CONTEXT);
   feature.set_location(SimpleFeature::COMPONENT_LOCATION);
   EXPECT_EQ(Feature::INVALID_LOCATION, feature.IsAvailableToContext(
       extension.get(), Feature::BLESSED_EXTENSION_CONTEXT,
@@ -496,7 +496,7 @@ TEST_F(ExtensionSimpleFeatureTest, ParseNull) {
   feature->Parse(value.get());
   EXPECT_TRUE(feature->whitelist()->empty());
   EXPECT_TRUE(feature->extension_types()->empty());
-  EXPECT_TRUE(feature->GetContexts()->empty());
+  EXPECT_TRUE(feature->contexts()->empty());
   EXPECT_EQ(SimpleFeature::UNSPECIFIED_LOCATION, feature->location());
   EXPECT_TRUE(feature->platforms()->empty());
   EXPECT_EQ(0, feature->min_manifest_version());
@@ -554,22 +554,17 @@ TEST_F(ExtensionSimpleFeatureTest, ParseContexts) {
   value->Set("contexts", contexts);
   scoped_ptr<SimpleFeature> feature(new SimpleFeature());
   feature->Parse(value.get());
-  EXPECT_EQ(5u, feature->GetContexts()->size());
-  EXPECT_TRUE(
-      feature->GetContexts()->count(Feature::BLESSED_EXTENSION_CONTEXT));
-  EXPECT_TRUE(
-      feature->GetContexts()->count(Feature::UNBLESSED_EXTENSION_CONTEXT));
-  EXPECT_TRUE(
-      feature->GetContexts()->count(Feature::CONTENT_SCRIPT_CONTEXT));
-  EXPECT_TRUE(
-      feature->GetContexts()->count(Feature::WEB_PAGE_CONTEXT));
-  EXPECT_TRUE(
-      feature->GetContexts()->count(Feature::BLESSED_WEB_PAGE_CONTEXT));
+  EXPECT_EQ(5u, feature->contexts()->size());
+  EXPECT_TRUE(feature->contexts()->count(Feature::BLESSED_EXTENSION_CONTEXT));
+  EXPECT_TRUE(feature->contexts()->count(Feature::UNBLESSED_EXTENSION_CONTEXT));
+  EXPECT_TRUE(feature->contexts()->count(Feature::CONTENT_SCRIPT_CONTEXT));
+  EXPECT_TRUE(feature->contexts()->count(Feature::WEB_PAGE_CONTEXT));
+  EXPECT_TRUE(feature->contexts()->count(Feature::BLESSED_WEB_PAGE_CONTEXT));
 
   value->SetString("contexts", "all");
   scoped_ptr<SimpleFeature> feature2(new SimpleFeature());
   feature2->Parse(value.get());
-  EXPECT_EQ(*(feature->GetContexts()), *(feature2->GetContexts()));
+  EXPECT_EQ(*(feature->contexts()), *(feature2->contexts()));
 }
 
 TEST_F(ExtensionSimpleFeatureTest, ParseLocation) {
@@ -625,7 +620,7 @@ TEST_F(ExtensionSimpleFeatureTest, Inheritance) {
   SimpleFeature feature;
   feature.whitelist()->insert("foo");
   feature.extension_types()->insert(Manifest::TYPE_THEME);
-  feature.GetContexts()->insert(Feature::BLESSED_EXTENSION_CONTEXT);
+  feature.contexts()->insert(Feature::BLESSED_EXTENSION_CONTEXT);
   feature.set_location(SimpleFeature::COMPONENT_LOCATION);
   feature.platforms()->insert(Feature::CHROMEOS_PLATFORM);
   feature.set_min_manifest_version(1);
@@ -637,7 +632,7 @@ TEST_F(ExtensionSimpleFeatureTest, Inheritance) {
   feature.Parse(&definition);
   EXPECT_EQ(1u, feature.whitelist()->size());
   EXPECT_EQ(1u, feature.extension_types()->size());
-  EXPECT_EQ(1u, feature.GetContexts()->size());
+  EXPECT_EQ(1u, feature.contexts()->size());
   EXPECT_EQ(1u, feature.whitelist()->count("foo"));
   EXPECT_EQ(SimpleFeature::COMPONENT_LOCATION, feature.location());
   EXPECT_EQ(1u, feature.platforms()->size());
@@ -661,11 +656,11 @@ TEST_F(ExtensionSimpleFeatureTest, Inheritance) {
   feature.Parse(&definition);
   EXPECT_EQ(1u, feature.whitelist()->size());
   EXPECT_EQ(1u, feature.extension_types()->size());
-  EXPECT_EQ(1u, feature.GetContexts()->size());
+  EXPECT_EQ(1u, feature.contexts()->size());
   EXPECT_EQ(1u, feature.whitelist()->count("bar"));
   EXPECT_EQ(1u, feature.extension_types()->count(Manifest::TYPE_EXTENSION));
   EXPECT_EQ(1u,
-            feature.GetContexts()->count(Feature::UNBLESSED_EXTENSION_CONTEXT));
+            feature.contexts()->count(Feature::UNBLESSED_EXTENSION_CONTEXT));
   EXPECT_EQ(2, feature.min_manifest_version());
   EXPECT_EQ(3, feature.max_manifest_version());
 }