core(console-messages): use source-location (#11899)

GoogleChrome · Mar 23, 2021 · b77b890 · b77b890
1 parent 49d6a55
commit b77b890
Show file tree

Hide file tree

Showing 13 changed files with 258 additions and 186 deletions.
diff --git a/lighthouse-core/audits/audit.js b/lighthouse-core/audits/audit.js
@@ -232,6 +232,22 @@ class Audit {
     };
   }
 
+  /**
+   * @param {LH.Artifacts.ConsoleMessage} entry
+   * @return {LH.Audit.Details.SourceLocationValue | undefined}
+   */
+  static makeSourceLocationFromConsoleMessage(entry) {
+    if (!entry.url) return;
+
+    return {
+      type: 'source-location',
+      url: entry.url,
+      urlProvider: 'network',
+      line: entry.lineNumber || 0,
+      column: entry.columnNumber || 0,
+    };
+  }
+
   /**
    * @param {number|null} score
    * @param {LH.Audit.ScoreDisplayMode} scoreDisplayMode

diff --git a/lighthouse-core/audits/deprecations.js b/lighthouse-core/audits/deprecations.js
@@ -57,33 +57,16 @@ class Deprecations extends Audit {
     const entries = artifacts.ConsoleMessages;
 
     const deprecations = entries.filter(log => log.source === 'deprecation').map(log => {
-      // HTML deprecations will have no url and no way to attribute to a specific line.
-      /** @type {LH.Audit.Details.SourceLocationValue=} */
-      let source;
-      if (log.url) {
-        // JS deprecations will have a stack trace.
-        // CSS deprecations only expose a line number.
-        const topCallFrame = log.stackTrace && log.stackTrace.callFrames[0];
-        const line = log.lineNumber || 0;
-        const column = topCallFrame ? topCallFrame.columnNumber : 0;
-        source = {
-          type: 'source-location',
-          url: log.url,
-          urlProvider: 'network',
-          line,
-          column,
-        };
-      }
       return {
         value: log.text,
-        source,
+        source: Audit.makeSourceLocationFromConsoleMessage(log),
       };
     });
 
     /** @type {LH.Audit.Details.Table['headings']} */
     const headings = [
       {key: 'value', itemType: 'text', text: str_(UIStrings.columnDeprecate)},
-      {key: 'source', itemType: 'source-location', text: str_(i18n.UIStrings.columnURL)},
+      {key: 'source', itemType: 'source-location', text: str_(i18n.UIStrings.columnSource)},
     ];
     const details = Audit.makeTableDetails(headings, deprecations);
 

