Adds a flag to identify whether a document is an ad subframe.

This CL adds a flag to SubresourceFilter which is currently set based
on the subresource filter rule set matching. Subsequently this will also
be set when the initiator of this frame is an ad resource even if
this does not match the subresource filter ruleset.

Note that the matching already takes place in the browser process and
this flag is set at commit time when the browser process communicates
the status of matching to the renderer process.

Note that this will work for both browser initiated and renderer
initiated subframes (in PlzNavigate) as is currently the case with
subresource filter matching.

BUG: 807640
Change-Id: If883be8209a7756c6024c1c9eda1a8d081f815a1
Reviewed-on: https://chromium-review.googlesource.com/868261
Commit-Queue: Shivani Sharma <shivanisha@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Nate Chapin <japhet@chromium.org>
Reviewed-by: Josh Karlin <jkarlin@chromium.org>
Reviewed-by: Charlie Harrison <csharrison@chromium.org>
Cr-Commit-Position: refs/heads/master@{#533294}

components/subresource_filter/content/browser/content_subresource_filter_throttle_manager.cc

@@ -73,7 +73,7 @@ void ContentSubresourceFilterThrottleManager::ReadyToCommitNavigation(
 
   // TODO(crbug.com/736249): Remove CHECKs in this file when the root cause of
   // the crash is found.
-  ActivationStateComputingNavigationThrottle* throttle = it->second;
+  ActivationStateComputingNavigationThrottle* throttle = it->second.throttle;
   CHECK_EQ(navigation_handle, throttle->navigation_handle());
 
   // Main frame throttles with disabled page-level activation will not have
@@ -93,10 +93,18 @@ void ContentSubresourceFilterThrottleManager::ReadyToCommitNavigation(
       "activation_state", filter->activation_state().ToTracedValue());
 
   throttle->WillSendActivationToRenderer();
+
+  // is_ad_subframe is guaranteed to have the correct value at this point since
+  // the ruleset checking and its notification is done before the navigation is
+  // resumed.
+  bool is_ad_subframe = it->second.is_ad_subframe;
+  DCHECK(!is_ad_subframe || level == ActivationLevel::DRYRUN);
+  DCHECK(!is_ad_subframe || !navigation_handle->IsInMainFrame());
+
   content::RenderFrameHost* frame_host =
       navigation_handle->GetRenderFrameHost();
   frame_host->Send(new SubresourceFilterMsg_ActivateForNextCommittedLoad(
-      frame_host->GetRoutingID(), filter->activation_state()));
+      frame_host->GetRoutingID(), filter->activation_state(), is_ad_subframe));
 }
 
 void ContentSubresourceFilterThrottleManager::DidFinishNavigation(
@@ -109,12 +117,14 @@ void ContentSubresourceFilterThrottleManager::DidFinishNavigation(
     return;
   }
 
-  auto throttle = ongoing_activation_throttles_.find(navigation_handle);
+  auto throttle_it = ongoing_activation_throttles_.find(navigation_handle);
   std::unique_ptr<AsyncDocumentSubresourceFilter> filter;
-  if (throttle != ongoing_activation_throttles_.end()) {
-    CHECK_EQ(navigation_handle, throttle->second->navigation_handle());
-    filter = throttle->second->ReleaseFilter();
-    ongoing_activation_throttles_.erase(throttle);
+  if (throttle_it != ongoing_activation_throttles_.end()) {
+    ActivationStateComputingNavigationThrottle* throttle =
+        throttle_it->second.throttle;
+    CHECK_EQ(navigation_handle, throttle->navigation_handle());
+    filter = throttle->ReleaseFilter();
+    ongoing_activation_throttles_.erase(throttle_it);
   }
 
   content::RenderFrameHost* frame_host =
@@ -196,8 +206,21 @@ void ContentSubresourceFilterThrottleManager::OnPageActivationComputed(
 
   auto it = ongoing_activation_throttles_.find(navigation_handle);
   if (it != ongoing_activation_throttles_.end()) {
-    it->second->NotifyPageActivationWithRuleset(EnsureRulesetHandle(),
-                                                activation_state);
+    it->second.throttle->NotifyPageActivationWithRuleset(EnsureRulesetHandle(),
+                                                         activation_state);
+  }
+}
+
+void ContentSubresourceFilterThrottleManager::OnSubframeNavigationEvaluated(
+    content::NavigationHandle* navigation_handle,
+    LoadPolicy load_policy) {
+  DCHECK(!navigation_handle->IsInMainFrame());
+  auto it = ongoing_activation_throttles_.find(navigation_handle);
+  if (it != ongoing_activation_throttles_.end()) {
+    // Note that is_ad_subframe is only relevant for
+    // LoadPolicy:WOULD_DISALLOW(dryrun mode), although also setting it for
+    // DISALLOW for completeness.
+    it->second.is_ad_subframe = load_policy != LoadPolicy::ALLOW;
   }
 }
 
@@ -215,7 +238,7 @@ void ContentSubresourceFilterThrottleManager::MaybeAppendNavigationThrottles(
   DCHECK(!base::ContainsKey(ongoing_activation_throttles_, navigation_handle));
   if (auto activation_throttle =
           MaybeCreateActivationStateComputingThrottle(navigation_handle)) {
-    ongoing_activation_throttles_[navigation_handle] =
+    ongoing_activation_throttles_[navigation_handle].throttle =
         activation_throttle.get();
     activation_throttle->set_destruction_closure(base::BindOnce(
         &ContentSubresourceFilterThrottleManager::OnActivationThrottleDestroyed,

components/subresource_filter/content/browser/content_subresource_filter_throttle_manager.h

@@ -102,6 +102,9 @@ class ContentSubresourceFilterThrottleManager
       content::NavigationHandle* navigation_handle,
       ActivationDecision activation_decision,
       const ActivationState& activation_state) override;
+  void OnSubframeNavigationEvaluated(
+      content::NavigationHandle* navigation_handle,
+      LoadPolicy load_policy) override;
 
  private:
   std::unique_ptr<SubframeNavigationFilteringThrottle>
@@ -147,8 +150,14 @@ class ContentSubresourceFilterThrottleManager
   // For each ongoing navigation that requires activation state computation,
   // keeps track of the throttle that is carrying out that computation, so that
   // the result can be retrieved when the navigation is ready to commit.
-  std::unordered_map<content::NavigationHandle*,
-                     ActivationStateComputingNavigationThrottle*>
+  // is_ad_subframe is set if SubframeNavigationFilteringThrottle finds that the
+  // subframe URL matches the ruleset.
+  struct OngoingThrottleInfo {
+    ActivationStateComputingNavigationThrottle* throttle = nullptr;
+    bool is_ad_subframe = false;
+  };
+
+  std::unordered_map<content::NavigationHandle*, OngoingThrottleInfo>
       ongoing_activation_throttles_;
 
   ScopedObserver<SubresourceFilterObserverManager, SubresourceFilterObserver>

components/subresource_filter/content/browser/content_subresource_filter_throttle_manager_unittest.cc

@@ -157,7 +157,8 @@ class ContentSubresourceFilterThrottleManagerTest
   }
 
   void ExpectActivationSignalForFrame(content::RenderFrameHost* rfh,
-                                      bool expect_activation) {
+                                      bool expect_activation,
+                                      bool expect_is_ad_subframe = false) {
     content::MockRenderProcessHost* render_process_host =
         static_cast<content::MockRenderProcessHost*>(rfh->GetProcess());
     const IPC::Message* message =
@@ -165,10 +166,12 @@ class ContentSubresourceFilterThrottleManagerTest
             SubresourceFilterMsg_ActivateForNextCommittedLoad::ID);
     ASSERT_EQ(expect_activation, !!message);
     if (expect_activation) {
-      std::tuple<ActivationState> args;
+      std::tuple<ActivationState, bool> args;
       SubresourceFilterMsg_ActivateForNextCommittedLoad::Read(message, &args);
       ActivationLevel level = std::get<0>(args).activation_level;
       EXPECT_NE(ActivationLevel::DISABLED, level);
+      bool is_ad_subframe = std::get<1>(args);
+      EXPECT_EQ(expect_is_ad_subframe, is_ad_subframe);
     }
     render_process_host->sink().ClearMessages();
   }
@@ -184,12 +187,14 @@ class ContentSubresourceFilterThrottleManagerTest
             url, render_frame_host);
   }
 
-  void CreateSubframeWithTestNavigation(const GURL& url,
-                                        content::RenderFrameHost* parent) {
+  content::RenderFrameHost* CreateSubframeWithTestNavigation(
+      const GURL& url,
+      content::RenderFrameHost* parent) {
     content::RenderFrameHost* subframe =
         content::RenderFrameHostTester::For(parent)->AppendChild(
             base::StringPrintf("subframe-%s", url.spec().c_str()));
     CreateTestNavigation(url, subframe);
+    return subframe;
   }
 
   void SimulateStartAndExpectResult(
@@ -346,7 +351,8 @@ TEST_P(ContentSubresourceFilterThrottleManagerTest,
   content::RenderFrameHost* child =
       SimulateCommitAndExpectResult(content::NavigationThrottle::PROCEED);
   // But it should still be activated.
-  ExpectActivationSignalForFrame(child, true /* expect_activation */);
+  ExpectActivationSignalForFrame(child, true /* expect_activation */,
+                                 true /* is_ad_subframe */);
 
   EXPECT_EQ(0, disallowed_notification_count());
 }
@@ -715,6 +721,30 @@ TEST_F(ContentSubresourceFilterThrottleManagerTest, LogActivation) {
                           base::ThreadTicks::IsSupported() ? 2 : 0);
 }
 
+// Check to make sure we don't send an IPC with the ad tag bit for ad frames
+// that are successfully filtered.
+TEST_P(ContentSubresourceFilterThrottleManagerTest,
+       ActivateMainFrameAndFilterSubframeNavigationTaggedAsAd) {
+  // Commit a navigation that triggers page level activation.
+  NavigateAndCommitMainFrame(GURL(kTestURLWithActivation));
+  ExpectActivationSignalForFrame(main_rfh(), true /* expect_activation */,
+                                 false /* is_ad_subframe */);
+
+  // A disallowed subframe navigation should be successfully filtered.
+  content::RenderFrameHost* subframe = CreateSubframeWithTestNavigation(
+      GURL("https://www.example.com/disallowed.html"), main_rfh());
+
+  SimulateStartAndExpectResult(
+      content::NavigationThrottle::BLOCK_REQUEST_AND_COLLAPSE);
+
+  EXPECT_EQ(1, disallowed_notification_count());
+
+  // Since the IPC is not actually sent, is_ad_subframe is not really checked
+  // but adding it here since we do tag even if its not a dryrun scenario.
+  ExpectActivationSignalForFrame(subframe, false /* expect_activation */,
+                                 true /* is_ad_subframe */);
+}
+
 // TODO(csharrison): Make sure the following conditions are exercised in tests:
 //
 // - Synchronous navigations to about:blank. These hit issues with the

components/subresource_filter/content/common/subresource_filter_messages.h

@@ -58,8 +58,9 @@ IPC_MESSAGE_CONTROL1(SubresourceFilterMsg_SetRulesetForProcess,
 // mojom::FrameNavigationControl::CommitNavigation.
 //
 // If no message arrives, the default behavior is ActivationLevel::DISABLED.
-IPC_MESSAGE_ROUTED1(SubresourceFilterMsg_ActivateForNextCommittedLoad,
-                    subresource_filter::ActivationState /* activation_state */)
+IPC_MESSAGE_ROUTED2(SubresourceFilterMsg_ActivateForNextCommittedLoad,
+                    subresource_filter::ActivationState /* activation_state */,
+                    bool /* is_ad_subframe */)
 
 // ----------------------------------------------------------------------------
 // Messages sent from the renderer to the browser.

components/subresource_filter/content/renderer/subresource_filter_agent.cc

@@ -37,7 +37,8 @@ SubresourceFilterAgent::SubresourceFilterAgent(
     UnverifiedRulesetDealer* ruleset_dealer)
     : content::RenderFrameObserver(render_frame),
       content::RenderFrameObserverTracker<SubresourceFilterAgent>(render_frame),
-      ruleset_dealer_(ruleset_dealer) {
+      ruleset_dealer_(ruleset_dealer),
+      is_ad_subframe_for_next_commit_(false) {
   DCHECK(ruleset_dealer);
 }
 
@@ -57,6 +58,11 @@ void SubresourceFilterAgent::SetSubresourceFilterForCommittedLoad(
   web_frame->GetDocumentLoader()->SetSubresourceFilter(filter.release());
 }
 
+void SubresourceFilterAgent::SetIsAdSubframeForDocument(bool is_ad_subframe) {
+  blink::WebLocalFrame* web_frame = render_frame()->GetWebFrame();
+  web_frame->GetDocumentLoader()->SetIsAdSubframe(is_ad_subframe);
+}
+
 void SubresourceFilterAgent::
     SignalFirstSubresourceDisallowedForCommittedLoad() {
   render_frame()->Send(new SubresourceFilterHostMsg_DidDisallowFirstSubresource(
@@ -84,8 +90,10 @@ ActivationState SubresourceFilterAgent::GetParentActivationState(
 }
 
 void SubresourceFilterAgent::OnActivateForNextCommittedLoad(
-    const ActivationState& activation_state) {
+    const ActivationState& activation_state,
+    bool is_ad_subframe) {
   activation_state_for_next_commit_ = activation_state;
+  is_ad_subframe_for_next_commit_ = is_ad_subframe;
 }
 
 void SubresourceFilterAgent::RecordHistogramsOnLoadCommitted(
@@ -146,9 +154,10 @@ void SubresourceFilterAgent::RecordHistogramsOnLoadFinished() {
   SendDocumentLoadStatistics(statistics);
 }
 
-void SubresourceFilterAgent::ResetActivatonStateForNextCommit() {
+void SubresourceFilterAgent::ResetInfoForNextCommit() {
   activation_state_for_next_commit_ =
       ActivationState(ActivationLevel::DISABLED);
+  is_ad_subframe_for_next_commit_ = false;
 }
 
 void SubresourceFilterAgent::OnDestruct() {
@@ -172,8 +181,9 @@ void SubresourceFilterAgent::DidCommitProvisionalLoad(
   const ActivationState activation_state =
       use_parent_activation ? GetParentActivationState(render_frame())
                             : activation_state_for_next_commit_;
+  bool is_ad_subframe = is_ad_subframe_for_next_commit_;
 
-  ResetActivatonStateForNextCommit();
+  ResetInfoForNextCommit();
 
   // Do not pollute the histograms for empty main frame documents.
   if (IsMainFrame() && !url.SchemeIsHTTPOrHTTPS() && !url.SchemeIsFile())
@@ -201,12 +211,14 @@ void SubresourceFilterAgent::DidCommitProvisionalLoad(
 
   filter_for_last_committed_load_ = filter->AsWeakPtr();
   SetSubresourceFilterForCommittedLoad(std::move(filter));
+  if (is_ad_subframe)
+    SetIsAdSubframeForDocument(true);
 }
 
 void SubresourceFilterAgent::DidFailProvisionalLoad(
     const blink::WebURLError& error) {
   // TODO(engedy): Add a test with `frame-ancestor` violation to exercise this.
-  ResetActivatonStateForNextCommit();
+  ResetInfoForNextCommit();
 }
 
 void SubresourceFilterAgent::DidFinishLoad() {

components/subresource_filter/content/renderer/subresource_filter_agent.h

@@ -52,6 +52,9 @@ class SubresourceFilterAgent
   virtual void SetSubresourceFilterForCommittedLoad(
       std::unique_ptr<blink::WebDocumentSubresourceFilter> filter);
 
+  // Sets in the underlying document, whether this is identified as an ad.
+  virtual void SetIsAdSubframeForDocument(bool is_ad_subframe);
+
   // Informs the browser that the first subresource load has been disallowed for
   // the most recently committed load. Not called if all resources are allowed.
   virtual void SignalFirstSubresourceDisallowedForCommittedLoad();
@@ -66,10 +69,11 @@ class SubresourceFilterAgent
   static ActivationState GetParentActivationState(
       content::RenderFrame* render_frame);
 
-  void OnActivateForNextCommittedLoad(const ActivationState& activation_state);
+  void OnActivateForNextCommittedLoad(const ActivationState& activation_state,
+                                      bool is_ad_subframe);
   void RecordHistogramsOnLoadCommitted(const ActivationState& activation_state);
   void RecordHistogramsOnLoadFinished();
-  void ResetActivatonStateForNextCommit();
+  void ResetInfoForNextCommit();
 
   // content::RenderFrameObserver:
   void OnDestruct() override;
@@ -85,6 +89,13 @@ class SubresourceFilterAgent
 
   ActivationState activation_state_for_next_commit_;
 
+  // This is received along with activation state in the
+  // SubresourceFilterMsg_ActivateForNextCommittedLoad IPC message. Specifies
+  // whether this is a subframe which is identified as an ad. Note that this
+  // will only be set in dry run mode because in blocking mode the frame would
+  // have been blocked.
+  bool is_ad_subframe_for_next_commit_;
+
   base::WeakPtr<WebDocumentSubresourceFilterImpl>
       filter_for_last_committed_load_;
 

components/subresource_filter/content/renderer/subresource_filter_agent_unittest.cc

@@ -59,6 +59,12 @@ class SubresourceFilterAgentUnderTest : public SubresourceFilterAgent {
     OnSetSubresourceFilterForCommittedLoadCalled();
   }
 
+  void SetIsAdSubframeForDocument(bool is_ad_subframe) override {
+    is_ad_subframe_ = is_ad_subframe;
+  }
+
+  bool GetIsAdSubframe() { return is_ad_subframe_; }
+
   blink::WebDocumentSubresourceFilter* filter() {
     return last_injected_filter_.get();
   }
@@ -68,6 +74,7 @@ class SubresourceFilterAgentUnderTest : public SubresourceFilterAgent {
   }
 
  private:
+  bool is_ad_subframe_ = false;
   std::unique_ptr<blink::WebDocumentSubresourceFilter> last_injected_filter_;
 
   DISALLOW_COPY_AND_ASSIGN(SubresourceFilterAgentUnderTest);
@@ -135,10 +142,12 @@ class SubresourceFilterAgentTest : public ::testing::Test {
     // No DidFinishLoad is called in this case.
   }
 
-  void StartLoadAndSetActivationState(ActivationState state) {
+  void StartLoadAndSetActivationState(ActivationState state,
+                                      bool is_ad_subframe = false) {
     agent_as_rfo()->DidStartProvisionalLoad(nullptr);
     EXPECT_TRUE(agent_as_rfo()->OnMessageReceived(
-        SubresourceFilterMsg_ActivateForNextCommittedLoad(0, state)));
+        SubresourceFilterMsg_ActivateForNextCommittedLoad(0, state,
+                                                          is_ad_subframe)));
     agent_as_rfo()->DidCommitProvisionalLoad(
         true /* is_new_navigation */, false /* is_same_document_navigation */);
   }
@@ -419,7 +428,8 @@ TEST_F(SubresourceFilterAgentTest,
   ActivationState state(ActivationLevel::ENABLED);
   state.measure_performance = true;
   EXPECT_TRUE(agent_as_rfo()->OnMessageReceived(
-      SubresourceFilterMsg_ActivateForNextCommittedLoad(0, state)));
+      SubresourceFilterMsg_ActivateForNextCommittedLoad(
+          0, state, false /* is_ad_subframe */)));
   agent_as_rfo()->DidFailProvisionalLoad(
       blink::WebURLError(net::ERR_FAILED, blink::WebURL()));
   agent_as_rfo()->DidStartProvisionalLoad(nullptr);
@@ -511,4 +521,16 @@ TEST_F(SubresourceFilterAgentTest,
   filter->ReportDisallowedLoad();
 }
 
+TEST_F(SubresourceFilterAgentTest, DryRun_DocumentIsAdSubframeTagging) {
+  base::HistogramTester histogram_tester;
+  ASSERT_NO_FATAL_FAILURE(
+      SetTestRulesetToDisallowURLsWithPathSuffix(kTestFirstURLPathSuffix));
+  ExpectSubresourceFilterGetsInjected();
+  StartLoadAndSetActivationState(ActivationState(ActivationLevel::DRYRUN),
+                                 true /* is_ad_subframe */);
+  ASSERT_TRUE(::testing::Mock::VerifyAndClearExpectations(agent()));
+
+  EXPECT_TRUE(agent()->GetIsAdSubframe());
+}
+
 }  // namespace subresource_filter

third_party/WebKit/Source/core/exported/WebDocumentLoaderImpl.cpp

@@ -198,6 +198,14 @@ void WebDocumentLoaderImpl::SetUserActivated() {
   DocumentLoader::SetUserActivated();
 }
 
+void WebDocumentLoaderImpl::SetIsAdSubframe(bool is_ad_subframe) {
+  GetSubresourceFilter()->SetIsAdSubframe(is_ad_subframe);
+}
+
+bool WebDocumentLoaderImpl::GetIsAdSubframe() const {
+  return GetSubresourceFilter()->GetIsAdSubframe();
+}
+
 void WebDocumentLoaderImpl::Trace(blink::Visitor* visitor) {
   DocumentLoader::Trace(visitor);
 }

third_party/WebKit/Source/core/exported/WebDocumentLoaderImpl.h

@@ -85,6 +85,8 @@ class CORE_EXPORT WebDocumentLoaderImpl final : public DocumentLoader,
   void SetSourceLocation(const WebSourceLocation&) override;
   void ResetSourceLocation() override;
   void SetUserActivated() override;
+  void SetIsAdSubframe(bool is_ad_subframe) override;
+  bool GetIsAdSubframe() const override;
 
   static WebNavigationType ToWebNavigationType(NavigationType);
 

third_party/WebKit/Source/core/loader/SubresourceFilter.cpp

@@ -45,7 +45,8 @@ SubresourceFilter::SubresourceFilter(
     ExecutionContext* execution_context,
     std::unique_ptr<WebDocumentSubresourceFilter> subresource_filter)
     : execution_context_(execution_context),
-      subresource_filter_(std::move(subresource_filter)) {}
+      subresource_filter_(std::move(subresource_filter)),
+      is_ad_subframe_(false) {}
 
 SubresourceFilter::~SubresourceFilter() = default;
 

third_party/WebKit/Source/core/loader/SubresourceFilter.h

@@ -34,6 +34,12 @@ class CORE_EXPORT SubresourceFilter final
                  SecurityViolationReportingPolicy);
   bool AllowWebSocketConnection(const KURL&);
 
+  void SetIsAdSubframe(bool is_ad_subframe) {
+    is_ad_subframe_ = is_ad_subframe;
+  }
+
+  bool GetIsAdSubframe() { return is_ad_subframe_; }
+
   virtual void Trace(blink::Visitor*);
 
  private:
@@ -45,6 +51,7 @@ class CORE_EXPORT SubresourceFilter final
 
   Member<ExecutionContext> execution_context_;
   std::unique_ptr<WebDocumentSubresourceFilter> subresource_filter_;
+  bool is_ad_subframe_;
 };
 
 }  // namespace blink

third_party/WebKit/public/web/WebDocumentLoader.h

@@ -144,6 +144,10 @@ class BLINK_EXPORT WebDocumentLoader {
   // initiated loads that may have had a user activation from the browser UI.
   virtual void SetUserActivated() = 0;
 
+  // Sets if the document is an ad identified subframe.
+  virtual void SetIsAdSubframe(bool is_ad_subframe) = 0;
+  virtual bool GetIsAdSubframe() const = 0;
+
  protected:
   ~WebDocumentLoader() = default;
 };