Commit 51fe5571 authored by Benedikt Meurer's avatar Benedikt Meurer Committed by V8 LUCI CQ

[inspector] Consistently pass around script ID as integer.

Within the inspector we should be consistent about passing the script ID
always as integer, and only convert to String16 when actually needed.
That (a) saves memory (and some runtime overhead) when stashing away
call frames, for example in case of async stack traces, and (b) reduces
confusion which representation to chose.

Bug: chromium:1162229
Change-Id: I9591931da0a307779372f36aba6e155ec22bbe3d
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2876856
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Commit-Queue: Yang Guo <yangguo@chromium.org>
Auto-Submit: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: 's avatarYang Guo <yangguo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#74410}
parent 9bd4492b
...@@ -105,8 +105,9 @@ class V8_EXPORT V8StackTrace { ...@@ -105,8 +105,9 @@ class V8_EXPORT V8StackTrace {
virtual StringView topSourceURL() const = 0; virtual StringView topSourceURL() const = 0;
virtual int topLineNumber() const = 0; virtual int topLineNumber() const = 0;
virtual int topColumnNumber() const = 0; virtual int topColumnNumber() const = 0;
virtual StringView topScriptId() const = 0; virtual int topScriptId() const = 0;
virtual int topScriptIdAsInteger() const = 0; V8_DEPRECATE_SOON("Use V8::StackTrace::topScriptId() instead.")
int topScriptIdAsInteger() const { return topScriptId(); }
virtual StringView topFunctionName() const = 0; virtual StringView topFunctionName() const = 0;
virtual ~V8StackTrace() = default; virtual ~V8StackTrace() = default;
......
...@@ -302,7 +302,8 @@ class InjectedScript::ProtocolPromiseHandler { ...@@ -302,7 +302,8 @@ class InjectedScript::ProtocolPromiseHandler {
exceptionDetails->setStackTrace( exceptionDetails->setStackTrace(
stack->buildInspectorObjectImpl(m_inspector->debugger())); stack->buildInspectorObjectImpl(m_inspector->debugger()));
if (stack && !stack->isEmpty()) if (stack && !stack->isEmpty())
exceptionDetails->setScriptId(toString16(stack->topScriptId())); exceptionDetails->setScriptId(
String16::fromInteger(stack->topScriptId()));
callback->sendSuccess(std::move(wrappedValue), std::move(exceptionDetails)); callback->sendSuccess(std::move(wrappedValue), std::move(exceptionDetails));
} }
......
...@@ -150,10 +150,11 @@ std::unique_ptr<protocol::Debugger::Location> currentDebugLocation( ...@@ -150,10 +150,11 @@ std::unique_ptr<protocol::Debugger::Location> currentDebugLocation(
V8InspectorImpl* inspector) { V8InspectorImpl* inspector) {
std::unique_ptr<V8StackTraceImpl> callStack = std::unique_ptr<V8StackTraceImpl> callStack =
inspector->debugger()->captureStackTrace(false /* fullStack */); inspector->debugger()->captureStackTrace(false /* fullStack */);
auto location = protocol::Debugger::Location::create() auto location =
.setScriptId(toString16(callStack->topScriptId())) protocol::Debugger::Location::create()
.setLineNumber(callStack->topLineNumber()) .setScriptId(String16::fromInteger(callStack->topScriptId()))
.build(); .setLineNumber(callStack->topLineNumber())
.build();
location->setColumnNumber(callStack->topColumnNumber()); location->setColumnNumber(callStack->topColumnNumber());
return location; return location;
} }
......
...@@ -177,7 +177,6 @@ std::unique_ptr<StringBuffer> V8StackTraceId::ToString() { ...@@ -177,7 +177,6 @@ std::unique_ptr<StringBuffer> V8StackTraceId::ToString() {
StackFrame::StackFrame(v8::Isolate* isolate, v8::Local<v8::StackFrame> v8Frame) StackFrame::StackFrame(v8::Isolate* isolate, v8::Local<v8::StackFrame> v8Frame)
: m_functionName(toProtocolString(isolate, v8Frame->GetFunctionName())), : m_functionName(toProtocolString(isolate, v8Frame->GetFunctionName())),
m_scriptId(v8Frame->GetScriptId()), m_scriptId(v8Frame->GetScriptId()),
m_scriptIdAsString(String16::fromInteger(v8Frame->GetScriptId())),
m_sourceURL( m_sourceURL(
toProtocolString(isolate, v8Frame->GetScriptNameOrSourceURL())), toProtocolString(isolate, v8Frame->GetScriptNameOrSourceURL())),
m_lineNumber(v8Frame->GetLineNumber() - 1), m_lineNumber(v8Frame->GetLineNumber() - 1),
...@@ -192,10 +191,6 @@ const String16& StackFrame::functionName() const { return m_functionName; } ...@@ -192,10 +191,6 @@ const String16& StackFrame::functionName() const { return m_functionName; }
int StackFrame::scriptId() const { return m_scriptId; } int StackFrame::scriptId() const { return m_scriptId; }
const String16& StackFrame::scriptIdAsString() const {
return m_scriptIdAsString;
}
const String16& StackFrame::sourceURL() const { return m_sourceURL; } const String16& StackFrame::sourceURL() const { return m_sourceURL; }
int StackFrame::lineNumber() const { return m_lineNumber; } int StackFrame::lineNumber() const { return m_lineNumber; }
...@@ -324,13 +319,7 @@ int V8StackTraceImpl::topColumnNumber() const { ...@@ -324,13 +319,7 @@ int V8StackTraceImpl::topColumnNumber() const {
return m_frames[0]->columnNumber() + 1; return m_frames[0]->columnNumber() + 1;
} }
StringView V8StackTraceImpl::topScriptId() const { int V8StackTraceImpl::topScriptId() const { return m_frames[0]->scriptId(); }
return toStringView(m_frames[0]->scriptIdAsString());
}
int V8StackTraceImpl::topScriptIdAsInteger() const {
return m_frames[0]->scriptId();
}
StringView V8StackTraceImpl::topFunctionName() const { StringView V8StackTraceImpl::topFunctionName() const {
return toStringView(m_frames[0]->functionName()); return toStringView(m_frames[0]->functionName());
......
...@@ -27,7 +27,6 @@ class StackFrame { ...@@ -27,7 +27,6 @@ class StackFrame {
const String16& functionName() const; const String16& functionName() const;
int scriptId() const; int scriptId() const;
const String16& scriptIdAsString() const;
const String16& sourceURL() const; const String16& sourceURL() const;
int lineNumber() const; // 0-based. int lineNumber() const; // 0-based.
int columnNumber() const; // 0-based. int columnNumber() const; // 0-based.
...@@ -38,7 +37,6 @@ class StackFrame { ...@@ -38,7 +37,6 @@ class StackFrame {
private: private:
String16 m_functionName; String16 m_functionName;
int m_scriptId; int m_scriptId;
String16 m_scriptIdAsString;
String16 m_sourceURL; String16 m_sourceURL;
int m_lineNumber; // 0-based. int m_lineNumber; // 0-based.
int m_columnNumber; // 0-based. int m_columnNumber; // 0-based.
...@@ -75,8 +73,7 @@ class V8StackTraceImpl : public V8StackTrace { ...@@ -75,8 +73,7 @@ class V8StackTraceImpl : public V8StackTrace {
StringView topSourceURL() const override; StringView topSourceURL() const override;
int topLineNumber() const override; // 1-based. int topLineNumber() const override; // 1-based.
int topColumnNumber() const override; // 1-based. int topColumnNumber() const override; // 1-based.
StringView topScriptId() const override; int topScriptId() const override;
int topScriptIdAsInteger() const override;
StringView topFunctionName() const override; StringView topFunctionName() const override;
std::unique_ptr<protocol::Runtime::API::StackTrace> buildInspectorObject() std::unique_ptr<protocol::Runtime::API::StackTrace> buildInspectorObject()
const override; const override;
......
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