Properly report soft navigation name in multiple entry cases

In cases where we had multiple entries operations (specifically, replaceState with a null URL followed by a pushState with the actual URL), we saw that the reported URL corresponds to the previous page, rather than to the navigated page's URL. This CL fixes that. It also changes the soft navigation heuristics to ignore history stack operations that don't modify the URL. Bug: 1371808 Change-Id: I0ab1002e8dd9ec13fa3153950521c7477fcbe2f1 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4046223 Commit-Queue: Yoav Weiss <[email protected]> Reviewed-by: Ian Clelland <[email protected]> Cr-Commit-Position: refs/heads/main@{#1074723}
web-platform-tests · Nov 22, 2022 · f5e9057 · f5e9057
1 parent f910727
commit f5e9057
Show file tree

Hide file tree

Showing 5 changed files with 96 additions and 30 deletions.
diff --git a/soft-navigation-heuristics/navigation-api-preventDefault.tentative.html b/soft-navigation-heuristics/navigation-api-preventDefault.tentative.html
@@ -16,16 +16,19 @@
   </main>
   <script>
     const link = document.getElementById("link");
-    testNavigationApiNotDetected(
-      "Aborted navigate event is not a soft navigation",
-      e => {
+    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();
-      }, link);
+      },
+      eventTarget: navigation,
+      eventName: "navigate",
+      link: link});
   </script>
 </body>
 </html>

diff --git a/soft-navigation-heuristics/replacestate-null-then-push.tentative.html b/soft-navigation-heuristics/replacestate-null-then-push.tentative.html
@@ -0,0 +1,34 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+<meta charset="utf-8">
+<title>Detect soft navigation with replaceState that has a null URL, then
+       pushState with the URL.</title>
+<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");
+    testSoftNavigation({
+      addContent: async (url) => {
+        await addImageToMain();
+        history.pushState({}, '', url);
+      },
+      link: link,
+      pushState: async (url) =>{
+        history.replaceState({}, '');
+      },
+      test: "Detect soft navigation with replaceState that has a null URL," +
+            " then pushState with the URL"});
+  </script>
+</body>
+</html>
+
+
diff --git a/soft-navigation-heuristics/replacestate.tentative.html b/soft-navigation-heuristics/replacestate.tentative.html
@@ -0,0 +1,28 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+<meta charset="utf-8">
+<title>Detect soft navigation with replaceState.</title>
+<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");
+    testSoftNavigation({
+      addContent: async () => {
+        await addImageToMain();
+      },
+      link: link,
+      pushState: (url)=>{history.replaceState({}, '', url);},
+      test: "Detect soft navigation with replaceState"});
+  </script>
+</body>
+</html>
+
diff --git a/soft-navigation-heuristics/resources/soft-navigation-helper.js b/soft-navigation-heuristics/resources/soft-navigation-helper.js
@@ -58,22 +58,24 @@ const testNavigationApi = (testName, navigateEventHandler, link) => {
   }, testName);
 };
 
-const testNavigationApiNotDetected =
-    (testName, navigateEventHandler, link) => {
-      promise_test(async t => {
-        const preClickLcp = await getLcpEntries();
-        navigation.addEventListener('navigate', navigateEventHandler);
-        click(link);
-        await new Promise((resolve, reject) => {
-          (new PerformanceObserver(() => reject())).observe({
-            type: 'soft-navigation'
-          });
-          t.step_timeout(resolve, 1000);
+const testSoftNavigationNotDetected = options => {
+    promise_test(async t => {
+      const preClickLcp = await getLcpEntries();
+      options.eventTarget.addEventListener(options.eventName, options.eventHandler);
+      click(options.link);
+      await new Promise((resolve, reject) => {
+        (new PerformanceObserver(() =>
+            reject("Soft navigation should not be triggered"))).observe({
+          type: 'soft-navigation',
+          buffered: true
         });
-        assert_equals(
-            document.softNavigations, 0, 'Soft Navigation not detected');
-      }, testName);
-    };
+        t.step_timeout(resolve, 1000);
+      });
+      assert_equals(
+          document.softNavigations, 0, 'Soft Navigation not detected');
+    }, options.testName);
+  };
+
 const runEntryValidations = async preClickLcp => {
   await doubleRaf();
   validatePaintEntries('first-contentful-paint');
@@ -109,14 +111,11 @@ const setEvent = (t, button, pushState, addContent, pushUrl, eventType) => {
     // Jump through a task, to ensure task tracking is working properly.
     await new Promise(r => t.step_timeout(r, 0));
 
-    // Fetch some content
-    const response = await fetch("/soft-navigation-heuristics/resources/content.json");
-    const json = await response.json();
-
+    const url = URL + "?" + counter;
     if (pushState) {
       // Change the URL
       if (pushUrl) {
-        pushState(URL + "?" + counter);
+        pushState(url);
       } else {
         pushState();
       }
@@ -125,7 +124,7 @@ const setEvent = (t, button, pushState, addContent, pushUrl, eventType) => {
     // Wait 10 ms to make sure the timestamps are correct.
     await new Promise(r => t.step_timeout(r, 10));
 
-    await addContent(json);
+    await addContent(url);
     ++counter;
 
     clicked = true;

diff --git a/soft-navigation-heuristics/soft-navigation-no-url.tentative.html b/soft-navigation-heuristics/soft-navigation-no-url.tentative.html
@@ -15,14 +15,16 @@
   </main>
   <script>
     const link = document.getElementById("link");
-    testSoftNavigation({
-      addContent: () => {
+    testSoftNavigationNotDetected({
+      eventHandler: url => {
         addTextToDivOnMain();
+        history.pushState({}, '');
       },
       link: link,
-      pushUrl: false,
-      test: "Test that a soft navigation is detected when a URL is not passed"
-        + " to the history API."});
+      eventName: "click",
+      eventTarget: link,
+      testName: "Test that a soft navigation is not detected when a URL is not"
+        + " passed to the history API."});
   </script>
 </body>
 </html>