Commit 8a7267ff authored by Devlin Cronin's avatar Devlin Cronin Committed by Commit Bot

[Extensions Features] Add a HashedExtensionId construct

Add a simple HashedExtensionId class as a wrapper around a hex-encoded
SHA1 hash of an extension id, and store this in the Manifest class (with
the regular extension id). This allows us to compute the hash of an
extension id once, rather than recomputing it each time we calculate
feature availability.

Previously, we would calculate this hash each time we'd examine a
feature for availability - which happens during extension initialization
and loading, bindings set up, function execution and more. (This was
even worse because we would check the blacklist even if the list was
empty, resulting in calculating the hash for that case no matter what).

Fun Stats (local sampling, so your mileage may vary):
Before, opening Chromium with default extensions resulted in ~350 calls
to crx_file::id_util::HashedIdInHex() in both the browser process *and*
each renderer process (so a minimum of ~700 calls to just open chrome,
with additional tabs triggering an additional ~350 calls).

Now, opening Chromium with default extensions results in <10 calls to
crx_file::id_util::HashedIdInHex() in each process.

With an average run speed (on a debug build [slow] of a Z840 [fast] -
so maybe representative-ish of real situations?) of 0.034ms in the
browser process and 0.303ms in the renderer process, this corresponds
to a 11.9ms speed up in browser startup and 106ms speed up in renderer
startup. Note that each extension adds to this linearly, so if the user
had an extension with content scripts running on each page, this would
save ~700 renderer calls, resulting in a 212ms speed up.

Yay!

Bug: 622035
Bug: 755441

