Propagation of ad tagging from parent frame to child frame.

This CL tags a subframe as an ad if its parent frame is tagged as well.
This extends the coverage of frames tagged as ads from just those that
match the ruleset to those that do not match but their parent frame is
tagged as an ad frame.

BUG: 807640
Change-Id: Ifee656298cdbe8bc16bcd24a16bc9a9905e2c2a3
Reviewed-on: https://chromium-review.googlesource.com/894542
Commit-Queue: Shivani Sharma <shivanisha@chromium.org>
Reviewed-by: Josh Karlin <jkarlin@chromium.org>
Reviewed-by: Charlie Harrison <csharrison@chromium.org>
Cr-Commit-Position: refs/heads/master@{#533323}

components/subresource_filter/content/browser/content_subresource_filter_throttle_manager.cc

@@ -56,6 +56,7 @@ void ContentSubresourceFilterThrottleManager::OnSubresourceFilterGoingAway() {
 void ContentSubresourceFilterThrottleManager::RenderFrameDeleted(
     content::RenderFrameHost* frame_host) {
   activated_frame_hosts_.erase(frame_host);
+  ad_frames_.erase(frame_host);
   DestroyRulesetHandleIfNoLongerUsed();
 }
 
@@ -103,6 +104,9 @@ void ContentSubresourceFilterThrottleManager::ReadyToCommitNavigation(
 
   content::RenderFrameHost* frame_host =
       navigation_handle->GetRenderFrameHost();
+  if (is_ad_subframe)
+    ad_frames_.insert(frame_host);
+
   frame_host->Send(new SubresourceFilterMsg_ActivateForNextCommittedLoad(
       frame_host->GetRoutingID(), filter->activation_state(), is_ad_subframe));
 }
@@ -216,11 +220,21 @@ void ContentSubresourceFilterThrottleManager::OnSubframeNavigationEvaluated(
     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;
+  if (it == ongoing_activation_throttles_.end())
+    return;
+
+  // 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;
+
+  // If this frame was not identified as an ad via ruleset matching, tag it
+  // based on whether its parent frame is an ad or not.
+  if (!it->second.is_ad_subframe) {
+    content::RenderFrameHost* parent_frame =
+        navigation_handle->GetParentFrame();
+    if (parent_frame && base::ContainsKey(ad_frames_, parent_frame))
+      it->second.is_ad_subframe = true;
   }
 }
 
@@ -247,6 +261,11 @@ void ContentSubresourceFilterThrottleManager::MaybeAppendNavigationThrottles(
   }
 }
 
+bool ContentSubresourceFilterThrottleManager::IsFrameTaggedAsAdForTesting(
+    content::RenderFrameHost* frame_host) {
+  return base::ContainsKey(ad_frames_, frame_host);
+}
+
 std::unique_ptr<SubframeNavigationFilteringThrottle>
 ContentSubresourceFilterThrottleManager::
     MaybeCreateSubframeNavigationFilteringThrottle(

components/subresource_filter/content/browser/content_subresource_filter_throttle_manager.h

@@ -5,13 +5,15 @@
 #ifndef COMPONENTS_SUBRESOURCE_FILTER_CONTENT_BROWSER_CONTENT_SUBRESOURCE_FILTER_THROTTLE_MANAGER_H_
 #define COMPONENTS_SUBRESOURCE_FILTER_CONTENT_BROWSER_CONTENT_SUBRESOURCE_FILTER_THROTTLE_MANAGER_H_
 
+#include <map>
 #include <memory>
-#include <unordered_map>
+#include <set>
 #include <vector>
 
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 #include "base/scoped_observer.h"
+#include "base/stl_util.h"
 #include "components/subresource_filter/content/browser/subresource_filter_observer.h"
 #include "components/subresource_filter/content/browser/verified_ruleset_dealer.h"
 #include "components/subresource_filter/core/common/activation_decision.h"
@@ -84,6 +86,8 @@ class ContentSubresourceFilterThrottleManager
     return ruleset_handle_.get();
   }
 
+  bool IsFrameTaggedAsAdForTesting(content::RenderFrameHost* frame_host);
+
  protected:
   // content::WebContentsObserver:
   void RenderFrameDeleted(content::RenderFrameHost* frame_host) override;
@@ -143,8 +147,8 @@ class ContentSubresourceFilterThrottleManager
   // For each RenderFrameHost where the last committed load has subresource
   // filtering activated, owns the corresponding AsyncDocumentSubresourceFilter.
   // It is possible for a frame to have a null filter.
-  std::unordered_map<content::RenderFrameHost*,
-                     std::unique_ptr<AsyncDocumentSubresourceFilter>>
+  std::map<content::RenderFrameHost*,
+           std::unique_ptr<AsyncDocumentSubresourceFilter>>
       activated_frame_hosts_;
 
   // For each ongoing navigation that requires activation state computation,
@@ -157,9 +161,13 @@ class ContentSubresourceFilterThrottleManager
     bool is_ad_subframe = false;
   };
 
-  std::unordered_map<content::NavigationHandle*, OngoingThrottleInfo>
+  std::map<content::NavigationHandle*, OngoingThrottleInfo>
       ongoing_activation_throttles_;
 
