From 40543949faa61279ddd3341accfe9ea866a5987e Mon Sep 17 00:00:00 2001 From: Hao Liu Date: Wed, 13 Sep 2023 10:44:08 -0700 Subject: [PATCH] Revert "[soft navigations] Enable keyboard shortcuts as soft navigation triggers" This reverts commit 6efe71286a014d3d3872bc990e3ea2d08dd46dba. Reason for revert: One check added in this CL causes crash. see crbug.com/1480047 Original change's description: > [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. > > [1] https://github.com/WICG/soft-navigations/issues/3 > > Bug: 1478772 > Change-Id: Ib423a3cfc09eaf4dd9a2221b3494ab1016fa8668 > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4839506 > Commit-Queue: Yoav Weiss > Reviewed-by: Ian Clelland > Cr-Commit-Position: refs/heads/main@{#1193004} Bug: 1478772 Change-Id: I3a518c165e6b19239a6bf7900e94c1ef9c3e5a5a Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4859802 Reviewed-by: Ian Clelland Commit-Queue: Hao Liu Owners-Override: Daniel Cheng Bot-Commit: Rubber Stamper Cr-Commit-Position: refs/heads/main@{#1196100} --- ...d-by-two-image-softnavs-lcp.tentative.html | 4 +-- .../keydown.tentative.html | 30 ---------------- .../navigate-child.html | 2 +- .../resources/soft-navigation-helper.js | 35 ++++++++----------- 4 files changed, 17 insertions(+), 54 deletions(-) delete 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 0e1b9a4d679cb9..25151cdd79ea4f 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(); - interact(link); + click(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(); - interact(link); + click(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 deleted file mode 100644 index fac86d71d2cc04..00000000000000 --- a/soft-navigation-heuristics/keydown.tentative.html +++ /dev/null @@ -1,30 +0,0 @@ - - - - -Detect hashchange event. - - - - - - - -
-
- First LCP! -
-
- - - diff --git a/soft-navigation-heuristics/navigate-child.html b/soft-navigation-heuristics/navigate-child.html index e3c17e2dbaa214..63a8adb208f2a0 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"); - interact(link); + click(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 64d0c265b6562c..edc9331c26375e 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 interacted; +var clicked; var timestamps = [] const MAX_CLICKS = 50; // Entries for one hard navigation + 50 soft navigations. @@ -20,7 +20,6 @@ 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'; promise_test(async t => { await waitInitialLCP(); @@ -30,8 +29,8 @@ const testSoftNavigation = const firstClick = (i === 0); let paint_entries_promise = waitOnPaintEntriesPromise(expectLCP && firstClick); - interacted = false; - interact(link, interactionType); + clicked = false; + click(link); await new Promise(resolve => { (new PerformanceObserver(() => resolve())).observe({ @@ -62,7 +61,7 @@ const testNavigationApi = (testName, navigateEventHandler, link) => { await waitInitialLCP(); const preClickLcp = await getLcpEntries(); let paint_entries_promise = waitOnPaintEntriesPromise(); - interact(link); + click(link); await new Promise(resolve => { (new PerformanceObserver(() => resolve())).observe({ type: 'soft-navigation' @@ -81,7 +80,7 @@ const testSoftNavigationNotDetected = options => { promise_test(async t => { const preClickLcp = await getLcpEntries(); options.eventTarget.addEventListener(options.eventName, options.eventHandler); - interact(options.link); + click(options.link); await new Promise((resolve, reject) => { (new PerformanceObserver(() => reject("Soft navigation should not be triggered"))).observe({ @@ -129,21 +128,15 @@ const runEntryValidations = } }; -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 click = link => { + if (test_driver) { + test_driver.click(link); + timestamps[counter] = {"syncPostClick": performance.now()}; + } +} const setEvent = (t, button, pushState, addContent, pushUrl, eventType) => { - const eventObject = - (eventType == 'click' || eventType == 'keydown') ? button : window; + const eventObject = (eventType == "click") ? button : window; eventObject.addEventListener(eventType, async e => { timestamps[counter]["eventStart"] = performance.now(); // Jump through a task, to ensure task tracking is working properly. @@ -165,7 +158,7 @@ const setEvent = (t, button, pushState, addContent, pushUrl, eventType) => { await addContent(url); ++counter; - interacted = true; + clicked = true; }); }; @@ -185,7 +178,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]["syncPostInteraction"], entryTimestamp); + assert_less_than_equal(timestamps[i]["syncPostClick"], entryTimestamp); assert_greater_than_equal( timestamps[i]['eventStart'], entryTimestamp, 'Event start timestamp matches');