[CrUX] Only set current page result on auto refresh

CrUXManager.ts will keep track of the current page CrUX data. However,
it should not override the current page data when the field data setup
checks different pages for valid CrUX data.

Fixed: 391990608
Change-Id: I1965e69c94879775bc455d363cb101c1dcf27d6a
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6198694
Auto-Submit: Adam Raine <[email protected]>
Reviewed-by: Connor Clark <[email protected]>
Commit-Queue: Adam Raine <[email protected]>

Author: Adam Raine

front_end/models/crux-manager/CrUXManager.test.ts

@@ -348,7 +348,7 @@ describeWithMockConnection('CrUXManager', () => {
         isPrimaryFrame: () => false,
       } as SDK.ResourceTreeModel.ResourceTreeFrame);
 
-      const result = await cruxManager.getFieldDataForCurrentPage();
+      const result = await cruxManager.getFieldDataForCurrentPageForTesting();
 
       assert.deepEqual(result.warnings, []);
       assert.strictEqual(getFieldDataMock.callCount, 1);
@@ -360,7 +360,7 @@ describeWithMockConnection('CrUXManager', () => {
       cruxManager.getConfigSetting().set(
           {enabled: false, override: 'https://example.com/override', overrideEnabled: true});
 
-      const result = await cruxManager.getFieldDataForCurrentPage();
+      const result = await cruxManager.getFieldDataForCurrentPageForTesting();
 
       assert.deepEqual(result.warnings, ['Field data is configured for a different URL than the current page.']);
       assert.strictEqual(getFieldDataMock.callCount, 1);
@@ -377,7 +377,7 @@ describeWithMockConnection('CrUXManager', () => {
         }],
       });
 
-      const result = await cruxManager.getFieldDataForCurrentPage();
+      const result = await cruxManager.getFieldDataForCurrentPageForTesting();
 
       assert.deepEqual(result.warnings, ['Field data is configured for a different URL than the current page.']);
       assert.strictEqual(getFieldDataMock.callCount, 1);
@@ -396,7 +396,7 @@ describeWithMockConnection('CrUXManager', () => {
         }],
       });
 
-      const result = await cruxManager.getFieldDataForCurrentPage();
+      const result = await cruxManager.getFieldDataForCurrentPageForTesting();
 
       assert.deepEqual(result.warnings, ['Field data is configured for a different URL than the current page.']);
       assert.strictEqual(getFieldDataMock.callCount, 1);
