[inspector] Fix positions for inline scripts with #sourceURL.

For inline scripts that have a `// #sourceURL=foo.js` annotation, the
V8 inspector (and by extension `Error.stack`) currently operates in
terms of the `foo.js`, i.e. doesn't give any hint about the actual
source, except for the line/column offsets reported upon scriptParsed.
However in case of stack frames (i.e. as part of `Error.stack` or as
part of the call frames reported via CDP), the line/column offsets are
relative to the actual source instead of relative to the `foo.js` part,
which - besides other things - makes post-processing of recorded stack
traces tricky (sometimes impossible).

This change adjusts the source positions reported for (inline) scripts
with sourceURL annotations to be relative to the (inline) script instead
of the surrounding document.

Bug: chromium:1183990
Fixed: chromium:578269
Change-Id: I74f2b93c22ec43ca796b6b51faa9df5b99cf03f9
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3069289
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Auto-Submit: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Simon Zünd <szuend@chromium.org>
Cr-Commit-Position: refs/heads/master@{#76097}

src/objects/script-inl.h

@@ -175,6 +175,10 @@ bool Script::HasValidSource() {
   return true;
 }
 
+bool Script::HasSourceURLComment() const {
+  return source_url().IsString() && String::cast(source_url()).length() != 0;
+}
+
 }  // namespace internal
 }  // namespace v8
 

src/objects/script.h

