Skip to content

Commit

Permalink
Reland "[soft-navigations] move startTime to processing end"
Browse files Browse the repository at this point in the history
This is a reland of commit eab7d4b6462aee496860f0dbe342066bd342f5b9

It better takes into account event bubbling, and the fact we see
multiple tasks in a single EventScope in those cases. It also tests this case specifically.

Original change's description:
> [soft-navigations] move startTime to processing end
>
> This CL aligns the implementation with [1], and sets the soft navigation
> start time to be the processingEnd of the relevant earlier event.
>
> It also cleans up a few bits of code that became useless, as well as
> issues around PerInteractionData copying that were buggy yet hidden
> before the timing of setting the different attributes was changed.
>
>
> [1] WICG/soft-navigations#14
>
> Change-Id: Id9f7ebf9f372a334a206f99258c882f10eeda2c5
> Bug: 1505059
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5059108
> Reviewed-by: Ian Clelland <[email protected]>
> Commit-Queue: Yoav Weiss <[email protected]>
> Cr-Commit-Position: refs/heads/main@{#1230490}

Bug: 1505059
Change-Id: I8c3664588c937e91643651d86d5a9635433e8e18
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5076713
Commit-Queue: Yoav Weiss <[email protected]>
Reviewed-by: Ian Clelland <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1231299}
  • Loading branch information
Yoav Weiss authored and chromium-wpt-export-bot committed Nov 30, 2023
1 parent 3262b0f commit 0b3dc93
Show file tree
Hide file tree
Showing 11 changed files with 52 additions and 12 deletions.
28 changes: 28 additions & 0 deletions soft-navigation-heuristics/click-event-bubbles.tentative.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
<!DOCTYPE HTML>
<html>
<head>
<meta charset="utf-8">
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="/resources/testdriver.js"></script>
<script src="/resources/testdriver-vendor.js"></script>
<script src="resources/soft-navigation-helper.js"></script>
</head>
<body>
<main id=main>
<a id=link>Click me!</a>
</main>
<script>
const link = document.getElementById("link");
// Adding a noop event that the "click" would bubble to.
document.getElementById("main").addEventListener("click", () => {});

testSoftNavigation({
addContent: () => {
addTextParagraphToMain("Lorem Ipsum");
},
link: link,
test: "Ensure event bubbling works well with soft navigations."});
</script>
</body>
</html>
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
});
testSoftNavigation({
eventPrepWork: url => {
timestamps[counter]["eventStart"] = performance.now();
addTextToDivOnMain();
history.pushState({}, '', 'foobar1.html');
// Here we're bypassing the regular test's event logic, as this test is
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,11 @@
<script>
const link = document.getElementById("link");
testNavigationApi("Test soft navigation when navigate event intecepts with { commit: 'after-transition' }", e => {
timestamps[counter]["eventStart"] = performance.now();
e.intercept({commit: "after-transition", handler: async () => {
await addImageToMain();
e.commit();
}});
timestamps[counter]["eventEnd"] = performance.now();
}, link);
</script>
</body>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,11 @@
<script>
const link = document.getElementById("link");
testNavigationApi("Test soft navigation with the Navigation API", e => {
timestamps[counter]["eventStart"] = performance.now();
e.intercept({handler: async () => {
await addImageToMain();
main.appendChild(img);
}});
timestamps[counter]["eventEnd"] = performance.now();
}, link);
</script>
</body>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,12 @@
testSoftNavigationNotDetected({
testName: "Aborted navigate event is not a soft navigation",
eventHandler: e => {
timestamps[counter]["eventStart"] = performance.now();
e.intercept({handler: async () => {
await addImageToMain();
main.appendChild(img);
}});
e.preventDefault();
timestamps[counter]["eventEnd"] = performance.now();
},
eventTarget: navigation,
eventName: "navigate",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,11 @@
<script>
const link = document.getElementById("link");
testNavigationApi("Test intercepted and rejected navigate event", e => {
timestamps[counter]["eventStart"] = performance.now();
e.intercept({handler: async () => {
await addImageToMain();
throw new Error("This navigation handler rejected");
}});
timestamps[counter]["eventEnd"] = performance.now();
}, link);
</script>
</body>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
await new Promise(r => step_timeout(r, 0));

const navigate_callback = e => {
timestamps[counter]["eventStart"] = performance.now();
e.intercept({
async handler() {
const lcp_promise = new Promise(resolve => {
Expand All @@ -41,6 +40,7 @@
await lcp_promise;
}
});
timestamps[counter]["eventEnd"] = performance.now();
};

const link = document.getElementById("link");
Expand Down
2 changes: 1 addition & 1 deletion soft-navigation-heuristics/navigation-api.tentative.html
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,10 @@
<script>
const link = document.getElementById("link");
testNavigationApi("Test soft navigation with the Navigation API", e => {
timestamps[counter]["eventStart"] = performance.now();
e.intercept({handler: async () => {
await addImageToMain();
}});
timestamps[counter]["eventEnd"] = performance.now();
}, link);
</script>
</body>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,10 @@
return t.popped == 2;
}
const link = document.getElementById("link");
link.addEventListener("click", () => history.back());
link.addEventListener("click", () => {
history.back();
timestamps[counter]["eventEnd"] = performance.now();
});
testSoftNavigation({
addContent: () => {
// Add the content to the main element
Expand Down
5 changes: 4 additions & 1 deletion soft-navigation-heuristics/popstate.tentative.html
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,10 @@
history.pushState({}, "", "another_one.html");

const link = document.getElementById("link");
link.addEventListener("click", () => history.back());
link.addEventListener("click", () => {
history.back();
timestamps[counter]["eventEnd"] = performance.now();
});
testSoftNavigation({
addContent: () => {
// Add the content to the main element
Expand Down
13 changes: 10 additions & 3 deletions soft-navigation-heuristics/resources/soft-navigation-helper.js
Original file line number Diff line number Diff line change
Expand Up @@ -144,10 +144,17 @@ const setEvent = (t, button, pushState, addContent, pushUrl, eventType, prepWork
const eventObject =
(eventType == 'click' || eventType.startsWith("key")) ? button : window;
eventObject.addEventListener(eventType, async e => {
let prepWorkFailed = false;
if (prepWork &&!prepWork(t)) {
prepWorkFailed = true;
}
// This is the end of the event's sync processing.
if (!timestamps[counter]["eventEnd"]) {
timestamps[counter]["eventEnd"] = performance.now();
}
if (prepWorkFailed) {
return;
}
timestamps[counter]["eventStart"] = performance.now();
// Jump through a task, to ensure task tracking is working properly.
await new Promise(r => t.step_timeout(r, 0));

Expand All @@ -165,9 +172,9 @@ const setEvent = (t, button, pushState, addContent, pushUrl, eventType, prepWork
await new Promise(r => t.step_timeout(r, 10));

await addContent(url);
++counter;

interacted = true;
++counter;
});
};

Expand All @@ -190,7 +197,7 @@ const validateSoftNavigationEntry = async (clicks, extraValidations,
assert_less_than_equal(timestamps[i]["syncPostInteraction"], entryTimestamp,
"Entry timestamp is lower than the post interaction one");
assert_greater_than_equal(
timestamps[i]['eventStart'], entryTimestamp,
entryTimestamp, timestamps[i]['eventEnd'],
'Event start timestamp matches');
assert_not_equals(entry.navigationId,
performance.getEntriesByType("navigation")[0].navigationId,
Expand Down

0 comments on commit 0b3dc93

Please sign in to comment.