From a884aebaa098c234b34b767dd0ad59cc53b28cca Mon Sep 17 00:00:00 2001
From: Yoav Weiss <yoavweiss@chromium.org>
Date: Tue, 10 Oct 2023 06:56:04 -0700
Subject: [PATCH] [soft-navigations] Take paint area into account

Currently, it's possible for a soft navigation to be triggered after a
non-visual element was appended to the DOM.
This CL modifies that, so that only if visual elements that comprise of
at least 20% of the initial (hard navigation) paint area were appended
to the DOM as a result of a user interaction, that would count as a soft
navigation.

Bug: 1479355
Change-Id: Id4609c6435c2ab3dde1a26fea14988e7cd171dad
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4846835
Reviewed-by: Ian Clelland <iclelland@chromium.org>
Commit-Queue: Yoav Weiss <yoavweiss@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1207615}
---
 ...d-by-two-image-softnavs-lcp.tentative.html |   3 -
 .../resources/soft-navigation-helper.js       | 137 ++++++++++--------
 ...-paint-larger-than-viewport.tentative.html |  70 +++++++++
 .../softnav-after-lcp-paint.tentative.html    |  42 ++++++
 .../softnav-before-lcp-paint.tentative.html   |  52 +++++++
 ...etween-lcp-render-and-paint.tentative.html |  36 +++++
 ...d-by-anim-image-softnav-lcp.tentative.html |  32 ++++
 .../visited-link.tentative.html               |  11 +-
 8 files changed, 320 insertions(+), 63 deletions(-)
 create mode 100644 soft-navigation-heuristics/softnav-after-lcp-paint-larger-than-viewport.tentative.html
 create mode 100644 soft-navigation-heuristics/softnav-after-lcp-paint.tentative.html
 create mode 100644 soft-navigation-heuristics/softnav-before-lcp-paint.tentative.html
 create mode 100644 soft-navigation-heuristics/softnav-between-lcp-render-and-paint.tentative.html
 create mode 100644 soft-navigation-heuristics/text-lcp-followed-by-anim-image-softnav-lcp.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..66a9e5136f6fed 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
@@ -19,9 +19,6 @@
     const link = document.getElementById("link");
 
     promise_test(async t => {
-      if (!performance.softNavPaintMetricsSupported) {
-        return;
-      }
       validatePaintEntries('first-contentful-paint', 1);
       validatePaintEntries('first-paint', 1);
       const preClickLcp = await getLcpEntries();
diff --git a/soft-navigation-heuristics/resources/soft-navigation-helper.js b/soft-navigation-heuristics/resources/soft-navigation-helper.js
index 1116ebccb359e3..6fdc1d100b929c 100644
--- a/soft-navigation-heuristics/resources/soft-navigation-helper.js
+++ b/soft-navigation-heuristics/resources/soft-navigation-helper.js
@@ -21,24 +21,24 @@ const testSoftNavigation =
       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 => {
         await waitInitialLCP();
         const preClickLcp = await getLcpEntries();
-        setEvent(t, link, pushState, addContent, pushUrl, eventType, eventPrepWork);
+        setEvent(t, link, pushState, addContent, pushUrl, eventType,
+                 eventPrepWork);
+        let first_navigation_id;
         for (let i = 0; i < clicks; ++i) {
           const firstClick = (i === 0);
           let paint_entries_promise =
-              waitOnPaintEntriesPromise(expectLCP && firstClick);
+              waitOnPaintEntriesPromise(firstClick);
           interacted = false;
           interact(link, interactionType);
 
-          await new Promise(resolve => {
-            (new PerformanceObserver(() => resolve())).observe({
-              type: 'soft-navigation'
-            });
-          });
+          const navigation_id = await waitOnSoftNav();
+          if (!first_navigation_id) {
+            first_navigation_id = navigation_id;
+          }
           // Ensure paint timing entries are fired before moving on to the next
           // click.
           await paint_entries_promise;
@@ -49,7 +49,7 @@ const testSoftNavigation =
         await validateSoftNavigationEntry(
             clicks, extraValidations, pushUrl);
 
-        await runEntryValidations(preClickLcp, clicks + 1, expectLCP);
+        await runEntryValidations(preClickLcp, first_navigation_id, clicks + 1, options.validate);
       }, testName);
     };
 
@@ -64,17 +64,13 @@ const testNavigationApi = (testName, navigateEventHandler, link) => {
     const preClickLcp = await getLcpEntries();
     let paint_entries_promise = waitOnPaintEntriesPromise();
     interact(link);
-    await new Promise(resolve => {
-      (new PerformanceObserver(() => resolve())).observe({
-        type: 'soft-navigation'
-      });
-    });
+    const first_navigation_id = await waitOnSoftNav();
     await navigated;
     await paint_entries_promise;
     assert_equals(document.softNavigations, 1, 'Soft Navigation detected');
     await validateSoftNavigationEntry(1, () => {}, 'foobar.html');
 
-    await runEntryValidations(preClickLcp);
+    await runEntryValidations(preClickLcp, first_navigation_id);
   }, testName);
 };
 
@@ -102,37 +98,34 @@ const testSoftNavigationNotDetected = options => {
   };
 
 const runEntryValidations =
-    async (preClickLcp, entries_expected_number = 2, expect_lcp = true) => {
-  await validatePaintEntries('first-contentful-paint', entries_expected_number);
-  await validatePaintEntries('first-paint', entries_expected_number);
+    async (preClickLcp, first_navigation_id, entries_expected_number = 2,
+           validate = null) => {
+  await validatePaintEntries('first-contentful-paint', entries_expected_number,
+                             first_navigation_id);
+  await validatePaintEntries('first-paint', entries_expected_number,
+                             first_navigation_id);
   const postClickLcp = await getLcpEntries();
   const postClickLcpWithoutSoftNavs = await getLcpEntriesWithoutSoftNavs();
-  if (expect_lcp) {
-    assert_greater_than(
-        postClickLcp.length, preClickLcp.length,
-        'Soft navigation should have triggered at least an LCP entry');
-  } else {
-    assert_equals(
-        postClickLcp.length, preClickLcp.length,
-        'Soft navigation should not have triggered an LCP entry');
+  assert_greater_than(
+      postClickLcp.length, preClickLcp.length,
+      'Soft navigation should have triggered at least an LCP entry');
+
+  if (validate) {
+    await validate();
   }
   assert_equals(
       postClickLcpWithoutSoftNavs.length, preClickLcp.length,
       'Soft navigation should not have triggered an LCP entry when the ' +
       'observer did not opt in');
-  if (expect_lcp) {
-    assert_not_equals(
-        postClickLcp[postClickLcp.length - 1].size,
-        preClickLcp[preClickLcp.length - 1].size,
-        'Soft navigation LCP element should not have identical size to the hard ' +
-            'navigation LCP element');
-  } else {
-    assert_equals(
-        postClickLcp[postClickLcp.length - 1].size,
-        preClickLcp[preClickLcp.length - 1].size,
-        'Soft navigation LCP element should have an identical size to the hard ' +
-            'navigation LCP element');
-  }
+  assert_not_equals(
+      postClickLcp[postClickLcp.length - 1].size,
+      preClickLcp[preClickLcp.length - 1].size,
+      'Soft navigation LCP element should not have identical size to the hard ' +
+          'navigation LCP element');
+  assert_equals(
+      postClickLcp[preClickLcp.length].navigationId,
+      first_navigation_id, 'Soft navigation LCP should have the same navigation ' +
+      'ID as the last soft nav entry')
 };
 
 const interact =
@@ -214,7 +207,7 @@ const validateSoftNavigationEntry = async (clicks, extraValidations,
 
 };
 
-const validatePaintEntries = async (type, entries_number) => {
+const validatePaintEntries = async (type, entries_number, first_navigation_id) => {
   if (!performance.softNavPaintMetricsSupported) {
     return;
   }
@@ -242,6 +235,12 @@ const validatePaintEntries = async (type, entries_number) => {
     assert_not_equals(entries[0].startTime, entries[1].startTime,
       "Entries have different timestamps for " + type);
   }
+  if (expected_entries_number > entries_without_softnavs.length) {
+    assert_equals(entries[entries_without_softnavs.length].navigationId,
+      first_navigation_id,
+      "First paint entry should have the same navigation ID as the last soft " +
+      "navigation entry");
+  }
 };
 
 const waitInitialLCP = () => {
@@ -253,6 +252,19 @@ const waitInitialLCP = () => {
   });
 }
 
+const waitOnSoftNav = () => {
+  return new Promise(resolve => {
+    (new PerformanceObserver(list => {
+       const entries = list.getEntries();
+       assert_equals(entries.length, 1,
+                     "Only one soft navigation entry");
+       resolve(entries[0].navigationId);
+    })).observe({
+      type: 'soft-navigation'
+    });
+  });
+};
+
 const getLcpEntries = async () => {
   const entries = await new Promise(resolve => {
     (new PerformanceObserver(list => resolve(
@@ -272,30 +284,41 @@ const getLcpEntriesWithoutSoftNavs = async () => {
   return entries;
 };
 
-const addImage = async (element) => {
+const addImage = async (element, url="blue.png") => {
   const img = new Image();
-  img.src = '/images/blue.png' + "?" + Math.random();
+  img.src = '/images/'+ url + "?" + Math.random();
   img.id="imagelcp";
   await img.decode();
   element.appendChild(img);
 };
-const addImageToMain = async () => {
-  await addImage(document.getElementById('main'));
+const addImageToMain = async (url="blue.png") => {
+  await addImage(document.getElementById('main'), url);
 };
 
-const addTextToDivOnMain =
-    () => {
-      const main = document.getElementById("main");
-      const prevDiv = document.getElementsByTagName("div")[0];
-      if (prevDiv) {
-        main.removeChild(prevDiv);
-      }
-      const div = document.createElement("div");
-      const text = document.createTextNode("Lorem Ipsum");
-      div.appendChild(text);
-      div.style = "font-size: 3em";
-      main.appendChild(div);
-    }
+const addTextParagraphToMain = (text, element_timing = "") => {
+  const main = document.getElementById("main");
+  const p = document.createElement("p");
+  const textNode = document.createTextNode(text);
+  p.appendChild(textNode);
+  if (element_timing) {
+    p.setAttribute("elementtiming", element_timing);
+  }
+  p.style = "font-size: 3em";
+  main.appendChild(p);
+  return p;
+};
+const addTextToDivOnMain = () => {
+  const main = document.getElementById("main");
+  const prevDiv = document.getElementsByTagName("div")[0];
+  if (prevDiv) {
+    main.removeChild(prevDiv);
+  }
+  const div = document.createElement("div");
+  const text = document.createTextNode("Lorem Ipsum");
+  div.appendChild(text);
+  div.style = "font-size: 3em";
+  main.appendChild(div);
+}
 
 const waitOnPaintEntriesPromise = (expectLCP = true) => {
   return new Promise((resolve, reject) => {
diff --git a/soft-navigation-heuristics/softnav-after-lcp-paint-larger-than-viewport.tentative.html b/soft-navigation-heuristics/softnav-after-lcp-paint-larger-than-viewport.tentative.html
new file mode 100644
index 00000000000000..3c930d8be4cd45
--- /dev/null
+++ b/soft-navigation-heuristics/softnav-after-lcp-paint-larger-than-viewport.tentative.html
@@ -0,0 +1,70 @@
+<!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="/soft-navigation-heuristics/resources/soft-navigation-helper.js"></script>
+<link rel="stylesheet" type="text/css" href="/fonts/ahem.css" />
+</head>
+<body>
+  <main id=main>
+    <div>
+      <a id=link><img id="initial_image1" src="/images/lcp-256x256.png?1" style="width: 90vw; height: 90vh">
+        <div id=extra></div>
+      </a>
+    </div>
+  </main>
+  <script>
+  (async () => {
+    const link = document.getElementById("link");
+
+    // Inject a second image that takes a large part of the viewport.
+    await new Promise(resolve => {
+      requestAnimationFrame(() => requestAnimationFrame(() => {
+        document.getElementById("extra").innerHTML = `
+          <div style="position: absolute; bottom: 0; right: 0">
+            <img id="initial_image2" src="/images/lcp-256x256.png?2" style="width: 90vw;height: 90vh">
+          </div>`;
+        resolve();
+      }));
+    });
+    // Wait until the second image is rendered.
+    await new Promise(resolve => {
+      requestAnimationFrame(() => requestAnimationFrame(() => {
+        resolve();
+      }));
+    });
+    testSoftNavigation({
+      addContent: async () => {
+        // Remove the initial image, so that the text would be painted on screen.
+        document.getElementById("initial_image1").remove();
+        document.getElementById("initial_image2").remove();
+        let lcp_element_painted;
+        const lcp_element_paint_promise = new Promise((r) => { lcp_element_painted = r; });
+        // Add an LCP element, but have it be small enough to not trigger the
+        // Soft Navigation heuristics.
+        const p = addTextParagraphToMain(
+          "Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod",
+          /*element_timing=*/"lcp");
+        (new PerformanceObserver(list => {
+          // Once the first element is fully painted:
+          lcp_element_painted();
+        })).observe({type: "element", buffered: true});
+        await lcp_element_paint_promise;
+        // Add a smaller element that gets us over that threshold.
+        addTextParagraphToMain("dolore magna aliqua.");
+      },
+      link: link,
+      test: "Test that an image LCP followed by a smaller soft navigation LCP"
+        + " properly queues an LCP entry, even when the soft navigation is"
+        + " detected after the LCP, even when initial paints significantly"
+        + " exceed the viewport dimensions."});
+  })();
+  </script>
+</body>
+</html>
+
+
diff --git a/soft-navigation-heuristics/softnav-after-lcp-paint.tentative.html b/soft-navigation-heuristics/softnav-after-lcp-paint.tentative.html
new file mode 100644
index 00000000000000..cf9a9942ca6453
--- /dev/null
+++ b/soft-navigation-heuristics/softnav-after-lcp-paint.tentative.html
@@ -0,0 +1,42 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+<meta charset="utf-8">
+<title>Detect simple soft navigation.</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>
+<link rel="stylesheet" type="text/css" href="/fonts/ahem.css" />
+</head>
+<body>
+  <main id=main>
+    <div>
+      <a id=link><img src="/images/lcp-256x256.png"></a>
+    </div>
+  </main>
+  <script>
+    const link = document.getElementById("link");
+    testSoftNavigation({
+      addContent: async () => {
+        let lcp_element_painted;
+        const lcp_element_paint_promise = new Promise((r) => { lcp_element_painted = r; });
+        // Add an LCP element, but have it be small enough to not trigger the
+        // Soft Navigation heuristics.
+        const p = addTextParagraphToMain("Lorem Ipsu", /*element_timing=*/"lcp");
+        (new PerformanceObserver(list => {
+          // Once the first element is fully painted:
+          lcp_element_painted();
+        })).observe({type: "element", buffered: true});
+        await lcp_element_paint_promise;
+        // Add a smaller element that gets us over that threshold.
+        addTextParagraphToMain("m");
+      },
+      link: link,
+      test: "Test that an image LCP followed by a smaller soft navigation LCP"
+        + " properly queues an LCP entry, even when the soft navigation is"
+        + " detected after the LCP."});
+  </script>
+</body>
+</html>
diff --git a/soft-navigation-heuristics/softnav-before-lcp-paint.tentative.html b/soft-navigation-heuristics/softnav-before-lcp-paint.tentative.html
new file mode 100644
index 00000000000000..d5dc4b74fb33b5
--- /dev/null
+++ b/soft-navigation-heuristics/softnav-before-lcp-paint.tentative.html
@@ -0,0 +1,52 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+<meta charset="utf-8">
+<title>Detect simple soft navigation.</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>
+<link rel="stylesheet" type="text/css" href="/fonts/ahem.css" />
+</head>
+<body>
+  <main id=main>
+    <div>
+      <a id=link><img src="/images/lcp-256x256.png"></a>
+    </div>
+  </main>
+  <script>
+    const link = document.getElementById("link");
+    testSoftNavigation({
+      addContent: async () => {
+        // Add an LCP element, large enough to trigger the Soft Navigation
+        // heuristics.
+        const p = addTextParagraphToMain("Lorem Ipsum", /*element_timing=*/"lcp");
+        p.id = "first_lcp";
+        // Once the first element is fully painted.
+        const observer = new PerformanceObserver(list => {
+          // Add a larger element to be the new LCP.
+          window.lcp_observer_promise = new Promise(resolve => {
+            (new PerformanceObserver(resolve)).observe({type: "element"});
+          });
+          const p2 = addTextParagraphToMain("LOREM IPSUMER", "real_lcp");
+          p2.id = "real_lcp";
+          observer.disconnect();
+        });
+        observer.observe({type: "element", buffered: true});
+      },
+      link: link,
+      validate: async () => {
+        await window.lcp_observer_promise;
+        const lcps = await getLcpEntries();
+        assert_greater_than_equal(lcps.length, 3, "Got at least 3 LCP entries");
+        assert_equals(lcps[lcps.length - 2].id, "first_lcp", "Got the first LCP");
+        assert_equals(lcps[lcps.length - 1].id, "real_lcp", "Got the real LCP");
+      },
+      test: "Test that an image LCP followed by 2 smaller soft navigation LCPs"
+        + " properly queues both LCP entries, even when the soft navigation"
+        + " is detected in between them."});
+  </script>
+</body>
+</html>
diff --git a/soft-navigation-heuristics/softnav-between-lcp-render-and-paint.tentative.html b/soft-navigation-heuristics/softnav-between-lcp-render-and-paint.tentative.html
new file mode 100644
index 00000000000000..56c7073de63036
--- /dev/null
+++ b/soft-navigation-heuristics/softnav-between-lcp-render-and-paint.tentative.html
@@ -0,0 +1,36 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+<meta charset="utf-8">
+<title>Detect simple soft navigation.</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>
+<link rel="stylesheet" type="text/css" href="/fonts/ahem.css" />
+</head>
+<body>
+  <main id=main>
+    <div>
+      <a id=link><img src="/images/lcp-256x256.png"></a>
+    </div>
+  </main>
+  <script>
+    const link = document.getElementById("link");
+    testSoftNavigation({
+      addContent: async () => {
+        // Add an LCP element, but have it be small enough to not trigger the
+        // Soft Navigation heuristics.
+        const p = addTextParagraphToMain("Lorem Ipsu");
+        requestAnimationFrame(() => {
+          const p2 = addTextParagraphToMain("m");
+        });
+      },
+      link: link,
+      test: "Test that an image LCP followed by a smaller soft navigation LCP"
+        + " properly queues an LCP entry, even when the soft navigation is"
+        + " detected between the LCP's render and paint."});
+  </script>
+</body>
+</html>
diff --git a/soft-navigation-heuristics/text-lcp-followed-by-anim-image-softnav-lcp.tentative.html b/soft-navigation-heuristics/text-lcp-followed-by-anim-image-softnav-lcp.tentative.html
new file mode 100644
index 00000000000000..0615b513e61319
--- /dev/null
+++ b/soft-navigation-heuristics/text-lcp-followed-by-anim-image-softnav-lcp.tentative.html
@@ -0,0 +1,32 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+<meta charset="utf-8">
+<title>Detect simple soft navigation.</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>
+    <div>
+      <a id=link style="font-size: 4em">Click me!</a>
+    </div>
+  </main>
+  <script>
+    const link = document.getElementById("link");
+    testSoftNavigation({
+      addContent: async () => {
+        const main = document.getElementById("main");
+        main.removeChild(document.getElementsByTagName("div")[0]);
+        await addImageToMain("anim-gr.png");
+      },
+      link: link,
+      test: "Test that a text LCP followup by a smaller soft navigation image"
+        + " LCP properly queues an LCP entry"});
+  </script>
+</body>
+</html>
+
diff --git a/soft-navigation-heuristics/visited-link.tentative.html b/soft-navigation-heuristics/visited-link.tentative.html
index e81c6f617345af..739d03b4f8e115 100644
--- a/soft-navigation-heuristics/visited-link.tentative.html
+++ b/soft-navigation-heuristics/visited-link.tentative.html
@@ -12,7 +12,7 @@
 <body>
   <main id=main>
   <a id=link>Click me!</a>
-  <a id=visited>link</a>
+  <a id=visited>link that is really long so it is the LCP</a>
   </main>
   <script>
     const visited = document.getElementById("visited");
@@ -26,7 +26,6 @@
       const div = document.createElement("div");
       const text = document.createTextNode("Lorem Ipsum");
       div.appendChild(text);
-      div.style="visibility: hidden";
       main.appendChild(div);
     }
     const link = document.getElementById("link");
@@ -35,7 +34,13 @@
         await visitFakeURLAndAddInvisibleText();
       },
       link: link,
-      validate: "no-lcp",
+      validate: async () => {
+        await new Promise(r => step_timeout(r, 100));
+        const postVisitedLcp = await getLcpEntries();
+        assert_not_equals(postVisitedLcp[postVisitedLcp.length - 1].id,
+                          "visited",
+                          "Soft Nav LCP ID should not be visited");
+      },
       test: "Test that a visited link doesn't trigger LCP after a soft " +
             "navigation is detected"});
   </script>