Skip to content

Commit

Permalink
[Layout Shift Normalization] Session window by inputs.
Browse files Browse the repository at this point in the history
This CL is for implementing a different session window strategy for layout shift normalization.
We start a new session window whenever there's a user input.

UKM privacy review: https://docs.google.com/document/d/15jUCzCdj4fftEID_Kn0YfzbMFYepNXiVemAxmxLv7JY/edit?usp=sharing&resourcekey=0-dGzuYuWOL5798S4T4-QIgg

Change-Id: I038b32ae02d43a416ec8858e1cf35df28247dec5
Bug: 1184073
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2731128
Commit-Queue: Hongbo Song <hbsong@google.com>
Reviewed-by: Michal Mocny <mmocny@chromium.org>
Reviewed-by: Steve Kobes <skobes@chromium.org>
Reviewed-by: Tom McKee <tommckee@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Reviewed-by: Robert Kaplow <rkaplow@chromium.org>
Reviewed-by: Alex Gough <ajgo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#862540}
  • Loading branch information
hbsong235 authored and Chromium LUCI CQ committed Mar 12, 2021
1 parent 1710729 commit 8f671f9
Show file tree
Hide file tree
Showing 24 changed files with 226 additions and 26 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -398,7 +398,7 @@ TEST_F(AMPPageLoadMetricsObserverTest, SubFrameMetrics_LayoutInstability) {
tester()->SimulateMetadataUpdate(metadata, subframe);

page_load_metrics::mojom::FrameRenderDataUpdate render_data(1.0, 0.5, 0, 0, 0,
0, {});
0, {}, {});
tester()->SimulateRenderDataUpdate(render_data, subframe);

// Navigate the main frame to trigger metrics recording.
Expand Down Expand Up @@ -446,7 +446,7 @@ TEST_F(AMPPageLoadMetricsObserverTest,

base::TimeTicks current_time = base::TimeTicks::Now();
page_load_metrics::mojom::FrameRenderDataUpdate render_data(0.65, 0.65, 0, 0,
0, 0, {});
0, 0, {}, {});

