Prepare Scripts handler for elided data source map urls

The `ScriptCatchup` v8 events have a sourceMapUrl field, which today can
contain data urls. These can be very large, especially for developer
builds. This can fill up the buffer quickly.

I'll be changing this trace event to elide sourceMapUrl if it is a data
url. To prepare for that, I created a test that will mimic the expected
trace format. Instead of the map coming from a trace event, the
Performance panel will grab it from the Script model and place it in the
metadata.

This also may be the source of some trace recordings missing data. For
example, this page[1] frequently drops trace events, perhaps because a
single trace event is too large. This page[2] has a slightly smaller
source map data url, and has no issue with recording.

[1] https://dupe-modules-lh-inline-data.surge.sh/
[2] https://dupe-modules-lh-inline-data.surge.sh/smaller

Bug: 409086863
Change-Id: If4d70d541d0e193543249aae8d3f4424868a5024
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6521671
Reviewed-by: Paul Irish <paulirish@chromium.org>
Commit-Queue: Connor Clark <cjamcl@chromium.org>

Author: Connor Clark

front_end/models/trace/handlers/ScriptsHandler.ts

@@ -27,8 +27,12 @@ export interface Script {
   url?: string;
   sourceUrl?: string;
   content?: string;
-  /** Note: this is the literal text given as the sourceMappingURL value. It has not been resolved relative to the script url. */
+  /** Note: this is the literal text given as the sourceMappingURL value. It has not been resolved relative to the script url.
+   * Since M138, data urls are never set here.
+   */
   sourceMapUrl?: string;
+  /** If true, the source map url was a data URL, so it got removed from the trace event. */
+  sourceMapUrlElided?: boolean;
   sourceMap?: SDK.SourceMap.SourceMap;
   request?: Types.Events.SyntheticNetworkRequest;
   /** Lazily generated - use getScriptGeneratedSizes to access. */
@@ -67,13 +71,21 @@ export function handleEvent(event: Types.Events.Event): void {
   }
 
   if (Types.Events.isV8SourceRundownEvent(event)) {
-    const {isolate, scriptId, url, sourceUrl, sourceMapUrl} = event.args.data;
+    const {isolate, scriptId, url, sourceUrl, sourceMapUrl, sourceMapUrlElided} = event.args.data;
     const script = getOrMakeScript(isolate, scriptId);
     script.url = url;
     if (sourceUrl) {
       script.sourceUrl = sourceUrl;
     }
-    if (sourceMapUrl) {
+
+    // Older traces may have data source map urls. Those can be very large, so a change
+    // was made to elide them from the trace.
+    // If elided, a fresh trace will fetch the source map from the Script model
+    // (see TimelinePanel getExistingSourceMap). If not fresh, the source map is resolved
+    // instead in this handler via `findCachedRawSourceMap`.
+    if (sourceMapUrlElided) {
+      script.sourceMapUrlElided = true;
+    } else if (sourceMapUrl) {
       script.sourceMapUrl = sourceMapUrl;
     }
     return;
@@ -199,17 +211,37 @@ export function getScriptGeneratedSizes(script: Script): GeneratedFileSizes|null
   return script.sizes ?? null;
 }
 
-function findCachedRawSourceMap(
-    sourceMapUrl: string, options: Types.Configuration.ParseOptions): SDK.SourceMap.SourceMapV3|undefined {
-  if (!sourceMapUrl) {
+function findCachedRawSourceMap(script: Script, options: Types.Configuration.ParseOptions): SDK.SourceMap.SourceMapV3|
+    undefined {
+  if (options.isFreshRecording || !options.metadata?.sourceMaps) {
+    // Exit if this is not a loaded trace w/ source maps in the metadata.
+    return;
+  }
+
+  // For elided data url source maps, search the metadata source maps by script url.
+  if (script.sourceMapUrlElided) {
+    if (!script.url) {
+      return;
+    }
+
+    const cachedSourceMap = options.metadata.sourceMaps.find(m => m.url === script.url);
+    if (cachedSourceMap) {
+      return cachedSourceMap.sourceMap;
+    }
+
     return;
   }
 
-  // If loading from disk, check the metadata for source maps.
-  // The metadata doesn't store data url source maps.
-  const isDataUrl = sourceMapUrl.startsWith('data:');
-  if (!options.isFreshRecording && options.metadata?.sourceMaps && !isDataUrl) {
-    const cachedSourceMap = options.metadata.sourceMaps.find(m => m.sourceMapUrl === sourceMapUrl);
+  if (!script.sourceMapUrl) {
+    return;
+  }
+
+  // Otherwise, search by source map url.
+  // Note: early enhanced traces may have this field set for data urls. Ignore those,
+  // as they were never stored in metadata sourcemap.
+  const isDataUrl = script.sourceMapUrl.startsWith('data:');
+  if (!isDataUrl) {
+    const cachedSourceMap = options.metadata.sourceMaps.find(m => m.sourceMapUrl === script.sourceMapUrl);
     if (cachedSourceMap) {
       return cachedSourceMap.sourceMap;
     }
@@ -243,7 +275,7 @@ export async function finalize(options: Types.Configuration.ParseOptions): Promi
     // No frame or url means the script came from somewhere we don't care about.
     // Note: scripts from inline <SCRIPT> elements use the url of the HTML document,
     // so aren't ignored.
-    if (!script.frame || !script.url || !script.sourceMapUrl) {
+    if (!script.frame || !script.url || (!script.sourceMapUrl && !script.sourceMapUrlElided)) {
       continue;
     }
 
@@ -259,22 +291,26 @@ export async function finalize(options: Types.Configuration.ParseOptions): Promi
       sourceUrl = Common.ParsedURL.ParsedURL.completeURL(frameUrl, script.sourceUrl) ?? script.sourceUrl;
     }
 
-    // Resolve the source map url. The value given by v8 may be relative, so resolve it here.
-    // This process should match the one in `SourceMapManager.attachSourceMap`.
-    const sourceMapUrl =
-        Common.ParsedURL.ParsedURL.completeURL(sourceUrl as Platform.DevToolsPath.UrlString, script.sourceMapUrl);
-    if (!sourceMapUrl) {
-      continue;
-    }
+    let sourceMapUrl;
+    if (script.sourceMapUrl) {
+      // Resolve the source map url. The value given by v8 may be relative, so resolve it here.
+      // This process should match the one in `SourceMapManager.attachSourceMap`.
+      sourceMapUrl =
+          Common.ParsedURL.ParsedURL.completeURL(sourceUrl as Platform.DevToolsPath.UrlString, script.sourceMapUrl);
+      if (!sourceMapUrl) {
+        continue;
+      }
 
-    script.sourceMapUrl = sourceMapUrl;
+      script.sourceMapUrl = sourceMapUrl;
+    }
 
     const params: Types.Configuration.ResolveSourceMapParams = {
       scriptId: script.scriptId,
-      scriptUrl: sourceUrl as Platform.DevToolsPath.UrlString,
-      sourceMapUrl: sourceMapUrl as Platform.DevToolsPath.UrlString,
+      scriptUrl: script.url as Platform.DevToolsPath.UrlString,
+      sourceUrl: sourceUrl as Platform.DevToolsPath.UrlString,
+      sourceMapUrl: sourceMapUrl ?? '' as Platform.DevToolsPath.UrlString,
       frame: script.frame as Protocol.Page.FrameId,
-      cachedRawSourceMap: findCachedRawSourceMap(sourceMapUrl, options),
+      cachedRawSourceMap: findCachedRawSourceMap(script, options),
     };
     const promise = options.resolveSourceMap(params).then(sourceMap => {
       if (sourceMap) {

front_end/models/trace/insights/DuplicatedJavaScript.test.ts

@@ -4,7 +4,8 @@
 
 import {describeWithEnvironment} from '../../../testing/EnvironmentHelpers.js';
 import {getFirstOrError, getInsightOrError, processTrace} from '../../../testing/InsightHelpers.js';
-import type * as Trace from '../trace.js';
+import {fetchFixture, TraceLoader} from '../../../testing/TraceLoader.js';
+import * as Trace from '../trace.js';
 
 describeWithEnvironment('DuplicatedJavaScript', function() {
   it('works (external source maps)', async () => {
@@ -47,12 +48,8 @@ describeWithEnvironment('DuplicatedJavaScript', function() {
     assert.deepEqual(insight.metricSavings, {FCP: 100, LCP: 100} as Trace.Insights.Types.MetricSavings);
   });
 
-  it('works (inline source maps)', async () => {
-    const {data, insights} = await processTrace(this, 'dupe-js-inline-maps.json.gz');
-    assert.strictEqual(insights.size, 1);
-    const insight = getInsightOrError(
-        'DuplicatedJavaScript', insights, getFirstOrError(data.Meta.navigationsByNavigationId.values()));
-
+  function assertInlineMapsTestCase(
+      insight: Trace.Insights.Models.DuplicatedJavaScript.DuplicatedJavaScriptInsightModel): void {
     const duplication = insight.duplicationGroupedByNodeModules;
     const results = Object.fromEntries(
         [...duplication.entries()].filter(v => v[1].estimatedDuplicateBytes > 1000 * 25).map(([key, data]) => {
@@ -85,5 +82,48 @@ describeWithEnvironment('DuplicatedJavaScript', function() {
     });
 
     assert.deepEqual(insight.metricSavings, {FCP: 50, LCP: 50} as Trace.Insights.Types.MetricSavings);
+  }
+
+  it('works (inline source maps in trace events)', async function() {
+    const {data, insights} = await processTrace(this, 'dupe-js-inline-maps.json.gz');
+    assert.strictEqual(insights.size, 1);
+    const insight = getInsightOrError(
+        'DuplicatedJavaScript', insights, getFirstOrError(data.Meta.navigationsByNavigationId.values()));
+
+    assertInlineMapsTestCase(insight);
+  });
+
+  it('works (inline source maps in metadata)', async function() {
+    // Load this trace in a way that mutating it is safe.
+    const traceText = await fetchFixture(
+        new URL('../../../panels/timeline/fixtures/traces/dupe-js-inline-maps.json.gz', import.meta.url));
+    const fileContents = JSON.parse(traceText) as Trace.Types.File.TraceFile;
+
+    // Remove the source map data urls from the trace, and move to metadata.
+    // This reflects how Chromium will elide data source map urls.
+    // The original trace here was recorded at a time where sourceMapUrl could be a
+    // large data url.
+    for (const event of fileContents.traceEvents) {
+      if (Trace.Types.Events.isV8SourceRundownEvent(event)) {
+        const {sourceMapUrl, url} = event.args.data;
+        if (sourceMapUrl?.startsWith('data:') && url) {
+          const sourceMap = await (await fetch(sourceMapUrl)).json();
+          fileContents.metadata.sourceMaps?.push({url, sourceMap});
+          event.args.data.sourceMapUrl = undefined;
+          event.args.data.sourceMapUrlElided = true;
+        }
+      }
+    }
+
+    const parsedTraceData = await TraceLoader.executeTraceEngineOnFileContents(fileContents);
+    const {parsedTrace: data, insights} = parsedTraceData;
+    if (!insights) {
+      throw new Error('invalid insights');
+    }
+
+    assert.strictEqual(insights.size, 1);
+    const insight = getInsightOrError(
+        'DuplicatedJavaScript', insights, getFirstOrError(data.Meta.navigationsByNavigationId.values()));
+    assertInlineMapsTestCase(insight);
   });
 });

front_end/models/trace/types/Configuration.ts

@@ -80,6 +80,8 @@ export interface ParseOptions {
 export interface ResolveSourceMapParams {
   scriptId: string;
   scriptUrl: Platform.DevToolsPath.UrlString;
+  /** The url as resolved by any sourceUrl comment. */
+  sourceUrl: Platform.DevToolsPath.UrlString;
   sourceMapUrl: Platform.DevToolsPath.UrlString;
   frame: Protocol.Page.FrameId;
   /** Set only if the raw source map was found on the provided metadata. Never set for source maps from data urls. */

front_end/models/trace/types/File.ts

@@ -198,7 +198,8 @@ export interface MetaData {
 
 interface MetadataSourceMap {
   url: string;
-  sourceMapUrl: string;
+  /** If not defined, then this was a data url. */
+  sourceMapUrl?: string;
   sourceMap: SDK.SourceMap.SourceMapV3;
 }
 

front_end/models/trace/types/TraceEvents.ts

@@ -3167,6 +3167,7 @@ export interface V8SourceRundownEvent extends Event {
       url?: string,
       sourceUrl?: string,
       sourceMapUrl?: string,
+      sourceMapUrlElided?: boolean,
     },
   };
 }

front_end/panels/timeline/TimelinePanel.ts

@@ -2422,6 +2422,18 @@ export class TimelinePanel extends UI.Panel.Panel implements Client, TimelineMod
   async #retainSourceMapsForEnhancedTrace(
       parsedTrace: Trace.Handlers.Types.ParsedTrace, metadata: Trace.Types.File.MetaData): Promise<void> {
     const handleScript = async(script: Trace.Handlers.ModelHandlers.Scripts.Script): Promise<void> => {
+      if (script.sourceMapUrlElided) {
+        if (metadata.sourceMaps?.find(m => m.url === script.url)) {
+          return;
+        }
+
+        const rawSourceMap = script.sourceMap?.json();
+        if (rawSourceMap && script.url) {
+          metadata.sourceMaps?.push({url: script.url, sourceMap: rawSourceMap});
+        }
+        return;
+      }
+
       if (!script.sourceMapUrl || script.sourceMapUrl.startsWith('data:')) {
         return;
       }
@@ -2435,7 +2447,7 @@ export class TimelinePanel extends UI.Panel.Panel implements Client, TimelineMod
       let rawSourceMap = script.sourceMap?.json();
 
       // If the raw map is not present for some reason, fetch it again.
-      if (!rawSourceMap) {
+      if (!rawSourceMap && !script.sourceMapUrlElided) {
         const initiator = {
           target: null,
           frameId: script.frame as Protocol.Page.FrameId,
@@ -2491,10 +2503,11 @@ export class TimelinePanel extends UI.Panel.Panel implements Client, TimelineMod
     }
 
     return async function resolveSourceMap(params: Trace.Types.Configuration.ResolveSourceMapParams) {
-      const {scriptId, scriptUrl, sourceMapUrl, frame, cachedRawSourceMap} = params;
+      const {scriptId, scriptUrl, sourceUrl, sourceMapUrl, frame, cachedRawSourceMap} = params;
 
       if (cachedRawSourceMap) {
-        return new SDK.SourceMap.SourceMap(scriptUrl, sourceMapUrl, cachedRawSourceMap);
+        return new SDK.SourceMap.SourceMap(
+            sourceUrl, sourceMapUrl ?? '' as Platform.DevToolsPath.UrlString, cachedRawSourceMap);
       }
 
       // For still-active frames, the source map is likely already fetched or at least in-flight.
@@ -2505,13 +2518,17 @@ export class TimelinePanel extends UI.Panel.Panel implements Client, TimelineMod
         }
       }
 
+      if (!sourceMapUrl) {
+        return null;
+      }
+
       // If loading from disk, check the metadata for source maps.
       // The metadata doesn't store data url source maps.
       const isDataUrl = sourceMapUrl.startsWith('data:');
       if (!isFreshRecording && metadata?.sourceMaps && !isDataUrl) {
         const cachedSourceMap = metadata.sourceMaps.find(m => m.sourceMapUrl === sourceMapUrl);
         if (cachedSourceMap) {
-          return new SDK.SourceMap.SourceMap(scriptUrl, sourceMapUrl, cachedSourceMap.sourceMap);
+          return new SDK.SourceMap.SourceMap(sourceUrl, sourceMapUrl, cachedSourceMap.sourceMap);
         }
       }
 
@@ -2521,7 +2538,7 @@ export class TimelinePanel extends UI.Panel.Panel implements Client, TimelineMod
         return null;
       }
 
-      if (!scriptUrl) {
+      if (!sourceUrl) {
         return null;
       }
 
@@ -2534,9 +2551,9 @@ export class TimelinePanel extends UI.Panel.Panel implements Client, TimelineMod
       // non-final navigations during the trace will never have their source maps
       // fetched by the debugger model. That's only ever done here.
 
-      const initiator = {target: null, frameId: frame, initiatorUrl: scriptUrl};
+      const initiator = {target: null, frameId: frame, initiatorUrl: sourceUrl};
       const payload = await SDK.SourceMapManager.tryLoadSourceMap(sourceMapUrl, initiator);
-      return payload ? new SDK.SourceMap.SourceMap(scriptUrl, sourceMapUrl, payload) : null;
+      return payload ? new SDK.SourceMap.SourceMap(sourceUrl, sourceMapUrl, payload) : null;
     };
   }
 

front_end/testing/TraceLoader.ts

@@ -245,15 +245,15 @@ export class TraceLoader {
             metadata,
             isFreshRecording: emulateFreshRecording,
             async resolveSourceMap(params) {
-              const {scriptUrl, sourceMapUrl, cachedRawSourceMap} = params;
+              const {sourceUrl, sourceMapUrl, cachedRawSourceMap} = params;
 
               if (cachedRawSourceMap) {
-                return new SDK.SourceMap.SourceMap(scriptUrl, sourceMapUrl, cachedRawSourceMap);
+                return new SDK.SourceMap.SourceMap(sourceUrl, sourceMapUrl, cachedRawSourceMap);
               }
 
               if (sourceMapUrl.startsWith('data:')) {
                 const rawSourceMap = await (await fetch(sourceMapUrl)).json();
-                return new SDK.SourceMap.SourceMap(scriptUrl, sourceMapUrl, rawSourceMap);
+                return new SDK.SourceMap.SourceMap(sourceUrl, sourceMapUrl, rawSourceMap);
               }
 
               return null;