From 7cd52d08e69a1aa9bd83a822de61f71e7c759c66 Mon Sep 17 00:00:00 2001
From: Connor Peet <connor@peet.io>
Date: Wed, 3 May 2023 11:06:48 -0700
Subject: [PATCH] fix: step into `eval` when `pauseForSourceMap` is true does
 not pause on next available line Fixes #1665

---
 src/adapter/threads.ts                        | 14 ++++----
 ...n-with-instrumentation-breakpoint-1665.txt | 26 ++++++++++++++
 src/test/breakpoints/breakpointsTest.ts       | 36 +++++++++++++++++++
 3 files changed, 70 insertions(+), 6 deletions(-)
 create mode 100644 src/test/breakpoints/breakpoints-does-not-interrupt-stepin-with-instrumentation-breakpoint-1665.txt

diff --git a/src/adapter/threads.ts b/src/adapter/threads.ts
index 3eb55f335..b9e26f502 100644
--- a/src/adapter/threads.ts
+++ b/src/adapter/threads.ts
@@ -836,6 +836,7 @@ export class Thread implements IVariableStoreLocationProvider {
         event.data.__rewriteAs = 'breakpoint';
       }
 
+      const expectedPauseReason = this._expectedPauseReason;
       if (scriptId && (await this._handleSourceMapPause(scriptId, location))) {
         // Pause if we just resolved a breakpoint that's on this
         // location; this won't have existed before now.
@@ -853,14 +854,15 @@ export class Thread implements IVariableStoreLocationProvider {
         )
       ) {
         // Check if there are any user-defined breakpoints on this line
-      } else if (
-        this._expectedPauseReason?.reason === 'step' &&
-        this._expectedPauseReason.direction === StepDirection.Over
-      ) {
-        // Check if we're in the middle of a step over, e.g. stepping over a
+      } else if (expectedPauseReason?.reason === 'step') {
+        // Check if we're in the middle of a step, e.g. stepping over a
         // function compilation. Stepping in should still remain paused,
         // and an instrumentation pause in step out should not be possible.
-        return this._cdp.Debugger.stepOut({});
+        if (expectedPauseReason.direction === StepDirection.In) {
+          // no-op
+        } else {
+          return this._cdp.Debugger.stepOut({});
+        }
       } else {
         // If none of this above, it's pure instrumentation.
         return this.resume();
diff --git a/src/test/breakpoints/breakpoints-does-not-interrupt-stepin-with-instrumentation-breakpoint-1665.txt b/src/test/breakpoints/breakpoints-does-not-interrupt-stepin-with-instrumentation-breakpoint-1665.txt
new file mode 100644
index 000000000..877e9dcc4
--- /dev/null
+++ b/src/test/breakpoints/breakpoints-does-not-interrupt-stepin-with-instrumentation-breakpoint-1665.txt
@@ -0,0 +1,26 @@
+Evaluating#1: test()
+{
+    allThreadsStopped : false
+    description : Paused on debugger statement
+    reason : pause
+    threadId : <number>
+}
+test @ <eval>/VM<xx>:4:13
+<anonymous> @ eval1.js:1:1
+{
+    allThreadsStopped : false
+    description : Paused
+    reason : step
+    threadId : <number>
+}
+test @ <eval>/VM<xx>:5:13
+<anonymous> @ eval1.js:1:1
+{
+    allThreadsStopped : false
+    description : Paused
+    reason : step
+    threadId : <number>
+}
+<anonymous> @ foo.js:2:15
+test @ <eval>/VM<xx>:5:15
+<anonymous> @ eval1.js:1:1
diff --git a/src/test/breakpoints/breakpointsTest.ts b/src/test/breakpoints/breakpointsTest.ts
index 8467a53f4..6b76143a1 100644
--- a/src/test/breakpoints/breakpointsTest.ts
+++ b/src/test/breakpoints/breakpointsTest.ts
@@ -1319,6 +1319,42 @@ describe('breakpoints', () => {
     },
   );
 
+  itIntegrates(
+    'does not interrupt stepIn with instrumentation breakpoint (#1665)',
+    async ({ r }) => {
+      const p = await r.launchAndLoad(`
+        <script>
+          function test() {
+            debugger;
+            f=eval(\`
+              (function (a, b) {
+                c = a + b;
+                return c;
+              });
+              //# sourceURL=foo.js
+              //# sourceMappingURL=foo.js.map
+            \`);
+            f(1, 2);
+          }
+        </script>`);
+
+      const evaluate = p.evaluate('test()');
+
+      const a = p.log(await p.dap.once('stopped')); // debugger statement
+      await p.logger.logStackTrace(a.threadId);
+      await p.dap.stepIn({ threadId: a.threadId });
+
+      const b = p.log(await p.dap.once('stopped')); // f=eval(...
+      await p.logger.logStackTrace(b.threadId);
+      await p.dap.stepIn({ threadId: b.threadId });
+
+      await waitForPause(p); // should now be on (function (a, b)
+
+      await evaluate;
+      p.assertLog();
+    },
+  );
+
   itIntegrates('deals with removed execution contexts (#1582)', async ({ r }) => {
     const p = await r.launchUrlAndLoad('iframe-1582/index.html');