+  // Set of frames that have been identified as ads, either through matching the
+  // ruleset or if their parent frame was an ad frame.
+  std::set<content::RenderFrameHost*> ad_frames_;
+
   ScopedObserver<SubresourceFilterObserverManager, SubresourceFilterObserver>
       scoped_observer_;
 

components/subresource_filter/content/browser/content_subresource_filter_throttle_manager_unittest.cc

@@ -745,6 +745,81 @@ TEST_P(ContentSubresourceFilterThrottleManagerTest,
                                  true /* is_ad_subframe */);
 }
 
+TEST_P(ContentSubresourceFilterThrottleManagerTest,
+       DryRun_FrameTaggingAsAdPropagatesToChildFrame) {
+  NavigateAndCommitMainFrame(GURL(kTestURLWithDryRun));
+  ExpectActivationSignalForFrame(main_rfh(), true /* expect_activation */);
+
+  // A disallowed subframe navigation should not be filtered in dry-run mode.
+  CreateSubframeWithTestNavigation(
+      GURL("https://www.example.com/disallowed.html"), main_rfh());
+  SimulateStartAndExpectResult(content::NavigationThrottle::PROCEED);
+  content::RenderFrameHost* child =
+      SimulateCommitAndExpectResult(content::NavigationThrottle::PROCEED);
+  EXPECT_TRUE(child);
+
+  // But it should still be activated.
+  ExpectActivationSignalForFrame(child, true /* expect_activation */,
+                                 true /* is_ad_subframe */);
+  EXPECT_TRUE(throttle_manager()->IsFrameTaggedAsAdForTesting(child));
+
+  // Create a subframe which is allowed as per ruleset but should still be
+  // tagged as ad because of its parent.
+  CreateSubframeWithTestNavigation(
+      GURL("https://www.example.com/allowed_by_ruleset.html"), child);
+  SimulateStartAndExpectResult(content::NavigationThrottle::PROCEED);
+  content::RenderFrameHost* grandchild =
+      SimulateCommitAndExpectResult(content::NavigationThrottle::PROCEED);
+  EXPECT_TRUE(grandchild);
+  ExpectActivationSignalForFrame(grandchild, true /* expect_activation */,
+                                 true /* is_ad_subframe */);
+  EXPECT_TRUE(throttle_manager()->IsFrameTaggedAsAdForTesting(grandchild));
+
+  // Verify that a 2nd level nested frame should also be tagged.
+  CreateSubframeWithTestNavigation(
+      GURL("https://www.example.com/great_grandchild_allowed_by_ruleset.html"),
+      child);
+  SimulateStartAndExpectResult(content::NavigationThrottle::PROCEED);
+  content::RenderFrameHost* greatGrandchild =
+      SimulateCommitAndExpectResult(content::NavigationThrottle::PROCEED);
+  EXPECT_TRUE(greatGrandchild);
+  ExpectActivationSignalForFrame(greatGrandchild, true /* expect_activation */,
+                                 true /* is_ad_subframe */);
+  EXPECT_TRUE(throttle_manager()->IsFrameTaggedAsAdForTesting(greatGrandchild));
+
+  EXPECT_EQ(0, disallowed_notification_count());
+}
+
+TEST_P(ContentSubresourceFilterThrottleManagerTest,
+       DryRun_AllowedsFrameNotTaggedAsAd) {
+  NavigateAndCommitMainFrame(GURL(kTestURLWithDryRun));
+  ExpectActivationSignalForFrame(main_rfh(), true /* expect_activation */);
+
+  CreateSubframeWithTestNavigation(
+      GURL("https://www.example.com/allowed_by_ruleset.html"), main_rfh());
+  SimulateStartAndExpectResult(content::NavigationThrottle::PROCEED);
+  content::RenderFrameHost* child =
+      SimulateCommitAndExpectResult(content::NavigationThrottle::PROCEED);
+  EXPECT_TRUE(child);
+  ExpectActivationSignalForFrame(child, true /* expect_activation */,
+                                 false /* is_ad_subframe */);
+  EXPECT_FALSE(throttle_manager()->IsFrameTaggedAsAdForTesting(child));
+
+  // Create a subframe which is allowed as per ruleset and should not be tagged
+  // as ad because its parent is not tagged as well.
+  CreateSubframeWithTestNavigation(
+      GURL("https://www.example.com/also_allowed_by_ruleset.html"), child);
+  SimulateStartAndExpectResult(content::NavigationThrottle::PROCEED);
+  content::RenderFrameHost* grandchild =
+      SimulateCommitAndExpectResult(content::NavigationThrottle::PROCEED);
+  EXPECT_TRUE(grandchild);
+  ExpectActivationSignalForFrame(grandchild, true /* expect_activation */,
+                                 false /* is_ad_subframe */);
+  EXPECT_FALSE(throttle_manager()->IsFrameTaggedAsAdForTesting(grandchild));
+
+  EXPECT_EQ(0, disallowed_notification_count());
+}
+
 // TODO(csharrison): Make sure the following conditions are exercised in tests:
 //
 // - Synchronous navigations to about:blank. These hit issues with the