Commit 72b53188 authored by Justin Miron's avatar Justin Miron Committed by Commit Bot

Reland "Propagate main frame document dimensions through frame intersections."

This is a reland of 2412286a. Is not
the culprit for https://crbug.com/1066455, the cause of the revert.

Original change's description:
> Propagate main frame document dimensions through frame intersections.
>
> This change calls OnMainFrameDocumentIntersectionChanged for the main
> frame. The RenderFrame resets the main frame's intersection when the
> frame commits a navigation to ensure we propagate a new intersection
> for each committed navigation (otherwise we may lose updates when the
> render frame observer propagates values for uncommitted loads to
> page load metrics).
>
> This allows consumers to compare frame intersections with the main
> frame's rect. This is a precursor to measuring page ad density.
>
> BUG=993453
>
> Change-Id: I689505c086ed0216af9f216ed4dd3d13f9c3ec1d
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2105622
> Commit-Queue: Justin Miron <justinmiron@google.com>
> Reviewed-by: Robert Sesek <rsesek@chromium.org>
> Reviewed-by: John Delaney <johnidel@chromium.org>
> Reviewed-by: Stefan Zager <szager@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#754626}

Bug: 993453
Change-Id: I5c08939d9cb8181eb4dac2d7fa5d55919fe848c5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2128984Reviewed-by: default avatarStefan Zager <szager@chromium.org>
Reviewed-by: default avatarRobert Sesek <rsesek@chromium.org>
Reviewed-by: default avatarJohn Delaney <johnidel@chromium.org>
Commit-Queue: Justin Miron <justinmiron@google.com>
Cr-Commit-Position: refs/heads/master@{#755080}
parent 493ca51b
...@@ -17,6 +17,7 @@ ...@@ -17,6 +17,7 @@
#include "base/memory/weak_ptr.h" #include "base/memory/weak_ptr.h"
#include "base/path_service.h" #include "base/path_service.h"
#include "base/run_loop.h" #include "base/run_loop.h"
#include "base/strings/string_number_conversions.h"
#include "base/strings/string_util.h" #include "base/strings/string_util.h"
#include "base/test/metrics/histogram_tester.h" #include "base/test/metrics/histogram_tester.h"
#include "base/test/scoped_feature_list.h" #include "base/test/scoped_feature_list.h"
...@@ -2490,6 +2491,44 @@ IN_PROC_BROWSER_TEST_F(PageLoadMetricsBrowserTest, PreCommitWebFeature) { ...@@ -2490,6 +2491,44 @@ IN_PROC_BROWSER_TEST_F(PageLoadMetricsBrowserTest, PreCommitWebFeature) {
static_cast<int32_t>(WebFeature::kSecureContextCheckFailed), 0); static_cast<int32_t>(WebFeature::kSecureContextCheckFailed), 0);
} }
IN_PROC_BROWSER_TEST_F(PageLoadMetricsBrowserTest,
MainFrameDocumentIntersectionsMainFrame) {
ASSERT_TRUE(embedded_test_server()->Start());
auto waiter = CreatePageLoadMetricsTestWaiter();
content::WebContents* web_contents =
browser()->tab_strip_model()->GetActiveWebContents();
// Evaluate the height and width of the page as the browser_test can
// vary the dimensions.
int document_height =
EvalJs(web_contents, "document.body.scrollHeight").ExtractInt();
int document_width =
EvalJs(web_contents, "document.body.scrollWidth").ExtractInt();
// Expectation is before NavigateToUrl for this test as the expectation can be
// met after NavigateToUrl and before the Wait.
waiter->AddMainFrameDocumentIntersectionExpectation(
gfx::Rect(0, 0, document_width,
document_height)); // Initial main frame rect.
ui_test_utils::NavigateToURL(
browser(),
embedded_test_server()->GetURL(
"/page_load_metrics/blank_with_positioned_iframe_writer.html"));
waiter->Wait();
// Create a |document_width|x|document_height| frame at 100,100, increasing
// the page width and height by 100.
waiter->AddMainFrameDocumentIntersectionExpectation(
gfx::Rect(0, 0, document_width + 100, document_height + 100));
EXPECT_TRUE(ExecJs(
web_contents,
content::JsReplace("createIframeAtRect(\"test\", 100, 100, $1, $2);",
document_width, document_height)));
waiter->Wait();
}
// Creates a single frame within the main frame and verifies the intersection // Creates a single frame within the main frame and verifies the intersection
// with the main frame. // with the main frame.
IN_PROC_BROWSER_TEST_F(PageLoadMetricsBrowserTest, IN_PROC_BROWSER_TEST_F(PageLoadMetricsBrowserTest,
......
...@@ -720,14 +720,6 @@ void MetricsWebContentsObserver::OnTimingUpdated( ...@@ -720,14 +720,6 @@ void MetricsWebContentsObserver::OnTimingUpdated(
error = true; error = true;
} }
if (!metadata->intersection_update.is_null()) {
mojo::ReportBadMessage(
"page_load_metrics.mojom.FrameMetadata does not report an "
"intersection "
"update for the main frame. ");
error = true;
}
if (error) if (error)
return; return;
} else if (!committed_load_) { } else if (!committed_load_) {
......
...@@ -370,7 +370,7 @@ class PageLoadMetricsObserver { ...@@ -370,7 +370,7 @@ class PageLoadMetricsObserver {
// Invoked when a frame's intersections with page elements changes and an // Invoked when a frame's intersections with page elements changes and an
// update is received. The main_frame_document_intersection_rect // update is received. The main_frame_document_intersection_rect
// returns an empty rect for out of view subframes and does not send updates // returns an empty rect for out of view subframes and the root document size
// for the main frame. // for the main frame.
// TODO(crbug/1048175): Expose intersections to observers via shared delegate. // TODO(crbug/1048175): Expose intersections to observers via shared delegate.
virtual void OnFrameIntersectionUpdate( virtual void OnFrameIntersectionUpdate(
......
...@@ -441,7 +441,7 @@ void PageLoadMetricsUpdateDispatcher::UpdateMetrics( ...@@ -441,7 +441,7 @@ void PageLoadMetricsUpdateDispatcher::UpdateMetrics(
bool is_main_frame = render_frame_host->GetParent() == nullptr; bool is_main_frame = render_frame_host->GetParent() == nullptr;
if (is_main_frame) { if (is_main_frame) {
UpdateMainFrameMetadata(std::move(new_metadata)); UpdateMainFrameMetadata(render_frame_host, std::move(new_metadata));
UpdateMainFrameTiming(std::move(new_timing)); UpdateMainFrameTiming(std::move(new_timing));
UpdateMainFrameRenderData(*render_data); UpdateMainFrameRenderData(*render_data);
} else { } else {
...@@ -535,34 +535,41 @@ void PageLoadMetricsUpdateDispatcher::UpdateSubFrameMetadata( ...@@ -535,34 +535,41 @@ void PageLoadMetricsUpdateDispatcher::UpdateSubFrameMetadata(
subframe_metadata_->behavior_flags |= subframe_metadata->behavior_flags; subframe_metadata_->behavior_flags |= subframe_metadata->behavior_flags;
client_->OnSubframeMetadataChanged(render_frame_host, *subframe_metadata); client_->OnSubframeMetadataChanged(render_frame_host, *subframe_metadata);
MaybeUpdateFrameIntersection(render_frame_host, subframe_metadata);
}
void PageLoadMetricsUpdateDispatcher::MaybeUpdateFrameIntersection(
content::RenderFrameHost* render_frame_host,
const mojom::FrameMetadataPtr& frame_metadata) {
// Handle intersection updates if included in the metadata. // Handle intersection updates if included in the metadata.
if (subframe_metadata->intersection_update.is_null()) { if (frame_metadata->intersection_update.is_null())
return; return;
}
// Do not notify intersections for untracked loads, // Do not notify intersections for untracked loads,
// subframe_navigation_start_offset_ excludes untracked loads. // subframe_navigation_start_offset_ excludes untracked loads.
// TODO(crbug/1061091): Document definition of untracked loads in page load // TODO(crbug/1061091): Document definition of untracked loads in page load
// metrics. // metrics.
const int frame_tree_node_id = render_frame_host->GetFrameTreeNodeId(); const int frame_tree_node_id = render_frame_host->GetFrameTreeNodeId();
if (subframe_navigation_start_offset_.find(frame_tree_node_id) == bool is_main_frame = render_frame_host->GetParent() == nullptr;
subframe_navigation_start_offset_.end()) { if (!is_main_frame &&
subframe_navigation_start_offset_.find(frame_tree_node_id) ==
subframe_navigation_start_offset_.end()) {
return; return;
} }
auto existing_intersection_it = auto existing_intersection_it =
frame_intersection_updates_.find(render_frame_host->GetFrameTreeNodeId()); frame_intersection_updates_.find(frame_tree_node_id);
// Check if we already have a frame intersection update for the frame, // Check if we already have a frame intersection update for the frame,
// dispatch updates for the first frame intersection update or if // dispatch updates for the first frame intersection update or if
// the intersection has changed. // the intersection has changed.
if (existing_intersection_it == frame_intersection_updates_.end() || if (existing_intersection_it == frame_intersection_updates_.end() ||
!existing_intersection_it->second.Equals( !existing_intersection_it->second.Equals(
*subframe_metadata->intersection_update)) { *frame_metadata->intersection_update)) {
frame_intersection_updates_[frame_tree_node_id] = frame_intersection_updates_[frame_tree_node_id] =
*subframe_metadata->intersection_update; *frame_metadata->intersection_update;
client_->OnFrameIntersectionUpdate(render_frame_host, client_->OnFrameIntersectionUpdate(render_frame_host,
*subframe_metadata->intersection_update); *frame_metadata->intersection_update);
} }
} }
...@@ -609,6 +616,7 @@ void PageLoadMetricsUpdateDispatcher::UpdateMainFrameTiming( ...@@ -609,6 +616,7 @@ void PageLoadMetricsUpdateDispatcher::UpdateMainFrameTiming(
} }
void PageLoadMetricsUpdateDispatcher::UpdateMainFrameMetadata( void PageLoadMetricsUpdateDispatcher::UpdateMainFrameMetadata(
content::RenderFrameHost* render_frame_host,
mojom::FrameMetadataPtr new_metadata) { mojom::FrameMetadataPtr new_metadata) {
if (main_frame_metadata_->Equals(*new_metadata)) if (main_frame_metadata_->Equals(*new_metadata))
return; return;
...@@ -624,6 +632,9 @@ void PageLoadMetricsUpdateDispatcher::UpdateMainFrameMetadata( ...@@ -624,6 +632,9 @@ void PageLoadMetricsUpdateDispatcher::UpdateMainFrameMetadata(
main_frame_metadata_ = std::move(new_metadata); main_frame_metadata_ = std::move(new_metadata);
client_->OnMainFrameMetadataChanged(); client_->OnMainFrameMetadataChanged();
if (!main_frame_metadata_.is_null())
MaybeUpdateFrameIntersection(render_frame_host, main_frame_metadata_);
} }
void PageLoadMetricsUpdateDispatcher::UpdatePageInputTiming( void PageLoadMetricsUpdateDispatcher::UpdatePageInputTiming(
......
...@@ -185,11 +185,16 @@ class PageLoadMetricsUpdateDispatcher { ...@@ -185,11 +185,16 @@ class PageLoadMetricsUpdateDispatcher {
void UpdateFrameCpuTiming(content::RenderFrameHost* render_frame_host, void UpdateFrameCpuTiming(content::RenderFrameHost* render_frame_host,
mojom::CpuTimingPtr new_timing); mojom::CpuTimingPtr new_timing);
void UpdateMainFrameMetadata(mojom::FrameMetadataPtr new_metadata); void UpdateMainFrameMetadata(content::RenderFrameHost* render_frame_host,
mojom::FrameMetadataPtr new_metadata);
void UpdateSubFrameMetadata(content::RenderFrameHost* render_frame_host, void UpdateSubFrameMetadata(content::RenderFrameHost* render_frame_host,
mojom::FrameMetadataPtr subframe_metadata); mojom::FrameMetadataPtr subframe_metadata);
void UpdatePageInputTiming(const mojom::InputTiming& input_timing_delta); void UpdatePageInputTiming(const mojom::InputTiming& input_timing_delta);
void MaybeUpdateFrameIntersection(
content::RenderFrameHost* render_frame_host,
const mojom::FrameMetadataPtr& frame_metadata);
void UpdatePageRenderData(const mojom::FrameRenderDataUpdate& render_data); void UpdatePageRenderData(const mojom::FrameRenderDataUpdate& render_data);
void UpdateMainFrameRenderData( void UpdateMainFrameRenderData(
const mojom::FrameRenderDataUpdate& render_data); const mojom::FrameRenderDataUpdate& render_data);
......
...@@ -137,9 +137,9 @@ struct PageLoadTiming { ...@@ -137,9 +137,9 @@ struct PageLoadTiming {
struct FrameIntersectionUpdate { struct FrameIntersectionUpdate {
// The frame's current intersection rect with the main frame document in the // The frame's current intersection rect with the main frame document in the
// coordinate system of the main frame's viewport. The intersection rect is // coordinate system of the main frame's viewport. The intersection rect is
// an empty rect when there is no intersection with the main frame. No // an empty rect when there is no intersection with the main frame and
// rect is returned for the main frame. This is only set the first time an // returns the document size of the root document for the main frame. This
// intersection changes and is null otherwise. // is only set the first time an intersection changes and is null otherwise.
gfx.mojom.Rect? main_frame_document_intersection_rect; gfx.mojom.Rect? main_frame_document_intersection_rect;
}; };
......
...@@ -5402,6 +5402,12 @@ void RenderFrameImpl::DidCommitNavigationInternal( ...@@ -5402,6 +5402,12 @@ void RenderFrameImpl::DidCommitNavigationInternal(
std::move(interface_params)); std::move(interface_params));
} }
} }
// Ensure we will propagate frame intersections when the main frame commits
// even if the intersection does not change across navigations.
if (IsMainFrame()) {
mainframe_document_intersection_rect_.reset();
}
} }
void RenderFrameImpl::PrepareFrameForCommit( void RenderFrameImpl::PrepareFrameForCommit(
......
...@@ -1067,9 +1067,17 @@ void LocalFrameView::RunIntersectionObserverSteps() { ...@@ -1067,9 +1067,17 @@ void LocalFrameView::RunIntersectionObserverSteps() {
return; return;
} }
if (frame_->IsMainFrame()) if (frame_->IsMainFrame()) {
EnsureOverlayInterstitialAdDetector().MaybeFireDetection(frame_.Get()); EnsureOverlayInterstitialAdDetector().MaybeFireDetection(frame_.Get());
// Report the main frame's document intersection with itself.
LayoutObject* layout_object = GetLayoutView();
IntRect main_frame_dimensions =
ToLayoutBox(layout_object)->PixelSnappedLayoutOverflowRect();
GetFrame().Client()->OnMainFrameDocumentIntersectionChanged(WebRect(
0, 0, main_frame_dimensions.Width(), main_frame_dimensions.Height()));
}
TRACE_EVENT0("blink,benchmark", TRACE_EVENT0("blink,benchmark",
"LocalFrameView::UpdateViewportIntersectionsForSubtree"); "LocalFrameView::UpdateViewportIntersectionsForSubtree");
SCOPED_UMA_AND_UKM_TIMER(EnsureUkmAggregator(), SCOPED_UMA_AND_UKM_TIMER(EnsureUkmAggregator(),
......
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