Revert "[RPP Observations] Add phases for each interaction"

This reverts commit a51d9f9.

Reason for revert: e2e test on mac consistently fails since this CL landed. First failure: https://ci.chromium.org/ui/p/devtools-frontend/builders/ci/Stand-alone%20Mac-arm64/4134/overview.

Original change's description:
> [RPP Observations] Add phases for each interaction
>
> - Interaction list items are expandable
> - Creates separate interaction if events happen in different frames
> - Interactions are restored from the buffer
> - `eventNames` property added to each interaction for future use
>
> Bug: 369097528, 365160880, 371052022
> Change-Id: Iab39cb300143aad3a6f2cc5ef5ecab79d7fd879e
> Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/5933344
> Reviewed-by: Paul Irish <[email protected]>
> Commit-Queue: Adam Raine <[email protected]>

Bug: 369097528, 365160880, 371052022
Change-Id: I7cd208c5e2620c682f9babaec573df4c0880cfb6
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/5938363
Auto-Submit: Simon Zünd <[email protected]>
Commit-Queue: Rubber Stamper <[email protected]>
Bot-Commit: Rubber Stamper <[email protected]>

Author: szuend

front_end/models/live-metrics/LiveMetrics.ts

@@ -4,7 +4,6 @@
 
 import * as Common from '../../core/common/common.js';
 import * as Host from '../../core/host/host.js';
-import * as Platform from '../../core/platform/platform.js';
 import * as Root from '../../core/root/root.js';
 import * as SDK from '../../core/sdk/sdk.js';
 import type * as Protocol from '../../generated/protocol.js';
@@ -27,8 +26,6 @@ class InjectedScript {
   }
 }
 
-export type InteractionMap = Map<Spec.UniqueInteractionId, Interaction>;
-
 export class LiveMetrics extends Common.ObjectWrapper.ObjectWrapper<EventTypes> implements SDK.TargetManager.Observer {
   #enabled = false;
   #target?: SDK.Target.Target;
@@ -37,7 +34,7 @@ export class LiveMetrics extends Common.ObjectWrapper.ObjectWrapper<EventTypes>
   #lcpValue?: LCPValue;
   #clsValue?: CLSValue;
   #inpValue?: INPValue;
-  #interactions: InteractionMap = new Map();
+  #interactions: Interaction[] = [];
   #layoutShifts: LayoutShift[] = [];
   #mutex = new Common.Mutex.Mutex();
 
@@ -67,7 +64,7 @@ export class LiveMetrics extends Common.ObjectWrapper.ObjectWrapper<EventTypes>
     return this.#inpValue;
   }
 