@@ -142,6 +142,9 @@ class Script : public TorqueGeneratedScript<Script, Struct> {
   // resource is accessible. Otherwise, always return true.
   inline bool HasValidSource();
 
+  // If the script has a non-empty sourceURL comment.
+  inline bool HasSourceURLComment() const;
+
   Object GetNameOrSourceURL();
 
   // Retrieve source position from where eval was called.

src/objects/stack-frame-info.cc

@@ -68,7 +68,11 @@ int StackFrameInfo::GetLineNumber(Handle<StackFrameInfo> info) {
   Handle<Script> script;
   if (GetScript(isolate, info).ToHandle(&script)) {
     int position = GetSourcePosition(info);
-    return Script::GetLineNumber(script, position) + 1;
+    int line_number = Script::GetLineNumber(script, position) + 1;
+    if (script->HasSourceURLComment()) {
+      line_number -= script->line_offset();
+    }
+    return line_number;
   }
   return Message::kNoLineNumberInfo;
 }
@@ -84,7 +88,13 @@ int StackFrameInfo::GetColumnNumber(Handle<StackFrameInfo> info) {
 #endif  // V8_ENABLE_WEBASSEMBLY
   Handle<Script> script;
   if (GetScript(isolate, info).ToHandle(&script)) {
-    return Script::GetColumnNumber(script, position) + 1;
+    int column_number = Script::GetColumnNumber(script, position) + 1;
+    if (script->HasSourceURLComment()) {
+      if (Script::GetLineNumber(script, position) == script->line_offset()) {
+        column_number -= script->column_offset();
+      }
+    }
+    return column_number;
   }
   return Message::kNoColumnInfo;
 }

test/inspector/debugger/async-instrumentation-expected.txt

@@ -9,12 +9,12 @@ test (test.js:21:2)
 
 foo (test.js:10:2)
 -- Promise.then --
-test (test.js:19:14)
+test (test.js:12:14)
 (anonymous) (expr1.js:0:0)
 
 foo (test.js:12:2)
 -- Promise.then --
-test (test.js:19:14)
+test (test.js:12:14)
 (anonymous) (expr1.js:0:0)
 
 

test/inspector/debugger/async-promise-late-then-expected.txt

 Checks async stack for late .then handlers with gc
 foo1 (test.js:11:2)
 -- Promise.then --
-test (test.js:18:14)
+test (test.js:10:14)
 (anonymous) (expr.js:0:0)
 
 foo1 (test.js:11:2)
 -- Promise.then --
-test (test.js:22:14)
+test (test.js:14:14)
 (anonymous) (expr.js:0:0)
 
 foo1 (test.js:11:2)
 -- Promise.then --
-test (test.js:24:14)
+test (test.js:16:14)
 (anonymous) (expr.js:0:0)
 

test/inspector/debugger/async-set-timeout-expected.txt

@@ -2,10 +2,10 @@ Checks that async stack contains setTimeout
 inner1 (test.js:11:4)
 foo1 (test.js:14:2)
 -- setTimeout --
-inner2 (test.js:18:4)
-foo2 (test.js:20:2)
+inner2 (test.js:11:4)
+foo2 (test.js:13:2)
 -- setTimeout --
-inner3 (test.js:25:4)
-foo3 (test.js:27:2)
+inner3 (test.js:18:4)
+foo3 (test.js:20:2)
 (anonymous) (expr.js:0:0)
 

test/inspector/debugger/async-stack-created-frame-expected.txt

@@ -3,78 +3,78 @@ Checks created frame for async call chain
 Running test: testPromise
 foo1 (test.js:10:2)
 -- Promise.then --
-promise (test.js:20:14)
+promise (test.js:12:14)
 (anonymous) (expr.js:0:0)
 
 
 Running test: testPromiseThen
 foo1 (test.js:10:2)
 -- Promise.then --
-promiseThen (test.js:28:14)
+promiseThen (test.js:20:14)
 (anonymous) (expr.js:0:0)
 
 foo2 (test.js:14:2)
 -- Promise.then --
-promiseThen (test.js:29:14)
+promiseThen (test.js:21:14)
 (anonymous) (expr.js:0:0)
 
 
 Running test: testPromiseThenThen
 foo1 (test.js:10:2)
 -- Promise.then --
-promiseThenThen (test.js:37:14)
+promiseThenThen (test.js:29:14)
 (anonymous) (expr.js:0:0)
 
 foo1 (test.js:10:2)
 -- Promise.then --
-promiseThenThen (test.js:38:14)
+promiseThenThen (test.js:30:14)
 (anonymous) (expr.js:0:0)
 
 foo2 (test.js:14:2)
 -- Promise.then --
-promiseThenThen (test.js:37:25)
+promiseThenThen (test.js:29:25)
 (anonymous) (expr.js:0:0)
 
 
 Running test: testPromiseResolve
 foo1 (test.js:10:2)
 -- Promise.then --
-promiseResolve (test.js:44:27)
+promiseResolve (test.js:36:27)
 (anonymous) (expr.js:0:0)
 
 
 Running test: testPromiseReject
 foo1 (test.js:10:2)
 -- Promise.catch --
-promiseReject (test.js:48:31)
+promiseReject (test.js:40:31)
 (anonymous) (expr.js:0:0)
 
 
 Running test: testPromiseAll
 foo1 (test.js:10:2)
 -- Promise.then --
-promiseAll (test.js:52:44)
+promiseAll (test.js:44:44)
 (anonymous) (expr.js:0:0)
 
 
 Running test: testPromiseRace
 foo1 (test.js:10:2)
 -- Promise.then --
-promiseRace (test.js:56:45)
+promiseRace (test.js:48:45)
 (anonymous) (expr.js:0:0)
 
 
 Running test: testThenableJob1
 foo1 (test.js:10:2)
 -- Promise.then --
-thenableJob1 (test.js:60:72)
+thenableJob1 (test.js:52:72)
 (anonymous) (expr.js:0:0)
 
 
 Running test: testThenableJob2
 foo1 (test.js:10:2)
 -- Promise.then --
-thenableJob2 (test.js:64:57)
+thenableJob2 (test.js:56:57)
 (anonymous) (expr.js:0:0)
 
 
@@ -82,10 +82,10 @@ Running test: testSetTimeouts
 foo1 (test.js:10:2)
 (anonymous) (test.js:72:25)
 -- setTimeout --
-(anonymous) (test.js:72:6)
+(anonymous) (test.js:64:6)
 -- setTimeout --
-(anonymous) (test.js:71:4)
+(anonymous) (test.js:63:4)
 -- setTimeout --
-setTimeouts (test.js:70:2)
+setTimeouts (test.js:62:2)
 (anonymous) (expr.js:0:0)
 

test/inspector/debugger/async-stack-for-promise-expected.txt

@@ -3,94 +3,94 @@ Checks that async chains for promises are correct.
 Running test: testPromise
 foo1 (test.js:9:2)
 -- Promise.then --
-promise (test.js:19:14)
+promise (test.js:12:14)
 (anonymous) (testPromise.js:0:0)
 
 
 Running test: testPromiseResolvedBySetTimeout
 foo1 (test.js:9:2)
 -- Promise.then --
-promiseResolvedBySetTimeout (test.js:27:14)
+promiseResolvedBySetTimeout (test.js:20:14)
 (anonymous) (testPromiseResolvedBySetTimeout.js:0:0)
 
 
 Running test: testPromiseAll
 foo1 (test.js:9:2)
 -- Promise.then --
-promiseAll (test.js:37:35)
+promiseAll (test.js:30:35)
 (anonymous) (testPromiseAll.js:0:0)
 
 
 Running test: testPromiseAllReverseOrder
 foo1 (test.js:9:2)
 -- Promise.then --
-promiseAllReverseOrder (test.js:48:35)
+promiseAllReverseOrder (test.js:41:35)
 (anonymous) (testPromiseAllReverseOrder.js:0:0)
 
 
 Running test: testPromiseRace
 foo1 (test.js:9:2)
 -- Promise.then --
-promiseRace (test.js:59:36)
+promiseRace (test.js:52:36)
 (anonymous) (testPromiseRace.js:0:0)
 
 
 Running test: testTwoChainedCallbacks
 foo1 (test.js:9:2)
 -- Promise.then --
-twoChainedCallbacks (test.js:68:14)
+twoChainedCallbacks (test.js:61:14)
 (anonymous) (testTwoChainedCallbacks.js:0:0)
 
 foo2 (test.js:13:2)
 -- Promise.then --
-twoChainedCallbacks (test.js:68:25)
+twoChainedCallbacks (test.js:61:25)
 (anonymous) (testTwoChainedCallbacks.js:0:0)
 
 
 Running test: testPromiseResolve
 foo1 (test.js:9:2)
 -- Promise.then --
-promiseResolve (test.js:74:27)
+promiseResolve (test.js:67:27)
 (anonymous) (testPromiseResolve.js:0:0)
 
 foo2 (test.js:13:2)
 -- Promise.then --
-promiseResolve (test.js:74:38)
+promiseResolve (test.js:67:38)
 (anonymous) (testPromiseResolve.js:0:0)
 
 
 Running test: testThenableJobResolvedInSetTimeout
 foo1 (test.js:9:2)
 -- Promise.then --
-thenableJobResolvedInSetTimeout (test.js:86:40)
+thenableJobResolvedInSetTimeout (test.js:79:40)
 (anonymous) (testThenableJobResolvedInSetTimeout.js:0:0)
 
 
 Running test: testThenableJobResolvedInSetTimeoutWithStack
 foo1 (test.js:9:2)
 -- Promise.then --
-thenableJobResolvedInSetTimeoutWithStack (test.js:104:40)
+thenableJobResolvedInSetTimeoutWithStack (test.js:97:40)
 (anonymous) (testThenableJobResolvedInSetTimeoutWithStack.js:0:0)
 
 
 Running test: testThenableJobResolvedByPromise
 foo1 (test.js:9:2)
 -- Promise.then --
-thenableJobResolvedByPromise (test.js:118:40)
+thenableJobResolvedByPromise (test.js:111:40)
 (anonymous) (testThenableJobResolvedByPromise.js:0:0)
 
 
 Running test: testThenableJobResolvedByPromiseWithStack
 foo1 (test.js:9:2)
 -- Promise.then --
-thenableJobResolvedByPromiseWithStack (test.js:136:40)
+thenableJobResolvedByPromiseWithStack (test.js:129:40)
 (anonymous) (testThenableJobResolvedByPromiseWithStack.js:0:0)
 
 
 Running test: testLateThenCallback
 foo1 (test.js:9:2)
 -- Promise.then --
-lateThenCallback (test.js:145:12)
+lateThenCallback (test.js:138:12)
 (anonymous) (testLateThenCallback.js:0:0)
 
 
@@ -98,36 +98,36 @@ Running test: testComplex
 inner1 (test.js:154:6)
 foo1 (test.js:156:4)
 -- Promise.then --
-complex (test.js:202:5)
+complex (test.js:195:5)
 (anonymous) (testComplex.js:0:0)
 
 (anonymous) (test.js:207:8)
 -- Promise.then --
-(anonymous) (test.js:206:8)
+(anonymous) (test.js:199:8)
 -- Promise.then --
-(anonymous) (test.js:205:6)
+(anonymous) (test.js:198:6)
 -- setTimeout --
-complex (test.js:204:2)
+complex (test.js:197:2)
 (anonymous) (testComplex.js:0:0)
 
 
 Running test: testReject
 foo1 (test.js:9:2)
 -- Promise.catch --
-reject (test.js:217:31)
+reject (test.js:210:31)
 (anonymous) (testReject.js:0:0)
 
 
 Running test: testFinally1
 foo1 (test.js:9:2)
 -- Promise.finally --
-finally1 (test.js:221:33)
+finally1 (test.js:214:33)
 (anonymous) (testFinally1.js:0:0)
 
 
 Running test: testFinally2
 foo1 (test.js:9:2)
 -- Promise.finally --
-finally2 (test.js:225:34)
+finally2 (test.js:218:34)
 (anonymous) (testFinally2.js:0:0)
 

test/inspector/debugger/continue-to-location-target-call-frames-expected.txt

@@ -8,10 +8,10 @@ asyncFact (test.js:9:2)
 
 asyncFact (test.js:11:2)
 -- async function --
-asyncFact (test.js:10:20)
-asyncFact (test.js:10:20)
-asyncFact (test.js:10:20)
-asyncFact (test.js:10:20)
+asyncFact (test.js:3:20)
+asyncFact (test.js:3:20)
+asyncFact (test.js:3:20)
+asyncFact (test.js:3:20)
 (anonymous) (expr.js:0:0)
 
 
@@ -23,7 +23,7 @@ asyncFact (test.js:9:2)
 
 asyncFact (test.js:11:2)
 -- async function --
-asyncFact (test.js:10:20)
+asyncFact (test.js:3:20)
 (anonymous) (expr.js:0:0)
 
 

test/inspector/debugger/script-parsed-for-runtime-evaluate-expected.txt

@@ -164,7 +164,7 @@ Runtime.evaluate compiled script with stack trace
                 [0] : {
                     columnNumber : 2
                     functionName : fooTop
-                    lineNumber : 10
+                    lineNumber : 2
                     scriptId : <scriptId>
                     url : top-frame.js
                 }
@@ -247,7 +247,7 @@ Runtime.evaluate compile script error with stack trace
                 [0] : {
                     columnNumber : 2
                     functionName : fooTopFail
-                    lineNumber : 20
+                    lineNumber : 2
                     scriptId : <scriptId>
                     url : top-frame-fail.js
                 }