Commit 42f44e6e authored by Sigurd Schneider's avatar Sigurd Schneider Committed by Chromium LUCI CQ

Provide scriptId for source locations if available

When capturing a stack trace, we were providing an invalid scriptId on
the corresponding source location. This CL gets the scriptId of the
top-most script, just as we get the column/line/url.

Bug: chromium:1158782
Change-Id: Iad46758f8b24f60b07cace13239bb9fb678b5b0a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2595528
Commit-Queue: Sigurd Schneider <sigurds@chromium.org>
Reviewed-by: default avatarPaul Lewis <aerotwist@chromium.org>
Reviewed-by: default avatarBenedikt Meurer <bmeurer@chromium.org>
Reviewed-by: default avatarMichael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#844467}
parent 73bd2a30
...@@ -41,9 +41,10 @@ std::unique_ptr<SourceLocation> SourceLocation::Capture( ...@@ -41,9 +41,10 @@ std::unique_ptr<SourceLocation> SourceLocation::Capture(
unsigned column_number) { unsigned column_number) {
std::unique_ptr<v8_inspector::V8StackTrace> stack_trace = std::unique_ptr<v8_inspector::V8StackTrace> stack_trace =
CaptureStackTrace(false); CaptureStackTrace(false);
if (stack_trace && !stack_trace->isEmpty()) if (stack_trace && !stack_trace->isEmpty()) {
return SourceLocation::CreateFromNonEmptyV8StackTrace( return SourceLocation::CreateFromNonEmptyV8StackTrace(
std::move(stack_trace), 0); std::move(stack_trace));
}
return std::make_unique<SourceLocation>(url, line_number, column_number, return std::make_unique<SourceLocation>(url, line_number, column_number,
std::move(stack_trace)); std::move(stack_trace));
} }
...@@ -53,9 +54,10 @@ std::unique_ptr<SourceLocation> SourceLocation::Capture( ...@@ -53,9 +54,10 @@ std::unique_ptr<SourceLocation> SourceLocation::Capture(
ExecutionContext* execution_context) { ExecutionContext* execution_context) {
std::unique_ptr<v8_inspector::V8StackTrace> stack_trace = std::unique_ptr<v8_inspector::V8StackTrace> stack_trace =
CaptureStackTrace(false); CaptureStackTrace(false);
if (stack_trace && !stack_trace->isEmpty()) if (stack_trace && !stack_trace->isEmpty()) {
return SourceLocation::CreateFromNonEmptyV8StackTrace( return SourceLocation::CreateFromNonEmptyV8StackTrace(
std::move(stack_trace), 0); std::move(stack_trace));
}
if (LocalDOMWindow* window = DynamicTo<LocalDOMWindow>(execution_context)) { if (LocalDOMWindow* window = DynamicTo<LocalDOMWindow>(execution_context)) {
Document* document = window->document(); Document* document = window->document();
...@@ -99,9 +101,10 @@ std::unique_ptr<SourceLocation> SourceLocation::FromMessage( ...@@ -99,9 +101,10 @@ std::unique_ptr<SourceLocation> SourceLocation::FromMessage(
message->GetStartColumn(isolate->GetCurrentContext()).To(&column_number)) message->GetStartColumn(isolate->GetCurrentContext()).To(&column_number))
++column_number; ++column_number;
if ((!script_id || !line_number) && stack_trace && !stack_trace->isEmpty()) if ((!script_id || !line_number) && stack_trace && !stack_trace->isEmpty()) {
return SourceLocation::CreateFromNonEmptyV8StackTrace( return SourceLocation::CreateFromNonEmptyV8StackTrace(
std::move(stack_trace), 0); std::move(stack_trace));
}
String url = ToCoreStringWithUndefinedOrNullCheck( String url = ToCoreStringWithUndefinedOrNullCheck(
message->GetScriptOrigin().ResourceName()); message->GetScriptOrigin().ResourceName());
...@@ -113,12 +116,12 @@ std::unique_ptr<SourceLocation> SourceLocation::FromMessage( ...@@ -113,12 +116,12 @@ std::unique_ptr<SourceLocation> SourceLocation::FromMessage(
// static // static
std::unique_ptr<SourceLocation> SourceLocation::CreateFromNonEmptyV8StackTrace( std::unique_ptr<SourceLocation> SourceLocation::CreateFromNonEmptyV8StackTrace(
std::unique_ptr<v8_inspector::V8StackTrace> stack_trace, std::unique_ptr<v8_inspector::V8StackTrace> stack_trace) {
int script_id) {
// Retrieve the data before passing the ownership to SourceLocation. // Retrieve the data before passing the ownership to SourceLocation.
String url = ToCoreString(stack_trace->topSourceURL()); String url = ToCoreString(stack_trace->topSourceURL());
unsigned line_number = stack_trace->topLineNumber(); unsigned line_number = stack_trace->topLineNumber();
unsigned column_number = stack_trace->topColumnNumber(); unsigned column_number = stack_trace->topColumnNumber();
int script_id = stack_trace->topScriptIdAsInteger();
return base::WrapUnique(new SourceLocation( return base::WrapUnique(new SourceLocation(
url, line_number, column_number, std::move(stack_trace), script_id)); url, line_number, column_number, std::move(stack_trace), script_id));
} }
...@@ -139,9 +142,10 @@ std::unique_ptr<SourceLocation> SourceLocation::FromFunction( ...@@ -139,9 +142,10 @@ std::unique_ptr<SourceLocation> SourceLocation::FromFunction(
std::unique_ptr<SourceLocation> SourceLocation::CaptureWithFullStackTrace() { std::unique_ptr<SourceLocation> SourceLocation::CaptureWithFullStackTrace() {
std::unique_ptr<v8_inspector::V8StackTrace> stack_trace = std::unique_ptr<v8_inspector::V8StackTrace> stack_trace =
CaptureStackTrace(true); CaptureStackTrace(true);
if (stack_trace && !stack_trace->isEmpty()) if (stack_trace && !stack_trace->isEmpty()) {
return SourceLocation::CreateFromNonEmptyV8StackTrace( return SourceLocation::CreateFromNonEmptyV8StackTrace(
std::move(stack_trace), 0); std::move(stack_trace));
}
return std::make_unique<SourceLocation>(String(), 0, 0, nullptr, 0); return std::make_unique<SourceLocation>(String(), 0, 0, nullptr, 0);
} }
...@@ -166,7 +170,7 @@ void SourceLocation::ToTracedValue(TracedValue* value, const char* name) const { ...@@ -166,7 +170,7 @@ void SourceLocation::ToTracedValue(TracedValue* value, const char* name) const {
value->BeginDictionary(); value->BeginDictionary();
value->SetString("functionName", value->SetString("functionName",
ToCoreString(stack_trace_->topFunctionName())); ToCoreString(stack_trace_->topFunctionName()));
value->SetString("scriptId", ToCoreString(stack_trace_->topScriptId())); value->SetInteger("scriptId", stack_trace_->topScriptIdAsInteger());
value->SetString("url", ToCoreString(stack_trace_->topSourceURL())); value->SetString("url", ToCoreString(stack_trace_->topSourceURL()));
value->SetInteger("lineNumber", stack_trace_->topLineNumber()); value->SetInteger("lineNumber", stack_trace_->topLineNumber());
value->SetInteger("columnNumber", stack_trace_->topColumnNumber()); value->SetInteger("columnNumber", stack_trace_->topColumnNumber());
......
...@@ -76,8 +76,7 @@ class CORE_EXPORT SourceLocation { ...@@ -76,8 +76,7 @@ class CORE_EXPORT SourceLocation {
private: private:
static std::unique_ptr<SourceLocation> CreateFromNonEmptyV8StackTrace( static std::unique_ptr<SourceLocation> CreateFromNonEmptyV8StackTrace(
std::unique_ptr<v8_inspector::V8StackTrace>, std::unique_ptr<v8_inspector::V8StackTrace>);
int script_id);
String url_; String url_;
unsigned line_number_; unsigned line_number_;
......
...@@ -21,5 +21,5 @@ Layout Properties: ...@@ -21,5 +21,5 @@ Layout Properties:
startTime : <number> startTime : <number>
type : "Layout" type : "Layout"
} }
Text details for Layout: forced-layout-in-microtask.js:21 Text details for Layout: test://evaluations/0/forced-layout-in-microtask.js:21
...@@ -21,7 +21,7 @@ Layout Properties: ...@@ -21,7 +21,7 @@ Layout Properties:
startTime : <number> startTime : <number>
type : "Layout" type : "Layout"
} }
Text details for Layout: timeline-layout.js:40 Text details for Layout: test://evaluations/0/timeline-layout.js:40
Layout Properties: Layout Properties:
{ {
data : { data : {
...@@ -43,5 +43,5 @@ Layout Properties: ...@@ -43,5 +43,5 @@ Layout Properties:
startTime : <number> startTime : <number>
type : "Layout" type : "Layout"
} }
Text details for Layout: timeline-layout.js:40 Text details for Layout: test://evaluations/0/timeline-layout.js:40
...@@ -13,7 +13,7 @@ RequestAnimationFrame Properties: ...@@ -13,7 +13,7 @@ RequestAnimationFrame Properties:
startTime : <number> startTime : <number>
type : "RequestAnimationFrame" type : "RequestAnimationFrame"
} }
Text details for RequestAnimationFrame: timeline-animation-frame.js:14 Text details for RequestAnimationFrame: test://evaluations/0/timeline-animation-frame.js:14
FireAnimationFrame Properties: FireAnimationFrame Properties:
{ {
data : { data : {
...@@ -25,7 +25,7 @@ FireAnimationFrame Properties: ...@@ -25,7 +25,7 @@ FireAnimationFrame Properties:
startTime : <number> startTime : <number>
type : "FireAnimationFrame" type : "FireAnimationFrame"
} }
Text details for FireAnimationFrame: timeline-animation-frame.js:14 Text details for FireAnimationFrame: test://evaluations/0/timeline-animation-frame.js:14
CancelAnimationFrame Properties: CancelAnimationFrame Properties:
{ {
data : { data : {
...@@ -39,5 +39,5 @@ CancelAnimationFrame Properties: ...@@ -39,5 +39,5 @@ CancelAnimationFrame Properties:
startTime : <number> startTime : <number>
type : "CancelAnimationFrame" type : "CancelAnimationFrame"
} }
Text details for CancelAnimationFrame: timeline-animation-frame.js:17 Text details for CancelAnimationFrame: test://evaluations/0/timeline-animation-frame.js:17
...@@ -15,7 +15,7 @@ TimerInstall Properties: ...@@ -15,7 +15,7 @@ TimerInstall Properties:
startTime : <number> startTime : <number>
type : "TimerInstall" type : "TimerInstall"
} }
Text details for TimerInstall: timeline-timer.js:14 Text details for TimerInstall: test://evaluations/0/timeline-timer.js:14
TimerInstall Properties: TimerInstall Properties:
{ {
data : { data : {
...@@ -31,7 +31,7 @@ TimerInstall Properties: ...@@ -31,7 +31,7 @@ TimerInstall Properties:
startTime : <number> startTime : <number>
type : "TimerInstall" type : "TimerInstall"
} }
Text details for TimerInstall: timeline-timer.js:15 Text details for TimerInstall: test://evaluations/0/timeline-timer.js:15
TimerFire Properties: TimerFire Properties:
{ {
data : { data : {
...@@ -43,7 +43,7 @@ TimerFire Properties: ...@@ -43,7 +43,7 @@ TimerFire Properties:
startTime : <number> startTime : <number>
type : "TimerFire" type : "TimerFire"
} }
Text details for TimerFire: timeline-timer.js:14 Text details for TimerFire: test://evaluations/0/timeline-timer.js:14
TimerFire Properties: TimerFire Properties:
{ {
data : { data : {
...@@ -55,7 +55,7 @@ TimerFire Properties: ...@@ -55,7 +55,7 @@ TimerFire Properties:
startTime : <number> startTime : <number>
type : "TimerFire" type : "TimerFire"
} }
Text details for TimerFire: timeline-timer.js:15 Text details for TimerFire: test://evaluations/0/timeline-timer.js:15
TimerFire Properties: TimerFire Properties:
{ {
data : { data : {
...@@ -67,7 +67,7 @@ TimerFire Properties: ...@@ -67,7 +67,7 @@ TimerFire Properties:
startTime : <number> startTime : <number>
type : "TimerFire" type : "TimerFire"
} }
Text details for TimerFire: timeline-timer.js:15 Text details for TimerFire: test://evaluations/0/timeline-timer.js:15
TimerRemove Properties: TimerRemove Properties:
{ {
data : { data : {
...@@ -81,7 +81,7 @@ TimerRemove Properties: ...@@ -81,7 +81,7 @@ TimerRemove Properties:
startTime : <number> startTime : <number>
type : "TimerRemove" type : "TimerRemove"
} }
Text details for TimerRemove: timeline-timer.js:22 Text details for TimerRemove: test://evaluations/0/timeline-timer.js:22
FunctionCall Properties: FunctionCall Properties:
{ {
data : { data : {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment