Skip to content

Commit

Permalink
Avoid firing LCP triggered after mouseover over LCP element.
Browse files Browse the repository at this point in the history
This CL adds a flag that turns off LCP entries that are loaded right
after a mouseover event on a previous LCP element.
The flag is currently disabled by default.

Bug: 1288027
Change-Id: Idd8c1d47ad4a3bb91cc583b89ab4d68f08213ff4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4061623
Reviewed-by: Koji Ishii <kojii@chromium.org>
Reviewed-by: Ian Clelland <iclelland@chromium.org>
Commit-Queue: Yoav Weiss <yoavweiss@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1077459}
  • Loading branch information
Yoav Weiss authored and Chromium LUCI CQ committed Nov 30, 2022
1 parent ae67301 commit bdcef78
Show file tree
Hide file tree
Showing 12 changed files with 290 additions and 6 deletions.
Expand Up @@ -20,7 +20,8 @@
return event => {
const div = document.createElement("div");
const [width, height] = size.split("x");
div.style = `background-image: url(/images/green-${size}.png); width: ${width}; height: ${height}`;
div.style = `background-image:
url(/images/green-${size}.png); width: ${width}px; height: ${height}px`;
document.body.appendChild(div);
}
};
Expand Down
Expand Up @@ -364,8 +364,10 @@ class MouseoverLCPTest : public MetricIntegrationTest,
"registerMouseover(" + background + ")")
.error,
"");
EXPECT_EQ(
EvalJs(web_contents()->GetPrimaryMainFrame(), "run_test(1)").error, "");
EXPECT_EQ(EvalJs(web_contents()->GetPrimaryMainFrame(),
"run_test(/*expected_entries=*/1)")
.error,
"");

// We should wait for the main frame's hit-test data to be ready before
// sending the mouse events below to avoid flakiness.
Expand All @@ -374,6 +376,17 @@ class MouseoverLCPTest : public MetricIntegrationTest,
content::MainThreadFrameObserver frame_observer(GetRenderWidgetHost());
frame_observer.Wait();

std::string get_timestamp = R"(
(async () => {
await new Promise(r => setTimeout(r, 200));
const timestamp = performance.now();
await new Promise(r => setTimeout(r, 200));
return timestamp;
})())";
double timestamp =
EvalJs(web_contents()->GetPrimaryMainFrame(), get_timestamp)
.ExtractDouble();

// Simulate a mouse move event which will generate a mouse over event.
EXPECT_TRUE(
ExecJs(web_contents(),
Expand All @@ -387,7 +400,6 @@ class MouseoverLCPTest : public MetricIntegrationTest,
"run_test(/*entries_expected= */" + entries + ")")
.error,
"");

if (x1 != x2 || y1 != y2) {
// Wait for 600ms before the second mouse move, as our heuristics wait for
// 500ms after a mousemove event on an LCP image.
Expand Down Expand Up @@ -416,6 +428,17 @@ class MouseoverLCPTest : public MetricIntegrationTest,
ExpectUKMPageLoadMetricFlagSet(
PageLoad::kPaintTiming_LargestContentfulPaintTypeName,
LargestContentfulPaintTypeToUKMFlags(flag_set), expected);
// If we never fired an entry for mouseover LCP, we should expect the UKM
// timestamps to match that.
if (entries == entries2 && entries == "1") {
ExpectUKMPageLoadMetricLowerThan(
PageLoad::kPaintTiming_NavigationToLargestContentfulPaint2Name,
timestamp);
} else {
ExpectUKMPageLoadMetricGreaterThan(
PageLoad::kPaintTiming_NavigationToLargestContentfulPaint2Name,
timestamp);
}
}

void SetUpCommandLine(base::CommandLine* command_line) override {
Expand Down Expand Up @@ -473,6 +496,42 @@ IN_PROC_BROWSER_TEST_P(MouseoverLCPTest,
/*expected=*/false);
}

class MouseoverLCPTestWithHeuristicFlag : public MouseoverLCPTest {
void SetUpCommandLine(base::CommandLine* command_line) override {
MouseoverLCPTest::SetUpCommandLine(command_line);
feature_list_.InitWithFeatures(
{blink::features::kLCPMouseoverHeuristics} /*enabled*/,
{} /*disabled*/);
}

base::test::ScopedFeatureList feature_list_;
};

INSTANTIATE_TEST_SUITE_P(All,
MouseoverLCPTestWithHeuristicFlag,
::testing::Values(false, true));
IN_PROC_BROWSER_TEST_P(MouseoverLCPTestWithHeuristicFlag,
LargestContentfulPaint_MouseoverOverLCPImageThenBody) {
test_mouseover("/mouseover.html?dispatch",
blink::LargestContentfulPaintType::kAfterMouseover,
/*entries=*/"1",
/*entries2=*/"2",
/*x1=*/10, /*y1=*/10,
/*x2=*/30, /*y2=*/10,
/*expected=*/false);
}

IN_PROC_BROWSER_TEST_P(MouseoverLCPTestWithHeuristicFlag,
LargestContentfulPaint_MouseoverOverLCPImageReplace) {
test_mouseover("/mouseover.html?replace",
blink::LargestContentfulPaintType::kAfterMouseover,
/*entries=*/"1",
/*entries2=*/"1",
/*x1=*/10, /*y1=*/10,
/*x2=*/10, /*y2=*/10,
/*expected=*/false);
}

