feat: lazily announce evaluated scripts

Closes #1939
microsoft · Feb 5, 2024 · 2d9fbaf · 2d9fbaf
1 parent 8eee78b
commit 2d9fbaf
Show file tree

Hide file tree

Showing 17 changed files with 170 additions and 70 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,6 +4,7 @@ This changelog records changes to stable releases since 1.50.2. "TBA" changes he
 
 ## Nightly (only)
 
+- feat: lazily announce evaluated scripts ([#1939](https://github.com/microsoft/vscode-js-debug/issues/1939))
 - fix: support object property shorthand in logpoints ([#1788](https://github.com/microsoft/vscode-js-debug/issues/1788))
 - fix: pages not loading in browser after attach browser disconnect ([#1795](https://github.com/microsoft/vscode-js-debug/issues/1795))
 - fix: skipFiles not matching/negating with special chars ([vscode#203408](https://github.com/microsoft/vscode/issues/203408))

diff --git a/src/adapter/profiling/sourceAnnotationHelper.ts b/src/adapter/profiling/sourceAnnotationHelper.ts
@@ -52,7 +52,7 @@ export class SourceAnnotationHelper {
         });
 
         return Promise.all(
-          locations.map(async loc => ({ ...loc, source: await loc.source.toDapShallow() })),
+          locations.map(async loc => ({ ...loc, source: await loc.source.toDap() })),
         );
       })(),
     });

diff --git a/src/adapter/source.ts b/src/adapter/source.ts
@@ -61,6 +61,17 @@ export class Source {
   private readonly _existingAbsolutePath: Promise<string | undefined>;
   private _scripts: ISourceScript[] = [];
 
+  /**
+   * Gets whether the source should be sent to the client lazily.
+   * This is true for evaluated scripts. (#1939)
+   */
+  public get sendLazy() {
+    return !this.url;
+  }
+
+  /** @internal */
+  public hasBeenAnnounced = false;
+
   /**
    * @param inlineScriptOffset Offset of the start location of the script in
    * its source file. This is used on scripts in HTML pages, where the script
@@ -245,15 +256,9 @@ export class Source {
 
   /**
    * Returns a DAP representation of the source.
+   * Has a side-effect of announcing the script if it has not yet been annoucned.
    */
-  async toDap(): Promise<Dap.Source> {
-    return this.toDapShallow();
-  }
-
-  /**
-   * Returns a DAP representation without including any nested sources.
-   */
-  public async toDapShallow(): Promise<Dap.Source> {
+  public async toDap(): Promise<Dap.Source> {
     const existingAbsolutePath = await this._existingAbsolutePath;
     const dap: Dap.Source = {
       name: this._name,
@@ -268,6 +273,11 @@ export class Source {
       dap.path = existingAbsolutePath;
     }
 
+    if (!this.hasBeenAnnounced) {
+      this.hasBeenAnnounced = true;
+      this._container.emitLoadedSource(this);
+    }
+
     return dap;
   }
 

diff --git a/src/adapter/sourceContainer.ts b/src/adapter/sourceContainer.ts
@@ -689,7 +689,9 @@ export class SourceContainer {
     }
 
     this.scriptSkipper.initializeSkippingValueForSource(source);
-    source.toDap().then(dap => this._dap.loadedSource({ reason: 'new', source: dap }));
+    if (!source.sendLazy) {
+      this.emitLoadedSource(source);
+    }
 
     if (isSourceWithSourceMap(source)) {
       this._finishAddSourceWithSourceMap(source);
@@ -828,7 +830,7 @@ export class SourceContainer {
       this._temporarilyDisabledSourceMaps.delete(source);
     }
 
-    if (!silent) {
+    if (!silent && source.hasBeenAnnounced) {
       source.toDap().then(dap => this._dap.loadedSource({ reason: 'removed', source: dap }));
     }
 
@@ -841,6 +843,14 @@ export class SourceContainer {
     }
   }
 
+  /**
+   * Sends a 'loadedSource' event for the given source.
+   */
+  public emitLoadedSource(source: Source): Promise<void> {
+    source.hasBeenAnnounced = true;
+    return source.toDap().then(dap => this._dap.loadedSource({ reason: 'new', source: dap }));
+  }
+
   async _addSourceMapSources(compiled: ISourceWithMap, map: SourceMap) {
     const todo: Promise<unknown>[] = [];
     for (const url of map.sources) {

diff --git a/src/adapter/stackTrace.ts b/src/adapter/stackTrace.ts
@@ -751,7 +751,7 @@ export class InlinedFrame implements IStackFrameElement {
       name: this.name,
       column: columnNumber,
       line: lineNumber,
-      source: await source.toDapShallow(),
+      source: await source.toDap(),
     });
   }
 

diff --git a/src/adapter/threads.ts b/src/adapter/threads.ts
@@ -1067,7 +1067,7 @@ export class Thread implements IVariableStoreLocationProvider {
         continue;
       }
 
-      firstSource ??= await first.source.toDapShallow();
+      firstSource ??= await first.source.toDap();
       if (!sourcesEqual(firstSource, target.source)) {
         continue;
       }
@@ -1092,7 +1092,7 @@ export class Thread implements IVariableStoreLocationProvider {
           continue;
         }
 
-        const source = (stackAsDap[i] ??= await r.source.toDapShallow());
+        const source = (stackAsDap[i] ??= await r.source.toDap());
         if (sourcesEqual(source, caller.source)) {
           return true;
         }

diff --git a/src/test/breakpoints/breakpoints-hit-count-can-change-after-set.txt b/src/test/breakpoints/breakpoints-hit-count-can-change-after-set.txt
@@ -1,12 +1,21 @@
+Evaluating#1:
+        function foo() {
+          for (let i = 0; i < 10; i++) {
+            console.log(i);
+            console.log(i);
+            console.log(i);
+          }
+        }
+
 {
     breakpoints : [
         [0] : {
             column : 13
             id : <number>
             line : 4
             source : {
-                name : <eval>/VM<xx>
-                path : <eval>/VM<xx>
+                name : localhost꞉8001/eval1.js
+                path : localhost꞉8001/eval1.js
                 sourceReference : <number>
             }
             verified : true
@@ -20,13 +29,13 @@
             id : <number>
             line : 4
             source : {
-                name : <eval>/VM<xx>
-                path : <eval>/VM<xx>
+                name : localhost꞉8001/eval1.js
+                path : localhost꞉8001/eval1.js
                 sourceReference : <number>
             }
             verified : true
         }
     ]
 }
-Evaluating#1: foo();
+Evaluating#2: foo();
 result: 7
diff --git a/src/test/breakpoints/breakpoints-hit-count-does-not-validate-or-hit-invalid-breakpoint.txt b/src/test/breakpoints/breakpoints-hit-count-does-not-validate-or-hit-invalid-breakpoint.txt
@@ -1,3 +1,12 @@
+Evaluating#1:
+        function foo() {
+          for (let i = 0; i < 10; i++) {
+            console.log(i);
+            console.log(i);
+            console.log(i);
+          }
+        }
+
 {
     breakpoints : [
         [0] : {
@@ -11,4 +20,4 @@
     category : stderr
     output : Invalid hit condition "potato". Expected an expression like "> 42" or "== 2".
 }
-Evaluating#1: foo();
+Evaluating#2: foo();
diff --git a/src/test/breakpoints/breakpoints-hit-count-implies-equal-1698.txt b/src/test/breakpoints/breakpoints-hit-count-implies-equal-1698.txt
@@ -1,17 +1,26 @@
+Evaluating#1:
+        function foo() {
+          for (let i = 0; i < 10; i++) {
+            console.log(i);
+            console.log(i);
+            console.log(i);
+          }
+        }
+
 {
     breakpoints : [
         [0] : {
             column : 13
             id : <number>
             line : 4
             source : {
-                name : <eval>/VM<xx>
-                path : <eval>/VM<xx>
+                name : localhost꞉8001/eval1.js
+                path : localhost꞉8001/eval1.js
                 sourceReference : <number>
             }
             verified : true
         }
     ]
 }
-Evaluating#1: foo();
+Evaluating#2: foo();
 result: 4
diff --git a/src/test/breakpoints/breakpoints-hit-count-works-for-valid.txt b/src/test/breakpoints/breakpoints-hit-count-works-for-valid.txt
@@ -1,17 +1,26 @@
+Evaluating#1:
+        function foo() {
+          for (let i = 0; i < 10; i++) {
+            console.log(i);
+            console.log(i);
+            console.log(i);
+          }
+        }
+
 {
     breakpoints : [
         [0] : {
             column : 13
             id : <number>
             line : 4
             source : {
-                name : <eval>/VM<xx>
-                path : <eval>/VM<xx>
+                name : localhost꞉8001/eval1.js
+                path : localhost꞉8001/eval1.js
                 sourceReference : <number>
             }
             verified : true
         }
     ]
 }
-Evaluating#1: foo();
+Evaluating#2: foo();
 result: 4
diff --git a/src/test/breakpoints/breakpoints-launched-overwrite.txt b/src/test/breakpoints/breakpoints-launched-overwrite.txt
@@ -1,10 +1,16 @@
+Evaluating#1:
+        function foo() {
+          var x = 3;
+          return 2;
+        }
+
 {
     allThreadsStopped : false
     description : Paused on breakpoint
     reason : breakpoint
     threadId : <number>
 }
-Window.foo @ <eval>/VM<xx>:3:19
+Window.foo @ localhost꞉8001/eval1.js:3:19
 <anonymous> @ localhost꞉8001/test.js:1:1
 {
     allThreadsStopped : false

diff --git a/src/test/breakpoints/breakpoints-launched-ref.txt b/src/test/breakpoints/breakpoints-launched-ref.txt
@@ -1,9 +1,14 @@
-Evaluating#1: foo();
+Evaluating#1:
+      function foo() {
+        return 2;
+      }
+
+Evaluating#2: foo();
 {
     allThreadsStopped : false
     description : Paused on breakpoint
     reason : breakpoint
     threadId : <number>
 }
-Window.foo @ <eval>/VM<xx>:3:11
-<anonymous> @ localhost꞉8001/eval1.js:1:1
+Window.foo @ localhost꞉8001/eval1.js:3:9
+<anonymous> @ localhost꞉8001/eval2.js:1:1
diff --git a/src/test/breakpoints/breakpoints-launched-remove.txt b/src/test/breakpoints/breakpoints-launched-remove.txt
@@ -1,3 +1,8 @@
+Evaluating#1:
+        function foo() {
+          return 2;
+        }
+
 {
     allThreadsStopped : false
     description : Paused on debugger statement

diff --git a/src/test/breakpoints/breakpointsTest.ts b/src/test/breakpoints/breakpointsTest.ts
@@ -203,13 +203,11 @@ describe('breakpoints', () => {
     itIntegrates('ref', async ({ r }) => {
       // Breakpoint in eval script set after launch using source reference.
       const p = await r.launchUrlAndLoad('index.html');
-      p.cdp.Runtime.evaluate({
-        expression: `
-        function foo() {
-          return 2;
-        }
-      `,
-      });
+      p.evaluate(`
+      function foo() {
+        return 2;
+      }
+    `);
       const { source } = await p.waitForSource('eval');
       source.path = undefined;
       await p.dap.setBreakpoints({ source, breakpoints: [{ line: 3 }] });
@@ -222,13 +220,11 @@ describe('breakpoints', () => {
     itIntegrates('remove', async ({ r }) => {
       // Breakpoint in eval script set after launch and immediately removed.
       const p = await r.launchUrlAndLoad('index.html');
-      p.cdp.Runtime.evaluate({
-        expression: `
+      p.evaluate(`
         function foo() {
           return 2;
         }
-      `,
-      });
+      `);
       const { source } = await p.waitForSource('eval');
       source.path = undefined;
       await p.dap.setBreakpoints({ source, breakpoints: [{ line: 3 }] });
@@ -241,14 +237,12 @@ describe('breakpoints', () => {
     itIntegrates('overwrite', async ({ r }) => {
       // Breakpoint in eval script set after launch and immediately overwritten.
       const p = await r.launchUrlAndLoad('index.html');
-      p.cdp.Runtime.evaluate({
-        expression: `
+      p.evaluate(`
         function foo() {
           var x = 3;
           return 2;
         }
-      `,
-      });
+      `);
       const { source } = await p.waitForSource('eval');
       source.path = undefined;
       await p.dap.setBreakpoints({ source, breakpoints: [{ line: 4 }] });
@@ -950,17 +944,15 @@ describe('breakpoints', () => {
   describe('hit count', () => {
     const doTest = async (r: TestRoot, run: (p: TestP, source: Dap.Source) => Promise<void>) => {
       const p = await r.launchUrlAndLoad('index.html');
-      p.cdp.Runtime.evaluate({
-        expression: `
+      p.evaluate(`
         function foo() {
           for (let i = 0; i < 10; i++) {
             console.log(i);
             console.log(i);
             console.log(i);
           }
         }
-      `,
-      });
+      `);
       const { source } = await p.waitForSource('eval');
       source.path = undefined;
       await run(p, source);

diff --git a/src/test/sources/sources-basic-sources.txt b/src/test/sources/sources-basic-sources.txt
@@ -52,20 +52,6 @@ Source event for dir/helloworld
 }
 console.log('Hello, world!');
 
----------
-Evaluating#2: 42
-
-Source event for eval
-{
-    reason : new
-    source : {
-        name : <eval>/VM<xx>
-        path : <eval>/VM<xx>
-        sourceReference : <number>
-    }
-}
-42
-
 ---------
 
 Loaded sources: {
@@ -100,10 +86,5 @@ Loaded sources: {
             path : ${workspaceFolder}/web/dir/helloworld.js
             sourceReference : <number>
         }
-        [6] : {
-            name : <eval>/VM<xx>
-            path : <eval>/VM<xx>
-            sourceReference : <number>
-        }
     ]
 }
-Original file line number
+Diff line change
@@ Expand Up / @@ -52,7 +52,7 @@ export class SourceAnnotationHelper { @@
             });
             return Promise.all(
-              locations.map(async loc => ({ ...loc, source: await loc.source.toDapShallow() })),
+              locations.map(async loc => ({ ...loc, source: await loc.source.toDap() })),
             );
           })(),
         });
@@ Expand Down @@