Skip to content

Commit

Permalink
Add a new signal IsCrossSiteFrame to DocumentCreated event.
Browse files Browse the repository at this point in the history
The signal indicates whether a non-top-level UKM source belongs to a cross-site security origin or a same-site security origin.
UKM proposal: https://docs.google.com/document/d/1_lodiN5x-rP2Wh-xQZJbVyOoTln8Vm9xBWatUJiXZqM/edit#

Bug: b/177260736
Change-Id: Ibc3113ee45835cc8e2b6829698267586a0e67795
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2678561
Reviewed-by: Aaron Colwell <acolwell@chromium.org>
Reviewed-by: Asanka Herath <asanka@chromium.org>
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Commit-Queue: Qingxin Wu <qingxinwu@google.com>
Cr-Commit-Position: refs/heads/master@{#858168}
  • Loading branch information
Qingxin Wu authored and Chromium LUCI CQ committed Feb 26, 2021
1 parent 3215a98 commit 6ab5abc
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 1 deletion.
10 changes: 10 additions & 0 deletions content/browser/renderer_host/render_frame_host_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10401,10 +10401,20 @@ void RenderFrameHostImpl::RecordDocumentCreatedUkmEvent(
!is_main_frame() &&
!GetMainFrame()->GetLastCommittedOrigin().IsSameOriginWith(origin);

// Compares the subframe site with the main frame site. In the case of
// nested subframes such as A(B(A)), the bottom-most frame A is expected to
// have |is_cross_site_frame| set to false, even though this frame is cross-
// site from its parent frame B. This value is only used in manual analysis.
bool is_cross_site_frame =
!is_main_frame() &&
(net::SchemefulSite(origin) !=
net::SchemefulSite(GetMainFrame()->GetLastCommittedOrigin()));

ukm::builders::DocumentCreated(document_ukm_source_id)
.SetNavigationSourceId(GetPageUkmSourceId())
.SetIsMainFrame(is_main_frame())
.SetIsCrossOriginFrame(is_cross_origin_frame)
.SetIsCrossSiteFrame(is_cross_site_frame)
.Record(ukm_recorder);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4731,7 +4731,7 @@ IN_PROC_BROWSER_TEST_F(RenderFrameHostImplBrowserTest, GetUkmSourceIds) {

const auto& document_created_entries =
recorder.GetEntriesByName("DocumentCreated");
// There should one DocumentCreated entry for each of the two frames.
// There should be one DocumentCreated entry for each of the two frames.
ASSERT_EQ(2u, document_created_entries.size());

auto* main_frame_document_created_entry =
Expand All @@ -4747,6 +4747,8 @@ IN_PROC_BROWSER_TEST_F(RenderFrameHostImplBrowserTest, GetUkmSourceIds) {
"IsMainFrame"));
EXPECT_FALSE(*recorder.GetEntryMetric(main_frame_document_created_entry,
"IsCrossOriginFrame"));
EXPECT_FALSE(*recorder.GetEntryMetric(main_frame_document_created_entry,
"IsCrossSiteFrame"));

EXPECT_EQ(page_ukm_source_id,
*recorder.GetEntryMetric(sub_frame_document_created_entry,
Expand All @@ -4755,6 +4757,8 @@ IN_PROC_BROWSER_TEST_F(RenderFrameHostImplBrowserTest, GetUkmSourceIds) {
"IsMainFrame"));
EXPECT_TRUE(*recorder.GetEntryMetric(sub_frame_document_created_entry,
"IsCrossOriginFrame"));
EXPECT_TRUE(*recorder.GetEntryMetric(sub_frame_document_created_entry,
"IsCrossSiteFrame"));

// Verify source creations. Main frame document source should have the URL;
// no source should have been created for the sub-frame document.
Expand All @@ -4770,6 +4774,29 @@ IN_PROC_BROWSER_TEST_F(RenderFrameHostImplBrowserTest, GetUkmSourceIds) {
}
}

IN_PROC_BROWSER_TEST_F(RenderFrameHostImplBrowserTest, CrossSiteFrame) {
ukm::TestAutoSetUkmRecorder recorder;
// This test site has one cross-origin but same-site iframe (b.x.com).
GURL main_frame_url(embedded_test_server()->GetURL(
"a.x.com", "/frame_tree/page_with_cross_origin_same_site_iframe.html"));
WebContents* web_contents = shell()->web_contents();
DocumentUkmSourceIdObserver observer(web_contents);

ASSERT_TRUE(NavigateToURL(shell(), main_frame_url));

auto* sub_frame_document_created_entry =
recorder.GetDocumentCreatedEntryForSourceId(
observer.GetSubFrameDocumentUkmSourceId());

// Verify the recorded values on the sub frame's DocumentCreated entry.
EXPECT_FALSE(*recorder.GetEntryMetric(sub_frame_document_created_entry,
"IsMainFrame"));
EXPECT_TRUE(*recorder.GetEntryMetric(sub_frame_document_created_entry,
"IsCrossOriginFrame"));
EXPECT_FALSE(*recorder.GetEntryMetric(sub_frame_document_created_entry,
"IsCrossSiteFrame"));
}

// TODO(https://crbug.com/794320): the code below is temporary and will be
// removed when Java Bridge is mojofied.
#if defined(OS_ANDROID)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<!DOCTYPE html>
<head>
</head>
<body>
This page contains a cross-origin but same-site iframe.
<iframe name="frame0" src="/cross-site/b.x.com/title1.html"></iframe>
</body>
</html>

6 changes: 6 additions & 0 deletions tools/metrics/ukm/ukm.xml
Original file line number Diff line number Diff line change
Expand Up @@ -4299,6 +4299,12 @@ be describing additional metrics about the same event.
1.
</summary>
</metric>
<metric name="IsCrossSiteFrame">
<summary>
Whether the document was in a cross site iframe. This can either be 0 or
1.
</summary>
</metric>
<metric name="IsMainFrame">
<summary>
Whether the document was in the main frame. This is can either be 0 or 1.
Expand Down

0 comments on commit 6ab5abc

Please sign in to comment.