-  get interactions(): InteractionMap {
+  get interactions(): Interaction[] {
     return this.#interactions;
   }
 
@@ -143,7 +140,7 @@ export class LiveMetrics extends Common.ObjectWrapper.ObjectWrapper<EventTypes>
 
     const allLayoutAffectedNodes = this.#layoutShifts.flatMap(shift => shift.affectedNodes);
     const toRefresh: Array<{node?: SDK.DOMModel.DOMNode}> =
-        [this.#lcpValue || {}, ...this.#interactions.values(), ...allLayoutAffectedNodes];
+        [this.#lcpValue || {}, ...this.#interactions, ...allLayoutAffectedNodes];
 
     const allPromises = toRefresh.map(item => {
       const node = item.node;
@@ -201,30 +198,20 @@ export class LiveMetrics extends Common.ObjectWrapper.ObjectWrapper<EventTypes>
         this.#inpValue = inpEvent;
         break;
       }
-      case 'InteractionEntry': {
-        const interaction: Interaction = Platform.MapUtilities.getWithDefault(
-            this.#interactions, webVitalsEvent.uniqueInteractionId,
-            () => ({
-              interactionType: webVitalsEvent.interactionType,
-              duration: webVitalsEvent.duration,
-              eventNames: [],
-              phases: webVitalsEvent.phases,
-              uniqueInteractionId: webVitalsEvent.uniqueInteractionId,
-            }));
-
-        // We can get multiple instances of the first input interaction since web-vitals.js installs
-        // an extra listener for events of type `first-input`. This is a simple way to de-dupe those
-        // events without adding complexity to the injected code.
-        if (!interaction.eventNames.includes(webVitalsEvent.eventName)) {
-          interaction.eventNames.push(webVitalsEvent.eventName);
-        }
-
+      case 'Interaction': {
+        const interaction: Interaction = {
+          duration: webVitalsEvent.duration,
+          interactionType: webVitalsEvent.interactionType,
+          uniqueInteractionId: webVitalsEvent.uniqueInteractionId,
+        };
         if (webVitalsEvent.nodeIndex !== undefined) {
           const node = await this.#resolveDomNode(webVitalsEvent.nodeIndex, executionContextId);
           if (node) {
             interaction.node = node;
           }
         }
+
+        this.#interactions.push(interaction);
         break;
       }
       case 'LayoutShift': {
@@ -248,7 +235,7 @@ export class LiveMetrics extends Common.ObjectWrapper.ObjectWrapper<EventTypes>
         this.#lcpValue = undefined;
         this.#clsValue = undefined;
         this.#inpValue = undefined;
-        this.#interactions.clear();
+        this.#interactions = [];
         this.#layoutShifts = [];
         break;
       }
@@ -335,7 +322,7 @@ export class LiveMetrics extends Common.ObjectWrapper.ObjectWrapper<EventTypes>
   }
 
   clearInteractions(): void {
-    this.#interactions.clear();
+    this.#interactions = [];
     this.#sendStatusUpdate();
   }
 
@@ -480,10 +467,8 @@ export interface LayoutShift {
 }
 
 export interface Interaction {
-  interactionType: Spec.InteractionEntryEvent['interactionType'];
-  eventNames: string[];
-  duration: Spec.InteractionEntryEvent['duration'];
-  phases: Spec.INPPhases;
+  interactionType: Spec.InteractionEvent['interactionType'];
+  duration: Spec.InteractionEvent['duration'];
   uniqueInteractionId: Spec.UniqueInteractionId;
   node?: SDK.DOMModel.DOMNode;
 }
@@ -492,7 +477,7 @@ export interface StatusEvent {
   lcp?: LCPValue;
   cls?: CLSValue;
   inp?: INPValue;
-  interactions: InteractionMap;
+  interactions: Interaction[];
   layoutShifts: LayoutShift[];
 }
 

front_end/models/live-metrics/web-vitals-injected/OnEachInteraction.ts

@@ -4,31 +4,68 @@
 
 /**
  * @fileoverview web-vitals.js doesn't provide a log of all interactions.
- * This solution is hacky but it was recommended by web-vitals devs:
- * b/371052022
+ * This was largely copied from the web vitals extension:
+ * https://github.com/GoogleChrome/web-vitals-extension/blob/main/src/browser_action/on-each-interaction.js
  */
 
-import * as WebVitals from '../../../third_party/web-vitals/web-vitals.js';
-
-export function onEachInteraction(onReport: (metric: WebVitals.INPMetricWithAttribution) => void): void {
-  WebVitals.entryPreProcessingCallbacks.push((entry: PerformanceEventTiming) => {
-    // Wait a microtask so this "pre" processing callback actually
-    // becomes a "post" processing callback.
-    void Promise.resolve().then(() => {
-      if (entry.interactionId) {
-        const interaction = WebVitals.attributeINP({
-          entries: [entry],
-          // The only value we really need for `attributeINP` is `entries`
-          // Everything else is included to fill out the type.
-          name: 'INP',
-          rating: 'good',
-          value: entry.duration,
-          delta: entry.duration,
-          navigationType: 'navigate',
-          id: 'N/A',
-        });
-        onReport(interaction);
-      }
-    });
+import type * as WebVitals from '../../../third_party/web-vitals/web-vitals.js';
+
+export interface InteractionWithAttribution {
+  attribution: {
+    interactionTargetElement: Node|null,
+    interactionType: WebVitals.INPAttribution['interactionType'],
+  };
+  entries: PerformanceEventTiming[];
+  value: number;
+}
+
+export function onEachInteraction(callback: (interaction: InteractionWithAttribution) => void): void {
+  const eventObserver = new PerformanceObserver(list => {
+    const entries = list.getEntries();
+    const interactions = new Map<number, PerformanceEventTiming[]>();
+
+    const performanceEventTimings = entries.filter((entry): entry is PerformanceEventTiming => 'interactionId' in entry)
+                                        .filter(entry => entry.interactionId);
+
+    for (const entry of performanceEventTimings) {
+      const interaction = interactions.get(entry.interactionId) || [];
+      interaction.push(entry);
+      interactions.set(entry.interactionId, interaction);
+    }
+
+    // Will report as a single interaction even if parts are in separate frames.
+    // Consider splitting by animation frame.
+    for (const interaction of interactions.values()) {
+      const longestEntry = interaction.reduce((prev, curr) => prev.duration >= curr.duration ? prev : curr);
+      const value = longestEntry.duration;
+
+      const firstEntryWithTarget = interaction.find(entry => entry.target);
+
+      callback({
+        attribution: {
+          interactionTargetElement: firstEntryWithTarget?.target ?? null,
+          interactionType: longestEntry.name.startsWith('key') ? 'keyboard' : 'pointer',
+        },
+        entries: interaction,
+        value,
+      });
+    }
+  });
+
+  eventObserver.observe({
+    type: 'first-input',
+    buffered: false,
+  });
+
+  eventObserver.observe({
+    type: 'event',
+    durationThreshold: 0,
+    // Interaction events can only be stored to the buffer if their duration is >=104ms.
+    // https://www.w3.org/TR/event-timing/#sec-events-exposed
+    //
+    // This means we can only collect a subset of interactions that happen before this observer is started.
+    // To avoid confusion, we only collect interactions that after this observer has started.
+    // Note: This DOES NOT affect the collection for the INP metric and so INP will still be restored from the buffer.
+    buffered: false,
   });
 }

front_end/models/live-metrics/web-vitals-injected/spec/spec.ts

@@ -62,18 +62,11 @@ export interface INPChangeEvent extends MetricChangeEvent {
   uniqueInteractionId: UniqueInteractionId;
 }
 
-/**
- * This event is not 1:1 with the interactions that the user sees in the interactions log.
- * It is 1:1 with a `PerformanceEventTiming` entry that will be combined by `uniqueInteractionId`
- * in the DevTools client.
- */
-export interface InteractionEntryEvent {
-  name: 'InteractionEntry';
+export interface InteractionEvent {
+  name: 'Interaction';
   interactionType: INPAttribution['interactionType'];
-  eventName: string;
   uniqueInteractionId: UniqueInteractionId;
   duration: number;
-  phases: INPPhases;
   nodeIndex?: number;
 }
 
@@ -88,5 +81,4 @@ export interface ResetEvent {
   name: 'reset';
 }
 
-export type WebVitalsEvent =
-    LCPChangeEvent|CLSChangeEvent|INPChangeEvent|InteractionEntryEvent|LayoutShiftEvent|ResetEvent;
+export type WebVitalsEvent = LCPChangeEvent|CLSChangeEvent|INPChangeEvent|InteractionEvent|LayoutShiftEvent|ResetEvent;

front_end/models/live-metrics/web-vitals-injected/web-vitals-injected.ts

@@ -152,23 +152,14 @@ function initialize(): void {
       interactionType: metric.attribution.interactionType,
     };
     sendEventToDevTools(event);
-  }, {reportAllChanges: true, durationThreshold: 0});
+  }, {reportAllChanges: true});
 
   onEachInteraction(interaction => {
-    // Multiple `InteractionEntry` events can be emitted for the same `uniqueInteractionId`
-    // However, it is easier to combine these entries in the DevTools client rather than in
-    // this injected code.
-    const event: Spec.InteractionEntryEvent = {
-      name: 'InteractionEntry',
+    const event: Spec.InteractionEvent = {
+      name: 'Interaction',
       duration: interaction.value,
-      phases: {
-        inputDelay: interaction.attribution.inputDelay,
-        processingDuration: interaction.attribution.processingDuration,
-        presentationDelay: interaction.attribution.presentationDelay,
-      },
       uniqueInteractionId: Spec.getUniqueInteractionId(interaction.entries),
       interactionType: interaction.attribution.interactionType,
-      eventName: interaction.entries[0].name,
     };
     const node = interaction.attribution.interactionTargetElement;
     if (node) {