Skip to content

Commit

Permalink
Fix ad tagging for MPArch fenced frames in subresource filter
Browse files Browse the repository at this point in the history
This is a followup to https://crrev.com/5a0d4e585a which changed the
subresource filter to treat MPArch fenced frames as child frames for the
purpose of ad filtering.

This CL updates ad tagging to match so that MPArch fenced frames,
despite having their own frame tree and main frame, are treated like
iframes. ShadowDOM fenced frames are implemented using a regular child
frame so their subresource filtering behavior is already correct. The
discussion below is specific to MPArch.

The main complication here is making script-based ad tagging work for
fenced frame roots. This works by searching the v8 stack for ad scripts
when a RenderFrame is created and relies on the RenderFrame being
created and initialized synchronously with the JavaScript call that
creates the frame. However, an MPArch fenced frame does not create a
RenderFrame in its embedder at all; its root RenderFrame is initialized
in a new frame tree and renderer at which point the creating v8 stack is
gone and inaccessible.

This CL makes fenced frame creation take a special path by inspecting
the v8 stack at the time the CreateFencedFrame call is made and tagging
it separately at that time.

The remainder of this CL simply generalizes GetParent checks to also
cross fenced frame boundaries, where necessary, and allow fenced frame
main frames in methods that previously assumed only non-main frames.

