Skip to content

Commit

Permalink
DeclarativeNetRequest: Remove IsAPIAvailable checks.
Browse files Browse the repository at this point in the history
The API is available to stable since M84. Also, we should generally be
relying on checking feature availability to extensions instead of
blanket api availability checks.

BUG=696822

Change-Id: If6917abbe89268bf143949157a00978994ee6cb2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2629112
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Commit-Queue: Karan Bhatia <karandeepb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#847438}
  • Loading branch information
Karandeep Bhatia authored and Chromium LUCI CQ committed Jan 27, 2021
1 parent 2208d93 commit d92523f
Show file tree
Hide file tree
Showing 10 changed files with 19 additions and 70 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
#include "extensions/browser/api/declarative_net_request/utils.h"
#include "extensions/common/api/declarative_net_request.h"
#include "extensions/common/api/declarative_net_request/dnr_manifest_data.h"
#include "extensions/common/api/declarative_net_request/utils.h"
#include "extensions/common/error_utils.h"
#include "extensions/common/file_util.h"
#include "extensions/common/install_warning.h"
Expand Down Expand Up @@ -398,17 +397,13 @@ FileBackedRulesetSource FileBackedRulesetSource::Clone() const {

IndexAndPersistJSONRulesetResult
FileBackedRulesetSource::IndexAndPersistJSONRulesetUnsafe() const {
DCHECK(IsAPIAvailable());

base::ElapsedTimer timer;
return IndexAndPersistRuleset(*this, ReadJSONRulesUnsafe(), timer);
}

void FileBackedRulesetSource::IndexAndPersistJSONRuleset(
data_decoder::DataDecoder* decoder,
IndexAndPersistJSONRulesetCallback callback) const {
DCHECK(IsAPIAvailable());

if (!base::PathExists(json_path_)) {
std::move(callback).Run(IndexAndPersistJSONRulesetResult::CreateErrorResult(
GetErrorWithFilename(json_path_, kFileDoesNotExistError)));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
#include "extensions/browser/api/declarative_net_request/constants.h"
#include "extensions/browser/api/declarative_net_request/utils.h"
#include "extensions/common/api/declarative_net_request.h"
#include "extensions/common/api/declarative_net_request/utils.h"
#include "net/http/http_util.h"
#include "third_party/re2/src/re2/re2.h"
#include "url/gurl.h"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,9 @@
#include "extensions/common/api/declarative_net_request.h"
#include "extensions/common/api/declarative_net_request/constants.h"
#include "extensions/common/api/declarative_net_request/dnr_manifest_data.h"
#include "extensions/common/api/declarative_net_request/utils.h"
#include "extensions/common/error_utils.h"
#include "extensions/common/extension_id.h"
#include "extensions/common/permissions/api_permission.h"
#include "tools/json_schema_compiler/util.h"

namespace extensions {
Expand All @@ -72,6 +72,11 @@ void LogLoadRulesetResult(LoadRulesetResult result) {
UMA_HISTOGRAM_ENUMERATION(kLoadRulesetResultHistogram, result);
}

bool HasAPIPermission(const Extension& extension) {
return extension.permissions_data()->HasAPIPermission(
APIPermission::kDeclarativeNetRequest);
}

// Returns whether the extension's allocation should be released. This would
// return true for cases where we expect the extension to be unloaded for a
// while or if the extension directory's contents changed in a reload.
Expand Down Expand Up @@ -393,12 +398,7 @@ RulesMonitorService::RulesMonitorService(
ruleset_manager_(browser_context),
action_tracker_(browser_context),
global_rules_tracker_(prefs_, extension_registry_) {
// Don't monitor extension lifecycle if the API is not available. This is
// useful since we base some of our actions (like loading dynamic ruleset on
// extension load) on the presence of certain extension prefs. These may still
// be remaining from an earlier install on which the feature was available.
if (IsAPIAvailable())
registry_observer_.Add(extension_registry_);
registry_observer_.Add(extension_registry_);
}

RulesMonitorService::~RulesMonitorService() = default;
Expand All @@ -425,6 +425,9 @@ void RulesMonitorService::OnExtensionWillBeInstalled(
const Extension* extension,
bool is_update,
const std::string& old_name) {
if (!HasAPIPermission(*extension))
return;

if (!is_update || Manifest::IsUnpackedLocation(extension->location()))
return;

Expand All @@ -442,6 +445,9 @@ void RulesMonitorService::OnExtensionLoaded(
const Extension* extension) {
DCHECK_EQ(context_, browser_context);

if (!HasAPIPermission(*extension))
return;

LoadRequestData load_data(extension->id());
int expected_ruleset_checksum;

Expand Down Expand Up @@ -511,6 +517,9 @@ void RulesMonitorService::OnExtensionUnloaded(
UnloadedExtensionReason reason) {
DCHECK_EQ(context_, browser_context);

if (!HasAPIPermission(*extension))
return;

// If the extension is unloaded for any reason other than an update, the
// unused rule allocation should not be kept for this extension the next
// time its rulesets are loaded, as it is no longer "the first load after an
Expand Down Expand Up @@ -539,6 +548,9 @@ void RulesMonitorService::OnExtensionUninstalled(
UninstallReason reason) {
DCHECK_EQ(context_, browser_context);

if (!HasAPIPermission(*extension))
return;

session_rules_.erase(extension->id());

// Skip if the extension will be reinstalled soon.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
#include "extensions/browser/extension_prefs.h"
#include "extensions/browser/extension_util.h"
#include "extensions/common/api/declarative_net_request.h"
#include "extensions/common/api/declarative_net_request/utils.h"
#include "extensions/common/constants.h"
#include "url/origin.h"

Expand Down Expand Up @@ -82,7 +81,6 @@ RulesetManager::~RulesetManager() {
void RulesetManager::AddRuleset(const ExtensionId& extension_id,
std::unique_ptr<CompositeMatcher> matcher) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
DCHECK(IsAPIAvailable());

bool inserted =
rulesets_
Expand All @@ -101,7 +99,6 @@ void RulesetManager::AddRuleset(const ExtensionId& extension_id,

void RulesetManager::RemoveRuleset(const ExtensionId& extension_id) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
DCHECK(IsAPIAvailable());

auto compare_by_id =
[&extension_id](const ExtensionRulesetData& ruleset_data) {
Expand Down Expand Up @@ -131,7 +128,6 @@ std::set<ExtensionId> RulesetManager::GetExtensionsWithRulesets() const {
CompositeMatcher* RulesetManager::GetMatcherForExtension(
const ExtensionId& extension_id) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
DCHECK(IsAPIAvailable());

// This is O(n) but it's ok since the number of extensions will be small and
// we have to maintain the rulesets sorted in decreasing order of installation
Expand Down Expand Up @@ -429,11 +425,6 @@ bool RulesetManager::ShouldEvaluateRequest(
// Ensure clients filter out sensitive requests.
DCHECK(!WebRequestPermissions::HideRequest(permission_helper_, request));

if (!IsAPIAvailable()) {
DCHECK(rulesets_.empty());
return false;
}

// Prevent extensions from modifying any resources on the chrome-extension
// scheme. Practically, this has the effect of not allowing an extension to
// modify its own resources (The extension wouldn't have the permission to
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
#include "extensions/browser/api/declarative_net_request/constants.h"
#include "extensions/browser/api/declarative_net_request/request_action.h"
#include "extensions/browser/api/declarative_net_request/utils.h"
#include "extensions/common/api/declarative_net_request/utils.h"

namespace extensions {
namespace declarative_net_request {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
#include "extensions/browser/api/declarative_net_request/ruleset_matcher.h"
#include "extensions/browser/api/declarative_net_request/utils.h"
#include "extensions/common/api/declarative_net_request/constants.h"
#include "extensions/common/api/declarative_net_request/utils.h"
#include "extensions/common/extension.h"
#include "third_party/flatbuffers/src/include/flatbuffers/flatbuffers.h"
#include "url/gurl.h"
Expand Down
2 changes: 0 additions & 2 deletions extensions/common/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,6 @@ if (enable_extensions) {
"api/declarative_net_request/dnr_manifest_data.h",
"api/declarative_net_request/dnr_manifest_handler.cc",
"api/declarative_net_request/dnr_manifest_handler.h",
"api/declarative_net_request/utils.cc",
"api/declarative_net_request/utils.h",
"api/extension_action/action_info.cc",
"api/extension_action/action_info.h",
"api/messaging/message.h",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
#include "extensions/common/api/declarative_net_request.h"
#include "extensions/common/api/declarative_net_request/constants.h"
#include "extensions/common/api/declarative_net_request/dnr_manifest_data.h"
#include "extensions/common/api/declarative_net_request/utils.h"
#include "extensions/common/error_utils.h"
#include "extensions/common/extension_resource.h"
#include "extensions/common/manifest_constants.h"
Expand All @@ -34,7 +33,6 @@ DNRManifestHandler::~DNRManifestHandler() = default;
bool DNRManifestHandler::Parse(Extension* extension, base::string16* error) {
DCHECK(extension->manifest()->HasKey(
dnr_api::ManifestKeys::kDeclarativeNetRequest));
DCHECK(IsAPIAvailable());

if (!PermissionsParser::HasAPIPermission(
extension, APIPermission::kDeclarativeNetRequest)) {
Expand Down Expand Up @@ -126,8 +124,6 @@ bool DNRManifestHandler::Parse(Extension* extension, base::string16* error) {
bool DNRManifestHandler::Validate(const Extension* extension,
std::string* error,
std::vector<InstallWarning>* warnings) const {
DCHECK(IsAPIAvailable());

DNRManifestData* data =
static_cast<DNRManifestData*>(extension->GetManifestData(
dnr_api::ManifestKeys::kDeclarativeNetRequest));
Expand Down
22 changes: 0 additions & 22 deletions extensions/common/api/declarative_net_request/utils.cc

This file was deleted.

18 changes: 0 additions & 18 deletions extensions/common/api/declarative_net_request/utils.h

This file was deleted.

0 comments on commit d92523f

Please sign in to comment.