Skip to content

Commit

Permalink
Support a "policy" extension location in extension features files.
Browse files Browse the repository at this point in the history
BUG=364536

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@264826 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
kalman@chromium.org committed Apr 18, 2014
1 parent 4890a17 commit e42ffb1
Show file tree
Hide file tree
Showing 16 changed files with 310 additions and 231 deletions.
2 changes: 1 addition & 1 deletion chrome/common/extensions/api/extension_api_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -706,7 +706,7 @@ TEST(ExtensionAPITest, DefaultConfigurationFeatures) {
EXPECT_TRUE(feature->GetContexts()->count(
Feature::BLESSED_EXTENSION_CONTEXT));

EXPECT_EQ(Feature::UNSPECIFIED_LOCATION, feature->location());
EXPECT_EQ(SimpleFeature::UNSPECIFIED_LOCATION, feature->location());
EXPECT_TRUE(feature->platforms()->empty());
EXPECT_EQ(0, feature->min_manifest_version());
EXPECT_EQ(0, feature->max_manifest_version());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ std::string ChromeChannelFeatureFilter::Parse(
Feature::Availability ChromeChannelFeatureFilter::IsAvailableToManifest(
const std::string& extension_id,
Manifest::Type type,
Feature::Location location,
Manifest::Location location,
int manifest_version,
Feature::Platform platfortm) const {
if (channel_has_been_set_ && channel_ < GetCurrentChannel()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ class ChromeChannelFeatureFilter : public SimpleFeatureFilter {
virtual Feature::Availability IsAvailableToManifest(
const std::string& extension_id,
Manifest::Type type,
Feature::Location location,
Manifest::Location location,
int manifest_version,
Feature::Platform platform) const OVERRIDE;

Expand Down
44 changes: 24 additions & 20 deletions extensions/common/features/base_feature_provider_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -170,29 +170,33 @@ TEST(BaseFeatureProviderTest, ComplexFeatures) {
// Make sure both rules are applied correctly.
{
ScopedCurrentChannel current_channel(VersionInfo::CHANNEL_BETA);
EXPECT_EQ(Feature::IS_AVAILABLE, feature->IsAvailableToManifest(
"1",
Manifest::TYPE_EXTENSION,
Feature::UNSPECIFIED_LOCATION,
Feature::UNSPECIFIED_PLATFORM).result());
EXPECT_EQ(Feature::IS_AVAILABLE, feature->IsAvailableToManifest(
"2",
Manifest::TYPE_LEGACY_PACKAGED_APP,
Feature::UNSPECIFIED_LOCATION,
Feature::UNSPECIFIED_PLATFORM).result());
EXPECT_EQ(
Feature::IS_AVAILABLE,
feature->IsAvailableToManifest("1",
Manifest::TYPE_EXTENSION,
Manifest::INVALID_LOCATION,
Feature::UNSPECIFIED_PLATFORM).result());
EXPECT_EQ(
Feature::IS_AVAILABLE,
feature->IsAvailableToManifest("2",
Manifest::TYPE_LEGACY_PACKAGED_APP,
Manifest::INVALID_LOCATION,
Feature::UNSPECIFIED_PLATFORM).result());
}
{
ScopedCurrentChannel current_channel(VersionInfo::CHANNEL_STABLE);
EXPECT_NE(Feature::IS_AVAILABLE, feature->IsAvailableToManifest(
"1",
Manifest::TYPE_EXTENSION,
Feature::UNSPECIFIED_LOCATION,
Feature::UNSPECIFIED_PLATFORM).result());
EXPECT_NE(Feature::IS_AVAILABLE, feature->IsAvailableToManifest(
"2",
Manifest::TYPE_LEGACY_PACKAGED_APP,
Feature::UNSPECIFIED_LOCATION,
Feature::UNSPECIFIED_PLATFORM).result());
EXPECT_NE(
Feature::IS_AVAILABLE,
feature->IsAvailableToManifest("1",
Manifest::TYPE_EXTENSION,
Manifest::INVALID_LOCATION,
Feature::UNSPECIFIED_PLATFORM).result());
EXPECT_NE(
Feature::IS_AVAILABLE,
feature->IsAvailableToManifest("2",
Manifest::TYPE_LEGACY_PACKAGED_APP,
Manifest::INVALID_LOCATION,
Feature::UNSPECIFIED_PLATFORM).result());
}
}

Expand Down
7 changes: 5 additions & 2 deletions extensions/common/features/complex_feature.cc
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,11 @@ ComplexFeature::~ComplexFeature() {
}

Feature::Availability ComplexFeature::IsAvailableToManifest(
const std::string& extension_id, Manifest::Type type, Location location,
int manifest_version, Platform platform) const {
const std::string& extension_id,
Manifest::Type type,
Manifest::Location location,
int manifest_version,
Platform platform) const {
Feature::Availability first_availability =
features_[0]->IsAvailableToManifest(
extension_id, type, location, manifest_version, platform);
Expand Down
2 changes: 1 addition & 1 deletion extensions/common/features/complex_feature.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ class ComplexFeature : public Feature {
// extensions::Feature:
virtual Availability IsAvailableToManifest(const std::string& extension_id,
Manifest::Type type,
Location location,
Manifest::Location location,
int manifest_version,
Platform platform) const OVERRIDE;

Expand Down
90 changes: 49 additions & 41 deletions extensions/common/features/complex_feature_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -70,33 +70,38 @@ TEST_F(ExtensionComplexFeatureTest, MultipleRulesWhitelist) {
scoped_ptr<ComplexFeature> feature(new ComplexFeature(features.Pass()));

// Test match 1st rule.
EXPECT_EQ(Feature::IS_AVAILABLE, feature->IsAvailableToManifest(
kIdFoo,
Manifest::TYPE_EXTENSION,
Feature::UNSPECIFIED_LOCATION,
Feature::UNSPECIFIED_PLATFORM,
Feature::GetCurrentPlatform()).result());
EXPECT_EQ(
Feature::IS_AVAILABLE,
feature->IsAvailableToManifest(kIdFoo,
Manifest::TYPE_EXTENSION,
Manifest::INVALID_LOCATION,
Feature::UNSPECIFIED_PLATFORM,
Feature::GetCurrentPlatform()).result());

// Test match 2nd rule.
EXPECT_EQ(Feature::IS_AVAILABLE, feature->IsAvailableToManifest(
kIdBar,
Manifest::TYPE_LEGACY_PACKAGED_APP,
Feature::UNSPECIFIED_LOCATION,
Feature::UNSPECIFIED_PLATFORM,
Feature::GetCurrentPlatform()).result());
EXPECT_EQ(
Feature::IS_AVAILABLE,
feature->IsAvailableToManifest(kIdBar,
Manifest::TYPE_LEGACY_PACKAGED_APP,
Manifest::INVALID_LOCATION,
Feature::UNSPECIFIED_PLATFORM,
Feature::GetCurrentPlatform()).result());

// Test whitelist with wrong extension type.
EXPECT_NE(Feature::IS_AVAILABLE, feature->IsAvailableToManifest(
kIdBar,
Manifest::TYPE_EXTENSION,
Feature::UNSPECIFIED_LOCATION,
Feature::UNSPECIFIED_PLATFORM,
Feature::GetCurrentPlatform()).result());
EXPECT_NE(Feature::IS_AVAILABLE, feature->IsAvailableToManifest(kIdFoo,
Manifest::TYPE_LEGACY_PACKAGED_APP,
Feature::UNSPECIFIED_LOCATION,
Feature::UNSPECIFIED_PLATFORM,
Feature::GetCurrentPlatform()).result());
EXPECT_NE(
Feature::IS_AVAILABLE,
feature->IsAvailableToManifest(kIdBar,
Manifest::TYPE_EXTENSION,
Manifest::INVALID_LOCATION,
Feature::UNSPECIFIED_PLATFORM,
Feature::GetCurrentPlatform()).result());
EXPECT_NE(
Feature::IS_AVAILABLE,
feature->IsAvailableToManifest(kIdFoo,
Manifest::TYPE_LEGACY_PACKAGED_APP,
Manifest::INVALID_LOCATION,
Feature::UNSPECIFIED_PLATFORM,
Feature::GetCurrentPlatform()).result());
}

TEST_F(ExtensionComplexFeatureTest, MultipleRulesChannels) {
Expand Down Expand Up @@ -126,34 +131,37 @@ TEST_F(ExtensionComplexFeatureTest, MultipleRulesChannels) {
// Test match 1st rule.
{
ScopedCurrentChannel current_channel(VersionInfo::CHANNEL_UNKNOWN);
EXPECT_EQ(Feature::IS_AVAILABLE, feature->IsAvailableToManifest(
"1",
Manifest::TYPE_EXTENSION,
Feature::UNSPECIFIED_LOCATION,
Feature::UNSPECIFIED_PLATFORM,
Feature::GetCurrentPlatform()).result());
EXPECT_EQ(
Feature::IS_AVAILABLE,
feature->IsAvailableToManifest("1",
Manifest::TYPE_EXTENSION,
Manifest::INVALID_LOCATION,
Feature::UNSPECIFIED_PLATFORM,
Feature::GetCurrentPlatform()).result());
}

// Test match 2nd rule.
{
ScopedCurrentChannel current_channel(VersionInfo::CHANNEL_BETA);
EXPECT_EQ(Feature::IS_AVAILABLE, feature->IsAvailableToManifest(
"2",
Manifest::TYPE_LEGACY_PACKAGED_APP,
Feature::UNSPECIFIED_LOCATION,
Feature::UNSPECIFIED_PLATFORM,
Feature::GetCurrentPlatform()).result());
EXPECT_EQ(
Feature::IS_AVAILABLE,
feature->IsAvailableToManifest("2",
Manifest::TYPE_LEGACY_PACKAGED_APP,
Manifest::INVALID_LOCATION,
Feature::UNSPECIFIED_PLATFORM,
Feature::GetCurrentPlatform()).result());
}

// Test feature not available to extensions above channel unknown.
{
ScopedCurrentChannel current_channel(VersionInfo::CHANNEL_BETA);
EXPECT_NE(Feature::IS_AVAILABLE, feature->IsAvailableToManifest(
"1",
Manifest::TYPE_EXTENSION,
Feature::UNSPECIFIED_LOCATION,
Feature::UNSPECIFIED_PLATFORM,
Feature::GetCurrentPlatform()).result());
EXPECT_NE(
Feature::IS_AVAILABLE,
feature->IsAvailableToManifest("1",
Manifest::TYPE_EXTENSION,
Manifest::INVALID_LOCATION,
Feature::UNSPECIFIED_PLATFORM,
Feature::GetCurrentPlatform()).result());
}
}

Expand Down
17 changes: 9 additions & 8 deletions extensions/common/features/feature.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include "base/lazy_instance.h"
#include "base/strings/string_util.h"
#include "base/strings/stringprintf.h"
#include "extensions/common/extension.h"

namespace extensions {

Expand All @@ -28,20 +29,20 @@ Feature::Platform Feature::GetCurrentPlatform() {
#endif
}

// static
Feature::Location Feature::ConvertLocation(Manifest::Location location) {
if (location == Manifest::COMPONENT)
return COMPONENT_LOCATION;
else
return UNSPECIFIED_LOCATION;
}

// static
Feature::Availability Feature::CreateAvailability(AvailabilityResult result,
const std::string& message) {
return Availability(result, message);
}

Feature::Availability Feature::IsAvailableToExtension(
const Extension* extension) {
return IsAvailableToManifest(extension->id(),
extension->GetType(),
extension->location(),
extension->manifest_version());
}

Feature::Feature() : no_parent_(false) {}

Feature::~Feature() {}
Expand Down
18 changes: 7 additions & 11 deletions extensions/common/features/feature.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,12 +46,6 @@ class Feature {
BLESSED_WEB_PAGE_CONTEXT,
};

// The location required of extensions the feature is supported in.
enum Location {
UNSPECIFIED_LOCATION,
COMPONENT_LOCATION
};

// The platforms the feature is supported in.
enum Platform {
UNSPECIFIED_PLATFORM,
Expand Down Expand Up @@ -101,6 +95,8 @@ class Feature {
virtual ~Feature();

// Used by ChromeV8Context until the feature system is fully functional.
// TODO(kalman): This is no longer used by ChromeV8Context, so what is the
// comment trying to say?
static Availability CreateAvailability(AvailabilityResult result,
const std::string& message);

Expand All @@ -112,9 +108,6 @@ class Feature {
// Gets the platform the code is currently running on.
static Platform GetCurrentPlatform();

// Gets the Feature::Location value for the specified Manifest::Location.
static Location ConvertLocation(Manifest::Location extension_location);

virtual std::set<Context>* GetContexts() = 0;

// Tests whether this is an internal API or not.
Expand All @@ -127,17 +120,20 @@ class Feature {
// manifest.
Availability IsAvailableToManifest(const std::string& extension_id,
Manifest::Type type,
Location location,
Manifest::Location location,
int manifest_version) const {
return IsAvailableToManifest(extension_id, type, location, manifest_version,
GetCurrentPlatform());
}
virtual Availability IsAvailableToManifest(const std::string& extension_id,
Manifest::Type type,
Location location,
Manifest::Location location,
int manifest_version,
Platform platform) const = 0;

// Returns true if the feature is available to |extension|.
Availability IsAvailableToExtension(const Extension* extension);

// Returns true if the feature is available to be used in the specified
// extension and context.
Availability IsAvailableToContext(const Extension* extension,
Expand Down
39 changes: 28 additions & 11 deletions extensions/common/features/simple_feature.cc
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ struct Mappings {
contexts["web_page"] = Feature::WEB_PAGE_CONTEXT;
contexts["blessed_web_page"] = Feature::BLESSED_WEB_PAGE_CONTEXT;

locations["component"] = Feature::COMPONENT_LOCATION;
locations["component"] = SimpleFeature::COMPONENT_LOCATION;
locations["policy"] = SimpleFeature::POLICY_LOCATION;

platforms["chromeos"] = Feature::CHROMEOS_PLATFORM;
platforms["linux"] = Feature::LINUX_PLATFORM;
Expand All @@ -44,7 +45,7 @@ struct Mappings {

std::map<std::string, Manifest::Type> extension_types;
std::map<std::string, Feature::Context> contexts;
std::map<std::string, Feature::Location> locations;
std::map<std::string, SimpleFeature::Location> locations;
std::map<std::string, Feature::Platform> platforms;
};

Expand Down Expand Up @@ -264,7 +265,7 @@ std::string SimpleFeature::Parse(const base::DictionaryValue* value) {
Feature::Availability SimpleFeature::IsAvailableToManifest(
const std::string& extension_id,
Manifest::Type type,
Location location,
Manifest::Location location,
int manifest_version,
Platform platform) const {
// Check extension type first to avoid granting platform app permissions
Expand All @@ -279,7 +280,8 @@ Feature::Availability SimpleFeature::IsAvailableToManifest(
}

// Component extensions can access any feature.
if (location == COMPONENT_LOCATION)
// TODO(kalman/asargent): Should this match EXTERNAL_COMPONENT too?
if (location == Manifest::COMPONENT)
return CreateAvailability(IS_AVAILABLE, type);

if (!whitelist_.empty()) {
Expand All @@ -298,7 +300,7 @@ Feature::Availability SimpleFeature::IsAvailableToManifest(
}
}

if (location_ != UNSPECIFIED_LOCATION && location_ != location)
if (!MatchesManifestLocation(location))
return CreateAvailability(INVALID_LOCATION, type);

if (!platforms_.empty() &&
Expand Down Expand Up @@ -329,12 +331,11 @@ Feature::Availability SimpleFeature::IsAvailableToContext(
const GURL& url,
SimpleFeature::Platform platform) const {
if (extension) {
Availability result = IsAvailableToManifest(
extension->id(),
extension->GetType(),
ConvertLocation(extension->location()),
extension->manifest_version(),
platform);
Availability result = IsAvailableToManifest(extension->id(),
extension->GetType(),
extension->location(),
extension->manifest_version(),
platform);
if (!result.is_available())
return result;
}
Expand Down Expand Up @@ -479,4 +480,20 @@ bool SimpleFeature::IsIdInWhitelist(const std::string& extension_id,
return false;
}

bool SimpleFeature::MatchesManifestLocation(
Manifest::Location manifest_location) const {
switch (location_) {
case SimpleFeature::UNSPECIFIED_LOCATION:
return true;
case SimpleFeature::COMPONENT_LOCATION:
// TODO(kalman/asargent): Should this include EXTERNAL_COMPONENT too?
return manifest_location == Manifest::COMPONENT;
case SimpleFeature::POLICY_LOCATION:
return manifest_location == Manifest::EXTERNAL_POLICY ||
manifest_location == Manifest::EXTERNAL_POLICY_DOWNLOAD;
}
NOTREACHED();
return false;
}

} // namespace extensions
Loading

0 comments on commit e42ffb1

Please sign in to comment.