Bug: 1263541
Change-Id: I92d6bfbd509ad7abdcef28bc6b0c80fd126acf99
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3537448
Reviewed-by: Alex Turner <alexmt@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Reviewed-by: Will Harris <wfh@chromium.org>
Reviewed-by: Josh Karlin <jkarlin@chromium.org>
Reviewed-by: Dave Tapuska <dtapuska@chromium.org>
Commit-Queue: David Bokan <bokan@chromium.org>
Cr-Commit-Position: refs/heads/main@{#988077}
  • Loading branch information
bokand authored and Chromium LUCI CQ committed Apr 1, 2022
1 parent 2ba12dc commit c53c9bb
Show file tree
Hide file tree
Showing 31 changed files with 681 additions and 127 deletions.
270 changes: 269 additions & 1 deletion chrome/browser/subresource_filter/ad_tagging_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -684,7 +684,7 @@ IN_PROC_BROWSER_TEST_F(
ui_test_utils::NavigateToURL(browser(), GetURL("frame_factory.html")));

// Create a frame and abort its initial load in vanilla script. The children
// of this vanilla frame should be taggged correctly.
// of this vanilla frame should be tagged correctly.
content::RenderFrameHost* vanilla_frame_with_aborted_load =
CreateFrameWithDocWriteAbortedLoad(GetWebContents());

Expand Down Expand Up @@ -1111,6 +1111,274 @@ INSTANTIATE_TEST_SUITE_P(
AdTaggingEventWithScriptInStackBrowserTest,
::testing::Bool());

class AdTaggingFencedFrameBrowserTest : public AdTaggingBrowserTest {
public:
AdTaggingFencedFrameBrowserTest() = default;
~AdTaggingFencedFrameBrowserTest() override = default;

content::test::FencedFrameTestHelper& fenced_frame_test_helper() {
return fenced_frame_test_helper_;
}

void SetUpOnMainThread() override {
AdTaggingBrowserTest::SetUpOnMainThread();
observer_ = std::make_unique<TestSubresourceFilterObserver>(web_contents());
}

bool EvaluatedSubframeLoad(const GURL url) {
return observer_->GetSubframeLoadPolicy(url).has_value();
}

bool IsAdSubframe(RenderFrameHost* host) {
return observer_->GetIsAdSubframe(host->GetFrameTreeNodeId());
}

RenderFrameHost* PrimaryMainFrame() { return web_contents()->GetMainFrame(); }

private:
content::test::FencedFrameTestHelper fenced_frame_test_helper_;
std::unique_ptr<TestSubresourceFilterObserver> observer_;
};

// Test that a fenced frame itself can be tagged as an ad and that iframes
// nested within it see it as an ancestor ad frame.
IN_PROC_BROWSER_TEST_F(AdTaggingFencedFrameBrowserTest,
FencedFrameMatchingSuffixRuleIsTagged) {
ASSERT_TRUE(ui_test_utils::NavigateToURL(
browser(), GetURL("frame_factory.html?primary")));

// Create a fenced frame which should be tagged as an ad.
const GURL kUrl = GetURL("frame_factory.html?fencedframe&ad=true");
RenderFrameHost* fenced_frame = CreateFencedFrame(PrimaryMainFrame(), kUrl);

EXPECT_TRUE(EvaluatedSubframeLoad(kUrl));
EXPECT_TRUE(IsAdSubframe(fenced_frame));
EXPECT_TRUE(EvidenceForFrameComprises(
fenced_frame, /*parent_is_ad=*/false,
blink::mojom::FilterListResult::kMatchedBlockingRule,
blink::mojom::FrameCreationStackEvidence::kNotCreatedByAdScript));

// Create an iframe in the fenced frame that doesn't match any rules. Since
// the fenced frame was tagged as an ad, this should be as well.
GURL kInnerUrl = GetURL("test_div.html");
RenderFrameHost* inner_frame = CreateSrcFrame(fenced_frame, kInnerUrl);

EXPECT_TRUE(EvaluatedSubframeLoad(kInnerUrl));
EXPECT_TRUE(IsAdSubframe(inner_frame));
// Note: kCreatedByAdScript is expected since any script running in an ad is
// considered ad script.
EXPECT_TRUE(EvidenceForFrameComprises(
inner_frame, /*parent_is_ad=*/true,
blink::mojom::FilterListResult::kMatchedNoRules,
blink::mojom::FrameCreationStackEvidence::kCreatedByAdScript));
}

// Test that a fenced frame created by ad script is tagged as an ad.
IN_PROC_BROWSER_TEST_F(AdTaggingFencedFrameBrowserTest,
FencedFrameCreatedByAdScriptIsTagged) {
ASSERT_TRUE(ui_test_utils::NavigateToURL(
browser(), GetURL("frame_factory.html?primary")));

{
// Create a fenced frame from an ad script so it should be tagged as an ad.
const GURL kUrl = GetURL("frame_factory.html?fencedframe");
RenderFrameHost* fenced_frame =
CreateFencedFrameFromAdScript(PrimaryMainFrame(), kUrl);

EXPECT_TRUE(EvaluatedSubframeLoad(kUrl));
EXPECT_TRUE(IsAdSubframe(fenced_frame));
EXPECT_TRUE(EvidenceForFrameComprises(
fenced_frame, /*parent_is_ad=*/false,
blink::mojom::FilterListResult::kMatchedNoRules,
blink::mojom::FrameCreationStackEvidence::kCreatedByAdScript));

// Create a plain frame in the fenced frame. Since the fenced frame was
// tagged as an ad, this should be as well.
const GURL kInnerUrl = GetURL("test_div.html");
RenderFrameHost* inner_frame = CreateSrcFrame(fenced_frame, kInnerUrl);

EXPECT_TRUE(EvaluatedSubframeLoad(kInnerUrl));
EXPECT_TRUE(IsAdSubframe(inner_frame));
EXPECT_TRUE(EvidenceForFrameComprises(
inner_frame, /*parent_is_ad=*/true,
blink::mojom::FilterListResult::kMatchedNoRules,
blink::mojom::FrameCreationStackEvidence::kCreatedByAdScript));
}

// Create a fenced frame from a non-ad script, ensure it isn't tagged.
{
const GURL kUrl = GetURL("frame_factory.html?non+ad+fencedframe");
RenderFrameHost* fenced_frame = CreateFencedFrame(PrimaryMainFrame(), kUrl);

EXPECT_TRUE(EvaluatedSubframeLoad(kUrl));
EXPECT_FALSE(IsAdSubframe(fenced_frame));
}
}

// Test that a fenced frame not matching any rules, created in an ad-frame is
// also tagged as an ad.
IN_PROC_BROWSER_TEST_F(AdTaggingFencedFrameBrowserTest,
FencedFrameWithAdParentIsTagged) {
ASSERT_TRUE(ui_test_utils::NavigateToURL(
browser(), GetURL("frame_factory.html?primary")));

{
// Create a fenced frame to an ad URL.
const GURL kAdUrl = GetURL("frame_factory.html?fencedframe&ad=true");
RenderFrameHost* ad_frame = CreateFencedFrame(PrimaryMainFrame(), kAdUrl);

ASSERT_TRUE(EvaluatedSubframeLoad(kAdUrl));
ASSERT_TRUE(IsAdSubframe(ad_frame));

// Create another fenced frame, nested in the first one, that doesn't match
// any rules. Since the first fenced frame was tagged as an ad, the inner
// one should be as well.
const GURL kInnerUrl = GetURL("test_div.html?nested+in+fenced+frame");
RenderFrameHost* inner_frame = CreateFencedFrame(ad_frame, kInnerUrl);

EXPECT_TRUE(EvaluatedSubframeLoad(kInnerUrl));
EXPECT_TRUE(IsAdSubframe(inner_frame));
// Note: kCreatedByAdScript is expected since any script running in an ad
// is considered ad script.
EXPECT_TRUE(EvidenceForFrameComprises(
inner_frame, /*parent_is_ad=*/true,
blink::mojom::FilterListResult::kMatchedNoRules,
blink::mojom::FrameCreationStackEvidence::kCreatedByAdScript));
}

{
// Create an iframe to an ad URL
const GURL kAdUrl = GetURL("frame_factory.html?iframe&ad=true");
RenderFrameHost* ad_frame = CreateSrcFrame(PrimaryMainFrame(), kAdUrl);

ASSERT_TRUE(EvaluatedSubframeLoad(kAdUrl));
ASSERT_TRUE(IsAdSubframe(ad_frame));

// Create a fenced frame, nested in the ad iframe, that doesn't match any
// rules. Since the iframe was tagged as an ad, the fenced frame should be
// as well.
const GURL kInnerUrl = GetURL("test_div.html?nested+in+iframe");
RenderFrameHost* inner_frame = CreateFencedFrame(ad_frame, kInnerUrl);

EXPECT_TRUE(EvaluatedSubframeLoad(kInnerUrl));
EXPECT_TRUE(IsAdSubframe(inner_frame));
// Note: kCreatedByAdScript is expected since any script running in an ad
// is considered ad script.
EXPECT_TRUE(EvidenceForFrameComprises(
inner_frame, /*parent_is_ad=*/true,
blink::mojom::FilterListResult::kMatchedNoRules,
blink::mojom::FrameCreationStackEvidence::kCreatedByAdScript));
}
}

// Test ad tagging for iframes nested within a fenced frame.
IN_PROC_BROWSER_TEST_F(AdTaggingFencedFrameBrowserTest,
IFrameNestedInFencedFrameIsTagged) {
ASSERT_TRUE(ui_test_utils::NavigateToURL(
browser(), GetURL("frame_factory.html?primary")));

// Create a fenced frame into which we'll nest iframes.
const GURL kOuterUrl = GetURL("frame_factory.html?fencedframe");
RenderFrameHost* fenced_frame =
CreateFencedFrame(PrimaryMainFrame(), kOuterUrl);

ASSERT_TRUE(EvaluatedSubframeLoad(kOuterUrl));
ASSERT_FALSE(IsAdSubframe(fenced_frame));

// Create an iframe inside the fenced frame that matches an ad rule. It
// should be tagged as an ad.
{
const GURL kAdUrl = GetURL("test_div.html?ad=true");
RenderFrameHost* inner_frame = CreateSrcFrame(fenced_frame, kAdUrl);

EXPECT_TRUE(EvaluatedSubframeLoad(kAdUrl));
EXPECT_TRUE(IsAdSubframe(inner_frame));
EXPECT_TRUE(EvidenceForFrameComprises(
inner_frame, /*parent_is_ad=*/false,
blink::mojom::FilterListResult::kMatchedBlockingRule,
blink::mojom::FrameCreationStackEvidence::kNotCreatedByAdScript));
}

// Create an iframe inside the fenced frame that doesn't match any rules but
// is created from an ad script. It should be tagged as an ad.
{
const GURL kNotAdUrl = GetURL("test_div.html?not+an+ad");
RenderFrameHost* inner_frame =
CreateSrcFrameFromAdScript(fenced_frame, kNotAdUrl);

EXPECT_TRUE(EvaluatedSubframeLoad(kNotAdUrl));
EXPECT_TRUE(IsAdSubframe(inner_frame));
EXPECT_TRUE(EvidenceForFrameComprises(
inner_frame, /*parent_is_ad=*/false,
blink::mojom::FilterListResult::kMatchedNoRules,
blink::mojom::FrameCreationStackEvidence::kCreatedByAdScript));
}

// Create an iframe inside the fenced frame that doesn't match any rules. It
// should not be tagged as an ad.
{
const GURL kNotAdUrl = GetURL("test_div.html?also+not+an+ad");
RenderFrameHost* inner_frame = CreateSrcFrame(fenced_frame, kNotAdUrl);

EXPECT_TRUE(EvaluatedSubframeLoad(kNotAdUrl));
EXPECT_FALSE(IsAdSubframe(inner_frame));
}
}

// Test ad tagging for fenced frames nested within a fenced frame.
IN_PROC_BROWSER_TEST_F(AdTaggingFencedFrameBrowserTest,
FencedFrameNestedInFencedFrameIsTagged) {
ASSERT_TRUE(ui_test_utils::NavigateToURL(
browser(), GetURL("frame_factory.html?primary")));

// Create a fenced frame into which we'll nest other fenced frames.
const GURL kOuterUrl = GetURL("frame_factory.html?fencedframe");
RenderFrameHost* fenced_frame =
CreateFencedFrame(PrimaryMainFrame(), kOuterUrl);

ASSERT_TRUE(EvaluatedSubframeLoad(kOuterUrl));
ASSERT_FALSE(IsAdSubframe(fenced_frame));

// Create a fenced frame inside the fenced frame that matches an ad rule. It
// should be tagged as an ad.
{
const GURL kAdUrl = GetURL("test_div.html?ad=true");
RenderFrameHost* inner_frame = CreateFencedFrame(fenced_frame, kAdUrl);

EXPECT_TRUE(EvaluatedSubframeLoad(kAdUrl));
EXPECT_TRUE(IsAdSubframe(inner_frame));
EXPECT_TRUE(EvidenceForFrameComprises(
inner_frame, /*parent_is_ad=*/false,
blink::mojom::FilterListResult::kMatchedBlockingRule,
blink::mojom::FrameCreationStackEvidence::kNotCreatedByAdScript));
}

// Create a fenced frame inside the fenced frame that doesn't match any rules
// but is created from an ad script. It should be tagged as an ad.
{
const GURL kNotAdUrl = GetURL("test_div.html?not+an+ad");
RenderFrameHost* inner_frame =
CreateFencedFrameFromAdScript(fenced_frame, kNotAdUrl);

EXPECT_TRUE(EvaluatedSubframeLoad(kNotAdUrl));
EXPECT_TRUE(IsAdSubframe(inner_frame));
EXPECT_TRUE(EvidenceForFrameComprises(
inner_frame, /*parent_is_ad=*/false,
blink::mojom::FilterListResult::kMatchedNoRules,
blink::mojom::FrameCreationStackEvidence::kCreatedByAdScript));
}

// Create a fenced frame inside the fenced frame that doesn't match any
// rules. It should not be tagged as an ad.
{
const GURL kNotAdUrl = GetURL("test_div.html?also+not+an+ad");
RenderFrameHost* inner_frame = CreateFencedFrame(fenced_frame, kNotAdUrl);

EXPECT_TRUE(EvaluatedSubframeLoad(kNotAdUrl));
EXPECT_FALSE(IsAdSubframe(inner_frame));
}
}

} // namespace

} // namespace subresource_filter
Original file line number Diff line number Diff line change
Expand Up @@ -1113,7 +1113,7 @@ TEST_F(AdsPageLoadMetricsObserverTest, CountAbortedNavigation) {
NavigationSimulator::CreateRendererInitiated(GURL(kAdUrl), subframe_ad);
// The sub-frame renavigates before it commits.
navigation_simulator->Start();
SetIsAdSubframe(subframe_ad, /*is_ad_subframe=*/true);
SetIsAdFrame(subframe_ad, /*is_ad_frame=*/true);
navigation_simulator->Fail(net::ERR_ABORTED);

// Load resources for the aborted frame (e.g., simulate the navigation
Expand Down Expand Up @@ -1142,7 +1142,7 @@ TEST_F(AdsPageLoadMetricsObserverTest, CountAbortedSecondNavigationForFrame) {
NavigationSimulator::CreateRendererInitiated(GURL(kAdUrl), sub_frame);
// The sub-frame renavigates before it commits.
navigation_simulator->Start();
SetIsAdSubframe(sub_frame, /*is_ad_subframe=*/true);
SetIsAdFrame(sub_frame, /*is_ad_frame=*/true);
navigation_simulator->Fail(net::ERR_ABORTED);

// Load resources for the aborted frame (e.g., simulate the navigation
Expand Down Expand Up @@ -1173,7 +1173,7 @@ TEST_F(AdsPageLoadMetricsObserverTest, TwoResourceLoadsBeforeCommit) {

// The sub-frame renavigates before it commits.
navigation_simulator->Start();
SetIsAdSubframe(subframe_ad, /*is_ad_subframe=*/true);
SetIsAdFrame(subframe_ad, /*is_ad_frame=*/true);
navigation_simulator->Fail(net::ERR_ABORTED);

// Renavigate the subframe to a successful commit. But again, the resource
Expand All @@ -1198,7 +1198,7 @@ TEST_F(AdsPageLoadMetricsObserverTest, UntaggingAdFrame) {
// Renavigate and untag the ad frame.
auto navigation_simulator =
NavigationSimulator::CreateRendererInitiated(GURL(kNonAdUrl), ad_frame);
SetIsAdSubframe(ad_frame, /*is_ad_subframe=*/false);
SetIsAdFrame(ad_frame, /*is_ad_frame=*/false);
navigation_simulator->Commit();

ResourceDataUpdate(navigation_simulator->GetFinalRenderFrameHost(),
Expand Down Expand Up @@ -1408,7 +1408,7 @@ TEST_F(AdsPageLoadMetricsObserverTest,
RenderFrameHostTester::For(main_frame)->AppendChild("frame_name");
auto navigation_simulator = NavigationSimulator::CreateRendererInitiated(
GURL("https://foo.com"), subframe);
SetIsAdSubframe(subframe, /*is_ad_subframe=*/true);
SetIsAdFrame(subframe, /*is_ad_frame=*/true);
navigation_simulator->Commit();

subframe = navigation_simulator->GetFinalRenderFrameHost();
Expand Down

0 comments on commit c53c9bb

Please sign in to comment.