Make StringBuffer::string return a StringView instead of a reference.

A StringView is pretty light, so this should be similar to
how absl::string_view is typically used, e.g. see the guidance here:
https://github.com/abseil/abseil-cpp/blob/master/absl/strings/string_view.h
I suspect this reasoning holds even though StringView (defined
just above StringBuffer in v8-inspector.h) carries an additional bool.
This yields a small simplification of the StringBuffer implementations.

Change-Id: I03f850049afe2327913070838f39649fcdfa6fa8
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2045110
Commit-Queue: Yang Guo <yangguo@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#66858}
This commit is contained in:
Johannes Henkel 2020-02-07 15:26:48 -08:00 committed by Commit Bot
parent 7a3c7b155a
commit 844fe8f7d9
8 changed files with 90 additions and 96 deletions

View File

@ -65,15 +65,15 @@ class V8_EXPORT StringView {
class V8_EXPORT StringBuffer {
public:
virtual ~StringBuffer() = default;
virtual const StringView& string() = 0;
virtual StringView string() const = 0;
// This method copies contents.
static std::unique_ptr<StringBuffer> create(const StringView&);
static std::unique_ptr<StringBuffer> create(StringView);
};
class V8_EXPORT V8ContextInfo {
public:
V8ContextInfo(v8::Local<v8::Context> context, int contextGroupId,
const StringView& humanReadableName)
StringView humanReadableName)
: context(context),
contextGroupId(contextGroupId),
humanReadableName(humanReadableName),
@ -132,37 +132,36 @@ class V8_EXPORT V8InspectorSession {
virtual void addInspectedObject(std::unique_ptr<Inspectable>) = 0;
// Dispatching protocol messages.
static bool canDispatchMethod(const StringView& method);
virtual void dispatchProtocolMessage(const StringView& message) = 0;
static bool canDispatchMethod(StringView method);
virtual void dispatchProtocolMessage(StringView message) = 0;
virtual std::vector<uint8_t> state() = 0;
virtual std::vector<std::unique_ptr<protocol::Schema::API::Domain>>
supportedDomains() = 0;
// Debugger actions.
virtual void schedulePauseOnNextStatement(const StringView& breakReason,
const StringView& breakDetails) = 0;
virtual void schedulePauseOnNextStatement(StringView breakReason,
StringView breakDetails) = 0;
virtual void cancelPauseOnNextStatement() = 0;
virtual void breakProgram(const StringView& breakReason,
const StringView& breakDetails) = 0;
virtual void breakProgram(StringView breakReason,
StringView breakDetails) = 0;
virtual void setSkipAllPauses(bool) = 0;
virtual void resume(bool setTerminateOnResume = false) = 0;
virtual void stepOver() = 0;
virtual std::vector<std::unique_ptr<protocol::Debugger::API::SearchMatch>>
searchInTextByLines(const StringView& text, const StringView& query,
bool caseSensitive, bool isRegex) = 0;
searchInTextByLines(StringView text, StringView query, bool caseSensitive,
bool isRegex) = 0;
// Remote objects.
virtual std::unique_ptr<protocol::Runtime::API::RemoteObject> wrapObject(
v8::Local<v8::Context>, v8::Local<v8::Value>, const StringView& groupName,
v8::Local<v8::Context>, v8::Local<v8::Value>, StringView groupName,
bool generatePreview) = 0;
virtual bool unwrapObject(std::unique_ptr<StringBuffer>* error,
const StringView& objectId, v8::Local<v8::Value>*,
StringView objectId, v8::Local<v8::Value>*,
v8::Local<v8::Context>*,
std::unique_ptr<StringBuffer>* objectGroup) = 0;
virtual void releaseObjectGroup(const StringView&) = 0;
virtual void triggerPreciseCoverageDeltaUpdate(
const StringView& occassion) = 0;
virtual void releaseObjectGroup(StringView) = 0;
virtual void triggerPreciseCoverageDeltaUpdate(StringView occassion) = 0;
};
class V8_EXPORT V8InspectorClient {
@ -240,7 +239,7 @@ struct V8_EXPORT V8StackTraceId {
V8StackTraceId(uintptr_t id, const std::pair<int64_t, int64_t> debugger_id);
V8StackTraceId(uintptr_t id, const std::pair<int64_t, int64_t> debugger_id,
bool should_pause);
explicit V8StackTraceId(const StringView&);
explicit V8StackTraceId(StringView);
V8StackTraceId& operator=(const V8StackTraceId&) = default;
V8StackTraceId& operator=(V8StackTraceId&&) noexcept = default;
~V8StackTraceId() = default;
@ -265,26 +264,26 @@ class V8_EXPORT V8Inspector {
virtual void idleFinished() = 0;
// Async stack traces instrumentation.
virtual void asyncTaskScheduled(const StringView& taskName, void* task,
virtual void asyncTaskScheduled(StringView taskName, void* task,
bool recurring) = 0;
virtual void asyncTaskCanceled(void* task) = 0;
virtual void asyncTaskStarted(void* task) = 0;
virtual void asyncTaskFinished(void* task) = 0;
virtual void allAsyncTasksCanceled() = 0;
virtual V8StackTraceId storeCurrentStackTrace(
const StringView& description) = 0;
virtual V8StackTraceId storeCurrentStackTrace(StringView description) = 0;
virtual void externalAsyncTaskStarted(const V8StackTraceId& parent) = 0;
virtual void externalAsyncTaskFinished(const V8StackTraceId& parent) = 0;
// Exceptions instrumentation.
virtual unsigned exceptionThrown(
v8::Local<v8::Context>, const StringView& message,
v8::Local<v8::Value> exception, const StringView& detailedMessage,
const StringView& url, unsigned lineNumber, unsigned columnNumber,
std::unique_ptr<V8StackTrace>, int scriptId) = 0;
virtual unsigned exceptionThrown(v8::Local<v8::Context>, StringView message,
v8::Local<v8::Value> exception,
StringView detailedMessage, StringView url,
unsigned lineNumber, unsigned columnNumber,
std::unique_ptr<V8StackTrace>,
int scriptId) = 0;
virtual void exceptionRevoked(v8::Local<v8::Context>, unsigned exceptionId,
const StringView& message) = 0;
StringView message) = 0;
// Connection.
class V8_EXPORT Channel {
@ -295,8 +294,9 @@ class V8_EXPORT V8Inspector {
virtual void sendNotification(std::unique_ptr<StringBuffer> message) = 0;
virtual void flushProtocolNotifications() = 0;
};
virtual std::unique_ptr<V8InspectorSession> connect(
int contextGroupId, Channel*, const StringView& state) = 0;
virtual std::unique_ptr<V8InspectorSession> connect(int contextGroupId,
Channel*,
StringView state) = 0;
// API methods.
virtual std::unique_ptr<V8StackTrace> createStackTrace(

View File

@ -100,41 +100,38 @@ namespace {
// default-constructed StringView instance.
class EmptyStringBuffer : public StringBuffer {
public:
const StringView& string() override { return string_; }
private:
StringView string_;
StringView string() const override { return StringView(); }
};
// Contains LATIN1 text data or CBOR encoded binary data in a vector.
class StringBuffer8 : public StringBuffer {
public:
explicit StringBuffer8(std::vector<uint8_t> data)
: data_(std::move(data)), string_(data_.data(), data_.size()) {}
explicit StringBuffer8(std::vector<uint8_t> data) : data_(std::move(data)) {}
const StringView& string() override { return string_; }
StringView string() const override {
return StringView(data_.data(), data_.size());
}
private:
std::vector<uint8_t> data_;
StringView string_;
};
// Contains a 16 bit string (String16).
class StringBuffer16 : public StringBuffer {
public:
explicit StringBuffer16(String16 data)
: data_(std::move(data)), string_(data_.characters16(), data_.length()) {}
explicit StringBuffer16(String16 data) : data_(std::move(data)) {}
const StringView& string() override { return string_; }
StringView string() const override {
return StringView(data_.characters16(), data_.length());
}
private:
String16 data_;
StringView string_;
};
} // namespace
// static
std::unique_ptr<StringBuffer> StringBuffer::create(const StringView& string) {
std::unique_ptr<StringBuffer> StringBuffer::create(StringView string) {
if (string.length() == 0) return std::make_unique<EmptyStringBuffer>();
if (string.is8Bit()) {
return std::make_unique<StringBuffer8>(std::vector<uint8_t>(

View File

@ -155,8 +155,7 @@ std::unique_ptr<V8StackTrace> V8InspectorImpl::createStackTrace(
}
std::unique_ptr<V8InspectorSession> V8InspectorImpl::connect(
int contextGroupId, V8Inspector::Channel* channel,
const StringView& state) {
int contextGroupId, V8Inspector::Channel* channel, StringView state) {
int sessionId = ++m_lastSessionId;
std::unique_ptr<V8InspectorSessionImpl> session =
V8InspectorSessionImpl::create(this, contextGroupId, sessionId, channel,
@ -256,9 +255,9 @@ void V8InspectorImpl::idleStarted() { m_isolate->SetIdle(true); }
void V8InspectorImpl::idleFinished() { m_isolate->SetIdle(false); }
unsigned V8InspectorImpl::exceptionThrown(
v8::Local<v8::Context> context, const StringView& message,
v8::Local<v8::Value> exception, const StringView& detailedMessage,
const StringView& url, unsigned lineNumber, unsigned columnNumber,
v8::Local<v8::Context> context, StringView message,
v8::Local<v8::Value> exception, StringView detailedMessage, StringView url,
unsigned lineNumber, unsigned columnNumber,
std::unique_ptr<V8StackTrace> stackTrace, int scriptId) {
int groupId = contextGroupId(context);
if (!groupId || m_muteExceptionsMap[groupId]) return 0;
@ -277,7 +276,7 @@ unsigned V8InspectorImpl::exceptionThrown(
void V8InspectorImpl::exceptionRevoked(v8::Local<v8::Context> context,
unsigned exceptionId,
const StringView& message) {
StringView message) {
int groupId = contextGroupId(context);
if (!groupId) return;
@ -292,8 +291,7 @@ std::unique_ptr<V8StackTrace> V8InspectorImpl::captureStackTrace(
return m_debugger->captureStackTrace(fullStack);
}
V8StackTraceId V8InspectorImpl::storeCurrentStackTrace(
const StringView& description) {
V8StackTraceId V8InspectorImpl::storeCurrentStackTrace(StringView description) {
return m_debugger->storeCurrentStackTrace(description);
}
@ -305,7 +303,7 @@ void V8InspectorImpl::externalAsyncTaskFinished(const V8StackTraceId& parent) {
m_debugger->externalAsyncTaskFinished(parent);
}
void V8InspectorImpl::asyncTaskScheduled(const StringView& taskName, void* task,
void V8InspectorImpl::asyncTaskScheduled(StringView taskName, void* task,
bool recurring) {
if (!task) return;
m_debugger->asyncTaskScheduled(taskName, task, recurring);

View File

@ -77,7 +77,7 @@ class V8InspectorImpl : public V8Inspector {
// V8Inspector implementation.
std::unique_ptr<V8InspectorSession> connect(int contextGroupId,
V8Inspector::Channel*,
const StringView& state) override;
StringView state) override;
void contextCreated(const V8ContextInfo&) override;
void contextDestroyed(v8::Local<v8::Context>) override;
v8::MaybeLocal<v8::Context> contextById(int contextId) override;
@ -85,25 +85,25 @@ class V8InspectorImpl : public V8Inspector {
void resetContextGroup(int contextGroupId) override;
void idleStarted() override;
void idleFinished() override;
unsigned exceptionThrown(v8::Local<v8::Context>, const StringView& message,
unsigned exceptionThrown(v8::Local<v8::Context>, StringView message,
v8::Local<v8::Value> exception,
const StringView& detailedMessage,
const StringView& url, unsigned lineNumber,
unsigned columnNumber, std::unique_ptr<V8StackTrace>,
StringView detailedMessage, StringView url,
unsigned lineNumber, unsigned columnNumber,
std::unique_ptr<V8StackTrace>,
int scriptId) override;
void exceptionRevoked(v8::Local<v8::Context>, unsigned exceptionId,
const StringView& message) override;
StringView message) override;
std::unique_ptr<V8StackTrace> createStackTrace(
v8::Local<v8::StackTrace>) override;
std::unique_ptr<V8StackTrace> captureStackTrace(bool fullStack) override;
void asyncTaskScheduled(const StringView& taskName, void* task,
void asyncTaskScheduled(StringView taskName, void* task,
bool recurring) override;
void asyncTaskCanceled(void* task) override;
void asyncTaskStarted(void* task) override;
void asyncTaskFinished(void* task) override;
void allAsyncTasksCanceled() override;
V8StackTraceId storeCurrentStackTrace(const StringView& description) override;
V8StackTraceId storeCurrentStackTrace(StringView description) override;
void externalAsyncTaskStarted(const V8StackTraceId& parent) override;
void externalAsyncTaskFinished(const V8StackTraceId& parent) override;

View File

@ -33,12 +33,12 @@ using v8_crdtp::cbor::CheckCBORMessage;
using v8_crdtp::json::ConvertCBORToJSON;
using v8_crdtp::json::ConvertJSONToCBOR;
bool IsCBORMessage(const StringView& msg) {
bool IsCBORMessage(StringView msg) {
return msg.is8Bit() && msg.length() >= 2 && msg.characters8()[0] == 0xd8 &&
msg.characters8()[1] == 0x5a;
}
Status ConvertToCBOR(const StringView& state, std::vector<uint8_t>* cbor) {
Status ConvertToCBOR(StringView state, std::vector<uint8_t>* cbor) {
return state.is8Bit()
? ConvertJSONToCBOR(
span<uint8_t>(state.characters8(), state.length()), cbor)
@ -46,7 +46,7 @@ Status ConvertToCBOR(const StringView& state, std::vector<uint8_t>* cbor) {
span<uint16_t>(state.characters16(), state.length()), cbor);
}
std::unique_ptr<protocol::DictionaryValue> ParseState(const StringView& state) {
std::unique_ptr<protocol::DictionaryValue> ParseState(StringView state) {
std::vector<uint8_t> converted;
span<uint8_t> cbor;
if (IsCBORMessage(state))
@ -63,7 +63,7 @@ std::unique_ptr<protocol::DictionaryValue> ParseState(const StringView& state) {
} // namespace
// static
bool V8InspectorSession::canDispatchMethod(const StringView& method) {
bool V8InspectorSession::canDispatchMethod(StringView method) {
return stringViewStartsWith(method,
protocol::Runtime::Metainfo::commandPrefix) ||
stringViewStartsWith(method,
@ -85,7 +85,7 @@ int V8ContextInfo::executionContextId(v8::Local<v8::Context> context) {
std::unique_ptr<V8InspectorSessionImpl> V8InspectorSessionImpl::create(
V8InspectorImpl* inspector, int contextGroupId, int sessionId,
V8Inspector::Channel* channel, const StringView& state) {
V8Inspector::Channel* channel, StringView state) {
return std::unique_ptr<V8InspectorSessionImpl>(new V8InspectorSessionImpl(
inspector, contextGroupId, sessionId, channel, state));
}
@ -94,7 +94,7 @@ V8InspectorSessionImpl::V8InspectorSessionImpl(V8InspectorImpl* inspector,
int contextGroupId,
int sessionId,
V8Inspector::Channel* channel,
const StringView& savedState)
StringView savedState)
: m_contextGroupId(contextGroupId),
m_sessionId(sessionId),
m_inspector(inspector),
@ -242,7 +242,7 @@ Response V8InspectorSessionImpl::findInjectedScript(
return findInjectedScript(objectId->contextId(), injectedScript);
}
void V8InspectorSessionImpl::releaseObjectGroup(const StringView& objectGroup) {
void V8InspectorSessionImpl::releaseObjectGroup(StringView objectGroup) {
releaseObjectGroup(toString16(objectGroup));
}
@ -256,7 +256,7 @@ void V8InspectorSessionImpl::releaseObjectGroup(const String16& objectGroup) {
}
bool V8InspectorSessionImpl::unwrapObject(
std::unique_ptr<StringBuffer>* error, const StringView& objectId,
std::unique_ptr<StringBuffer>* error, StringView objectId,
v8::Local<v8::Value>* object, v8::Local<v8::Context>* context,
std::unique_ptr<StringBuffer>* objectGroup) {
String16 objectGroupString;
@ -294,8 +294,7 @@ Response V8InspectorSessionImpl::unwrapObject(const String16& objectId,
std::unique_ptr<protocol::Runtime::API::RemoteObject>
V8InspectorSessionImpl::wrapObject(v8::Local<v8::Context> context,
v8::Local<v8::Value> value,
const StringView& groupName,
bool generatePreview) {
StringView groupName, bool generatePreview) {
return wrapObject(context, value, toString16(groupName), generatePreview);
}
@ -342,8 +341,7 @@ void V8InspectorSessionImpl::reportAllContexts(V8RuntimeAgentImpl* agent) {
});
}
void V8InspectorSessionImpl::dispatchProtocolMessage(
const StringView& message) {
void V8InspectorSessionImpl::dispatchProtocolMessage(StringView message) {
using v8_crdtp::span;
using v8_crdtp::SpanFrom;
span<uint8_t> cbor;
@ -434,7 +432,7 @@ V8InspectorSession::Inspectable* V8InspectorSessionImpl::inspectedObject(
}
void V8InspectorSessionImpl::schedulePauseOnNextStatement(
const StringView& breakReason, const StringView& breakDetails) {
StringView breakReason, StringView breakDetails) {
std::vector<uint8_t> cbor;
ConvertToCBOR(breakDetails, &cbor);
m_debuggerAgent->schedulePauseOnNextStatement(
@ -447,8 +445,8 @@ void V8InspectorSessionImpl::cancelPauseOnNextStatement() {
m_debuggerAgent->cancelPauseOnNextStatement();
}
void V8InspectorSessionImpl::breakProgram(const StringView& breakReason,
const StringView& breakDetails) {
void V8InspectorSessionImpl::breakProgram(StringView breakReason,
StringView breakDetails) {
std::vector<uint8_t> cbor;
ConvertToCBOR(breakDetails, &cbor);
m_debuggerAgent->breakProgram(
@ -468,8 +466,7 @@ void V8InspectorSessionImpl::resume(bool terminateOnResume) {
void V8InspectorSessionImpl::stepOver() { m_debuggerAgent->stepOver(); }
std::vector<std::unique_ptr<protocol::Debugger::API::SearchMatch>>
V8InspectorSessionImpl::searchInTextByLines(const StringView& text,
const StringView& query,
V8InspectorSessionImpl::searchInTextByLines(StringView text, StringView query,
bool caseSensitive, bool isRegex) {
// TODO(dgozman): search may operate on StringView and avoid copying |text|.
std::vector<std::unique_ptr<protocol::Debugger::SearchMatch>> matches =
@ -482,7 +479,7 @@ V8InspectorSessionImpl::searchInTextByLines(const StringView& text,
}
void V8InspectorSessionImpl::triggerPreciseCoverageDeltaUpdate(
const StringView& occassion) {
StringView occassion) {
m_profilerAgent->triggerPreciseCoverageDeltaUpdate(toString16(occassion));
}

View File

@ -32,9 +32,11 @@ using protocol::Response;
class V8InspectorSessionImpl : public V8InspectorSession,
public protocol::FrontendChannel {
public:
static std::unique_ptr<V8InspectorSessionImpl> create(
V8InspectorImpl*, int contextGroupId, int sessionId,
V8Inspector::Channel*, const StringView& state);
static std::unique_ptr<V8InspectorSessionImpl> create(V8InspectorImpl*,
int contextGroupId,
int sessionId,
V8Inspector::Channel*,
StringView state);
~V8InspectorSessionImpl() override;
V8InspectorImpl* inspector() const { return m_inspector; }
@ -64,39 +66,38 @@ class V8InspectorSessionImpl : public V8InspectorSession,
void releaseObjectGroup(const String16& objectGroup);
// V8InspectorSession implementation.
void dispatchProtocolMessage(const StringView& message) override;
void dispatchProtocolMessage(StringView message) override;
std::vector<uint8_t> state() override;
std::vector<std::unique_ptr<protocol::Schema::API::Domain>> supportedDomains()
override;
void addInspectedObject(
std::unique_ptr<V8InspectorSession::Inspectable>) override;
void schedulePauseOnNextStatement(const StringView& breakReason,
const StringView& breakDetails) override;
void schedulePauseOnNextStatement(StringView breakReason,
StringView breakDetails) override;
void cancelPauseOnNextStatement() override;
void breakProgram(const StringView& breakReason,
const StringView& breakDetails) override;
void breakProgram(StringView breakReason, StringView breakDetails) override;
void setSkipAllPauses(bool) override;
void resume(bool terminateOnResume = false) override;
void stepOver() override;
std::vector<std::unique_ptr<protocol::Debugger::API::SearchMatch>>
searchInTextByLines(const StringView& text, const StringView& query,
bool caseSensitive, bool isRegex) override;
void releaseObjectGroup(const StringView& objectGroup) override;
bool unwrapObject(std::unique_ptr<StringBuffer>*, const StringView& objectId,
searchInTextByLines(StringView text, StringView query, bool caseSensitive,
bool isRegex) override;
void releaseObjectGroup(StringView objectGroup) override;
bool unwrapObject(std::unique_ptr<StringBuffer>*, StringView objectId,
v8::Local<v8::Value>*, v8::Local<v8::Context>*,
std::unique_ptr<StringBuffer>* objectGroup) override;
std::unique_ptr<protocol::Runtime::API::RemoteObject> wrapObject(
v8::Local<v8::Context>, v8::Local<v8::Value>, const StringView& groupName,
v8::Local<v8::Context>, v8::Local<v8::Value>, StringView groupName,
bool generatePreview) override;
V8InspectorSession::Inspectable* inspectedObject(unsigned num);
static const unsigned kInspectedObjectBufferSize = 5;
void triggerPreciseCoverageDeltaUpdate(const StringView& occassion) override;
void triggerPreciseCoverageDeltaUpdate(StringView occassion) override;
private:
V8InspectorSessionImpl(V8InspectorImpl*, int contextGroupId, int sessionId,
V8Inspector::Channel*, const StringView& state);
V8Inspector::Channel*, StringView state);
protocol::DictionaryValue* agentState(const String16& name);
// protocol::FrontendChannel implementation.

View File

@ -128,7 +128,7 @@ V8StackTraceId::V8StackTraceId(uintptr_t id,
bool should_pause)
: id(id), debugger_id(debugger_id), should_pause(should_pause) {}
V8StackTraceId::V8StackTraceId(const StringView& json)
V8StackTraceId::V8StackTraceId(StringView json)
: id(0), debugger_id(V8DebuggerId().pair()) {
if (json.length() == 0) return;
std::vector<uint8_t> cbor;

View File

@ -457,13 +457,14 @@ namespace {
class StringBufferImpl : public v8_inspector::StringBuffer {
public:
StringBufferImpl(v8::Isolate* isolate, v8::Local<v8::String> string)
: data_(ToVector(isolate, string)),
view_(data_.begin(), data_.length()) {}
const v8_inspector::StringView& string() override { return view_; }
: data_(ToVector(isolate, string)) {}
v8_inspector::StringView string() const override {
return v8_inspector::StringView(data_.begin(), data_.length());
}
private:
v8::internal::Vector<uint16_t> data_;
v8_inspector::StringView view_;
};
} // anonymous namespace