Correctly handle source maps when hydrating

btoa/base64 is not valid for utf-8. Use encodeURIComponent instead.

Bug: 337909145
Change-Id: Ic46a00e0ae8de5c25fd62cf2a8a685e578685f74
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6518811
Commit-Queue: Connor Clark <cjamcl@chromium.org>
Reviewed-by: Paul Irish <paulirish@chromium.org>
Auto-Submit: Connor Clark <cjamcl@chromium.org>

Author: Connor Clark

front_end/core/sdk/EnhancedTracesParser.test.ts

@@ -162,7 +162,7 @@ describe('EnhancedTracesParser', () => {
     url: 'http://localhost:8080/index.html',
     hasSourceURL: false,
     sourceURL: undefined,
-    sourceMapURL: undefined,
+    sourceMapURL: 'http://localhost:8080/source.map.json',
     pid: 8050,
     auxData: {
       frameId: '21D58E83A5C17916277166140F6A464B' as Protocol.Page.FrameId,
@@ -184,7 +184,7 @@ describe('EnhancedTracesParser', () => {
     url: 'http://localhost:8080/index.html',
     hasSourceURL: false,
     sourceURL: undefined,
-    sourceMapURL: undefined,
+    sourceMapURL: 'http://localhost:8080/source.map.json',
     pid: 8050,
     auxData: {
       frameId: '21D58E83A5C17916277166140F6A464B' as Protocol.Page.FrameId,
@@ -206,7 +206,7 @@ describe('EnhancedTracesParser', () => {
     url: 'http://localhost:8080/index.html',
     hasSourceURL: false,
     sourceURL: undefined,
-    sourceMapURL: undefined,
+    sourceMapURL: 'http://localhost:8080/source.map.json',
     pid: 8052,
   };
 
@@ -259,7 +259,7 @@ describe('EnhancedTracesParser', () => {
     assert.lengthOf(scripts, 6);
     for (const script of scripts) {
       if (script.scriptId === '1' && script.isolate === '12345') {
-        assert.deepEqual(script, {...script1, sourceMapURL: undefined});
+        assert.deepEqual(script, script1);
       } else if (script.scriptId === '2' && script.isolate === '12345') {
         assert.deepEqual(script, script2);
       } else if (script.scriptId === '1' && script.isolate === '6789') {
@@ -288,7 +288,7 @@ describe('EnhancedTracesParser', () => {
         assert.lengthOf(scripts, 4);
         for (const script of scripts) {
           if (script.scriptId === '1') {
-            assert.deepEqual(script, {...script1, sourceMapURL: undefined});
+            assert.deepEqual(script, script1);
           } else if (script.scriptId === '2') {
             assert.deepEqual(script, script2);
           } else if (script.scriptId === '3') {

front_end/core/sdk/EnhancedTracesParser.ts

@@ -43,14 +43,15 @@ interface TraceEventScriptRundown extends RehydratingTraceBase {
       isolate: string,
       executionContextId: Protocol.Runtime.ExecutionContextId,
       scriptId: Protocol.Runtime.ScriptId,
-      startLine: number,
-      startColumn: number,
-      endLine: number,
-      endColumn: number,
+      // These don't actually get set in v8.
       url: string,
       hash: string,
       isModule: boolean,
       hasSourceUrl: boolean,
+      startLine?: number,
+      startColumn?: number,
+      endLine?: number,
+      endColumn?: number,
       sourceUrl?: string,
       sourceMapUrl?: string,
     },
@@ -186,10 +187,10 @@ export class EnhancedTracesParser {
             scriptId: data.scriptId,
             isolate: data.isolate,
             executionContextId: data.executionContextId,
-            startLine: data.startLine,
-            startColumn: data.startColumn,
-            endLine: data.endLine,
-            endColumn: data.endColumn,
+            startLine: data.startLine ?? 0,
+            startColumn: data.startColumn ?? 0,
+            endLine: data.endLine ?? 0,
+            endColumn: data.endColumn ?? 0,
             hash: data.hash,
             isModule: data.isModule,
             url: data.url,
@@ -274,28 +275,27 @@ export class EnhancedTracesParser {
       // Encoded as a data url so that the debugger model makes no network request.
       // NOTE: consider passing directly as object and hacking `parsedScriptSource` in DebuggerModel.ts to handle
       // this fake event. Would avoid a lot of wasteful (de)serialization. Maybe add SDK.Script.hydratedSourceMap.
-      script.sourceMapURL = this.getEncodedSourceMapUrl(script);
+      this.resolveSourceMap(script);
     }
 
     return this.groupContextsAndScriptsUnderTarget(this.#targets, this.#executionContexts, this.#scripts);
   }
 
-  private getEncodedSourceMapUrl(script: RehydratingScript): string|undefined {
+  private resolveSourceMap(script: RehydratingScript): void {
     if (script.sourceMapURL?.startsWith('data:')) {
-      return script.sourceMapURL;
+      return;
     }
 
     const sourceMap = this.getSourceMapFromMetadata(script);
     if (!sourceMap) {
       return;
     }
 
-    try {
-      return `data:text/plain;base64,${btoa(JSON.stringify(sourceMap))}`;
-    } catch {
-      // TODO(cjamcl): getting InvalidCharacterError (try loading dupe-js.json.gz).
-      return;
-    }
+    // Note: this encoding + re-parsing overhead cost ~10ms per 1MB of JSON on my
+    // Mac M1 Pro.
+    // See https://crrev.com/c/6490409/comments/f294c12a_69781e24
+    const payload = encodeURIComponent(JSON.stringify(sourceMap));
+    script.sourceMapURL = `data:application/json;charset=utf-8,${payload}`;
   }
 
   private getSourceMapFromMetadata(script: RehydratingScript): SourceMapV3|undefined {

front_end/panels/recorder/RecorderController.ts

@@ -597,6 +597,7 @@ export class RecorderController extends LitElement {
       await UI.InspectorView.InspectorView.instance().showPanel(event.data?.targetPanel as string);
       switch (event.data?.targetPanel) {
         case Components.RecordingView.TargetPanel.PERFORMANCE_PANEL:
+          // Note: this is not passing any metadata to the Performance panel.
           Timeline.TimelinePanel.TimelinePanel.instance().loadFromEvents(events as Trace.Types.Events.Event[]);
           break;
       }

front_end/panels/timeline/TimelineLoader.ts

@@ -78,7 +78,15 @@ export class TimelineLoader implements Common.StringOutputStream.OutputStream {
   static loadFromEvents(events: Trace.Types.Events.Event[], client: Client): TimelineLoader {
     const loader = new TimelineLoader(client);
     window.setTimeout(async () => {
-      void loader.addEvents(events);
+      void loader.addEvents(events, null);
+    });
+    return loader;
+  }
+
+  static loadFromTraceFile(traceFile: Trace.Types.File.TraceFile, client: Client): TimelineLoader {
+    const loader = new TimelineLoader(client);
+    window.setTimeout(async () => {
+      void loader.addEvents(traceFile.traceEvents, traceFile.metadata);
     });
     return loader;
   }
@@ -92,7 +100,7 @@ export class TimelineLoader implements Common.StringOutputStream.OutputStream {
           profile, Trace.Types.Events.ThreadID(1));
 
       window.setTimeout(async () => {
-        void loader.addEvents(contents.traceEvents);
+        void loader.addEvents(contents.traceEvents, null);
       });
     } catch (e) {
       console.error(e.stack);
@@ -166,7 +174,9 @@ export class TimelineLoader implements Common.StringOutputStream.OutputStream {
     }
   }
 
-  async addEvents(events: readonly Trace.Types.Events.Event[]): Promise<void> {
+  async addEvents(events: readonly Trace.Types.Events.Event[], metadata: Trace.Types.File.MetaData|null):
+      Promise<void> {
+    this.#metadata = metadata;
     this.client?.loadingStarted();
     /**
      * See the `eventsPerChunk` comment in `models/trace/types/Configuration.ts`.

front_end/panels/timeline/TimelinePanel.ts

@@ -793,6 +793,14 @@ export class TimelinePanel extends UI.Panel.Panel implements Client, TimelineMod
     this.loader = TimelineLoader.loadFromEvents(events, this);
   }
 
+  loadFromTraceFile(traceFile: Trace.Types.File.TraceFile): void {
+    if (this.state !== State.IDLE) {
+      return;
+    }
+    this.prepareToLoadTimeline();
+    this.loader = TimelineLoader.loadFromTraceFile(traceFile, this);
+  }
+
   getFlameChart(): TimelineFlameChartView {
     return this.flameChart;
   }
@@ -2979,10 +2987,8 @@ export class LoadTimelineHandler implements Common.QueryParamHandler.QueryParamH
 
 export class TraceRevealer implements Common.Revealer.Revealer<SDK.TraceObject.TraceObject> {
   async reveal(trace: SDK.TraceObject.TraceObject): Promise<void> {
-    // TODO(cjamcl): This needs to be given a TraceFile, so that metadata is loaded too. Important
-    // for source maps (which otherwise won't be saved on export).
     await UI.ViewManager.ViewManager.instance().showView('timeline');
-    TimelinePanel.instance().loadFromEvents(trace.traceEvents);
+    TimelinePanel.instance().loadFromTraceFile(trace);
   }
 }