[RPP] Don't use scriptId as key in ScriptsHandler

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

Author: Connor Clark

front_end/models/trace/extras/ScriptDuplication.test.ts

@@ -25,6 +25,7 @@ async function loadScriptFixture(
   const compiledUrl = Platform.DevToolsPath.urlString`${name}.js`;
   const mapUrl = Platform.DevToolsPath.urlString`${name}.js.map`;
   return {
+    isolate: 'iso',
     scriptId: `1.${name}` as Protocol.Runtime.ScriptId,
     frame: 'abcdef',
     ts: 0 as Trace.Types.Timing.Micro,
@@ -232,19 +233,17 @@ describeWithEnvironment('ScriptDuplication', function() {
 
   describe('computeScriptDuplication', () => {
     it('works (simple, no duplication)', async () => {
-      const scripts = [await loadScriptFixture('foo.min')];
       const scriptsData: Trace.Handlers.ModelHandlers.Scripts.ScriptsData = {
-        scripts: new Map(scripts.map(s => ([s.scriptId, s]))),
+        scripts: [await loadScriptFixture('foo.min')],
       };
 
       const results = Object.fromEntries(Trace.Extras.ScriptDuplication.computeScriptDuplication(scriptsData));
       assert.deepEqual(results, {});
     });
 
     it('works (complex, lots of duplication)', async () => {
-      const scripts = [await loadScriptFixture('coursehero-bundle-1'), await loadScriptFixture('coursehero-bundle-2')];
       const scriptsData: Trace.Handlers.ModelHandlers.Scripts.ScriptsData = {
-        scripts: new Map(scripts.map(s => ([s.scriptId, s]))),
+        scripts: [await loadScriptFixture('coursehero-bundle-1'), await loadScriptFixture('coursehero-bundle-2')],
       };
 
       const results = Object.fromEntries(

front_end/models/trace/extras/ScriptDuplication.ts

@@ -175,7 +175,7 @@ function computeLastGeneratedColumnMap(map: SDK.SourceMap.SourceMap): Map<SDK.So
  */
 export function computeScriptDuplication(scriptsData: Handlers.ModelHandlers.Scripts.ScriptsData): ScriptDuplication {
   const sizesMap = new Map<Handlers.ModelHandlers.Scripts.Script, GeneratedFileSizes>();
-  for (const script of scriptsData.scripts.values()) {
+  for (const script of scriptsData.scripts) {
     if (script.content && script.sourceMap) {
       sizesMap.set(script, computeGeneratedFileSizes(script));
     }

front_end/models/trace/handlers/ScriptsHandler.test.ts

@@ -35,25 +35,36 @@ describe('ScriptsHandler', () => {
 
   it('collects scripts contents', async function() {
     const data = Trace.Handlers.ModelHandlers.Scripts.data();
-    assert.deepEqual([...data.scripts.values()], [
+    assert.deepEqual([...data.scripts], [
       {
+        isolate: '12345',
         scriptId: '1',
-        frame: '3E1717BE677B75D0536E292E00D6A34A',
+        frame: '21D58E83A5C17916277166140F6A464B',
         request: undefined,
         ts: 50442438976,
         url: 'http://localhost:8080/index.html',
-        content: ' text ',
+        content: 'source text 1',
         sourceMapUrl: 'http://localhost:8080/source.map.json',
         sourceMap: {test: 'http://localhost:8080/source.map.json'}
       },
       {
+        isolate: '12345',
         scriptId: '2',
         frame: '21D58E83A5C17916277166140F6A464B',
         request: undefined,
         ts: 50442438976,
         url: 'http://localhost:8080/index.html',
         content: 'source text 2'
       },
+      {
+        isolate: '6789',
+        scriptId: '1',
+        frame: '3E1717BE677B75D0536E292E00D6A34A',
+        content: ' text ',
+        request: undefined,
+        ts: 50442438976,
+        url: 'http://localhost:8080/index.html',
+      }
     ]);
   });
 });

front_end/models/trace/handlers/ScriptsHandler.ts

@@ -15,10 +15,11 @@ import type {HandlerName} from './types.js';
 
 export interface ScriptsData {
   /** Note: this is only populated when the "Enhanced Traces" feature is enabled. */
-  scripts: Map<Protocol.Runtime.ScriptId, Script>;
+  scripts: Script[];
 }
 
 export interface Script {
+  isolate: string;
   scriptId: Protocol.Runtime.ScriptId;
   frame: string;
   ts: Types.Timing.Micro;
@@ -31,7 +32,7 @@ export interface Script {
   request?: Types.Events.SyntheticNetworkRequest;
 }
 
-const scriptById = new Map<Protocol.Runtime.ScriptId, Script>();
+const scriptById = new Map<string, Script>();
 
 export function deps(): HandlerName[] {
   return ['Meta', 'NetworkRequests'];
@@ -42,23 +43,25 @@ export function reset(): void {
 }
 
 export function handleEvent(event: Types.Events.Event): void {
-  const getOrMakeScript = (scriptIdAsNumber: number): Script => {
+  const getOrMakeScript = (isolate: string, scriptIdAsNumber: number): Script => {
     const scriptId = String(scriptIdAsNumber) as Protocol.Runtime.ScriptId;
-    return Platform.MapUtilities.getWithDefault(scriptById, scriptId, () => ({scriptId, frame: '', ts: 0} as Script));
+    const key = `${isolate}.${scriptId}`;
+    return Platform.MapUtilities.getWithDefault(
+        scriptById, key, () => ({isolate, scriptId, frame: '', ts: 0} as Script));
   };
 
   if (Types.Events.isTargetRundownEvent(event) && event.args.data) {
-    const {scriptId, frame} = event.args.data;
-    const script = getOrMakeScript(scriptId);
+    const {isolate, scriptId, frame} = event.args.data;
+    const script = getOrMakeScript(isolate, scriptId);
     script.frame = frame;
     script.ts = event.ts;
 
     return;
   }
 
   if (Types.Events.isV8SourceRundownEvent(event)) {
-    const {scriptId, url, sourceUrl, sourceMapUrl} = event.args.data;
-    const script = getOrMakeScript(scriptId);
+    const {isolate, scriptId, url, sourceUrl, sourceMapUrl} = event.args.data;
+    const script = getOrMakeScript(isolate, scriptId);
     script.url = url;
     if (sourceUrl) {
       script.sourceUrl = sourceUrl;
@@ -70,15 +73,15 @@ export function handleEvent(event: Types.Events.Event): void {
   }
 
   if (Types.Events.isV8SourceRundownSourcesScriptCatchupEvent(event)) {
-    const {scriptId, sourceText} = event.args.data;
-    const script = getOrMakeScript(scriptId);
+    const {isolate, scriptId, sourceText} = event.args.data;
+    const script = getOrMakeScript(isolate, scriptId);
     script.content = sourceText;
     return;
   }
 
   if (Types.Events.isV8SourceRundownSourcesLargeScriptCatchupEvent(event)) {
-    const {scriptId, sourceText} = event.args.data;
-    const script = getOrMakeScript(scriptId);
+    const {isolate, scriptId, sourceText} = event.args.data;
+    const script = getOrMakeScript(isolate, scriptId);
     script.content = (script.content ?? '') + sourceText;
     return;
   }
@@ -161,6 +164,6 @@ export async function finalize(options: Types.Configuration.ParseOptions): Promi
 
 export function data(): ScriptsData {
   return {
-    scripts: scriptById,
+    scripts: [...scriptById.values()],
   };
 }

front_end/models/trace/insights/DuplicateJavaScript.ts

@@ -56,7 +56,7 @@ function finalize(partialModel: PartialInsightModel<DuplicateJavaScriptInsightMo
 
 export function generateInsight(
     parsedTrace: RequiredData<typeof deps>, context: InsightSetContext): DuplicateJavaScriptInsightModel {
-  const scriptEntries = [...parsedTrace.Scripts.scripts].filter(([_, script]) => {
+  const scripts = parsedTrace.Scripts.scripts.filter(script => {
     if (!context.navigation) {
       return false;
     }
@@ -68,6 +68,6 @@ export function generateInsight(
     return Helpers.Timing.timestampIsInBounds(context.bounds, script.ts);
   });
 
-  const duplication = Extras.ScriptDuplication.computeScriptDuplication({scripts: new Map(scriptEntries)});
+  const duplication = Extras.ScriptDuplication.computeScriptDuplication({scripts});
   return finalize({duplication});
 }

front_end/testing/TraceHelpers.ts

@@ -752,7 +752,7 @@ export function getBaseTraceParseModelData(overrides: Partial<ParsedTrace> = {})
       asyncCallToScheduler: new Map(),
     },
     Scripts: {
-      scripts: new Map(),
+      scripts: [],
     },
     ...overrides,
   };