diff --git a/lighthouse-core/audits/dobetterweb/geolocation-on-start.js b/lighthouse-core/audits/dobetterweb/geolocation-on-start.js
@@ -51,8 +51,7 @@ class GeolocationOnStart extends ViolationAudit {
 
     /** @type {LH.Audit.Details.Table['headings']} */
     const headings = [
-      {key: 'url', itemType: 'url', text: str_(i18n.UIStrings.columnURL)},
-      {key: 'label', itemType: 'text', text: str_(i18n.UIStrings.columnLocation)},
+      {key: 'source', itemType: 'source-location', text: str_(i18n.UIStrings.columnSource)},
     ];
     // TODO(bckenny): there should actually be a ts error here. results[0].stackTrace
     // should violate the results type. Shouldn't be removed from details items regardless.

diff --git a/lighthouse-core/audits/dobetterweb/no-document-write.js b/lighthouse-core/audits/dobetterweb/no-document-write.js
@@ -67,8 +67,7 @@ class NoDocWriteAudit extends ViolationAudit {
 
     /** @type {LH.Audit.Details.Table['headings']} */
     const headings = [
-      {key: 'url', itemType: 'url', text: str_(i18n.UIStrings.columnURL)},
-      {key: 'label', itemType: 'text', text: str_(i18n.UIStrings.columnLocation)},
+      {key: 'source', itemType: 'source-location', text: str_(i18n.UIStrings.columnSource)},
     ];
     // TODO(bckenny): see TODO in geolocation-on-start
     const details = ViolationAudit.makeTableDetails(headings, results);

diff --git a/lighthouse-core/audits/dobetterweb/notification-on-start.js b/lighthouse-core/audits/dobetterweb/notification-on-start.js
@@ -50,8 +50,7 @@ class NotificationOnStart extends ViolationAudit {
 
     /** @type {LH.Audit.Details.Table['headings']} */
     const headings = [
-      {key: 'url', itemType: 'url', text: str_(i18n.UIStrings.columnURL)},
-      {key: 'label', itemType: 'text', text: str_(i18n.UIStrings.columnLocation)},
+      {key: 'source', itemType: 'source-location', text: str_(i18n.UIStrings.columnSource)},
     ];
     // TODO(bckenny): see TODO in geolocation-on-start
     const details = ViolationAudit.makeTableDetails(headings, results);

diff --git a/lighthouse-core/audits/dobetterweb/uses-passive-event-listeners.js b/lighthouse-core/audits/dobetterweb/uses-passive-event-listeners.js
@@ -50,8 +50,7 @@ class PassiveEventsAudit extends ViolationAudit {
 
     /** @type {LH.Audit.Details.Table['headings']} */
     const headings = [
-      {key: 'url', itemType: 'url', text: str_(i18n.UIStrings.columnURL)},
-      {key: 'label', itemType: 'text', text: str_(i18n.UIStrings.columnLocation)},
+      {key: 'source', itemType: 'source-location', text: str_(i18n.UIStrings.columnSource)},
     ];
     // TODO(bckenny): see TODO in geolocation-on-start
     const details = ViolationAudit.makeTableDetails(headings, results);

diff --git a/lighthouse-core/audits/errors-in-console.js b/lighthouse-core/audits/errors-in-console.js
@@ -81,14 +81,16 @@ class ErrorLogs extends Audit {
     /** @type {AuditOptions} */
     const auditOptions = context.options;
 
-    /** @type {Array<{source: string, description: string|undefined, url: string|undefined}>} */
+    /** @type {Array<{source: string, description: string|undefined, sourceLocation: LH.Audit.Details.SourceLocationValue|undefined}>} */
     const consoleRows = artifacts.ConsoleMessages
       .filter(item => item.level === 'error')
       .map(item => {
         return {
           source: item.source,
           description: item.text,
+          // TODO: remove for v8 (url is covered in sourceLocation)
           url: item.url,
+          sourceLocation: Audit.makeSourceLocationFromConsoleMessage(item),
         };
       });
 
@@ -97,7 +99,7 @@ class ErrorLogs extends Audit {
 
     /** @type {LH.Audit.Details.Table['headings']} */
     const headings = [
-      {key: 'url', itemType: 'url', text: str_(i18n.UIStrings.columnURL)},
+      {key: 'sourceLocation', itemType: 'source-location', text: str_(i18n.UIStrings.columnSource)},
       {key: 'description', itemType: 'code', text: str_(i18n.UIStrings.columnDescription)},
     ];
 

diff --git a/lighthouse-core/audits/no-unload-listeners.js b/lighthouse-core/audits/no-unload-listeners.js
@@ -47,7 +47,7 @@ class NoUnloadListeners extends Audit {
 
     /** @type {LH.Audit.Details.Table['headings']} */
     const headings = [
-      {key: 'source', itemType: 'source-location', text: str_(i18n.UIStrings.columnURL)},
+      {key: 'source', itemType: 'source-location', text: str_(i18n.UIStrings.columnSource)},
     ];
 
     // Look up scriptId to script URL via the JsUsage artifact.

diff --git a/lighthouse-core/audits/violation-audit.js b/lighthouse-core/audits/violation-audit.js
@@ -11,21 +11,32 @@ class ViolationAudit extends Audit {
   /**
    * @param {LH.Artifacts} artifacts
    * @param {RegExp} pattern
-   * @return {Array<{label: string, url?: string}>}
+   * @return {Array<{source: LH.Audit.Details.SourceLocationValue}>}
    */
   static getViolationResults(artifacts, pattern) {
+    /**
+     * @template T
+     * @param {T} value
+     * @return {value is Exclude<T, undefined>}
+     */
+    function filterUndefined(value) {
+      return value !== undefined;
+    }
+
     const seen = new Set();
     return artifacts.ConsoleMessages
         .filter(entry => entry.url && entry.source === 'violation' && pattern.test(entry.text))
-        .map(entry => ({label: `line: ${entry.lineNumber}`, url: entry.url}))
-        .filter(entry => {
-          // Filter out duplicate entries by URL/label since they are not differentiable to the user
+        .map(Audit.makeSourceLocationFromConsoleMessage)
+        .filter(filterUndefined)
+        .filter(source => {
+          // Filter out duplicate entries since they are not differentiable to the user
           // @see https://github.com/GoogleChrome/lighthouse/issues/5218
-          const key = `${entry.url}!${entry.label}`;
+          const key = `${source.url}!${source.line}!${source.column}`;
           if (seen.has(key)) return false;
           seen.add(key);
           return true;
-        });
+        })
+        .map(source => ({source}));
   }
 }
 

diff --git a/lighthouse-core/gather/gatherers/console-messages.js b/lighthouse-core/gather/gatherers/console-messages.js
@@ -60,13 +60,15 @@ class ConsoleMessages extends Gatherer {
       // Only gather warnings and errors for brevity.
       return;
     }
+
     /** @type {LH.Crdp.Runtime.RemoteObject[]} */
     const args = event.args || [];
     const text = args.map(remoteObjectToString).join(' ');
     if (!text && !event.stackTrace) {
       // No useful information from Chrome. Skip.
       return;
     }
+
     const {url, lineNumber, columnNumber} =
       event.stackTrace && event.stackTrace.callFrames[0] || {};
     /** @type {LH.Artifacts.ConsoleMessage} */
@@ -116,6 +118,11 @@ class ConsoleMessages extends Gatherer {
    */
   onLogEntry(event) {
     const {source, level, text, stackTrace, timestamp, url, lineNumber} = event.entry;
+
+    // JS events have a stack trace, which we use to get the column.
+    // CSS/HTML events only expose a line number.
+    const {columnNumber} = event.entry.stackTrace && event.entry.stackTrace.callFrames[0] || {};
+
     this._logEntries.push({
       eventType: 'protocolLog',
       source,
@@ -125,6 +132,7 @@ class ConsoleMessages extends Gatherer {
       timestamp,
       url,
       lineNumber,
+      columnNumber,
     });
   }