Change-Id: Idd5655982ad03ab55882d7b44a177a41fe1cb02c
Reviewed-on: https://chromium-review.googlesource.com/619663
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: default avatarIstiaque Ahmed <lazyboy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#495984}
parent 3d6de897
...@@ -23,6 +23,7 @@ ...@@ -23,6 +23,7 @@
#include "extensions/browser/extensions_browser_client.h" #include "extensions/browser/extensions_browser_client.h"
#include "extensions/common/features/feature.h" #include "extensions/common/features/feature.h"
#include "extensions/common/features/feature_provider.h" #include "extensions/common/features/feature_provider.h"
#include "extensions/common/hashed_extension_id.h"
namespace extensions { namespace extensions {
...@@ -77,8 +78,10 @@ void ActivityLogAPI::Shutdown() { ...@@ -77,8 +78,10 @@ void ActivityLogAPI::Shutdown() {
// static // static
bool ActivityLogAPI::IsExtensionWhitelisted(const std::string& extension_id) { bool ActivityLogAPI::IsExtensionWhitelisted(const std::string& extension_id) {
return FeatureProvider::GetPermissionFeatures()-> // TODO(devlin): Pass in a HashedExtensionId to avoid this conversion.
GetFeature("activityLogPrivate")->IsIdInWhitelist(extension_id); return FeatureProvider::GetPermissionFeatures()
->GetFeature("activityLogPrivate")
->IsIdInWhitelist(HashedExtensionId(extension_id));
} }
void ActivityLogAPI::OnListenerAdded(const EventListenerInfo& details) { void ActivityLogAPI::OnListenerAdded(const EventListenerInfo& details) {
......
...@@ -148,6 +148,8 @@ if (enable_extensions) { ...@@ -148,6 +148,8 @@ if (enable_extensions) {
"file_util.cc", "file_util.cc",
"file_util.h", "file_util.h",
"guest_view/extensions_guest_view_messages.h", "guest_view/extensions_guest_view_messages.h",
"hashed_extension_id.cc",
"hashed_extension_id.h",
"host_id.cc", "host_id.cc",
"host_id.h", "host_id.h",
"image_util.cc", "image_util.cc",
......
...@@ -392,6 +392,10 @@ const std::string& Extension::id() const { ...@@ -392,6 +392,10 @@ const std::string& Extension::id() const {
return manifest_->extension_id(); return manifest_->extension_id();
} }
const HashedExtensionId& Extension::hashed_id() const {
return manifest_->hashed_id();
}
const std::string Extension::VersionString() const { const std::string Extension::VersionString() const {
return version()->GetString(); return version()->GetString();
} }
...@@ -455,7 +459,7 @@ bool Extension::InitExtensionID(extensions::Manifest* manifest, ...@@ -455,7 +459,7 @@ bool Extension::InitExtensionID(extensions::Manifest* manifest,
int creation_flags, int creation_flags,
base::string16* error) { base::string16* error) {
if (!explicit_id.empty()) { if (!explicit_id.empty()) {
manifest->set_extension_id(explicit_id); manifest->SetExtensionId(explicit_id);
return true; return true;
} }
...@@ -468,7 +472,7 @@ bool Extension::InitExtensionID(extensions::Manifest* manifest, ...@@ -468,7 +472,7 @@ bool Extension::InitExtensionID(extensions::Manifest* manifest,
return false; return false;
} }
std::string extension_id = crx_file::id_util::GenerateId(public_key_bytes); std::string extension_id = crx_file::id_util::GenerateId(public_key_bytes);
manifest->set_extension_id(extension_id); manifest->SetExtensionId(extension_id);
return true; return true;
} }
...@@ -484,7 +488,7 @@ bool Extension::InitExtensionID(extensions::Manifest* manifest, ...@@ -484,7 +488,7 @@ bool Extension::InitExtensionID(extensions::Manifest* manifest,
NOTREACHED() << "Could not create ID from path."; NOTREACHED() << "Could not create ID from path.";
return false; return false;
} }
manifest->set_extension_id(extension_id); manifest->SetExtensionId(extension_id);
return true; return true;
} }
} }
......
...@@ -17,6 +17,7 @@ ...@@ -17,6 +17,7 @@
#include "base/threading/thread_checker.h" #include "base/threading/thread_checker.h"
#include "extensions/common/extension_id.h" #include "extensions/common/extension_id.h"
#include "extensions/common/extension_resource.h" #include "extensions/common/extension_resource.h"
#include "extensions/common/hashed_extension_id.h"
#include "extensions/common/install_warning.h" #include "extensions/common/install_warning.h"
#include "extensions/common/manifest.h" #include "extensions/common/manifest.h"
#include "extensions/common/url_pattern_set.h" #include "extensions/common/url_pattern_set.h"
...@@ -239,6 +240,7 @@ class Extension : public base::RefCountedThreadSafe<Extension> { ...@@ -239,6 +240,7 @@ class Extension : public base::RefCountedThreadSafe<Extension> {
const GURL& url() const { return extension_url_; } const GURL& url() const { return extension_url_; }
Manifest::Location location() const; Manifest::Location location() const;
const ExtensionId& id() const; const ExtensionId& id() const;
const HashedExtensionId& hashed_id() const;
const base::Version* version() const { return version_.get(); } const base::Version* version() const { return version_.get(); }
const std::string& version_name() const { return version_name_; } const std::string& version_name() const { return version_name_; }
const std::string VersionString() const; const std::string VersionString() const;
......
...@@ -33,21 +33,21 @@ ComplexFeature::~ComplexFeature() { ...@@ -33,21 +33,21 @@ ComplexFeature::~ComplexFeature() {
} }
Feature::Availability ComplexFeature::IsAvailableToManifest( Feature::Availability ComplexFeature::IsAvailableToManifest(
const std::string& extension_id, const HashedExtensionId& hashed_id,
Manifest::Type type, Manifest::Type type,
Manifest::Location location, Manifest::Location location,
int manifest_version, int manifest_version,
Platform platform) const { Platform platform) const {
Feature::Availability first_availability = Feature::Availability first_availability =
features_[0]->IsAvailableToManifest( features_[0]->IsAvailableToManifest(hashed_id, type, location,
extension_id, type, location, manifest_version, platform); manifest_version, platform);
if (first_availability.is_available()) if (first_availability.is_available())
return first_availability; return first_availability;
for (FeatureList::const_iterator it = features_.begin() + 1; for (FeatureList::const_iterator it = features_.begin() + 1;
it != features_.end(); ++it) { it != features_.end(); ++it) {
Availability availability = (*it)->IsAvailableToManifest( Availability availability = (*it)->IsAvailableToManifest(
extension_id, type, location, manifest_version, platform); hashed_id, type, location, manifest_version, platform);
if (availability.is_available()) if (availability.is_available())
return availability; return availability;
} }
...@@ -78,21 +78,21 @@ Feature::Availability ComplexFeature::IsAvailableToContext( ...@@ -78,21 +78,21 @@ Feature::Availability ComplexFeature::IsAvailableToContext(
return first_availability; return first_availability;
} }
bool ComplexFeature::IsIdInBlacklist(const std::string& extension_id) const { bool ComplexFeature::IsIdInBlacklist(const HashedExtensionId& hashed_id) const {
for (FeatureList::const_iterator it = features_.begin(); for (FeatureList::const_iterator it = features_.begin();
it != features_.end(); it != features_.end();
++it) { ++it) {
if ((*it)->IsIdInBlacklist(extension_id)) if ((*it)->IsIdInBlacklist(hashed_id))
return true; return true;
} }
return false; return false;
} }
bool ComplexFeature::IsIdInWhitelist(const std::string& extension_id) const { bool ComplexFeature::IsIdInWhitelist(const HashedExtensionId& hashed_id) const {
for (FeatureList::const_iterator it = features_.begin(); for (FeatureList::const_iterator it = features_.begin();
it != features_.end(); it != features_.end();
++it) { ++it) {
if ((*it)->IsIdInWhitelist(extension_id)) if ((*it)->IsIdInWhitelist(hashed_id))
return true; return true;
} }
return false; return false;
......
...@@ -28,7 +28,7 @@ class ComplexFeature : public Feature { ...@@ -28,7 +28,7 @@ class ComplexFeature : public Feature {
~ComplexFeature() override; ~ComplexFeature() override;
// extensions::Feature: // extensions::Feature:
Availability IsAvailableToManifest(const std::string& extension_id, Availability IsAvailableToManifest(const HashedExtensionId& hashed_id,
Manifest::Type type, Manifest::Type type,
Manifest::Location location, Manifest::Location location,
int manifest_version, int manifest_version,
...@@ -39,8 +39,8 @@ class ComplexFeature : public Feature { ...@@ -39,8 +39,8 @@ class ComplexFeature : public Feature {
const GURL& url, const GURL& url,
Platform platform) const override; Platform platform) const override;
bool IsIdInBlacklist(const std::string& extension_id) const override; bool IsIdInBlacklist(const HashedExtensionId& hashed_id) const override;
bool IsIdInWhitelist(const std::string& extension_id) const override; bool IsIdInWhitelist(const HashedExtensionId& hashed_id) const override;
protected: protected:
// Feature: // Feature:
......
...@@ -15,14 +15,14 @@ ...@@ -15,14 +15,14 @@
namespace extensions { namespace extensions {
TEST(ComplexFeatureTest, MultipleRulesWhitelist) { TEST(ComplexFeatureTest, MultipleRulesWhitelist) {
const std::string kIdFoo("fooabbbbccccddddeeeeffffgggghhhh"); const HashedExtensionId kIdFoo("fooabbbbccccddddeeeeffffgggghhhh");
const std::string kIdBar("barabbbbccccddddeeeeffffgggghhhh"); const HashedExtensionId kIdBar("barabbbbccccddddeeeeffffgggghhhh");
std::vector<Feature*> features; std::vector<Feature*> features;
{ {
// Rule: "extension", whitelist "foo". // Rule: "extension", whitelist "foo".
std::unique_ptr<SimpleFeature> simple_feature(new SimpleFeature()); std::unique_ptr<SimpleFeature> simple_feature(new SimpleFeature());
simple_feature->set_whitelist({kIdFoo.c_str()}); simple_feature->set_whitelist({kIdFoo.value().c_str()});
simple_feature->set_extension_types({Manifest::TYPE_EXTENSION}); simple_feature->set_extension_types({Manifest::TYPE_EXTENSION});
features.push_back(simple_feature.release()); features.push_back(simple_feature.release());
} }
...@@ -30,7 +30,7 @@ TEST(ComplexFeatureTest, MultipleRulesWhitelist) { ...@@ -30,7 +30,7 @@ TEST(ComplexFeatureTest, MultipleRulesWhitelist) {
{ {
// Rule: "legacy_packaged_app", whitelist "bar". // Rule: "legacy_packaged_app", whitelist "bar".
std::unique_ptr<SimpleFeature> simple_feature(new SimpleFeature()); std::unique_ptr<SimpleFeature> simple_feature(new SimpleFeature());
simple_feature->set_whitelist({kIdBar.c_str()}); simple_feature->set_whitelist({kIdBar.value().c_str()});
simple_feature->set_extension_types({Manifest::TYPE_LEGACY_PACKAGED_APP}); simple_feature->set_extension_types({Manifest::TYPE_LEGACY_PACKAGED_APP});
features.push_back(simple_feature.release()); features.push_back(simple_feature.release());
} }
...@@ -94,31 +94,34 @@ TEST(ComplexFeatureTest, Dependencies) { ...@@ -94,31 +94,34 @@ TEST(ComplexFeatureTest, Dependencies) {
std::unique_ptr<ComplexFeature> feature(new ComplexFeature(&features)); std::unique_ptr<ComplexFeature> feature(new ComplexFeature(&features));
// Available to extensions because of the content_security_policy rule. // Available to extensions because of the content_security_policy rule.
EXPECT_EQ( EXPECT_EQ(Feature::IS_AVAILABLE,
Feature::IS_AVAILABLE, feature
feature->IsAvailableToManifest("extensionid", ->IsAvailableToManifest(HashedExtensionId(std::string(32, 'a')),
Manifest::TYPE_EXTENSION, Manifest::TYPE_EXTENSION,
Manifest::INVALID_LOCATION, Manifest::INVALID_LOCATION,
Feature::UNSPECIFIED_PLATFORM, Feature::UNSPECIFIED_PLATFORM,
Feature::GetCurrentPlatform()).result()); Feature::GetCurrentPlatform())
.result());
// Available to platform apps because of the serial rule. // Available to platform apps because of the serial rule.
EXPECT_EQ( EXPECT_EQ(Feature::IS_AVAILABLE,
Feature::IS_AVAILABLE, feature
feature->IsAvailableToManifest("platformappid", ->IsAvailableToManifest(HashedExtensionId(std::string(32, 'b')),
Manifest::TYPE_PLATFORM_APP, Manifest::TYPE_PLATFORM_APP,
Manifest::INVALID_LOCATION, Manifest::INVALID_LOCATION,
Feature::UNSPECIFIED_PLATFORM, Feature::UNSPECIFIED_PLATFORM,
Feature::GetCurrentPlatform()).result()); Feature::GetCurrentPlatform())
.result());
// Not available to hosted apps. // Not available to hosted apps.
EXPECT_EQ( EXPECT_EQ(Feature::INVALID_TYPE,
Feature::INVALID_TYPE, feature
feature->IsAvailableToManifest("hostedappid", ->IsAvailableToManifest(HashedExtensionId(std::string(32, 'c')),
Manifest::TYPE_HOSTED_APP, Manifest::TYPE_HOSTED_APP,
Manifest::INVALID_LOCATION, Manifest::INVALID_LOCATION,
Feature::UNSPECIFIED_PLATFORM, Feature::UNSPECIFIED_PLATFORM,
Feature::GetCurrentPlatform()).result()); Feature::GetCurrentPlatform())
.result());
} }
} // namespace extensions } // namespace extensions
...@@ -33,15 +33,13 @@ Feature::Platform Feature::GetCurrentPlatform() { ...@@ -33,15 +33,13 @@ Feature::Platform Feature::GetCurrentPlatform() {
Feature::Availability Feature::IsAvailableToExtension( Feature::Availability Feature::IsAvailableToExtension(
const Extension* extension) const { const Extension* extension) const {
return IsAvailableToManifest(extension->id(), return IsAvailableToManifest(extension->hashed_id(), extension->GetType(),
extension->GetType(),
extension->location(), extension->location(),
extension->manifest_version()); extension->manifest_version());
} }
Feature::Availability Feature::IsAvailableToEnvironment() const { Feature::Availability Feature::IsAvailableToEnvironment() const {
return IsAvailableToManifest("", // extension_id return IsAvailableToManifest(HashedExtensionId(), Manifest::TYPE_UNKNOWN,
Manifest::TYPE_UNKNOWN,
Manifest::INVALID_LOCATION, Manifest::INVALID_LOCATION,
-1); // manifest_version -1); // manifest_version
} }
......
...@@ -10,6 +10,7 @@ ...@@ -10,6 +10,7 @@
#include "base/strings/string_piece.h" #include "base/strings/string_piece.h"
#include "base/values.h" #include "base/values.h"
#include "extensions/common/hashed_extension_id.h"
#include "extensions/common/manifest.h" #include "extensions/common/manifest.h"
class GURL; class GURL;
...@@ -109,14 +110,14 @@ class Feature { ...@@ -109,14 +110,14 @@ class Feature {
// Returns true if the feature is available to be parsed into a new extension // Returns true if the feature is available to be parsed into a new extension
// manifest. // manifest.
Availability IsAvailableToManifest(const std::string& extension_id, Availability IsAvailableToManifest(const HashedExtensionId& hashed_id,
Manifest::Type type, Manifest::Type type,
Manifest::Location location, Manifest::Location location,
int manifest_version) const { int manifest_version) const {
return IsAvailableToManifest(extension_id, type, location, manifest_version, return IsAvailableToManifest(hashed_id, type, location, manifest_version,
GetCurrentPlatform()); GetCurrentPlatform());
} }
virtual Availability IsAvailableToManifest(const std::string& extension_id, virtual Availability IsAvailableToManifest(const HashedExtensionId& hashed_id,
Manifest::Type type, Manifest::Type type,
Manifest::Location location, Manifest::Location location,
int manifest_version, int manifest_version,
...@@ -148,8 +149,8 @@ class Feature { ...@@ -148,8 +149,8 @@ class Feature {
// method instead. // method instead.
Availability IsAvailableToEnvironment() const; Availability IsAvailableToEnvironment() const;
virtual bool IsIdInBlacklist(const std::string& extension_id) const = 0; virtual bool IsIdInBlacklist(const HashedExtensionId& hashed_id) const = 0;
virtual bool IsIdInWhitelist(const std::string& extension_id) const = 0; virtual bool IsIdInWhitelist(const HashedExtensionId& hashed_id) const = 0;
protected: protected:
std::string name_; std::string name_;
......
...@@ -32,10 +32,10 @@ namespace { ...@@ -32,10 +32,10 @@ namespace {
struct WhitelistInfo { struct WhitelistInfo {
WhitelistInfo() WhitelistInfo()
: extension_id( : hashed_id(HashedIdInHex(
base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII( base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
switches::kWhitelistedExtensionID)) {} switches::kWhitelistedExtensionID))) {}
std::string extension_id; std::string hashed_id;
}; };
// A singleton copy of the --whitelisted-extension-id so that we don't need to // A singleton copy of the --whitelisted-extension-id so that we don't need to
// copy it from the CommandLine each time. // copy it from the CommandLine each time.
...@@ -43,14 +43,14 @@ base::LazyInstance<WhitelistInfo>::Leaky g_whitelist_info = ...@@ -43,14 +43,14 @@ base::LazyInstance<WhitelistInfo>::Leaky g_whitelist_info =
LAZY_INSTANCE_INITIALIZER; LAZY_INSTANCE_INITIALIZER;
Feature::Availability IsAvailableToManifestForBind( Feature::Availability IsAvailableToManifestForBind(
const std::string& extension_id, const HashedExtensionId& hashed_id,
Manifest::Type type, Manifest::Type type,
Manifest::Location location, Manifest::Location location,
int manifest_version, int manifest_version,
Feature::Platform platform, Feature::Platform platform,
const Feature* feature) { const Feature* feature) {
return feature->IsAvailableToManifest( return feature->IsAvailableToManifest(hashed_id, type, location,
extension_id, type, location, manifest_version, platform); manifest_version, platform);
} }
Feature::Availability IsAvailableToContextForBind(const Extension* extension, Feature::Availability IsAvailableToContextForBind(const Extension* extension,
...@@ -183,28 +183,26 @@ bool IsCommandLineSwitchEnabled(base::CommandLine* command_line, ...@@ -183,28 +183,26 @@ bool IsCommandLineSwitchEnabled(base::CommandLine* command_line,
return false; return false;
} }
bool IsWhitelistedForTest(const std::string& extension_id) { bool IsWhitelistedForTest(const HashedExtensionId& hashed_id) {
// TODO(jackhou): Delete the commandline whitelisting mechanism. // TODO(jackhou): Delete the commandline whitelisting mechanism.
// Since it is only used it tests, ideally it should not be set via the // Since it is only used it tests, ideally it should not be set via the
// commandline. At the moment the commandline is used as a mechanism to pass // commandline. At the moment the commandline is used as a mechanism to pass
// the id to the renderer process. // the id to the renderer process.
const std::string& whitelisted_extension_id = const std::string& whitelisted_id = g_whitelist_info.Get().hashed_id;
g_whitelist_info.Get().extension_id; return !whitelisted_id.empty() && whitelisted_id == hashed_id.value();
return !whitelisted_extension_id.empty() &&
whitelisted_extension_id == extension_id;
} }
} // namespace } // namespace
SimpleFeature::ScopedThreadUnsafeWhitelistForTest:: SimpleFeature::ScopedThreadUnsafeWhitelistForTest::
ScopedThreadUnsafeWhitelistForTest(const std::string& id) ScopedThreadUnsafeWhitelistForTest(const std::string& id)
: previous_id_(g_whitelist_info.Get().extension_id) { : previous_id_(g_whitelist_info.Get().hashed_id) {
g_whitelist_info.Get().extension_id = id; g_whitelist_info.Get().hashed_id = HashedIdInHex(id);
} }
SimpleFeature::ScopedThreadUnsafeWhitelistForTest:: SimpleFeature::ScopedThreadUnsafeWhitelistForTest::
~ScopedThreadUnsafeWhitelistForTest() { ~ScopedThreadUnsafeWhitelistForTest() {
g_whitelist_info.Get().extension_id = previous_id_; g_whitelist_info.Get().hashed_id = previous_id_;
} }
SimpleFeature::SimpleFeature() SimpleFeature::SimpleFeature()
...@@ -217,7 +215,7 @@ SimpleFeature::SimpleFeature() ...@@ -217,7 +215,7 @@ SimpleFeature::SimpleFeature()
SimpleFeature::~SimpleFeature() {} SimpleFeature::~SimpleFeature() {}
Feature::Availability SimpleFeature::IsAvailableToManifest( Feature::Availability SimpleFeature::IsAvailableToManifest(
const std::string& extension_id, const HashedExtensionId& hashed_id,
Manifest::Type type, Manifest::Type type,
Manifest::Location location, Manifest::Location location,
int manifest_version, int manifest_version,
...@@ -228,15 +226,12 @@ Feature::Availability SimpleFeature::IsAvailableToManifest( ...@@ -228,15 +226,12 @@ Feature::Availability SimpleFeature::IsAvailableToManifest(
if (!environment_availability.is_available()) if (!environment_availability.is_available())
return environment_availability; return environment_availability;
Availability manifest_availability = Availability manifest_availability =
GetManifestAvailability(extension_id, type, location, manifest_version); GetManifestAvailability(hashed_id, type, location, manifest_version);
if (!manifest_availability.is_available()) if (!manifest_availability.is_available())
return manifest_availability; return manifest_availability;
return CheckDependencies(base::Bind(&IsAvailableToManifestForBind, return CheckDependencies(base::Bind(&IsAvailableToManifestForBind, hashed_id,
extension_id, type, location, manifest_version,
type,
location,
manifest_version,
platform)); platform));
} }
...@@ -253,7 +248,7 @@ Feature::Availability SimpleFeature::IsAvailableToContext( ...@@ -253,7 +248,7 @@ Feature::Availability SimpleFeature::IsAvailableToContext(
if (extension) { if (extension) {
Availability manifest_availability = GetManifestAvailability( Availability manifest_availability = GetManifestAvailability(
extension->id(), extension->GetType(), extension->location(), extension->hashed_id(), extension->GetType(), extension->location(),
extension->manifest_version()); extension->manifest_version());
if (!manifest_availability.is_available()) if (!manifest_availability.is_available())
return manifest_availability; return manifest_availability;
...@@ -403,12 +398,12 @@ bool SimpleFeature::IsInternal() const { ...@@ -403,12 +398,12 @@ bool SimpleFeature::IsInternal() const {
return is_internal_; return is_internal_;
} }
bool SimpleFeature::IsIdInBlacklist(const std::string& extension_id) const { bool SimpleFeature::IsIdInBlacklist(const HashedExtensionId& hashed_id) const {
return IsIdInList(extension_id, blacklist_); return IsIdInList(hashed_id, blacklist_);
} }
bool SimpleFeature::IsIdInWhitelist(const std::string& extension_id) const { bool SimpleFeature::IsIdInWhitelist(const HashedExtensionId& hashed_id) const {
return IsIdInList(extension_id, whitelist_); return IsIdInList(hashed_id, whitelist_);
} }
// static // static
...@@ -426,13 +421,12 @@ bool SimpleFeature::IsIdInArray(const std::string& extension_id, ...@@ -426,13 +421,12 @@ bool SimpleFeature::IsIdInArray(const std::string& extension_id,
} }
// static // static
bool SimpleFeature::IsIdInList(const std::string& extension_id, bool SimpleFeature::IsIdInList(const HashedExtensionId& hashed_id,
const std::vector<std::string>& list) { const std::vector<std::string>& list) {
if (!IsValidExtensionId(extension_id)) if (!IsValidHashedExtensionId(hashed_id))
return false; return false;
return (base::ContainsValue(list, extension_id) || return base::ContainsValue(list, hashed_id.value());
base::ContainsValue(list, HashedIdInHex(extension_id)));
} }
bool SimpleFeature::MatchesManifestLocation( bool SimpleFeature::MatchesManifestLocation(
...@@ -490,6 +484,13 @@ bool SimpleFeature::IsValidExtensionId(const std::string& extension_id) { ...@@ -490,6 +484,13 @@ bool SimpleFeature::IsValidExtensionId(const std::string& extension_id) {
return (extension_id.length() == 32); return (extension_id.length() == 32);
} }
// static
bool SimpleFeature::IsValidHashedExtensionId(
const HashedExtensionId& hashed_id) {
// As above, just the bare-bones check.
return hashed_id.value().length() == 40;
}
void SimpleFeature::set_blacklist( void SimpleFeature::set_blacklist(
std::initializer_list<const char* const> blacklist) { std::initializer_list<const char* const> blacklist) {
blacklist_.assign(blacklist.begin(), blacklist.end()); blacklist_.assign(blacklist.begin(), blacklist.end());
...@@ -558,7 +559,7 @@ Feature::Availability SimpleFeature::GetEnvironmentAvailability( ...@@ -558,7 +559,7 @@ Feature::Availability SimpleFeature::GetEnvironmentAvailability(
} }
Feature::Availability SimpleFeature::GetManifestAvailability( Feature::Availability SimpleFeature::GetManifestAvailability(
const std::string& extension_id, const HashedExtensionId& hashed_id,
Manifest::Type type, Manifest::Type type,
Manifest::Location location, Manifest::Location location,
int manifest_version) const { int manifest_version) const {
...@@ -573,7 +574,7 @@ Feature::Availability SimpleFeature::GetManifestAvailability( ...@@ -573,7 +574,7 @@ Feature::Availability SimpleFeature::GetManifestAvailability(
return CreateAvailability(INVALID_TYPE, type); return CreateAvailability(INVALID_TYPE, type);
} }
if (IsIdInBlacklist(extension_id)) if (!blacklist_.empty() && IsIdInBlacklist(hashed_id))
return CreateAvailability(FOUND_IN_BLACKLIST); return CreateAvailability(FOUND_IN_BLACKLIST);
// TODO(benwells): don't grant all component extensions. // TODO(benwells): don't grant all component extensions.
...@@ -583,8 +584,8 @@ Feature::Availability SimpleFeature::GetManifestAvailability( ...@@ -583,8 +584,8 @@ Feature::Availability SimpleFeature::GetManifestAvailability(
if (component_extensions_auto_granted_ && location == Manifest::COMPONENT) if (component_extensions_auto_granted_ && location == Manifest::COMPONENT)
return CreateAvailability(IS_AVAILABLE); return CreateAvailability(IS_AVAILABLE);
if (!whitelist_.empty() && !IsIdInWhitelist(extension_id) && if (!whitelist_.empty() && !IsIdInWhitelist(hashed_id) &&
!IsWhitelistedForTest(extension_id)) { !IsWhitelistedForTest(hashed_id)) {
return CreateAvailability(NOT_FOUND_IN_WHITELIST); return CreateAvailability(NOT_FOUND_IN_WHITELIST);
} }
......
...@@ -70,7 +70,7 @@ class SimpleFeature : public Feature { ...@@ -70,7 +70,7 @@ class SimpleFeature : public Feature {
} }
// extension::Feature: // extension::Feature:
Availability IsAvailableToManifest(const std::string& extension_id, Availability IsAvailableToManifest(const HashedExtensionId& hashed_id,
Manifest::Type type, Manifest::Type type,
Manifest::Location location, Manifest::Location location,
int manifest_version, int manifest_version,
...@@ -80,8 +80,8 @@ class SimpleFeature : public Feature { ...@@ -80,8 +80,8 @@ class SimpleFeature : public Feature {
const GURL& url, const GURL& url,
Platform platform) const override; Platform platform) const override;
bool IsInternal() const override; bool IsInternal() const override;
bool IsIdInBlacklist(const std::string& extension_id) const override; bool IsIdInBlacklist(const HashedExtensionId& hashed_id) const override;
bool IsIdInWhitelist(const std::string& extension_id) const override; bool IsIdInWhitelist(const HashedExtensionId& hashed_id) const override;
static bool IsIdInArray(const std::string& extension_id, static bool IsIdInArray(const std::string& extension_id,
const char* const array[], const char* const array[],
...@@ -207,7 +207,7 @@ class SimpleFeature : public Feature { ...@@ -207,7 +207,7 @@ class SimpleFeature : public Feature {
// Holds String to Enum value mappings. // Holds String to Enum value mappings.
struct Mappings; struct Mappings;
static bool IsIdInList(const std::string& extension_id, static bool IsIdInList(const HashedExtensionId& hashed_id,
const std::vector<std::string>& list); const std::vector<std::string>& list);
bool MatchesManifestLocation(Manifest::Location manifest_location) const; bool MatchesManifestLocation(Manifest::Location manifest_location) const;
...@@ -220,6 +220,7 @@ class SimpleFeature : public Feature { ...@@ -220,6 +220,7 @@ class SimpleFeature : public Feature {
const base::Callback<Availability(const Feature*)>& checker) const; const base::Callback<Availability(const Feature*)>& checker) const;
static bool IsValidExtensionId(const std::string& extension_id); static bool IsValidExtensionId(const std::string& extension_id);
static bool IsValidHashedExtensionId(const HashedExtensionId& hashed_id);
// Returns the availability of the feature with respect to the basic // Returns the availability of the feature with respect to the basic
// environment Chrome is running in. // environment Chrome is running in.
...@@ -231,7 +232,7 @@ class SimpleFeature : public Feature { ...@@ -231,7 +232,7 @@ class SimpleFeature : public Feature {
// Returns the availability of the feature with respect to a given extension's // Returns the availability of the feature with respect to a given extension's
// properties. // properties.
Availability GetManifestAvailability(const std::string& extension_id, Availability GetManifestAvailability(const HashedExtensionId& hashed_id,
Manifest::Type type, Manifest::Type type,
Manifest::Location location, Manifest::Location location,
int manifest_version) const; int manifest_version) const;
......
// Copyright 2017 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "extensions/common/hashed_extension_id.h"
#include "components/crx_file/id_util.h"
namespace extensions {
HashedExtensionId::HashedExtensionId() {}
HashedExtensionId::HashedExtensionId(const ExtensionId& original_id)
: value_(crx_file::id_util::HashedIdInHex(original_id)) {}
HashedExtensionId::HashedExtensionId(HashedExtensionId&& other) = default;
HashedExtensionId::HashedExtensionId(const HashedExtensionId& other) = default;
HashedExtensionId& HashedExtensionId::operator=(HashedExtensionId&& other) =
default;
HashedExtensionId& HashedExtensionId::operator=(
const HashedExtensionId& other) = default;
} // namespace extensions
// Copyright 2017 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#ifndef EXTENSIONS_COMMON_HASHED_EXTENSION_ID_H_
#define EXTENSIONS_COMMON_HASHED_EXTENSION_ID_H_
#include <string>
#include "extensions/common/extension_id.h"
namespace extensions {
// A wrapper around a hex-encoded SHA1 hash of an extension ID. This struct is
// primarily to enforce type-safety, but also offers handy construction. The
// hashed ID of an extension is used to determine feature availability.
class HashedExtensionId {
public:
// Default constructor to initialize with an empty value. It'd be nice to get
// rid of this, but certain objects (like Manifest) don't have a valid ID at
// construction.
HashedExtensionId();
// Initialize a HashedExtensionId, given the original.
explicit HashedExtensionId(const ExtensionId& original_id);
HashedExtensionId(HashedExtensionId&& other);
HashedExtensionId(const HashedExtensionId& other);
HashedExtensionId& operator=(HashedExtensionId&& other);
HashedExtensionId& operator=(const HashedExtensionId& other);
const std::string& value() const { return value_; }
private:
// Not const to allow for assignment.
std::string value_;
};
} // namespace extensions
#endif // EXTENSIONS_COMMON_HASHED_EXTENSION_ID_H_
...@@ -11,6 +11,7 @@ ...@@ -11,6 +11,7 @@
#include "base/strings/string_split.h" #include "base/strings/string_split.h"
#include "base/strings/stringprintf.h" #include "base/strings/stringprintf.h"
#include "base/strings/utf_string_conversions.h" #include "base/strings/utf_string_conversions.h"
#include "components/crx_file/id_util.h"
#include "extensions/common/error_utils.h" #include "extensions/common/error_utils.h"
#include "extensions/common/features/feature.h" #include "extensions/common/features/feature.h"
#include "extensions/common/features/feature_provider.h" #include "extensions/common/features/feature_provider.h"
...@@ -134,6 +135,11 @@ Manifest::Manifest(Location location, ...@@ -134,6 +135,11 @@ Manifest::Manifest(Location location,
Manifest::~Manifest() { Manifest::~Manifest() {
} }
void Manifest::SetExtensionId(const ExtensionId& id) {
extension_id_ = id;
hashed_id_ = HashedExtensionId(id);
}
bool Manifest::ValidateManifest( bool Manifest::ValidateManifest(
std::string* error, std::string* error,
std::vector<InstallWarning>* warnings) const { std::vector<InstallWarning>* warnings) const {
...@@ -153,7 +159,7 @@ bool Manifest::ValidateManifest( ...@@ -153,7 +159,7 @@ bool Manifest::ValidateManifest(
continue; continue;
Feature::Availability result = map_entry.second->IsAvailableToManifest( Feature::Availability result = map_entry.second->IsAvailableToManifest(
extension_id_, type_, location_, GetManifestVersion()); hashed_id_, type_, location_, GetManifestVersion());
if (!result.is_available()) if (!result.is_available())
warnings->push_back(InstallWarning(result.message(), map_entry.first)); warnings->push_back(InstallWarning(result.message(), map_entry.first));
} }
...@@ -218,7 +224,7 @@ bool Manifest::GetList( ...@@ -218,7 +224,7 @@ bool Manifest::GetList(
Manifest* Manifest::DeepCopy() const { Manifest* Manifest::DeepCopy() const {
Manifest* manifest = new Manifest( Manifest* manifest = new Manifest(
location_, std::unique_ptr<base::DictionaryValue>(value_->DeepCopy())); location_, std::unique_ptr<base::DictionaryValue>(value_->DeepCopy()));
manifest->set_extension_id(extension_id_); manifest->SetExtensionId(extension_id_);
return manifest; return manifest;
} }
...@@ -252,8 +258,9 @@ bool Manifest::CanAccessKey(const std::string& key) const { ...@@ -252,8 +258,9 @@ bool Manifest::CanAccessKey(const std::string& key) const {
if (!feature) if (!feature)
return true; return true;
return feature->IsAvailableToManifest( return feature
extension_id_, type_, location_, GetManifestVersion()) ->IsAvailableToManifest(hashed_id_, type_, location_,
GetManifestVersion())
.is_available(); .is_available();
} }
......
...@@ -13,6 +13,8 @@ ...@@ -13,6 +13,8 @@
#include "base/macros.h" #include "base/macros.h"
#include "base/strings/string16.h" #include "base/strings/string16.h"
#include "base/values.h" #include "base/values.h"
#include "extensions/common/extension_id.h"
#include "extensions/common/hashed_extension_id.h"
namespace extensions { namespace extensions {
struct InstallWarning; struct InstallWarning;
...@@ -123,8 +125,10 @@ class Manifest { ...@@ -123,8 +125,10 @@ class Manifest {
Manifest(Location location, std::unique_ptr<base::DictionaryValue> value); Manifest(Location location, std::unique_ptr<base::DictionaryValue> value);
virtual ~Manifest(); virtual ~Manifest();
const std::string& extension_id() const { return extension_id_; } void SetExtensionId(const ExtensionId& id);
void set_extension_id(const std::string& id) { extension_id_ = id; }
const ExtensionId& extension_id() const { return extension_id_; }
const HashedExtensionId& hashed_id() const { return hashed_id_; }
Location location() const { return location_; } Location location() const { return location_; }
...@@ -191,6 +195,10 @@ class Manifest { ...@@ -191,6 +195,10 @@ class Manifest {
// key, or as a hash of the path in the case of unpacked extensions. // key, or as a hash of the path in the case of unpacked extensions.
std::string extension_id_; std::string extension_id_;
// The hex-encoding of the SHA1 of the extension id; used to determine feature
// availability.
HashedExtensionId hashed_id_;
// The location the extension was loaded from. // The location the extension was loaded from.
Location location_; Location location_;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment