Skip to content

Commit e54d723

Browse files
shaseleymertcanaltin
shaseley
authored and
mertcanaltin
committed
[soft navs] Fix image pseudo element handling in pre-paint mode
NotifyBackgroundImagePaint() is called with the "generating node", which for pseudo elements is the parent node, not the node itself, but SoftNavigationPaintAttributionTracker was tracking the pseudo element, which is node the image type layout object. This means unless the node was directly added, we aren't tracking paints for it. This CL fixes this by using the generating node during the pre-paint walk. This also lifts the generating node function into a helper class and uses that in both places. This also lifts the IsImageType() helper into that utils class, resolving a TODO. Note: node-with-image-pseudo-element-and-text.html fails with pre-paint mode disabled because the current stable mode ignores repaints, and the same node is considered painted for both the text and image. Fixed: 444653105 Change-Id: Ic8d9927620b687fe57a396591bdebca27215ffdb Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6944138 Reviewed-by: Jeremy Roman <[email protected]> Commit-Queue: Scott Haseley <[email protected]> Reviewed-by: Michal Mocny <[email protected]> Cr-Commit-Position: refs/heads/main@{#1515701}
1 parent 789953e commit e54d723

File tree

2 files changed

+144
-0
lines changed

2 files changed

+144
-0
lines changed

‎soft-navigation-heuristics/detection/tentative/node-with-image-pseudo-element-and-text.html‎

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
<!DOCTYPE html>
2+
<meta charset="utf-8" />
3+
<script src="/resources/testharness.js"></script>
4+
<script src="/resources/testharnessreport.js"></script>
5+
<script src="/resources/testdriver.js"></script>
6+
<script src="/resources/testdriver-vendor.js"></script>
7+
<script src="/soft-navigation-heuristics/resources/soft-navigation-test-helper.js"></script>
8+
9+
<style>
10+
.content::before {
11+
content: "";
12+
display: block;
13+
width: 256px;
14+
height: 256px;
15+
background-image: url(/images/lcp-256x256.png);
16+
background-size: contain;
17+
background-repeat: no-repeat;
18+
background-position: center;
19+
}
20+
</style>
21+
22+
<button id="navigateButton">Click here!</button>
23+
<div id="container"></div>
24+
25+
<script>
26+
promise_test(async t => {
27+
const url = 'soft-nav-1';
28+
29+
navigateButton.addEventListener('click', async () => {
30+
const firstIcpPromise = SoftNavigationTestHelper.getPerformanceEntries(
31+
'interaction-contentful-paint',
32+
/* minNumEntries= */ 1);
33+
34+
container.innerHTML = `<div><div id="content">Text</div></div>`;
35+
history.pushState({}, '', url);
36+
37+
// After the ICP entry for the text is produced, add the 'content' class
38+
// to cause an image paint for the same node. We expect an ICP entry for
39+
// this as well.
40+
await (new SoftNavigationTestHelper(t)).withTimeoutMessage(
41+
firstIcpPromise, 'Timed out waiting for initial ICP', /*timeout=*/ 3000);
42+
content.classList.add('content');
43+
}, {once: true});
44+
45+
// Set up the PerformanceObservers before clicking to avoid races,
46+
// and use unbuffered here so we don't get duplicate entries.
47+
const softNavPromise = SoftNavigationTestHelper.getPerformanceEntries(
48+
'soft-navigation',
49+
/* minNumEntries= */ 1);
50+
const icpPromise = SoftNavigationTestHelper.getPerformanceEntries(
51+
'interaction-contentful-paint',
52+
/* minNumEntries= */ 2);
53+
54+
if (test_driver) {
55+
test_driver.click(navigateButton);
56+
}
57+
58+
const helper = new SoftNavigationTestHelper(t);
59+
60+
// Check if we detected a soft nav.
61+
const softNavs = await helper.withTimeoutMessage(
62+
softNavPromise, "Soft navigation not detected.", /*timeout=*/ 3000);
63+
assert_equals(softNavs.length, 1, 'Expected exactly one soft navigation.');
64+
assert_true(
65+
softNavs[0].name.endsWith(url),
66+
`Unexpected Soft Navigation URL. Expected url to end with ${url} but got ${softNavs[0].name}`);
67+
68+
// Check that we get two ICP entries, one for the text and one for the image.
69+
const icps = await helper.withTimeoutMessage(
70+
icpPromise, 'ICPs not detected.', /*timeout=*/ 3000);
71+
assert_equals(icps.length, 2, 'Expected exactly two ICP entries.');
72+
73+
assert_equals(icps[0].id, 'content', 'Expected first ICP candidate to be "content"');
74+
assert_equals(icps[0].url, '', 'Expected first ICP url to be empty');
75+
76+
assert_equals(icps[1].id, 'content', 'Expected second ICP candidate to be "content"');
77+
assert_true(
78+
icps[1].url.endsWith('/images/lcp-256x256.png'),
79+
'Expected second ICP url to be the image url');
80+
}, 'Soft Navigation Detection supports adding nodes with image pseudo elements and text')
81+
</script>

‎soft-navigation-heuristics/detection/tentative/node-with-image-pseudo-element.html‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
<!DOCTYPE html>
2+
<meta charset="utf-8" />
3+
<script src="/resources/testharness.js"></script>
4+
<script src="/resources/testharnessreport.js"></script>
5+
<script src="/resources/testdriver.js"></script>
6+
<script src="/resources/testdriver-vendor.js"></script>
7+
<script src="/soft-navigation-heuristics/resources/soft-navigation-test-helper.js"></script>
8+
9+
<style>
10+
.content::before {
11+
content: "";
12+
display: block;
13+
width: 256px;
14+
height: 256px;
15+
background-image: url(/images/lcp-256x256.png);
16+
background-size: contain;
17+
background-repeat: no-repeat;
18+
background-position: center;
19+
}
20+
</style>
21+
22+
<button id="navigateButton">Click here!</button>
23+
<div id="container"></div>
24+
25+
<script>
26+
promise_test(async t => {
27+
const url = 'soft-nav-1';
28+
29+
navigateButton.addEventListener('click', async () => {
30+
container.innerHTML = `<div><div id="content">Text</div></div>`;
31+
history.pushState({}, '', url);
32+
}, {once: true});
33+
34+
// Set up the PerformanceObservers before clicking to avoid races,
35+
// and use unbuffered here so we don't get duplicate entries.
36+
const softNavPromise = SoftNavigationTestHelper.getPerformanceEntries(
37+
"soft-navigation",
38+
/* minNumEntries= */ 1);
39+
const icpPromise = SoftNavigationTestHelper.getPerformanceEntries(
40+
"interaction-contentful-paint",
41+
/* minNumEntries= */ 1);
42+
43+
if (test_driver) {
44+
test_driver.click(navigateButton);
45+
}
46+
47+
const helper = new SoftNavigationTestHelper(t);
48+
49+
// Check if we detected a soft nav.
50+
const softNavs = await helper.withTimeoutMessage(
51+
softNavPromise, "Soft navigation not detected.", /*timeout=*/ 3000);
52+
assert_equals(softNavs.length, 1, 'Expected exactly one soft navigation.');
53+
assert_true(
54+
softNavs[0].name.endsWith(url),
55+
`Unexpected Soft Navigation URL. Expected url to end with ${url} but got ${softNavs[0].name}`);
56+
57+
// Check that we only get one ICP entry, and that it's for the expected node.
58+
const icps = await helper.withTimeoutMessage(
59+
icpPromise, 'ICP not detected.', /*timeout=*/ 3000);
60+
assert_equals(icps.length, 1, 'Expected exactly one ICP entry.');
61+
assert_equals(icps[0].id, 'content', 'Expected ICP candidate to be "content"');
62+
}, 'Soft Navigation Detection supports adding nodes with image pseudo elements')
63+
</script>

0 commit comments

Comments
 (0)