[RPP Observations] Warn if LCP increases due to emulation changes

Bug: 360379617
Change-Id: Ib9ff899faf78cc3181f17616192dc8ad8a8dc008
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6043070
Commit-Queue: Adam Raine <[email protected]>
Reviewed-by: Connor Clark <[email protected]>

Author: Adam Raine

front_end/models/emulation/BUILD.gn

@@ -32,6 +32,7 @@ devtools_entrypoint("bundle") {
 
   visibility = [
     ":*",
+    "../*",
     "../../panels/emulation/*",
   ]
 

front_end/models/emulation/DeviceModeModel.ts

@@ -187,6 +187,21 @@ export class DeviceModeModel extends Common.ObjectWrapper.ObjectWrapper<EventTyp
     return deviceModeModelInstance;
   }
 
+  /**
+   * This wraps `instance()` in a try/catch because in some DevTools entry points
+   * (such as worker_app.ts) the Emulation panel is not included and as such
+   * the below code fails; it tries to instantiate the model which requires
+   * reading the value of a setting which has not been registered.
+   * See crbug.com/361515458 for an example bug that this resolves.
+   */
+  static tryInstance(opts?: {forceNew: boolean}): DeviceModeModel|null {
+    try {
+      return this.instance(opts);
+    } catch {
+      return null;
+    }
+  }
+
   static widthValidator(value: string): {
     valid: boolean,
     errorMessage: (string|undefined),

front_end/models/live-metrics/BUILD.gn

@@ -13,6 +13,7 @@ devtools_module("live-metrics") {
     "../../core/host:bundle",
     "../../core/sdk:bundle",
     "../../generated:protocol",
+    "../emulation:bundle",
     "./web-vitals-injected:web-vitals-injected",
     "./web-vitals-injected/spec:bundle",
   ]

front_end/models/live-metrics/LiveMetrics.ts

@@ -4,13 +4,26 @@
 
 import * as Common from '../../core/common/common.js';
 import * as Host from '../../core/host/host.js';
+import * as i18n from '../../core/i18n/i18n.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';
+import * as EmulationModel from '../../models/emulation/emulation.js';
 
 import * as Spec from './web-vitals-injected/spec/spec.js';
 
+const UIStrings = {
+  /**
+   * @description Warning text indicating that the Largest Contentful Paint (LCP) performance metric was affected by the user changing the simulated device.
+   */
+  lcpEmulationWarning:
+      'Simulating a new device after the page loads can affect LCP. Reload the page after simulating a new device for accurate LCP data.',
+};
+
+const str_ = i18n.i18n.registerUIStrings('models/live-metrics/LiveMetrics.ts', UIStrings);
+const i18nString = i18n.i18n.getLocalizedString.bind(undefined, str_);
+
 const LIVE_METRICS_WORLD_NAME = 'DevTools Performance Metrics';
 
 let liveMetricsInstance: LiveMetrics;
@@ -40,7 +53,9 @@ export class LiveMetrics extends Common.ObjectWrapper.ObjectWrapper<EventTypes>
   #interactions: InteractionMap = new Map();
   #interactionsByGroupId = new Map<Spec.InteractionEntryGroupId, Interaction[]>();
   #layoutShifts: LayoutShift[] = [];
+  #lastEmulationChangeTime?: number;
   #mutex = new Common.Mutex.Mutex();
+  #deviceModeModel = EmulationModel.DeviceModeModel.DeviceModeModel.tryInstance();
 
   private constructor() {
     super();
@@ -137,6 +152,10 @@ export class LiveMetrics extends Common.ObjectWrapper.ObjectWrapper<EventTypes>
     return true;
   }
 
+  #onEmulationChanged(): void {
+    this.#lastEmulationChangeTime = Date.now();
+  }
+
   /**
    * DOM nodes can't be sent over a runtime binding, so we have to retrieve
    * them separately.
@@ -232,9 +251,11 @@ export class LiveMetrics extends Common.ObjectWrapper.ObjectWrapper<EventTypes>
       webVitalsEvent: Spec.WebVitalsEvent, executionContextId: Protocol.Runtime.ExecutionContextId): Promise<void> {
     switch (webVitalsEvent.name) {
       case 'LCP': {
+        const warnings: string[] = [];
         const lcpEvent: LCPValue = {
           value: webVitalsEvent.value,
           phases: webVitalsEvent.phases,
+          warnings,
         };
         if (webVitalsEvent.nodeIndex !== undefined) {
           const node = await this.#resolveDomNode(webVitalsEvent.nodeIndex, executionContextId);
@@ -243,6 +264,10 @@ export class LiveMetrics extends Common.ObjectWrapper.ObjectWrapper<EventTypes>
           }
         }
 
+        if (this.#lastEmulationChangeTime && Date.now() - this.#lastEmulationChangeTime < 500) {
+          warnings.push(i18nString(UIStrings.lcpEmulationWarning));
+        }
+
         this.#lcpValue = lcpEvent;
         break;
       }
@@ -495,6 +520,9 @@ export class LiveMetrics extends Common.ObjectWrapper.ObjectWrapper<EventTypes>
     });
     this.#scriptIdentifier = identifier;
 
+    this.#deviceModeModel?.addEventListener(
+        EmulationModel.DeviceModeModel.Events.UPDATED, this.#onEmulationChanged, this);
+
     this.#enabled = true;
   }
 
@@ -526,6 +554,9 @@ export class LiveMetrics extends Common.ObjectWrapper.ObjectWrapper<EventTypes>
     }
     this.#scriptIdentifier = undefined;
 
+    this.#deviceModeModel?.removeEventListener(
+        EmulationModel.DeviceModeModel.Events.UPDATED, this.#onEmulationChanged, this);
+
     this.#enabled = false;
   }
 }
@@ -536,7 +567,10 @@ export const enum Events {
 
 export type InteractionId = `interaction-${number}-${number}`;
 
-export type MetricValue = Pick<Spec.MetricChangeEvent, 'value'>;
+export interface MetricValue {
+  value: number;
+  warnings?: string[];
+}
 
 export interface LCPValue extends MetricValue {
   phases: Spec.LCPPhases;

front_end/panels/timeline/TimelinePanel.ts

@@ -2156,20 +2156,6 @@ export class TimelinePanel extends UI.Panel.Panel implements Client, TimelineMod
     this.flameChart.getMainFlameChart().update();
   }
 
-  #deviceModeModel(): EmulationModel.DeviceModeModel.DeviceModeModel|null {
-    // This is wrapped in a try/catch because in some DevTools entry points
-    // (such as worker_app.ts) the Emulation panel is not included and as such
-    // the below code fails; it tries to instantiate the model which requires
-    // reading the value of a setting which has not been registered.
-    // In this case, we fallback to 'ALL'. See crbug.com/361515458 for an
-    // example bug that this resolves.
-    try {
-      return EmulationModel.DeviceModeModel.DeviceModeModel.instance();
-    } catch {
-      return null;
-    }
-  }
-
   /**
    * This is called with we are done loading a trace from a file, or after we
    * have recorded a fresh trace.
@@ -2211,7 +2197,7 @@ export class TimelinePanel extends UI.Panel.Panel implements Client, TimelineMod
     }
 
     if (!metadata) {
-      const deviceModeModel = this.#deviceModeModel();
+      const deviceModeModel = EmulationModel.DeviceModeModel.DeviceModeModel.tryInstance();
       let emulatedDeviceTitle;
       if (deviceModeModel?.type() === EmulationModel.DeviceModeModel.Type.Device) {
         emulatedDeviceTitle = deviceModeModel.device()?.title ?? undefined;

front_end/panels/timeline/components/LiveMetricsView.ts

@@ -308,6 +308,7 @@ export class LiveMetricsView extends LegacyWrapper.LegacyWrapper.WrappableCompon
   #interactionsListEl?: HTMLElement;
   #layoutShiftsListEl?: HTMLElement;
   #listIsScrolling = false;
+  #deviceModeModel = EmulationModel.DeviceModeModel.DeviceModeModel.tryInstance();
 
   constructor() {
     super();
@@ -396,8 +397,8 @@ export class LiveMetricsView extends LegacyWrapper.LegacyWrapper.WrappableCompon
     const cruxManager = CrUXManager.CrUXManager.instance();
     cruxManager.addEventListener(CrUXManager.Events.FIELD_DATA_CHANGED, this.#onFieldDataChanged, this);
 
-    const emulationModel = this.#deviceModeModel();
-    emulationModel?.addEventListener(EmulationModel.DeviceModeModel.Events.UPDATED, this.#onEmulationChanged, this);
+    this.#deviceModeModel?.addEventListener(
+        EmulationModel.DeviceModeModel.Events.UPDATED, this.#onEmulationChanged, this);
 
     if (cruxManager.getConfigSetting().get().enabled) {
       void this.#refreshFieldDataForCurrentPage();
@@ -411,27 +412,13 @@ export class LiveMetricsView extends LegacyWrapper.LegacyWrapper.WrappableCompon
     void ComponentHelpers.ScheduledRender.scheduleRender(this, this.#render);
   }
 
-  #deviceModeModel(): EmulationModel.DeviceModeModel.DeviceModeModel|null {
-    // This is wrapped in a try/catch because in some DevTools entry points
-    // (such as worker_app.ts) the Emulation panel is not included and as such
-    // the below code fails; it tries to instantiate the model which requires
-    // reading the value of a setting which has not been registered.
-    // In this case, we fallback to 'ALL'. See crbug.com/361515458 for an
-    // example bug that this resolves.
-    try {
-      return EmulationModel.DeviceModeModel.DeviceModeModel.instance();
-    } catch {
-      return null;
-    }
-  }
-
   disconnectedCallback(): void {
     LiveMetrics.LiveMetrics.instance().removeEventListener(LiveMetrics.Events.STATUS, this.#onMetricStatus, this);
 
     const cruxManager = CrUXManager.CrUXManager.instance();
     cruxManager.removeEventListener(CrUXManager.Events.FIELD_DATA_CHANGED, this.#onFieldDataChanged, this);
 
-    this.#deviceModeModel()?.removeEventListener(
+    this.#deviceModeModel?.removeEventListener(
         EmulationModel.DeviceModeModel.Events.UPDATED, this.#onEmulationChanged, this);
   }
 
@@ -448,6 +435,7 @@ export class LiveMetricsView extends LegacyWrapper.LegacyWrapper.WrappableCompon
         fieldValue: fieldData?.percentiles?.p75,
         histogram: fieldData?.histogram,
         tooltipContainer: this.#tooltipContainerEl,
+        warnings: this.#lcpValue?.warnings,
         phases: phases && [
           [i18nString(UIStrings.timeToFirstByte), phases.timeToFirstByte],
           [i18nString(UIStrings.resourceLoadDelay), phases.resourceLoadDelay],
@@ -482,6 +470,7 @@ export class LiveMetricsView extends LegacyWrapper.LegacyWrapper.WrappableCompon
         fieldValue: fieldData?.percentiles?.p75,
         histogram: fieldData?.histogram,
         tooltipContainer: this.#tooltipContainerEl,
+        warnings: this.#clsValue?.warnings,
       } as MetricCardData}>
         ${clusterIsVisible ? html`
           <div class="related-info" slot="extra-info">
@@ -512,6 +501,7 @@ export class LiveMetricsView extends LegacyWrapper.LegacyWrapper.WrappableCompon
         fieldValue: fieldData?.percentiles?.p75,
         histogram: fieldData?.histogram,
         tooltipContainer: this.#tooltipContainerEl,
+        warnings: this.#inpValue?.warnings,
         phases: phases && [
           [i18nString(UIStrings.inputDelay), phases.inputDelay],
           [i18nString(UIStrings.processingDuration), phases.processingDuration],
@@ -756,13 +746,11 @@ export class LiveMetricsView extends LegacyWrapper.LegacyWrapper.WrappableCompon
   }
 
   #getAutoDeviceScope(): CrUXManager.DeviceScope {
-    const emulationModel = this.#deviceModeModel();
-
-    if (emulationModel === null) {
+    if (this.#deviceModeModel === null) {
       return 'ALL';
     }
 
-    if (emulationModel.isMobile()) {
+    if (this.#deviceModeModel.isMobile()) {
       if (this.#cruxPageResult?.[`${this.#fieldPageScope}-PHONE`]) {
         return 'PHONE';
       }

front_end/panels/timeline/components/MetricCard.test.ts

@@ -40,6 +40,10 @@ function getDetailedCompareText(view: Element): HTMLElement|null {
   return view.shadowRoot!.querySelector('.detailed-compare-text');
 }
 
+function getWarnings(view: Element): string[] {
+  return Array.from(view.shadowRoot!.querySelectorAll('.warning')).map(w => w.textContent!);
+}
+
 function getEnvironmentRecs(view: Element): string[] {
   const recs = Array.from(view.shadowRoot!.querySelectorAll('.environment-recs li'));
   return recs.map(rec => rec.textContent!);
@@ -177,6 +181,25 @@ describeWithMockConnection('MetricCard', () => {
     assert.match(histogramLabels[2], /Poor\s+\(>4.00 s\)/);
   });
 
+  it('should show warnings', async () => {
+    const view = new Components.MetricCard.MetricCard();
+    view.data = {
+      metric: 'LCP',
+      localValue: 2000,
+      fieldValue: 1,
+      histogram: createMockHistogram(),
+      warnings: ['LCP warning'],
+    };
+
+    renderElementIntoDOM(view);
+    await coordinator.done();
+
+    const warnings = getWarnings(view);
+    assert.deepStrictEqual(warnings, [
+      'LCP warning',
+    ]);
+  });
+
   describe('phase table', () => {
     it('should not show if there is no phase data', async () => {
       const view = new Components.MetricCard.MetricCard();

front_end/panels/timeline/components/MetricCard.ts

@@ -149,6 +149,7 @@ export interface MetricCardData {
   histogram?: CrUXManager.MetricResponse['histogram'];
   tooltipContainer?: HTMLElement;
   phases?: Array<[string, number]>;
+  warnings?: string[];
 }
 
 export class MetricCard extends HTMLElement {
@@ -670,6 +671,9 @@ export class MetricCard extends HTMLElement {
         </div>
         ${fieldEnabled ? html`<hr class="divider">` : nothing}
         ${this.#renderCompareString()}
+        ${this.#data.warnings?.map(warning => html`
+          <div class="warning">${warning}</div>
+        `)}
         ${this.#renderEnvironmentRecommendations()}
         <slot name="extra-info"></slot>
       </div>

front_end/panels/timeline/components/liveMetricsView.css

@@ -279,7 +279,7 @@
 
 .field-data-warning {
   margin-top: 4px;
-  color: var(--color-error-text);
+  color: var(--sys-color-error);
 }
 
 .collection-period-range {

front_end/panels/timeline/components/metricCard.css

@@ -56,6 +56,11 @@
   font-weight: var(--ref-typeface-weight-medium);
 }
 
+.warning {
+  margin-top: 4px;
+  color: var(--sys-color-error);
+}
+
 .good-bg {
   background-color: var(--app-color-performance-good);
 }