From 04ab10bfca7454a6f6d968cb6c9c697fcdea9de2 Mon Sep 17 00:00:00 2001 From: Yoav Weiss Date: Mon, 2 Oct 2023 02:41:54 -0700 Subject: [PATCH] Reland: [soft navigations] Enable keyboard shortcuts as soft navigation triggers Following the discussion on issue #3 [1], this CL adds support to soft navigations triggered by keyboard shortcuts, by adding unfocused keydown events to the events that can trigger the soft navigation heuristic. This is a reland of [2], rebased and which fixes the unguarded ScriptState access in event_dispatcher, which caused a crash. [1] https://github.com/WICG/soft-navigations/issues/3 [2] https://chromium-review.googlesource.com/c/chromium/src/+/4839506 Bug: 1478772, 1480047 Change-Id: I6428e0635222366d880dd908f04f2273b6bf8b44 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4900577 Reviewed-by: Ian Clelland Commit-Queue: Yoav Weiss Cr-Commit-Position: refs/heads/main@{#1203903} --- ...d-by-two-image-softnavs-lcp.tentative.html | 4 +-- .../keydown.tentative.html | 30 ++++++++++++++++ .../navigate-child.html | 2 +- .../resources/soft-navigation-helper.js | 36 +++++++++++-------- 4 files changed, 54 insertions(+), 18 deletions(-) create mode 100644 soft-navigation-heuristics/keydown.tentative.html diff --git a/soft-navigation-heuristics/image-lcp-followed-by-two-image-softnavs-lcp.tentative.html b/soft-navigation-heuristics/image-lcp-followed-by-two-image-softnavs-lcp.tentative.html index 25151cdd79ea4f..0e1b9a4d679cb9 100644 --- a/soft-navigation-heuristics/image-lcp-followed-by-two-image-softnavs-lcp.tentative.html +++ b/soft-navigation-heuristics/image-lcp-followed-by-two-image-softnavs-lcp.tentative.html @@ -31,7 +31,7 @@ const first_click_paint_promise = waitOnPaintEntriesPromise(); - click(link); + interact(link); await new Promise(resolve => { (new PerformanceObserver(resolve)).observe({ type: 'soft-navigation' @@ -54,7 +54,7 @@ const second_click_paint_promise = waitOnPaintEntriesPromise(); const preClickLcp2 = await getLcpEntries(); - click(link); + interact(link); await new Promise(resolve => { (new PerformanceObserver(() => resolve())).observe({ type: 'soft-navigation' diff --git a/soft-navigation-heuristics/keydown.tentative.html b/soft-navigation-heuristics/keydown.tentative.html new file mode 100644 index 00000000000000..fac86d71d2cc04 --- /dev/null +++ b/soft-navigation-heuristics/keydown.tentative.html @@ -0,0 +1,30 @@ + + + + +Detect hashchange event. + + + + + + + +
+
+ First LCP! +
+
+ + + diff --git a/soft-navigation-heuristics/navigate-child.html b/soft-navigation-heuristics/navigate-child.html index 63a8adb208f2a0..e3c17e2dbaa214 100644 --- a/soft-navigation-heuristics/navigate-child.html +++ b/soft-navigation-heuristics/navigate-child.html @@ -20,7 +20,7 @@ await new Promise(r => t.step_timeout(r, 10)); } const link = document.getElementById("link"); - click(link); + interact(link); while (!child.location.href.includes("2")) { await new Promise(r => t.step_timeout(r, 10)); } diff --git a/soft-navigation-heuristics/resources/soft-navigation-helper.js b/soft-navigation-heuristics/resources/soft-navigation-helper.js index d119e02272514e..37097b276955d7 100644 --- a/soft-navigation-heuristics/resources/soft-navigation-helper.js +++ b/soft-navigation-heuristics/resources/soft-navigation-helper.js @@ -1,5 +1,5 @@ var counter = 0; -var clicked; +var interacted; var timestamps = [] const MAX_CLICKS = 50; // Entries for one hard navigation + 50 soft navigations. @@ -20,6 +20,7 @@ const testSoftNavigation = const testName = options.testName; const pushUrl = readValue(options.pushUrl, true); const eventType = readValue(options.eventType, "click"); + const interactionType = readValue(options.interactionType, 'click'); const expectLCP = options.validate != 'no-lcp'; const eventPrepWork = options.eventPrepWork; promise_test(async t => { @@ -30,8 +31,8 @@ const testSoftNavigation = const firstClick = (i === 0); let paint_entries_promise = waitOnPaintEntriesPromise(expectLCP && firstClick); - clicked = false; - click(link); + interacted = false; + interact(link, interactionType); await new Promise(resolve => { (new PerformanceObserver(() => resolve())).observe({ @@ -62,7 +63,7 @@ const testNavigationApi = (testName, navigateEventHandler, link) => { await waitInitialLCP(); const preClickLcp = await getLcpEntries(); let paint_entries_promise = waitOnPaintEntriesPromise(); - click(link); + interact(link); await new Promise(resolve => { (new PerformanceObserver(() => resolve())).observe({ type: 'soft-navigation' @@ -81,7 +82,7 @@ const testSoftNavigationNotDetected = options => { promise_test(async t => { const preClickLcp = await getLcpEntries(); options.eventTarget.addEventListener(options.eventName, options.eventHandler); - click(options.link); + interact(options.link); await new Promise((resolve, reject) => { (new PerformanceObserver(() => reject("Soft navigation should not be triggered"))).observe({ @@ -129,16 +130,21 @@ const runEntryValidations = } }; -const click = link => { - if (test_driver) { - test_driver.click(link); - timestamps[counter] = {"syncPostClick": performance.now()}; - } -} +const interact = + (link, interactionType = 'click') => { + if (test_driver) { + if (interactionType == 'click') { + test_driver.click(link); + } else { + test_driver.send_keys(link, 'j'); + } + timestamps[counter] = {"syncPostInteraction": performance.now()}; + } + } const setEvent = (t, button, pushState, addContent, pushUrl, eventType, prepWork) => { - const eventObject = (eventType == "click") ? button : window; - + const eventObject = + (eventType == 'click' || eventType == 'keydown') ? button : window; eventObject.addEventListener(eventType, async e => { if (prepWork &&!prepWork(t)) { return; @@ -163,7 +169,7 @@ const setEvent = (t, button, pushState, addContent, pushUrl, eventType, prepWork await addContent(url); ++counter; - clicked = true; + interacted = true; }); }; @@ -183,7 +189,7 @@ const validateSoftNavigationEntry = async (clicks, extraValidations, assert_true(entry.name.includes(pushUrl ? URL : document.location.href), "The soft navigation name is properly set"); const entryTimestamp = entry.startTime; - assert_less_than_equal(timestamps[i]["syncPostClick"], entryTimestamp); + assert_less_than_equal(timestamps[i]["syncPostInteraction"], entryTimestamp); assert_greater_than_equal( timestamps[i]['eventStart'], entryTimestamp, 'Event start timestamp matches');