Commit 061ce628 authored by Charlie Harrison's avatar Charlie Harrison Committed by Commit Bot

[subresource_filter] enable main frame speculative activation computation

This patch adds the ability for main frame throttles to receive
potentially multiple calls to notify of page-level activation. In these
cases, there is some logic at the end of the navigation to "normalize"
the computed ActivationState to apply to the most recent page-level
ActivationState given.

This patch should have no observable effects to subresource_filter
logic, with or without ad frame.

Bug: 809504
Change-Id: Icb85e36c737fd092db4b7a134aabd03a1382b875
Reviewed-on: https://chromium-review.googlesource.com/961534Reviewed-by: default avatarJosh Karlin <jkarlin@chromium.org>
Commit-Queue: Charlie Harrison <csharrison@chromium.org>
Cr-Commit-Position: refs/heads/master@{#545167}
parent c08aec2f
...@@ -62,8 +62,6 @@ void ActivationStateComputingNavigationThrottle:: ...@@ -62,8 +62,6 @@ void ActivationStateComputingNavigationThrottle::
VerifiedRuleset::Handle* ruleset_handle, VerifiedRuleset::Handle* ruleset_handle,
const ActivationState& page_activation_state) { const ActivationState& page_activation_state) {
DCHECK(navigation_handle()->IsInMainFrame()); DCHECK(navigation_handle()->IsInMainFrame());
DCHECK(!parent_activation_state_);
DCHECK(!ruleset_handle_);
DCHECK_NE(ActivationLevel::DISABLED, page_activation_state.activation_level); DCHECK_NE(ActivationLevel::DISABLED, page_activation_state.activation_level);
parent_activation_state_ = page_activation_state; parent_activation_state_ = page_activation_state;
ruleset_handle_ = ruleset_handle; ruleset_handle_ = ruleset_handle;
...@@ -71,14 +69,14 @@ void ActivationStateComputingNavigationThrottle:: ...@@ -71,14 +69,14 @@ void ActivationStateComputingNavigationThrottle::
content::NavigationThrottle::ThrottleCheckResult content::NavigationThrottle::ThrottleCheckResult
ActivationStateComputingNavigationThrottle::WillStartRequest() { ActivationStateComputingNavigationThrottle::WillStartRequest() {
if (!navigation_handle()->IsInMainFrame()) if (parent_activation_state_)
CheckActivationState(); CheckActivationState();
return content::NavigationThrottle::PROCEED; return content::NavigationThrottle::PROCEED;
} }
content::NavigationThrottle::ThrottleCheckResult content::NavigationThrottle::ThrottleCheckResult
ActivationStateComputingNavigationThrottle::WillRedirectRequest() { ActivationStateComputingNavigationThrottle::WillRedirectRequest() {
if (!navigation_handle()->IsInMainFrame()) if (parent_activation_state_)
CheckActivationState(); CheckActivationState();
return content::NavigationThrottle::PROCEED; return content::NavigationThrottle::PROCEED;
} }
...@@ -100,6 +98,8 @@ ActivationStateComputingNavigationThrottle::WillProcessResponse() { ...@@ -100,6 +98,8 @@ ActivationStateComputingNavigationThrottle::WillProcessResponse() {
// check. // check.
if (async_filter_ && async_filter_->has_activation_state()) { if (async_filter_ && async_filter_->has_activation_state()) {
LogDelayMetrics(base::TimeDelta::FromMilliseconds(0)); LogDelayMetrics(base::TimeDelta::FromMilliseconds(0));
if (navigation_handle()->IsInMainFrame())
UpdateWithMoreAccurateState();
return content::NavigationThrottle::PROCEED; return content::NavigationThrottle::PROCEED;
} }
DCHECK(!defer_timer_); DCHECK(!defer_timer_);
...@@ -142,10 +142,22 @@ void ActivationStateComputingNavigationThrottle::OnActivationStateComputed( ...@@ -142,10 +142,22 @@ void ActivationStateComputingNavigationThrottle::OnActivationStateComputed(
ActivationState state) { ActivationState state) {
if (defer_timer_) { if (defer_timer_) {
LogDelayMetrics(defer_timer_->Elapsed()); LogDelayMetrics(defer_timer_->Elapsed());
if (navigation_handle()->IsInMainFrame())
UpdateWithMoreAccurateState();
Resume(); Resume();
} }
} }
void ActivationStateComputingNavigationThrottle::UpdateWithMoreAccurateState() {
// This method is only needed for main frame navigations that are notified of
// page activation more than once. Even for those that are updated once, it
// should be a no-op.
DCHECK(navigation_handle()->IsInMainFrame());
DCHECK(parent_activation_state_);
DCHECK(async_filter_);
async_filter_->UpdateWithMoreAccurateState(*parent_activation_state_);
}
void ActivationStateComputingNavigationThrottle::LogDelayMetrics( void ActivationStateComputingNavigationThrottle::LogDelayMetrics(
base::TimeDelta delay) const { base::TimeDelta delay) const {
UMA_HISTOGRAM_CUSTOM_MICRO_TIMES( UMA_HISTOGRAM_CUSTOM_MICRO_TIMES(
......
...@@ -30,8 +30,9 @@ class AsyncDocumentSubresourceFilter; ...@@ -30,8 +30,9 @@ class AsyncDocumentSubresourceFilter;
// //
// Note: for performance, activation computation for subframes is done // Note: for performance, activation computation for subframes is done
// speculatively at navigation start and at every redirect. This is to reduce // speculatively at navigation start and at every redirect. This is to reduce
// the wait time (most likely to 0) by WillProcessResponse time. // the wait time (most likely to 0) by WillProcessResponse time. For main
// TODO(crbug.com/809504): Implement speculation for main frames as well. // frames, speculation will be done at the next navigation stage after
// NotifyPageActivationWithRuleset is called.
class ActivationStateComputingNavigationThrottle class ActivationStateComputingNavigationThrottle
: public content::NavigationThrottle { : public content::NavigationThrottle {
public: public:
...@@ -58,6 +59,10 @@ class ActivationStateComputingNavigationThrottle ...@@ -58,6 +59,10 @@ class ActivationStateComputingNavigationThrottle
// navigation for main frames. // navigation for main frames.
// //
// Should never be called with DISABLED activation. // Should never be called with DISABLED activation.
//
// Note: can be called multiple times, at any point in the navigation to
// update the page state. |page_activation_state| will be merged into any
// previously computed activation state.
void NotifyPageActivationWithRuleset( void NotifyPageActivationWithRuleset(
VerifiedRuleset::Handle* ruleset_handle, VerifiedRuleset::Handle* ruleset_handle,
const ActivationState& page_activation_state); const ActivationState& page_activation_state);
...@@ -89,6 +94,13 @@ class ActivationStateComputingNavigationThrottle ...@@ -89,6 +94,13 @@ class ActivationStateComputingNavigationThrottle
void CheckActivationState(); void CheckActivationState();
void OnActivationStateComputed(ActivationState state); void OnActivationStateComputed(ActivationState state);
// In the case when main frame navigations get notified of ActivationState
// multiple times, a method is needed for overriding previously computed
// results with a more accurate ActivationState.
//
// This must be called at the end of the WillProcessResponse stage.
void UpdateWithMoreAccurateState();
void LogDelayMetrics(base::TimeDelta delay) const; void LogDelayMetrics(base::TimeDelta delay) const;
ActivationStateComputingNavigationThrottle( ActivationStateComputingNavigationThrottle(
......
...@@ -58,6 +58,9 @@ ActivationState ComputeActivationState( ...@@ -58,6 +58,9 @@ ActivationState ComputeActivationState(
url_pattern_index::proto::ACTIVATION_TYPE_GENERICBLOCK)) { url_pattern_index::proto::ACTIVATION_TYPE_GENERICBLOCK)) {
activation_state.generic_blocking_rules_disabled = true; activation_state.generic_blocking_rules_disabled = true;
} }
// Careful note: any new state computed for ActivationState in this method
// must also update UpdateWithMoreAccurateState..
return activation_state; return activation_state;
} }
...@@ -158,6 +161,24 @@ void AsyncDocumentSubresourceFilter::ReportDisallowedLoad() { ...@@ -158,6 +161,24 @@ void AsyncDocumentSubresourceFilter::ReportDisallowedLoad() {
std::move(first_disallowed_load_callback_).Run(); std::move(first_disallowed_load_callback_).Run();
} }
void AsyncDocumentSubresourceFilter::UpdateWithMoreAccurateState(
const ActivationState& updated_page_state) {
// DISABLED activation level implies that the ruleset is somehow invalid. Make
// sure that we don't update the state in that case.
if (activation_state_->activation_level == ActivationLevel::DISABLED)
return;
// TODO(csharrison): Split ActivationState into multiple structs, with one
// that includes members that are inherited from the parent without change,
// and one that includes members that need to be computed.
bool filtering_disabled = activation_state_->filtering_disabled_for_document;
bool generic_disabled = activation_state_->generic_blocking_rules_disabled;
activation_state_ = updated_page_state;
activation_state_->filtering_disabled_for_document = filtering_disabled;
activation_state_->generic_blocking_rules_disabled = generic_disabled;
}
const ActivationState& AsyncDocumentSubresourceFilter::activation_state() const ActivationState& AsyncDocumentSubresourceFilter::activation_state()
const { const {
CHECK(activation_state_); CHECK(activation_state_);
......
...@@ -116,6 +116,11 @@ class AsyncDocumentSubresourceFilter { ...@@ -116,6 +116,11 @@ class AsyncDocumentSubresourceFilter {
// |task_runner|. // |task_runner|.
void ReportDisallowedLoad(); void ReportDisallowedLoad();
// Should only be called for main frames. Updates |activation_state_| with the
// more accurate |updated_page_state|, but retains ruleset specific properties
// like document whitelisting.
void UpdateWithMoreAccurateState(const ActivationState& updated_page_state);
// Must be called after activation state computation is finished. // Must be called after activation state computation is finished.
const ActivationState& activation_state() const; const ActivationState& activation_state() const;
......
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