@@ -406,7 +406,7 @@ describeWithMockConnection('CrUXManager', () => {
     it('should use inspected URL if main document is unavailable', async () => {
       target.setInspectedURL(urlString`https://example.com/inspected`);
 
-      const result = await cruxManager.getFieldDataForCurrentPage();
+      const result = await cruxManager.getFieldDataForCurrentPageForTesting();
 
       assert.deepEqual(result.warnings, []);
       assert.strictEqual(getFieldDataMock.callCount, 1);
@@ -416,7 +416,7 @@ describeWithMockConnection('CrUXManager', () => {
     it('should wait for inspected URL if main document and inspected URL are unavailable', async () => {
       target.setInspectedURL(Platform.DevToolsPath.EmptyUrlString);
 
-      const finishPromise = cruxManager.getFieldDataForCurrentPage();
+      const finishPromise = cruxManager.getFieldDataForCurrentPageForTesting();
 
       await triggerMicroTaskQueue();
 
@@ -430,7 +430,8 @@ describeWithMockConnection('CrUXManager', () => {
     });
 
     it('getSelectedFieldMetricData - should take from selected page scope', async () => {
-      await cruxManager.getFieldDataForCurrentPage();
+      cruxManager.getConfigSetting().set({enabled: true});
+      await cruxManager.refresh();
 
       let data: CrUXManager.MetricResponse|undefined;
 
@@ -448,7 +449,8 @@ describeWithMockConnection('CrUXManager', () => {
     });
 
     it('should take from selected device scope', async () => {
-      await cruxManager.getFieldDataForCurrentPage();
+      cruxManager.getConfigSetting().set({enabled: true});
+      await cruxManager.refresh();
       cruxManager.fieldPageScope = 'url';
 
       let data: CrUXManager.MetricResponse|undefined;
@@ -470,7 +472,8 @@ describeWithMockConnection('CrUXManager', () => {
       cruxManager.fieldDeviceOption = 'AUTO';
       assert.strictEqual(cruxManager.getSelectedDeviceScope(), 'ALL');
 
-      await cruxManager.getFieldDataForCurrentPage();
+      cruxManager.getConfigSetting().set({enabled: true});
+      await cruxManager.refresh();
       assert.strictEqual(cruxManager.getSelectedDeviceScope(), 'DESKTOP');
 
       for (const device of EmulationModel.EmulatedDevices.EmulatedDevicesList.instance().standard()) {
@@ -493,7 +496,7 @@ describeWithMockConnection('CrUXManager', () => {
       cruxManager.addEventListener(CrUXManager.Events.FIELD_DATA_CHANGED, event => {
         eventBodies.push(event.data);
       });
-      getFieldDataMock = sinon.stub(cruxManager, 'getFieldDataForCurrentPage');
+      getFieldDataMock = sinon.stub(cruxManager, 'getFieldDataForPage');
       getFieldDataMock.resolves({
         'origin-ALL': null,
         'origin-DESKTOP': null,
@@ -503,6 +506,7 @@ describeWithMockConnection('CrUXManager', () => {
         'url-DESKTOP': null,
         'url-PHONE': null,
         'url-TABLET': null,
+        warnings: [],
       });
     });
 

front_end/models/crux-manager/CrUXManager.ts

@@ -202,7 +202,6 @@ export class CrUXManager extends Common.ObjectWrapper.ObjectWrapper<EventTypes>
       }
 
       await Promise.all(promises);
-      this.#pageResult = pageResult;
     } catch (err) {
       console.error(err);
     } finally {
@@ -228,6 +227,10 @@ export class CrUXManager extends Common.ObjectWrapper.ObjectWrapper<EventTypes>
     }
   }
 
+  async getFieldDataForCurrentPageForTesting(): Promise<PageResult> {
+    return this.#getFieldDataForCurrentPage();
+  }
+
   /**
    * In general, this function should use the main document URL
    * (i.e. the URL after all redirects but before SPA navigations)
@@ -237,13 +240,12 @@ export class CrUXManager extends Common.ObjectWrapper.ObjectWrapper<EventTypes>
    * back to the currently inspected URL (i.e. what is displayed in the omnibox) if
    * the main document URL cannot be found.
    */
-  async getFieldDataForCurrentPage(): Promise<PageResult> {
+  async #getFieldDataForCurrentPage(): Promise<PageResult> {
     const currentUrl = this.#mainDocumentUrl || await this.#getInspectedURL();
     const urlForCrux = this.#configSetting.get().overrideEnabled ? this.#configSetting.get().override || '' :
                                                                    this.#getMappedUrl(currentUrl);
 
     const result = await this.getFieldDataForPage(urlForCrux);
-    this.#pageResult = result;
     if (currentUrl !== urlForCrux) {
       result.warnings.push(i18nString(UIStrings.fieldOverrideWarning));
     }
@@ -289,7 +291,7 @@ export class CrUXManager extends Common.ObjectWrapper.ObjectWrapper<EventTypes>
       return;
     }
 
-    this.#pageResult = await this.getFieldDataForCurrentPage();
+    this.#pageResult = await this.#getFieldDataForCurrentPage();
     this.dispatchEventToListeners(Events.FIELD_DATA_CHANGED, this.#pageResult);
   }
 

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

@@ -717,7 +717,7 @@ describeWithMockConnection('LiveMetricsView', () => {
         warnings: [],
       };
 
-      sinon.stub(CrUXManager.CrUXManager.instance(), 'getFieldDataForCurrentPage').callsFake(async () => mockFieldData);
+      sinon.stub(CrUXManager.CrUXManager.instance(), 'getFieldDataForPage').callsFake(async () => mockFieldData);
       CrUXManager.CrUXManager.instance().getConfigSetting().set({enabled: true, override: ''});
     });