Ensure one-time event listeners only run once (#538)

* Ensure one-time event listeners only run once

* Refactor whenIdle to make the logic more clear

* Restore original runOnce and whenIdle order

* Fix comment

src/attribution/onINP.ts

@@ -22,7 +22,7 @@ import {
   longestInteractionMap,
 } from '../lib/interactions.js';
 import {observe} from '../lib/observe.js';
-import {whenIdle} from '../lib/whenIdle.js';
+import {whenIdleOrHidden} from '../lib/whenIdleOrHidden.js';
 import {onINP as unattributedOnINP} from '../onINP.js';
 import {
   INPAttribution,
@@ -76,10 +76,8 @@ const entryToEntriesGroupMap: WeakMap<
 // A mapping of interactionIds to the target Node.
 export const interactionTargetMap: Map<number, Node> = new Map();
 
-// A reference to the idle task used to clean up entries from the above
-// variables. If the value is -1 it means no task is queue, and if it's
-// greater than -1 the value corresponds to the idle callback handle.
-let idleHandle: number = -1;
+// A boolean flag indicating whether or not a cleanup task has been queued.
+let cleanupPending = false;
 
 /**
  * Adds new LoAF entries to the `pendingLoAFs` list.
@@ -159,8 +157,9 @@ const groupEntriesByRenderTime = (entry: PerformanceEventTiming) => {
 
 const queueCleanup = () => {
   // Queue cleanup of entries that are not part of any INP candidates.
-  if (idleHandle < 0) {
-    idleHandle = whenIdle(cleanupEntries);
+  if (!cleanupPending) {
+    whenIdleOrHidden(cleanupEntries);
+    cleanupPending = true;
   }
 };
 
@@ -206,8 +205,7 @@ const cleanupEntries = () => {
     return loafsToKeep.has(loaf);
   });
 
-  // Reset the idle callback handle so it can be queued again.
-  idleHandle = -1;
+  cleanupPending = false;
 };
 
 entryPreProcessingCallbacks.push(

src/lib/whenIdle.ts → src/lib/whenIdleOrHidden.ts

@@ -14,25 +14,19 @@
  * limitations under the License.
  */
 
-import {onHidden} from './onHidden.js';
-import {runOnce} from './runOnce.js';
-
 /**
  * Runs the passed callback during the next idle period, or immediately
  * if the browser's visibility state is (or becomes) hidden.
  */
-export const whenIdle = (cb: () => void): number => {
+export const whenIdleOrHidden = (cb: () => void) => {
   const rIC = globalThis.requestIdleCallback || setTimeout;
 
-  let handle = -1;
-  cb = runOnce(cb);
   // If the document is hidden, run the callback immediately, otherwise
   // race an idle callback with the next `visibilitychange` event.
   if (document.visibilityState === 'hidden') {
     cb();
   } else {
-    handle = rIC(cb);
-    onHidden(cb);
+    rIC(cb);
+    document.addEventListener('visibilitychange', cb, {once: true});
   }
-  return handle;
 };

src/onCLS.ts

@@ -19,7 +19,6 @@ import {initMetric} from './lib/initMetric.js';
 import {observe} from './lib/observe.js';
 import {bindReporter} from './lib/bindReporter.js';
 import {doubleRAF} from './lib/doubleRAF.js';
-import {onHidden} from './lib/onHidden.js';
 import {runOnce} from './lib/runOnce.js';
 import {onFCP} from './onFCP.js';
 import {CLSMetric, MetricRatingThresholds, ReportOpts} from './types.js';
@@ -107,9 +106,11 @@ export const onCLS = (
           opts!.reportAllChanges,
         );
 
-        onHidden(() => {
-          handleEntries(po.takeRecords() as CLSMetric['entries']);
-          report(true);
+        document.addEventListener('visibilitychange', () => {
+          if (document.visibilityState === 'hidden') {
+            handleEntries(po.takeRecords() as CLSMetric['entries']);
+            report(true);
+          }
         });
 
         // Only report after a bfcache restore if the `PerformanceObserver`

src/onINP.ts

@@ -24,10 +24,9 @@ import {
   resetInteractions,
 } from './lib/interactions.js';
 import {observe} from './lib/observe.js';
-import {onHidden} from './lib/onHidden.js';
 import {initInteractionCountPolyfill} from './lib/polyfills/interactionCountPolyfill.js';
 import {whenActivated} from './lib/whenActivated.js';
-import {whenIdle} from './lib/whenIdle.js';
+import {whenIdleOrHidden} from './lib/whenIdleOrHidden.js';
 
 import {INPMetric, MetricRatingThresholds, ReportOpts} from './types.js';
 
@@ -89,7 +88,7 @@ export const onINP = (
       // have been dispatched. Note: there is currently an experiment
       // running in Chrome (EventTimingKeypressAndCompositionInteractionId)
       // 123+ that if rolled out fully may make this no longer necessary.
-      whenIdle(() => {
+      whenIdleOrHidden(() => {
         for (const entry of entries) {
           processInteractionEntry(entry);
         }
@@ -126,9 +125,11 @@ export const onINP = (
       // where the first interaction is less than the `durationThreshold`.
       po.observe({type: 'first-input', buffered: true});
 
-      onHidden(() => {
-        handleEntries(po.takeRecords() as INPMetric['entries']);
-        report(true);
+      document.addEventListener('visibilitychange', () => {
+        if (document.visibilityState === 'hidden') {
+          handleEntries(po.takeRecords() as INPMetric['entries']);
+          report(true);
+        }
       });
 
       // Only report after a bfcache restore if the `PerformanceObserver`

src/onLCP.ts

@@ -21,10 +21,9 @@ import {getActivationStart} from './lib/getActivationStart.js';
 import {getVisibilityWatcher} from './lib/getVisibilityWatcher.js';
 import {initMetric} from './lib/initMetric.js';
 import {observe} from './lib/observe.js';
-import {onHidden} from './lib/onHidden.js';
 import {runOnce} from './lib/runOnce.js';
 import {whenActivated} from './lib/whenActivated.js';
-import {whenIdle} from './lib/whenIdle.js';
+import {whenIdleOrHidden} from './lib/whenIdleOrHidden.js';
 import {LCPMetric, MetricRatingThresholds, ReportOpts} from './types.js';
 
 /** Thresholds for LCP. See https://web.dev/articles/lcp#what_is_a_good_lcp_score */
@@ -85,27 +84,33 @@ export const onLCP = (
         opts!.reportAllChanges,
       );
 
-      const stopListening = runOnce(() => {
-        if (!reportedMetricIDs[metric.id]) {
-          handleEntries(po!.takeRecords() as LCPMetric['entries']);
-          po!.disconnect();
-          reportedMetricIDs[metric.id] = true;
-          report(true);
-        }
-      });
+      // Ensure this logic only runs once, and wrap it in an idle callback
+      // so the callback is run in a separate task to avoid extending the
+      // keyboard/click handler to reduce INP impact.
+      // https://github.com/GoogleChrome/web-vitals/issues/383
+      const stopListening = () =>
+        whenIdleOrHidden(
+          runOnce(() => {
+            if (!reportedMetricIDs[metric.id]) {
+              handleEntries(po!.takeRecords() as LCPMetric['entries']);
+              po!.disconnect();
+              reportedMetricIDs[metric.id] = true;
+              report(true);
+            }
+          }),
+        );
 
-      // Stop listening after input. Note: while scrolling is an input that
-      // stops LCP observation, it's unreliable since it can be programmatically
-      // generated. See: https://github.com/GoogleChrome/web-vitals/issues/75
-      for (const type of ['keydown', 'click']) {
-        // Wrap in a setTimeout so the callback is run in a separate task
-        // to avoid extending the keyboard/click handler to reduce INP impact
-        // https://github.com/GoogleChrome/web-vitals/issues/383
-        addEventListener(type, () => whenIdle(stopListening), true);
+      // Stop listening after input or visibilitychange.
+      // Note: while scrolling is an input that stops LCP observation, it's
+      // unreliable since it can be programmatically generated.
+      // See: https://github.com/GoogleChrome/web-vitals/issues/75
+      for (const type of ['keydown', 'click', 'visibilitychange']) {
+        addEventListener(type, () => stopListening(), {
+          capture: true,
+          once: true,
+        });
       }
 
-      onHidden(stopListening);
-
       // Only report after a bfcache restore if the `PerformanceObserver`
       // successfully registered.
       onBFCacheRestore((event) => {