render_data.new_layout_shifts.emplace_back(
page_load_metrics::mojom::LayoutShift::New(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1023,7 +1023,11 @@ void UkmPageLoadMetricsObserver::ReportLayoutStability() {
.SetLayoutInstability_AverageCumulativeShiftScore_SessionWindow_Gap5000ms(
page_load_metrics::LayoutShiftUkmValue(
normalized_cls_data
.session_windows_gap5000ms_maxMax_average_cls));
.session_windows_gap5000ms_maxMax_average_cls))
.SetLayoutInstability_MaxCumulativeShiftScore_SessionWindowByInputs_Gap1000ms_Max5000ms(
page_load_metrics::LayoutShiftUkmValue(
normalized_cls_data
.session_windows_by_inputs_gap1000ms_max5000ms_max_cls));
}
builder.Record(ukm::UkmRecorder::Get());

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1473,17 +1473,28 @@ TEST_F(UkmPageLoadMetricsObserverTest, CpuTimeMetrics) {

TEST_F(UkmPageLoadMetricsObserverTest, LayoutInstability) {
NavigateAndCommit(GURL(kTestUrl1));
base::TimeTicks time_origin = base::TimeTicks::Now();
page_load_metrics::mojom::FrameRenderDataUpdate render_data(
1.0, 1.0, 0, 0, 0, 0, {},
{time_origin - base::TimeDelta::FromMilliseconds(3000)});
render_data.new_layout_shifts.emplace_back(
page_load_metrics::mojom::LayoutShift::New(
time_origin - base::TimeDelta::FromMilliseconds(4000), 0.5));
render_data.new_layout_shifts.emplace_back(
page_load_metrics::mojom::LayoutShift::New(
time_origin - base::TimeDelta::FromMilliseconds(3500), 0.5));

page_load_metrics::mojom::FrameRenderDataUpdate render_data(1.0, 1.0, 0, 0, 0,
0, {});
tester()->SimulateRenderDataUpdate(render_data);

// Simulate hiding the tab (the report should include shifts after hide).
web_contents()->WasHidden();

render_data.layout_shift_delta = 1.5;
render_data.layout_shift_delta_before_input_or_scroll = 0.0;
tester()->SimulateRenderDataUpdate(render_data);
page_load_metrics::mojom::FrameRenderDataUpdate render_data_2(1.5, 0.0, 0, 0,
0, 0, {}, {});
render_data_2.new_layout_shifts.emplace_back(
page_load_metrics::mojom::LayoutShift::New(
time_origin - base::TimeDelta::FromMilliseconds(2500), 1.5));
tester()->SimulateRenderDataUpdate(render_data_2);

// Simulate closing the tab.
DeleteContents();
Expand All @@ -1503,6 +1514,36 @@ TEST_F(UkmPageLoadMetricsObserverTest, LayoutInstability) {
PageLoad::
kLayoutInstability_CumulativeShiftScore_MainFrame_BeforeInputOrScrollName,
100);
ukm_recorder.ExpectEntryMetric(
ukm_entry,
PageLoad::
kLayoutInstability_MaxCumulativeShiftScore_SessionWindow_Gap1000ms_Max5000msName,
250);
ukm_recorder.ExpectEntryMetric(
ukm_entry,
PageLoad::
kLayoutInstability_MaxCumulativeShiftScore_SessionWindow_Gap1000msName,
250);
ukm_recorder.ExpectEntryMetric(
ukm_entry,
PageLoad::
kLayoutInstability_MaxCumulativeShiftScore_SlidingWindow_Duration1000msName,
200);
ukm_recorder.ExpectEntryMetric(
ukm_entry,
PageLoad::
kLayoutInstability_MaxCumulativeShiftScore_SlidingWindow_Duration300msName,
150);
ukm_recorder.ExpectEntryMetric(
ukm_entry,
PageLoad::
kLayoutInstability_AverageCumulativeShiftScore_SessionWindow_Gap5000msName,
250);
ukm_recorder.ExpectEntryMetric(
ukm_entry,
PageLoad::
kLayoutInstability_MaxCumulativeShiftScore_SessionWindowByInputs_Gap1000ms_Max5000msName,
150);
ukm_recorder.ExpectEntryMetric(kv.second.get(),
PageLoad::kNavigation_PageEndReason3Name,
page_load_metrics::END_CLOSE);
Expand Down Expand Up @@ -1596,7 +1637,7 @@ TEST_F(UkmPageLoadMetricsObserverTest, LayoutInstabilitySubframeAggregation) {

// Simulate layout instability in the main frame.
page_load_metrics::mojom::FrameRenderDataUpdate render_data(1.0, 1.0, 0, 0, 0,
0, {});
0, {}, {});
tester()->SimulateRenderDataUpdate(render_data);

RenderFrameHost* subframe =
Expand Down Expand Up @@ -2114,7 +2155,7 @@ TEST_F(UkmPageLoadMetricsObserverTest, CLSNeverForegroundedNoReport) {
NavigateAndCommit(GURL(kTestUrl1));

page_load_metrics::mojom::FrameRenderDataUpdate render_data(1.0, 1.0, 0, 0, 0,
0, {});
0, {}, {});
tester()->SimulateRenderDataUpdate(render_data);

// Simulate closing the tab.
Expand Down Expand Up @@ -2149,7 +2190,7 @@ void CLSUkmPageLoadMetricsObserverTest::SimulateShiftDelta(
float delta,
content::RenderFrameHost* frame) {
page_load_metrics::mojom::FrameRenderDataUpdate render_data(delta, delta, 0,
0, 0, 0, {});
0, 0, 0, {}, {});
tester()->SimulateRenderDataUpdate(render_data, frame);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ TEST_F(FromGWSPageLoadMetricsObserverTest, SearchPreviousCommittedUrl1) {

tester()->SimulateTimingUpdate(timing);
page_load_metrics::mojom::FrameRenderDataUpdate render_data(1.0, 1.0, 0, 0, 0,
0, {});
0, {}, {});
tester()->SimulateRenderDataUpdate(render_data);
render_data.layout_shift_delta = 1.5;
render_data.layout_shift_delta_before_input_or_scroll = 0.0;
Expand Down Expand Up @@ -613,7 +613,7 @@ TEST_F(FromGWSPageLoadMetricsObserverTest,
NavigateAndCommit(GURL("https://www.google.com/search#q=test"));
NavigateAndCommit(GURL(kExampleUrl));
page_load_metrics::mojom::FrameRenderDataUpdate render_data(1.0, 1.0, 0, 0, 0,
0, {});
0, {}, {});
tester()->SimulateRenderDataUpdate(render_data);

web_contents()->WasHidden();
Expand Down
63 changes: 51 additions & 12 deletions components/page_load_metrics/browser/layout_shift_normalization.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,16 @@ constexpr auto MAX_SHIFT_BUFFER_SIZE = 300;
LayoutShiftNormalization::LayoutShiftNormalization() = default;
LayoutShiftNormalization::~LayoutShiftNormalization() = default;

void LayoutShiftNormalization::AddInputTimeStamps(
const std::vector<base::TimeTicks>& input_timestamps) {
recent_input_timestamps_.insert(recent_input_timestamps_.end(),
input_timestamps.begin(),
input_timestamps.end());
std::inplace_merge(recent_input_timestamps_.begin(),
recent_input_timestamps_.end() - input_timestamps.size(),
recent_input_timestamps_.end());
}

void LayoutShiftNormalization::AddNewLayoutShifts(
const std::vector<page_load_metrics::mojom::LayoutShiftPtr>& new_shifts,
base::TimeTicks current_time,
Expand Down Expand Up @@ -96,20 +106,27 @@ void LayoutShiftNormalization::UpdateSlidingWindow(
[](auto time, auto const& window) { return time < window.start_time; });
sliding_windows->erase(sliding_windows->begin(), first_non_stale_window);
}

void LayoutShiftNormalization::UpdateSessionWindow(
SessionWindow* session_window,
base::TimeDelta gap,
base::TimeDelta max_duration,
std::vector<std::pair<base::TimeTicks, double>>::const_iterator begin,
std::vector<std::pair<base::TimeTicks, double>>::const_iterator end,
std::vector<base::TimeTicks>& input_timestamps,
double& max_score,
uint32_t& count) {
for (auto it = begin; it != end; ++it) {
if ((it->first - session_window->last_time > gap) ||
(it->first - session_window->start_time > max_duration)) {
(it->first - session_window->start_time > max_duration) ||
(!input_timestamps.empty() && input_timestamps.front() <= it->first)) {
session_window->start_time = it->first;
session_window->layout_shift_score = 0;
++count;
while (!input_timestamps.empty() &&
input_timestamps.front() <= it->first) {
input_timestamps.erase(input_timestamps.begin());
}
}
session_window->last_time = it->first;
session_window->layout_shift_score += it->second;
Expand Down Expand Up @@ -141,6 +158,7 @@ void LayoutShiftNormalization::UpdateWindowCLS(
double cumulative_layout_shift_score) {
double dummy_max = 0.0;
uint32_t dummy_count = 0;
std::vector<base::TimeTicks> dummy_input_timestamps;
// Update Sliding Windows.
UpdateSlidingWindow(
&sliding_300ms_, base::TimeDelta::FromMilliseconds(300), current_time,
Expand All @@ -157,21 +175,34 @@ void LayoutShiftNormalization::UpdateWindowCLS(
UpdateSessionWindow(
&session_gap1000ms_max5000ms_, base::TimeDelta::FromMilliseconds(1000),
base::TimeDelta::FromMilliseconds(5000), first, first_non_stale,
dummy_input_timestamps,
normalized_cls_data_.session_windows_gap1000ms_max5000ms_max_cls,
dummy_count);
UpdateSessionWindow(
&session_gap1000ms_, base::TimeDelta::FromMilliseconds(1000),
base::TimeDelta::Max(), first, first_non_stale,
base::TimeDelta::Max(), first, first_non_stale, dummy_input_timestamps,
normalized_cls_data_.session_windows_gap1000ms_maxMax_max_cls,
dummy_count);
UpdateSessionWindow(&session_gap5000ms_,
base::TimeDelta::FromMilliseconds(5000),
base::TimeDelta::Max(), first, first_non_stale, dummy_max,
session_gap5000ms_count_);
UpdateSessionWindow(&session_by_inputs_gap1000ms_max5000ms_,
base::TimeDelta::FromMilliseconds(1000),
base::TimeDelta::FromMilliseconds(5000), first,
first_non_stale, recent_input_timestamps_,
potential_max_cls_session_by_inputs_gap1000ms_max5000ms_,
dummy_count);
normalized_cls_data_.session_windows_by_inputs_gap1000ms_max5000ms_max_cls =
potential_max_cls_session_by_inputs_gap1000ms_max5000ms_;
UpdateSessionWindow(
&session_gap5000ms_, base::TimeDelta::FromMilliseconds(5000),
base::TimeDelta::Max(), first, first_non_stale, dummy_input_timestamps,
dummy_max, session_gap5000ms_count_);

auto tmp_session_gap1000ms_max5000ms = session_gap1000ms_max5000ms_;
auto tmp_session_gap1000ms_ = session_gap1000ms_;
auto tmp_session_gap5000ms_ = session_gap5000ms_;
auto tmp_session_gap5000ms_count_ = session_gap5000ms_count_;
auto tmp_session_by_inputs_gap1000ms_max5000ms_ =
session_by_inputs_gap1000ms_max5000ms_;
auto tmp_recent_input_timestamps_ = recent_input_timestamps_;

UpdateSlidingWindow(
&tmp_sliding_300ms, base::TimeDelta::FromMilliseconds(300), current_time,
Expand All @@ -184,18 +215,26 @@ void LayoutShiftNormalization::UpdateWindowCLS(
UpdateSessionWindow(
&tmp_session_gap1000ms_max5000ms, base::TimeDelta::FromMilliseconds(1000),
base::TimeDelta::FromMilliseconds(5000), first_non_stale, last,
dummy_input_timestamps,
normalized_cls_data_.session_windows_gap1000ms_max5000ms_max_cls,
dummy_count);
UpdateSessionWindow(
&tmp_session_gap1000ms_, base::TimeDelta::FromMilliseconds(1000),
base::TimeDelta::Max(), first_non_stale, last,
base::TimeDelta::Max(), first_non_stale, last, dummy_input_timestamps,
normalized_cls_data_.session_windows_gap1000ms_maxMax_max_cls,
dummy_count);
UpdateSessionWindow(&tmp_session_gap5000ms_,
base::TimeDelta::FromMilliseconds(5000),
base::TimeDelta::Max(), first_non_stale, last, dummy_max,
tmp_session_gap5000ms_count_);

UpdateSessionWindow(
&tmp_session_by_inputs_gap1000ms_max5000ms_,
base::TimeDelta::FromMilliseconds(1000),
base::TimeDelta::FromMilliseconds(5000), first_non_stale, last,
tmp_recent_input_timestamps_,
normalized_cls_data_
.session_windows_by_inputs_gap1000ms_max5000ms_max_cls,
dummy_count);
UpdateSessionWindow(
&tmp_session_gap5000ms_, base::TimeDelta::FromMilliseconds(5000),
base::TimeDelta::Max(), first_non_stale, last, dummy_input_timestamps,
dummy_max, tmp_session_gap5000ms_count_);
normalized_cls_data_.session_windows_gap5000ms_maxMax_average_cls =
cumulative_layout_shift_score / tmp_session_gap5000ms_count_;
}
Expand Down
10 changes: 10 additions & 0 deletions components/page_load_metrics/browser/layout_shift_normalization.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ class LayoutShiftNormalization {
return normalized_cls_data_;
}

void AddInputTimeStamps(const std::vector<base::TimeTicks>& input_timestamps);
void AddNewLayoutShifts(
const std::vector<page_load_metrics::mojom::LayoutShiftPtr>& new_shifts,
base::TimeTicks current_time,
Expand Down Expand Up @@ -63,6 +64,7 @@ class LayoutShiftNormalization {
base::TimeDelta max_duration,
std::vector<std::pair<base::TimeTicks, double>>::const_iterator begin,
std::vector<std::pair<base::TimeTicks, double>>::const_iterator end,
std::vector<base::TimeTicks>& input_timestamps,
double& max_score,
uint32_t& count);

Expand All @@ -72,12 +74,20 @@ class LayoutShiftNormalization {
// This vector is maintained in sorted order.
std::vector<std::pair<base::TimeTicks, double>> recent_layout_shifts_;

// This vector which contains input timestamps is maintained in sorted order.
std::vector<base::TimeTicks> recent_input_timestamps_;

// Sliding window vectors are maintained in sorted order.
std::vector<SlidingWindow> sliding_300ms_;
std::vector<SlidingWindow> sliding_1000ms_;
SessionWindow session_gap1000ms_max5000ms_;
SessionWindow session_gap1000ms_;
SessionWindow session_gap5000ms_;
SessionWindow session_by_inputs_gap1000ms_max5000ms_;
// A new input in non-stale data can split the session window and make the
// max_cls smaller. We need to store the "max_cls" calculated by the stale
// data.
double potential_max_cls_session_by_inputs_gap1000ms_max5000ms_ = 0.0;
uint32_t session_gap5000ms_count_ = 0;

DISALLOW_COPY_AND_ASSIGN(LayoutShiftNormalization);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,12 @@ class LayoutShiftNormalizationTest : public testing::Test {
layout_shift_normalization_.AddNewLayoutShifts(
new_shifts, current_time, cumulative_layoutshift_score_);
}

void AddInputTimeStamps(
const std::vector<base::TimeTicks>& input_timestamps) {
layout_shift_normalization_.AddInputTimeStamps(input_timestamps);
}

const page_load_metrics::NormalizedCLSData& normalized_cls_data() const {
return layout_shift_normalization_.normalized_cls_data();
}
Expand Down Expand Up @@ -143,4 +149,30 @@ TEST_F(LayoutShiftNormalizationTest, MultipleShiftsFromDifferentTimes) {
EXPECT_EQ(normalized_cls_data().session_windows_gap5000ms_maxMax_average_cls,
3.5);
EXPECT_EQ(normalized_cls_data().data_tainted, false);
}

TEST_F(LayoutShiftNormalizationTest, SessionWindowByInputs) {
base::TimeTicks time_origin = base::TimeTicks::Now();
std::vector<page_load_metrics::mojom::LayoutShiftPtr> new_shifts;
// Insert new layout shifts. The insertion order matters.
InsertNewLayoutShifts(new_shifts, time_origin,
{{2100, 1.5}, {1800, 1.5}, {1300, 1.5}, {1000, 1.5}});
// Update CLS normalization data.
AddInputTimeStamps({time_origin - base::TimeDelta::FromMilliseconds(2200),
time_origin - base::TimeDelta::FromMilliseconds(1100),
time_origin - base::TimeDelta::FromMilliseconds(800)});
AddNewLayoutShifts(new_shifts, time_origin);

EXPECT_EQ(normalized_cls_data().sliding_windows_duration300ms_max_cls, 3.0);
EXPECT_EQ(normalized_cls_data().sliding_windows_duration1000ms_max_cls, 4.5);
EXPECT_EQ(normalized_cls_data().session_windows_gap1000ms_max5000ms_max_cls,
6.0);
EXPECT_EQ(normalized_cls_data().session_windows_gap1000ms_maxMax_max_cls,
6.0);
EXPECT_EQ(normalized_cls_data().session_windows_gap5000ms_maxMax_average_cls,
6.0);
EXPECT_EQ(normalized_cls_data()
.session_windows_by_inputs_gap1000ms_max5000ms_max_cls,
4.5);
EXPECT_EQ(normalized_cls_data().data_tainted, false);
}
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,11 @@ struct NormalizedCLSData {
// is not bigger than 5000ms.
double session_windows_gap5000ms_maxMax_average_cls = 0.0;

// Maximum CLS of session windows. The gap between two consecutive shifts is
// not bigger than 1000ms or segmented by a user input. The maximum window
// size is 5000ms.
double session_windows_by_inputs_gap1000ms_max5000ms_max_cls = 0.0;

// If true, will not report the data in UKM.
bool data_tainted = false;
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -735,6 +735,7 @@ void PageLoadMetricsUpdateDispatcher::UpdateMobileFriendliness(
void PageLoadMetricsUpdateDispatcher::UpdatePageRenderData(
const mojom::FrameRenderDataUpdate& render_data) {
page_render_data_.layout_shift_score += render_data.layout_shift_delta;
layout_shift_normalization_.AddInputTimeStamps(render_data.input_timestamps);
layout_shift_normalization_.AddNewLayoutShifts(
render_data.new_layout_shifts, base::TimeTicks::Now(),
page_render_data_.layout_shift_score);
Expand Down
3 changes: 3 additions & 0 deletions components/page_load_metrics/common/page_load_metrics.mojom
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,9 @@ struct FrameRenderDataUpdate {

// New layout shifts with timestamps.
array<LayoutShift> new_layout_shifts;

// Recent input timestamps for layout shift tracking.
array<mojo_base.mojom.TimeTicks> input_timestamps;
};

// Metrics about the time spent in tasks (cpu time) by a frame.
Expand Down
Loading

0 comments on commit 8f671f9

Please sign in to comment.