class LargestContentfulPaintTypeTypeTest : public MetricIntegrationTest {
private:
std::unique_ptr<page_load_metrics::PageLoadMetricsTestWaiter>
Expand Down
Expand Up @@ -140,14 +140,42 @@ std::unique_ptr<HttpResponse> MetricIntegrationTest::HandleRequest(
return std::move(response);
}

const ukm::mojom::UkmEntryPtr MetricIntegrationTest::GetEntry() {
auto merged_entries =
ukm_recorder().GetMergedEntriesByName(PageLoad::kEntryName);
EXPECT_EQ(1ul, merged_entries.size());
const auto& kv = merged_entries.begin();
return std::move(kv->second);
}

void MetricIntegrationTest::ExpectUKMPageLoadMetric(StringPiece metric_name,
int64_t expected_value) {
TestUkmRecorder::ExpectEntryMetric(GetEntry().get(), metric_name,
expected_value);
}

void MetricIntegrationTest::ExpectUKMPageLoadMetricGreaterThan(
base::StringPiece metric_name,
int64_t expected_value) {
// TODO(yoav): figure out why GetEntry fails on the bots.
auto merged_entries =
ukm_recorder().GetMergedEntriesByName(PageLoad::kEntryName);
EXPECT_EQ(1ul, merged_entries.size());
const auto& kv = merged_entries.begin();
TestUkmRecorder::ExpectEntryMetric(kv->second.get(), metric_name,
expected_value);
const int64_t* value =
TestUkmRecorder::GetEntryMetric(kv->second.get(), metric_name);
EXPECT_GT(*value, expected_value);
}
void MetricIntegrationTest::ExpectUKMPageLoadMetricLowerThan(
base::StringPiece metric_name,
int64_t expected_value) {
auto merged_entries =
ukm_recorder().GetMergedEntriesByName(PageLoad::kEntryName);
EXPECT_EQ(1ul, merged_entries.size());
const auto& kv = merged_entries.begin();
const int64_t* value =
TestUkmRecorder::GetEntryMetric(kv->second.get(), metric_name);
EXPECT_LT(*value, expected_value);
}

int64_t MetricIntegrationTest::GetUKMPageLoadMetricFlagSet(
Expand Down
Expand Up @@ -93,6 +93,10 @@ class MetricIntegrationTest : public InProcessBrowserTest {
// metric name and value.
void ExpectUKMPageLoadMetric(base::StringPiece metric_name,
int64_t expected_value);
void ExpectUKMPageLoadMetricGreaterThan(base::StringPiece metric_name,
int64_t expected_value);
void ExpectUKMPageLoadMetricLowerThan(base::StringPiece metric_name,
int64_t expected_value);

int64_t GetUKMPageLoadMetricFlagSet(base::StringPiece metric_name);

Expand Down Expand Up @@ -149,6 +153,8 @@ class MetricIntegrationTest : public InProcessBrowserTest {
base::TimeDelta delay,
const net::test_server::HttpRequest& request);

const ukm::mojom::UkmEntryPtr GetEntry();

absl::optional<ukm::TestAutoSetUkmRecorder> ukm_recorder_;
absl::optional<base::HistogramTester> histogram_tester_;
};
Expand Down
Expand Up @@ -561,6 +561,10 @@ bool ImageRecordsManager::RecordFirstPaintAndReturnIsPending(
bpp < features::kMinimumEntropyForLCP.Get()) {
return false;
}
if (RuntimeEnabledFeatures::LCPMouseoverHeuristicsEnabled() &&
is_loaded_after_mouseover) {
return false;
}

std::unique_ptr<ImageRecord> record = CreateImageRecord(
*record_id.first, record_id.second, visual_size, frame_visual_rect,
Expand Down
Expand Up @@ -1654,6 +1654,10 @@
name: "LCPAnimatedImagesWebExposed",
status: "test",
},
{
name: "LCPMouseoverHeuristics",
base_feature: "LCPMouseoverHeuristics"
},
{
name: "LCPMultipleUpdatesPerElement",
base_feature: "LCPMultipleUpdatesPerElement"
Expand Down
4 changes: 4 additions & 0 deletions third_party/blink/web_tests/TestExpectations
Expand Up @@ -5505,6 +5505,10 @@ crbug.com/1015130 external/wpt/largest-contentful-paint/first-paint-equals-lcp-t

crbug.com/1000051 media/controls/volume-slider.html [ Failure Pass Timeout ]

# Test for LCP's mouseover heuristics. Tested only under virtual/lcp-mouseover-heuristics.
crbug.com/1288027 external/wpt/largest-contentful-paint/mouseover-heuristics-element.tentative.html [ Skip Timeout ]
crbug.com/1288027 external/wpt/largest-contentful-paint/mouseover-heuristics-background.tentative.html [ Skip Timeout ]

# Sheriff 2019-09-04
crbug.com/1000396 [ Win ] media/video-remove-insert-repaints.html [ Failure Pass ]

Expand Down
12 changes: 12 additions & 0 deletions third_party/blink/web_tests/VirtualTestSuites
Expand Up @@ -1291,6 +1291,18 @@
],
"expires": "Jul 1, 2023"
},
{
"prefix": "lcp-mouseover-heuristics",
"platforms": ["Linux", "Mac", "Win"],
"bases": [
"external/wpt/largest-contentful-paint/mouseover-heuristics-element.tentative.html",
"external/wpt/largest-contentful-paint/mouseover-heuristics-background.tentative.html"
],
"args": [
"--enable-features=LCPMouseoverHeuristics"
],
"expires": "Jul 1, 2023"
},
{
"prefix": "css-highlight-inheritance",
"platforms": ["Linux", "Mac", "Win"],
Expand Down
@@ -0,0 +1,19 @@
<!DOCTYPE html>
<html>
<head>
<title>LCP mouseover heuristics ignore background-based zoom widgets</title>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="/resources/testdriver.js"></script>
<script src="/resources/testdriver-actions.js"></script>
<script src="/resources/testdriver-vendor.js"></script>
<script src="resources/mouseover-utils.js"></script>
</head>
<body>
<img src="/images/green-16x16.png" id=image>
<span id=span style="display: inline-block;width: 15px; height: 15px"></span>
<script>
run_mouseover_test(/*background=*/true);
</script>
</body>

@@ -0,0 +1,18 @@
<!DOCTYPE html>
<html>
<head>
<title>LCP mouseover heuristics ignore element-based zoom widgets</title>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="/resources/testdriver.js"></script>
<script src="/resources/testdriver-actions.js"></script>
<script src="/resources/testdriver-vendor.js"></script>
<script src="resources/mouseover-utils.js"></script>
</head>
<body>
<img src="/images/green-16x16.png" id=image>
<span id=span style="display: inline-block;width: 15px; height: 15px"></span>
<script>
run_mouseover_test(/*background=*/false);
</script>
</body>
@@ -0,0 +1,128 @@
let counter = 0;
const loadImage = size => {
return event => {
let zoom;
if (location.search.includes("replace")) {
zoom = document.getElementById("image");
} else {
zoom = new Image();
}
zoom.src=`/images/green-${size}.png`;
++counter;
zoom.elementTiming = "zoom" + counter;
document.body.appendChild(zoom);
}
};
const loadBackgroundImage = size => {
return event => {
const div = document.createElement("div");
const [width, height] = size.split("x");
++counter;
div.style = `background-image:
url(/images/green-${size}.png?${counter}); width: ${width}px; height: ${height}px`;
div.elementTiming = "zoom" + counter;
document.body.appendChild(div);
}
};

const registerMouseover = background => {
const image = document.getElementById("image");
const span = document.getElementById("span");
const func = background ? loadBackgroundImage : loadImage;
image.addEventListener("mouseover", func("100x50"));
span.addEventListener("mouseover", func("256x256"));
}

const dispatch_mouseover = () => {
span.dispatchEvent(new Event("mouseover"))
};

const wait_for_lcp_entries = async entries_expected => {
await new Promise(resolve => {
let entries_seen = 0;
const PO = new PerformanceObserver(list => {
const entries = list.getEntries();
for (let entry of entries) {
if (entry.url) {
entries_seen++;
}
}
if (entries_seen == entries_expected) {
PO.disconnect();
resolve()
} else if (entries_seen > entries_expected) {
PO.disconnect();
reject();
}
});
PO.observe({type: "largest-contentful-paint", buffered: true});
});
};
const wait_for_element_timing_entry = async identifier => {
await new Promise(resolve => {
const PO = new PerformanceObserver(list => {
const entries = list.getEntries();
for (let entry of entries) {
if (entry.identifier == identifier) {
PO.disconnect();
resolve()
}
}
});
PO.observe({type: "element", buffered: true});
});
};
const wait_for_resource_timing_entry = async name => {
await new Promise(resolve => {
const PO = new PerformanceObserver(list => {
const entries = list.getEntries();
for (let entry of entries) {
if (entry.name.includes(name)) {
PO.disconnect();
resolve()
}
}
});
PO.observe({type: "resource", buffered: true});
});
};

const run_mouseover_test = background => {
promise_test(async t => {
// await the first LCP entry
await wait_for_lcp_entries(1);
// Hover over the image
registerMouseover(background);
if (test_driver) {
await new test_driver.Actions().pointerMove(0, 0, {origin: image}).send();
}
if (!background) {
await wait_for_element_timing_entry("zoom1");
} else {
await wait_for_resource_timing_entry("png?1");
await new Promise(r => requestAnimationFrame(r));
}
// There's only a single LCP entry, because the zoom was skipped.
await wait_for_lcp_entries(1);

// Wait 600 ms as the heuristic in 500 ms.
// This will no longer be necessary once the heuristic relies on Task
// Attribution.
await new Promise(r => step_timeout(r, 600));

// Hover over the span.
if (test_driver) {
await new test_driver.Actions().pointerMove(0, 0, {origin: span}).send();
}
if (!background) {
await wait_for_element_timing_entry("zoom2");
} else {
await wait_for_resource_timing_entry("png?2");
await new Promise(r => requestAnimationFrame(r));
}
// There are 2 LCP entries, as the image loaded due to span hover is a
// valid LCP candidate.
await wait_for_lcp_entries(2);
}, `LCP mouseover heuristics ignore ${background ?
"background" : "element"}-based zoom widgets`);
}
@@ -0,0 +1 @@
A virtual test suite for LCP's mouseover heuristics, which are not yet at the experimental stage.

0 comments on commit bdcef78

Please sign in to comment.