Commit 8ab445a5 authored by jam's avatar jam Committed by Commit bot

Fix SubResourceFilter to work with PlzNavigate.

There are two main fixes:
-use the new NavigationHandle callbacks on WebContentsObserver which are compatible with PlzNavigate
-update the renderer side so that handles the new order of IPCs: while ReadyToCommitNavigation means the browser side can send its IPC before the internal content ones, with PlzNavigate one content IPC will fire both DidStartProvisionalLoad & DidCommitProvisionalLoad. So add extra logic to the renderer side to handle this.

This fixes
SubresourceFilterBrowserTest.MainFrameActivation
SubresourceFilterBrowserTest.MainFrameActivationOnStartup
SubresourceFilterBrowserTest.SubFrameActivation
with PlzNavigate.

BUG=504347, 653807

Review-Url: https://codereview.chromium.org/2446503002
Cr-Commit-Position: refs/heads/master@{#427085}
parent e3b47824
...@@ -140,8 +140,6 @@ ...@@ -140,8 +140,6 @@
#include "components/security_interstitials/core/ssl_error_ui.h" #include "components/security_interstitials/core/ssl_error_ui.h"
#include "components/signin/core/common/profile_management_switches.h" #include "components/signin/core/common/profile_management_switches.h"
#include "components/startup_metric_utils/browser/startup_metric_host_impl.h" #include "components/startup_metric_utils/browser/startup_metric_host_impl.h"
#include "components/subresource_filter/content/browser/content_subresource_filter_driver_factory.h"
#include "components/subresource_filter/content/browser/subresource_filter_navigation_throttle.h"
#include "components/translate/core/common/translate_switches.h" #include "components/translate/core/common/translate_switches.h"
#include "components/url_formatter/url_fixer.h" #include "components/url_formatter/url_fixer.h"
#include "components/variations/variations_associated_data.h" #include "components/variations/variations_associated_data.h"
...@@ -3149,20 +3147,6 @@ ChromeContentBrowserClient::CreateThrottlesForNavigation( ...@@ -3149,20 +3147,6 @@ ChromeContentBrowserClient::CreateThrottlesForNavigation(
throttles.push_back(new extensions::ExtensionNavigationThrottle(handle)); throttles.push_back(new extensions::ExtensionNavigationThrottle(handle));
#endif #endif
subresource_filter::ContentSubresourceFilterDriverFactory*
subresource_filter_driver_factory =
subresource_filter::ContentSubresourceFilterDriverFactory::
FromWebContents(handle->GetWebContents());
if (subresource_filter_driver_factory && handle->IsInMainFrame() &&
handle->GetURL().SchemeIsHTTPOrHTTPS()) {
// TODO(melandory): Activation logic should be moved to the
// WebContentsObserver, once ReadyToCommitNavigation is available on
// pre-PlzNavigate world (tracking bug: https://crbug.com/621856).
throttles.push_back(
subresource_filter::SubresourceFilterNavigationThrottle::Create(
handle));
}
return throttles; return throttles;
} }
......
...@@ -10,8 +10,6 @@ static_library("browser") { ...@@ -10,8 +10,6 @@ static_library("browser") {
"content_subresource_filter_driver.h", "content_subresource_filter_driver.h",
"content_subresource_filter_driver_factory.cc", "content_subresource_filter_driver_factory.cc",
"content_subresource_filter_driver_factory.h", "content_subresource_filter_driver_factory.h",
"subresource_filter_navigation_throttle.cc",
"subresource_filter_navigation_throttle.h",
] ]
deps = [ deps = [
"//base", "//base",
...@@ -31,7 +29,6 @@ source_set("unit_tests") { ...@@ -31,7 +29,6 @@ source_set("unit_tests") {
sources = [ sources = [
"content_ruleset_distributor_unittest.cc", "content_ruleset_distributor_unittest.cc",
"content_subresource_filter_driver_factory_unittest.cc", "content_subresource_filter_driver_factory_unittest.cc",
"subresource_filter_navigation_throttle_unittests.cc",
] ]
deps = [ deps = [
":browser", ":browser",
......
...@@ -16,11 +16,12 @@ ContentSubresourceFilterDriver::ContentSubresourceFilterDriver( ...@@ -16,11 +16,12 @@ ContentSubresourceFilterDriver::ContentSubresourceFilterDriver(
ContentSubresourceFilterDriver::~ContentSubresourceFilterDriver() {} ContentSubresourceFilterDriver::~ContentSubresourceFilterDriver() {}
void ContentSubresourceFilterDriver::ActivateForProvisionalLoad( void ContentSubresourceFilterDriver::ActivateForProvisionalLoad(
ActivationState activation_state) { ActivationState activation_state,
const GURL& url) {
// Must use legacy IPC to ensure the activation message arrives in-order, i.e. // Must use legacy IPC to ensure the activation message arrives in-order, i.e.
// before the load is committed on the renderer side. // before the load is committed on the renderer side.
render_frame_host_->Send(new SubresourceFilterMsg_ActivateForProvisionalLoad( render_frame_host_->Send(new SubresourceFilterMsg_ActivateForProvisionalLoad(
render_frame_host_->GetRoutingID(), activation_state)); render_frame_host_->GetRoutingID(), activation_state, url));
} }
} // namespace subresource_filter } // namespace subresource_filter
...@@ -8,6 +8,8 @@ ...@@ -8,6 +8,8 @@
#include "base/macros.h" #include "base/macros.h"
#include "components/subresource_filter/core/common/activation_state.h" #include "components/subresource_filter/core/common/activation_state.h"
class GURL;
namespace content { namespace content {
class RenderFrameHost; class RenderFrameHost;
} // namespace content } // namespace content
...@@ -24,7 +26,8 @@ class ContentSubresourceFilterDriver { ...@@ -24,7 +26,8 @@ class ContentSubresourceFilterDriver {
// Instructs the agent on the renderer to set up the subresource filter for // Instructs the agent on the renderer to set up the subresource filter for
// the currently ongoing provisional document load in the frame. // the currently ongoing provisional document load in the frame.
virtual void ActivateForProvisionalLoad(ActivationState activation_state); virtual void ActivateForProvisionalLoad(ActivationState activation_state,
const GURL& url);
private: private:
// The RenderFrameHost that this driver belongs to. // The RenderFrameHost that this driver belongs to.
......
...@@ -10,6 +10,7 @@ ...@@ -10,6 +10,7 @@
#include "components/subresource_filter/core/browser/subresource_filter_client.h" #include "components/subresource_filter/core/browser/subresource_filter_client.h"
#include "components/subresource_filter/core/browser/subresource_filter_features.h" #include "components/subresource_filter/core/browser/subresource_filter_features.h"
#include "components/subresource_filter/core/common/activation_list.h" #include "components/subresource_filter/core/common/activation_list.h"
#include "content/public/browser/navigation_handle.h"
#include "content/public/browser/render_frame_host.h" #include "content/public/browser/render_frame_host.h"
#include "content/public/browser/web_contents.h" #include "content/public/browser/web_contents.h"
#include "ipc/ipc_message_macros.h" #include "ipc/ipc_message_macros.h"
...@@ -71,13 +72,12 @@ void ContentSubresourceFilterDriverFactory::OnFirstSubresourceLoadDisallowed() { ...@@ -71,13 +72,12 @@ void ContentSubresourceFilterDriverFactory::OnFirstSubresourceLoadDisallowed() {
bool ContentSubresourceFilterDriverFactory::IsWhitelisted( bool ContentSubresourceFilterDriverFactory::IsWhitelisted(
const GURL& url) const { const GURL& url) const {
return whitelisted_set().find(url.host()) != whitelisted_set().end(); return whitelisted_hosts_.find(url.host()) != whitelisted_hosts_.end();
} }
bool ContentSubresourceFilterDriverFactory::IsHit(const GURL& url) const { bool ContentSubresourceFilterDriverFactory::IsHit(const GURL& url) const {
return safe_browsing_blacklisted_patterns_set().find(url.host() + return safe_browsing_blacklisted_patterns_.find(url.host() + url.path()) !=
url.path()) != safe_browsing_blacklisted_patterns_.end();
safe_browsing_blacklisted_patterns_set().end();
} }
...@@ -113,15 +113,6 @@ void ContentSubresourceFilterDriverFactory::AddToActivationHitsSet( ...@@ -113,15 +113,6 @@ void ContentSubresourceFilterDriverFactory::AddToActivationHitsSet(
safe_browsing_blacklisted_patterns_.insert(url.host() + url.path()); safe_browsing_blacklisted_patterns_.insert(url.host() + url.path());
} }
void ContentSubresourceFilterDriverFactory::ReadyToCommitMainFrameNavigation(
content::RenderFrameHost* render_frame_host,
const GURL& url) {
if (ShouldActivateForMainFrameURL(url)) {
set_activation_state(GetMaximumActivationState());
ActivateForFrameHostIfNeeded(render_frame_host, url);
}
}
bool ContentSubresourceFilterDriverFactory::ShouldActivateForMainFrameURL( bool ContentSubresourceFilterDriverFactory::ShouldActivateForMainFrameURL(
const GURL& url) const { const GURL& url) const {
if (GetCurrentActivationScope() == ActivationScope::ALL_SITES) if (GetCurrentActivationScope() == ActivationScope::ALL_SITES)
...@@ -136,7 +127,7 @@ void ContentSubresourceFilterDriverFactory::ActivateForFrameHostIfNeeded( ...@@ -136,7 +127,7 @@ void ContentSubresourceFilterDriverFactory::ActivateForFrameHostIfNeeded(
const GURL& url) { const GURL& url) {
if (activation_state_ != ActivationState::DISABLED) { if (activation_state_ != ActivationState::DISABLED) {
DriverFromFrameHost(render_frame_host) DriverFromFrameHost(render_frame_host)
->ActivateForProvisionalLoad(GetMaximumActivationState()); ->ActivateForProvisionalLoad(GetMaximumActivationState(), url);
} }
} }
...@@ -172,20 +163,23 @@ void ContentSubresourceFilterDriverFactory::RenderFrameDeleted( ...@@ -172,20 +163,23 @@ void ContentSubresourceFilterDriverFactory::RenderFrameDeleted(
frame_drivers_.erase(render_frame_host); frame_drivers_.erase(render_frame_host);
} }
void ContentSubresourceFilterDriverFactory::DidStartProvisionalLoadForFrame( void ContentSubresourceFilterDriverFactory::DidStartNavigation(
content::RenderFrameHost* render_frame_host, content::NavigationHandle* navigation_handle) {
const GURL& validated_url,
bool is_error_page,
bool is_iframe_srcdoc) {
safe_browsing_blacklisted_patterns_.clear(); safe_browsing_blacklisted_patterns_.clear();
if (!render_frame_host->GetParent()) { if (navigation_handle->IsInMainFrame()) {
client_->ToggleNotificationVisibility(false); client_->ToggleNotificationVisibility(false);
set_activation_state(ActivationState::DISABLED); activation_state_ = ActivationState::DISABLED;
} else {
ActivateForFrameHostIfNeeded(render_frame_host, validated_url);
} }
} }
void ContentSubresourceFilterDriverFactory::ReadyToCommitNavigation(
content::NavigationHandle* navigation_handle) {
content::RenderFrameHost* render_frame_host =
navigation_handle->GetRenderFrameHost();
GURL url = navigation_handle->GetURL();
ReadyToCommitNavigationInternal(render_frame_host, url);
}
bool ContentSubresourceFilterDriverFactory::OnMessageReceived( bool ContentSubresourceFilterDriverFactory::OnMessageReceived(
const IPC::Message& message, const IPC::Message& message,
content::RenderFrameHost* render_frame_host) { content::RenderFrameHost* render_frame_host) {
...@@ -198,9 +192,19 @@ bool ContentSubresourceFilterDriverFactory::OnMessageReceived( ...@@ -198,9 +192,19 @@ bool ContentSubresourceFilterDriverFactory::OnMessageReceived(
return handled; return handled;
} }
void ContentSubresourceFilterDriverFactory::set_activation_state( void ContentSubresourceFilterDriverFactory::ReadyToCommitNavigationInternal(
const ActivationState& new_activation_state) { content::RenderFrameHost* render_frame_host,
activation_state_ = new_activation_state; const GURL& url) {
if (!render_frame_host->GetParent()) {
if (ShouldActivateForMainFrameURL(url)) {
activation_state_ = GetMaximumActivationState();
ActivateForFrameHostIfNeeded(render_frame_host, url);
} else {
activation_state_ = ActivationState::DISABLED;
}
} else {
ActivateForFrameHostIfNeeded(render_frame_host, url);
}
} }
} // namespace subresource_filter } // namespace subresource_filter
...@@ -76,25 +76,9 @@ class ContentSubresourceFilterDriverFactory ...@@ -76,25 +76,9 @@ class ContentSubresourceFilterDriverFactory
// Reloads the page and inserts the url to the whitelist. // Reloads the page and inserts the url to the whitelist.
void OnReloadRequested(); void OnReloadRequested();
// Checks if all preconditions are fulfilled and if so, activates filtering
// for the given |render_frame_host|. |url| is used to check web site specific
// preconditions and should be the web URL of the page where caller is
// intended to activate the Safe Browsing Subresource Filter.
// TODO(melandory) While due to crbug.com/621856 we cannot yet get rid of
// SubresourceFilterNavigationThrottle, it would still make sense to change
// its semantics so that its only responsibility is to emulate
// DidRedirectNavigation and ReadyToCommitNavigation for us before we get
// these from WebContentsObserver for free. Then, the throttle would no longer
// contain any subresource filter specific logic, and those pieces of logic
// would all be moved into here.
void ReadyToCommitMainFrameNavigation(
content::RenderFrameHost* render_frame_host,
const GURL& url);
const HostSet& safe_browsing_blacklisted_patterns_set() const { const HostSet& safe_browsing_blacklisted_patterns_set() const {
return safe_browsing_blacklisted_patterns_; return safe_browsing_blacklisted_patterns_;
} }
const HostSet& whitelisted_set() const { return whitelisted_hosts_; }
ActivationState activation_state() { return activation_state_; } ActivationState activation_state() { return activation_state_; }
private: private:
...@@ -117,11 +101,10 @@ class ContentSubresourceFilterDriverFactory ...@@ -117,11 +101,10 @@ class ContentSubresourceFilterDriverFactory
// content::WebContentsObserver: // content::WebContentsObserver:
void RenderFrameCreated(content::RenderFrameHost* render_frame_host) override; void RenderFrameCreated(content::RenderFrameHost* render_frame_host) override;
void RenderFrameDeleted(content::RenderFrameHost* render_frame_host) override; void RenderFrameDeleted(content::RenderFrameHost* render_frame_host) override;
void DidStartProvisionalLoadForFrame( void DidStartNavigation(
content::RenderFrameHost* render_frame_host, content::NavigationHandle* navigation_handle) override;
const GURL& validated_url, void ReadyToCommitNavigation(
bool is_error_page, content::NavigationHandle* navigation_handle) override;
bool is_iframe_srcdoc) override;
bool OnMessageReceived(const IPC::Message& message, bool OnMessageReceived(const IPC::Message& message,
content::RenderFrameHost* render_frame_host) override; content::RenderFrameHost* render_frame_host) override;
...@@ -131,7 +114,11 @@ class ContentSubresourceFilterDriverFactory ...@@ -131,7 +114,11 @@ class ContentSubresourceFilterDriverFactory
void ActivateForFrameHostIfNeeded(content::RenderFrameHost* render_frame_host, void ActivateForFrameHostIfNeeded(content::RenderFrameHost* render_frame_host,
const GURL& url); const GURL& url);
void set_activation_state(const ActivationState& new_activation_state); // Internal implementation of ReadyToCommitNavigation which doesn't use
// NavigationHandle to ease unit tests.
void ReadyToCommitNavigationInternal(
content::RenderFrameHost* render_frame_host,
const GURL& url);
bool IsHit(const GURL& url) const; bool IsHit(const GURL& url) const;
......
...@@ -88,7 +88,7 @@ class MockSubresourceFilterDriver : public ContentSubresourceFilterDriver { ...@@ -88,7 +88,7 @@ class MockSubresourceFilterDriver : public ContentSubresourceFilterDriver {
~MockSubresourceFilterDriver() override = default; ~MockSubresourceFilterDriver() override = default;
MOCK_METHOD1(ActivateForProvisionalLoad, void(ActivationState)); MOCK_METHOD2(ActivateForProvisionalLoad, void(ActivationState, const GURL&));
private: private:
DISALLOW_COPY_AND_ASSIGN(MockSubresourceFilterDriver); DISALLOW_COPY_AND_ASSIGN(MockSubresourceFilterDriver);
...@@ -170,22 +170,19 @@ class ContentSubresourceFilterDriverFactoryTest ...@@ -170,22 +170,19 @@ class ContentSubresourceFilterDriverFactoryTest
content::RenderFrameHost* rfh, content::RenderFrameHost* rfh,
const GURL& url, const GURL& url,
bool should_activate) { bool should_activate) {
EXPECT_CALL(*driver, ActivateForProvisionalLoad(::testing::_)) EXPECT_CALL(*driver, ActivateForProvisionalLoad(::testing::_, ::testing::_))
.Times(should_activate); .Times(should_activate);
factory()->ReadyToCommitMainFrameNavigation(rfh, url); factory()->ReadyToCommitNavigationInternal(rfh, url);
::testing::Mock::VerifyAndClearExpectations(driver); ::testing::Mock::VerifyAndClearExpectations(driver);
} }
void NavigateAndCommitSubframe(const GURL& url, bool should_activate) { void NavigateAndCommitSubframe(const GURL& url, bool should_activate) {
EXPECT_CALL(*subframe_driver(), ActivateForProvisionalLoad(::testing::_)) EXPECT_CALL(*subframe_driver(),
ActivateForProvisionalLoad(::testing::_, ::testing::_))
.Times(should_activate); .Times(should_activate);
EXPECT_CALL(*client(), ToggleNotificationVisibility(::testing::_)).Times(0); EXPECT_CALL(*client(), ToggleNotificationVisibility(::testing::_)).Times(0);
factory()->DidStartProvisionalLoadForFrame( factory()->ReadyToCommitNavigationInternal(subframe_rfh(), url);
subframe_rfh(), url /* validated_url */, false /* is_error_page */,
false /* is_iframe_srcdoc */);
factory()->DidCommitProvisionalLoadForFrame(
subframe_rfh(), url, ui::PageTransition::PAGE_TRANSITION_AUTO_SUBFRAME);
::testing::Mock::VerifyAndClearExpectations(subframe_driver()); ::testing::Mock::VerifyAndClearExpectations(subframe_driver());
::testing::Mock::VerifyAndClearExpectations(client()); ::testing::Mock::VerifyAndClearExpectations(client());
} }
...@@ -214,8 +211,10 @@ class ContentSubresourceFilterDriverFactoryTest ...@@ -214,8 +211,10 @@ class ContentSubresourceFilterDriverFactoryTest
BlacklistURLWithRedirectsNavigateMainFrameAndSubrame( BlacklistURLWithRedirectsNavigateMainFrameAndSubrame(
bad_url, redirects, GURL(kExampleUrl), should_activate); bad_url, redirects, GURL(kExampleUrl), should_activate);
EXPECT_CALL(*driver(), ActivateForProvisionalLoad(::testing::_)).Times(0); EXPECT_CALL(*driver(),
EXPECT_CALL(*client(), ToggleNotificationVisibility(::testing::_)).Times(0); ActivateForProvisionalLoad(::testing::_, ::testing::_))
.Times(0);
EXPECT_CALL(*client(), ToggleNotificationVisibility(::testing::_)).Times(1);
content::RenderFrameHostTester::For(main_rfh()) content::RenderFrameHostTester::For(main_rfh())
->SimulateNavigationCommit(GURL(kExampleUrl)); ->SimulateNavigationCommit(GURL(kExampleUrl));
::testing::Mock::VerifyAndClearExpectations(driver()); ::testing::Mock::VerifyAndClearExpectations(driver());
......
// Copyright 2016 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 "components/subresource_filter/content/browser/subresource_filter_navigation_throttle.h"
#include "components/subresource_filter/content/browser/content_subresource_filter_driver.h"
#include "components/subresource_filter/content/browser/content_subresource_filter_driver_factory.h"
#include "components/subresource_filter/core/browser/subresource_filter_features.h"
#include "components/subresource_filter/core/common/activation_state.h"
#include "content/public/browser/navigation_handle.h"
namespace subresource_filter {
// static
std::unique_ptr<content::NavigationThrottle>
SubresourceFilterNavigationThrottle::Create(content::NavigationHandle* handle) {
return std::unique_ptr<content::NavigationThrottle>(
new SubresourceFilterNavigationThrottle(handle));
}
SubresourceFilterNavigationThrottle::SubresourceFilterNavigationThrottle(
content::NavigationHandle* handle)
: content::NavigationThrottle(handle) {}
SubresourceFilterNavigationThrottle::~SubresourceFilterNavigationThrottle() {}
content::NavigationThrottle::ThrottleCheckResult
SubresourceFilterNavigationThrottle::WillProcessResponse() {
if (!navigation_handle()->GetURL().SchemeIsHTTPOrHTTPS())
return NavigationThrottle::PROCEED;
ContentSubresourceFilterDriverFactory::FromWebContents(
navigation_handle()->GetWebContents())
->ReadyToCommitMainFrameNavigation(
navigation_handle()->GetRenderFrameHost(),
navigation_handle()->GetURL());
return NavigationThrottle::PROCEED;
}
} // namespace subresource_filter
// Copyright 2016 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 COMPONENTS_SUBRESOURCE_FILTER_CONTENT_BROWSER_SUBRESOURCE_FILTER_NAVIGATION_THROTTLE_
#define COMPONENTS_SUBRESOURCE_FILTER_CONTENT_BROWSER_SUBRESOURCE_FILTER_NAVIGATION_THROTTLE_
#include <memory>
#include "base/macros.h"
#include "content/public/browser/navigation_throttle.h"
#include "url/gurl.h"
namespace content {
class NavigationHandle;
} // content
namespace subresource_filter {
// This class has two resposibilities:
// * it tracks the redirects that happen after Safe Browsing detects that the
// page being loaded contains deceptive embedded content. If such a redirect
// happens and leads to new domains, these are also put on the activation list
// of the tab.
// * it creates a ContentSubresourceFilterDriver for the tab when the final site
// of a (potentially empty) redirect chain is reached and any URL of the
// redirect chain was on the activation list.
// This throttle is active only for main frame http or https navigations and
// doesn't act on subframe navigations or subresource loads.
class SubresourceFilterNavigationThrottle : public content::NavigationThrottle {
public:
static std::unique_ptr<content::NavigationThrottle> Create(
content::NavigationHandle* handle);
~SubresourceFilterNavigationThrottle() override;
// content::NavigationThrottle implementation:
ThrottleCheckResult WillProcessResponse() override;
private:
explicit SubresourceFilterNavigationThrottle(
content::NavigationHandle* navigation_handle);
DISALLOW_COPY_AND_ASSIGN(SubresourceFilterNavigationThrottle);
};
} // namespace subresource_filter
#endif // COMPONENTS_SUBRESOURCE_FILTER_CONTENT_BROWSER_SUBRESOURCE_FILTER_NAVIGATION_THROTTLE_
...@@ -9,6 +9,7 @@ ...@@ -9,6 +9,7 @@
#include "ipc/ipc_message_macros.h" #include "ipc/ipc_message_macros.h"
#include "ipc/ipc_message.h" #include "ipc/ipc_message.h"
#include "ipc/ipc_platform_file.h" #include "ipc/ipc_platform_file.h"
#include "url/ipc/url_param_traits.h"
#define IPC_MESSAGE_START SubresourceFilterMsgStart #define IPC_MESSAGE_START SubresourceFilterMsgStart
...@@ -29,8 +30,9 @@ IPC_MESSAGE_CONTROL1(SubresourceFilterMsg_SetRulesetForProcess, ...@@ -29,8 +30,9 @@ IPC_MESSAGE_CONTROL1(SubresourceFilterMsg_SetRulesetForProcess,
// ongoing provisional document load in a frame. The message must arrive after // ongoing provisional document load in a frame. The message must arrive after
// the provisional load starts, but before it is committed on the renderer side. // the provisional load starts, but before it is committed on the renderer side.
// If no message arrives, the default behavior is ActivationState::DISABLED. // If no message arrives, the default behavior is ActivationState::DISABLED.
IPC_MESSAGE_ROUTED1(SubresourceFilterMsg_ActivateForProvisionalLoad, IPC_MESSAGE_ROUTED2(SubresourceFilterMsg_ActivateForProvisionalLoad,
subresource_filter::ActivationState /* activation_state */); subresource_filter::ActivationState /* activation_state */,
GURL /* url */);
// ---------------------------------------------------------------------------- // ----------------------------------------------------------------------------
// Messages sent from the renderer to the browser. // Messages sent from the renderer to the browser.
......
...@@ -11,6 +11,7 @@ ...@@ -11,6 +11,7 @@
#include "components/subresource_filter/content/renderer/document_subresource_filter.h" #include "components/subresource_filter/content/renderer/document_subresource_filter.h"
#include "components/subresource_filter/content/renderer/ruleset_dealer.h" #include "components/subresource_filter/content/renderer/ruleset_dealer.h"
#include "components/subresource_filter/core/common/memory_mapped_ruleset.h" #include "components/subresource_filter/core/common/memory_mapped_ruleset.h"
#include "content/public/common/browser_side_navigation_policy.h"
#include "content/public/renderer/render_frame.h" #include "content/public/renderer/render_frame.h"
#include "ipc/ipc_message.h" #include "ipc/ipc_message.h"
#include "third_party/WebKit/public/web/WebDataSource.h" #include "third_party/WebKit/public/web/WebDataSource.h"
...@@ -57,9 +58,11 @@ void SubresourceFilterAgent:: ...@@ -57,9 +58,11 @@ void SubresourceFilterAgent::
render_frame()->GetRoutingID())); render_frame()->GetRoutingID()));
} }
void SubresourceFilterAgent::ActivateForProvisionalLoad( void SubresourceFilterAgent::OnActivateForProvisionalLoad(
ActivationState activation_state) { ActivationState activation_state,
const GURL& url) {
activation_state_for_provisional_load_ = activation_state; activation_state_for_provisional_load_ = activation_state;
url_for_provisional_load_ = url;
} }
void SubresourceFilterAgent::RecordHistogramsOnLoadCommitted() { void SubresourceFilterAgent::RecordHistogramsOnLoadCommitted() {
...@@ -95,7 +98,21 @@ void SubresourceFilterAgent::OnDestruct() { ...@@ -95,7 +98,21 @@ void SubresourceFilterAgent::OnDestruct() {
} }
void SubresourceFilterAgent::DidStartProvisionalLoad() { void SubresourceFilterAgent::DidStartProvisionalLoad() {
activation_state_for_provisional_load_ = ActivationState::DISABLED; // With PlzNavigate, DidStartProvisionalLoad and DidCommitProvisionalLoad will
// both be called in response to the one commit IPC from the browser. That
// means that they will come after OnActivateForProvisionalLoad. So we have to
// have extra logic to check that the response to OnActivateForProvisionalLoad
// isn't removed in that case.
blink::WebDataSource* ds =
render_frame() ? render_frame()->GetWebFrame()->provisionalDataSource()
: nullptr;
if (!content::IsBrowserSideNavigationEnabled() ||
(!ds ||
static_cast<GURL>(ds->request().url()) != url_for_provisional_load_)) {
activation_state_for_provisional_load_ = ActivationState::DISABLED;
} else {
url_for_provisional_load_ = GURL();
}
filter_for_last_committed_load_.reset(); filter_for_last_committed_load_.reset();
} }
...@@ -132,7 +149,7 @@ bool SubresourceFilterAgent::OnMessageReceived(const IPC::Message& message) { ...@@ -132,7 +149,7 @@ bool SubresourceFilterAgent::OnMessageReceived(const IPC::Message& message) {
bool handled = true; bool handled = true;
IPC_BEGIN_MESSAGE_MAP(SubresourceFilterAgent, message) IPC_BEGIN_MESSAGE_MAP(SubresourceFilterAgent, message)
IPC_MESSAGE_HANDLER(SubresourceFilterMsg_ActivateForProvisionalLoad, IPC_MESSAGE_HANDLER(SubresourceFilterMsg_ActivateForProvisionalLoad,
ActivateForProvisionalLoad) OnActivateForProvisionalLoad)
IPC_MESSAGE_UNHANDLED(handled = false) IPC_MESSAGE_UNHANDLED(handled = false)
IPC_END_MESSAGE_MAP() IPC_END_MESSAGE_MAP()
return handled; return handled;
......
...@@ -12,6 +12,7 @@ ...@@ -12,6 +12,7 @@
#include "base/memory/weak_ptr.h" #include "base/memory/weak_ptr.h"
#include "components/subresource_filter/core/common/activation_state.h" #include "components/subresource_filter/core/common/activation_state.h"
#include "content/public/renderer/render_frame_observer.h" #include "content/public/renderer/render_frame_observer.h"
#include "url/gurl.h"
class GURL; class GURL;
...@@ -56,7 +57,8 @@ class SubresourceFilterAgent ...@@ -56,7 +57,8 @@ class SubresourceFilterAgent
virtual void SignalFirstSubresourceDisallowedForCommittedLoad(); virtual void SignalFirstSubresourceDisallowedForCommittedLoad();
private: private:
void ActivateForProvisionalLoad(ActivationState activation_state); void OnActivateForProvisionalLoad(ActivationState activation_state,
const GURL& url);
void RecordHistogramsOnLoadCommitted(); void RecordHistogramsOnLoadCommitted();
void RecordHistogramsOnLoadFinished(); void RecordHistogramsOnLoadFinished();
...@@ -72,6 +74,7 @@ class SubresourceFilterAgent ...@@ -72,6 +74,7 @@ class SubresourceFilterAgent
RulesetDealer* ruleset_dealer_; RulesetDealer* ruleset_dealer_;
ActivationState activation_state_for_provisional_load_; ActivationState activation_state_for_provisional_load_;
GURL url_for_provisional_load_;
base::WeakPtr<DocumentSubresourceFilter> filter_for_last_committed_load_; base::WeakPtr<DocumentSubresourceFilter> filter_for_last_committed_load_;
DISALLOW_COPY_AND_ASSIGN(SubresourceFilterAgent); DISALLOW_COPY_AND_ASSIGN(SubresourceFilterAgent);
......
...@@ -105,7 +105,8 @@ class SubresourceFilterAgentTest : public ::testing::Test { ...@@ -105,7 +105,8 @@ class SubresourceFilterAgentTest : public ::testing::Test {
void StartLoadAndSetActivationState(ActivationState activation_state) { void StartLoadAndSetActivationState(ActivationState activation_state) {
agent_as_rfo()->DidStartProvisionalLoad(); agent_as_rfo()->DidStartProvisionalLoad();
EXPECT_TRUE(agent_as_rfo()->OnMessageReceived( EXPECT_TRUE(agent_as_rfo()->OnMessageReceived(
SubresourceFilterMsg_ActivateForProvisionalLoad(0, activation_state))); SubresourceFilterMsg_ActivateForProvisionalLoad(0, activation_state,
GURL())));
agent_as_rfo()->DidCommitProvisionalLoad( agent_as_rfo()->DidCommitProvisionalLoad(
true /* is_new_navigation */, false /* is_same_page_navigation */); true /* is_new_navigation */, false /* is_same_page_navigation */);
} }
...@@ -232,7 +233,7 @@ TEST_F(SubresourceFilterAgentTest, ...@@ -232,7 +233,7 @@ TEST_F(SubresourceFilterAgentTest,
agent_as_rfo()->DidStartProvisionalLoad(); agent_as_rfo()->DidStartProvisionalLoad();
EXPECT_TRUE(agent_as_rfo()->OnMessageReceived( EXPECT_TRUE(agent_as_rfo()->OnMessageReceived(
SubresourceFilterMsg_ActivateForProvisionalLoad( SubresourceFilterMsg_ActivateForProvisionalLoad(
0, ActivationState::ENABLED))); 0, ActivationState::ENABLED, GURL())));
agent_as_rfo()->DidStartProvisionalLoad(); agent_as_rfo()->DidStartProvisionalLoad();
agent_as_rfo()->DidCommitProvisionalLoad(true /* is_new_navigation */, agent_as_rfo()->DidCommitProvisionalLoad(true /* is_new_navigation */,
false /* is_same_page_navigation */); false /* is_same_page_navigation */);
......
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