AuroraMiddleware/v8
1
0
Fork 0
Code Issues 2 Pull Requests Releases Wiki Activity

[cleanup] Replace raw pointer with std::weak_ptr for EvaluateCallback

Browse Source 
This CL replaces the raw pointer in the `ProtocolPromiseHandler` to the
`EvaluateCallback` with a std::weak_ptr. This better matches the
semantics. If the `ProtocolPromiseHandler` is able to lock the
shared_ptr, we still have to remove it from the `InjectedScript`
since the `ProtocolPromiseHandler` sends the response.

R=jarin@chormium.org

Bug: chromium:1366843
Change-Id: I7f371dbd5423f88105981996584ccba5f814dcdb
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3933352
Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Commit-Queue: Simon Zünd <szuend@chromium.org>
Cr-Commit-Position: refs/heads/main@{#83508}
This commit is contained in:
Simon Zünd 2022-10-04 08:04:56 +02:00 committed by V8 LUCI CQ
parent 94e8282325
commit ed5db2eaec
3 changed files with 33 additions and 37 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

59
src/inspector/injected-script.cc
View File

@ -74,24 +74,24 @@ class InjectedScript::ProtocolPromiseHandler {
v8::Local<v8::Context> context, v8::Local<v8::Value> value,
int executionContextId, const String16& objectGroup,
WrapMode wrapMode, bool replMode, bool throwOnSideEffect,
EvaluateCallback* callback) {
std::weak_ptr<EvaluateCallback> callback) {
InjectedScript::ContextScope scope(session, executionContextId);
Response response = scope.initialize();
if (!response.IsSuccess()) return;
v8::Local<v8::Promise::Resolver> resolver;
if (!v8::Promise::Resolver::New(context).ToLocal(&resolver)) {
std::unique_ptr<EvaluateCallback> ownedCallback =
scope.injectedScript()->takeEvaluateCallback(callback);
CHECK(ownedCallback);
ownedCallback->sendFailure(Response::InternalError());
std::shared_ptr<EvaluateCallback> cb = callback.lock();
CHECK(cb);
scope.injectedScript()->deleteEvaluateCallback(cb);
cb->sendFailure(Response::InternalError());
return;
}
if (!resolver->Resolve(context, value).FromMaybe(false)) {
std::unique_ptr<EvaluateCallback> ownedCallback =
scope.injectedScript()->takeEvaluateCallback(callback);
CHECK(ownedCallback);
ownedCallback->sendFailure(Response::InternalError());
std::shared_ptr<EvaluateCallback> cb = callback.lock();
CHECK(cb);
scope.injectedScript()->deleteEvaluateCallback(cb);
cb->sendFailure(Response::InternalError());
return;
}
@ -117,10 +117,10 @@ class InjectedScript::ProtocolPromiseHandler {
// Re-initialize after returning from JS.
Response response = scope.initialize();
if (!response.IsSuccess()) return;
std::unique_ptr<EvaluateCallback> ownedCallback =
scope.injectedScript()->takeEvaluateCallback(callback);
if (!ownedCallback) return;
ownedCallback->sendFailure(Response::InternalError());
std::shared_ptr<EvaluateCallback> cb = callback.lock();
if (!cb) return;
scope.injectedScript()->deleteEvaluateCallback(cb);
cb->sendFailure(Response::InternalError());
}
}
@ -157,7 +157,8 @@ class InjectedScript::ProtocolPromiseHandler {
ProtocolPromiseHandler(V8InspectorSessionImpl* session,
int executionContextId, const String16& objectGroup,
WrapMode wrapMode, bool replMode,
bool throwOnSideEffect, EvaluateCallback* callback,
bool throwOnSideEffect,
std::weak_ptr<EvaluateCallback> callback,
v8::MaybeLocal<v8::Promise> maybeEvaluationResult)
: m_inspector(session->inspector()),
m_sessionId(session->sessionId()),
@ -198,9 +199,9 @@ class InjectedScript::ProtocolPromiseHandler {
Response response = scope.initialize();
if (!response.IsSuccess()) return;
std::unique_ptr<EvaluateCallback> callback =
scope.injectedScript()->takeEvaluateCallback(m_callback);
std::shared_ptr<EvaluateCallback> callback = m_callback.lock();
if (!callback) return;
scope.injectedScript()->deleteEvaluateCallback(callback);
// In REPL mode the result is additionally wrapped in an object.
// The evaluation result can be found at ".repl_result".
@ -242,9 +243,9 @@ class InjectedScript::ProtocolPromiseHandler {
InjectedScript::ContextScope scope(session, m_executionContextId);
Response response = scope.initialize();
if (!response.IsSuccess()) return;
std::unique_ptr<EvaluateCallback> callback =
scope.injectedScript()->takeEvaluateCallback(m_callback);
std::shared_ptr<EvaluateCallback> callback = m_callback.lock();
if (!callback) return;
scope.injectedScript()->deleteEvaluateCallback(callback);
std::unique_ptr<protocol::Runtime::RemoteObject> wrappedValue;
response = scope.injectedScript()->wrapObject(result, m_objectGroup,
m_wrapMode, &wrappedValue);
@ -337,9 +338,9 @@ class InjectedScript::ProtocolPromiseHandler {
InjectedScript::ContextScope scope(session, m_executionContextId);
Response response = scope.initialize();
if (!response.IsSuccess()) return;
std::unique_ptr<EvaluateCallback> callback =
scope.injectedScript()->takeEvaluateCallback(m_callback);
std::shared_ptr<EvaluateCallback> callback = m_callback.lock();
if (!callback) return;
scope.injectedScript()->deleteEvaluateCallback(callback);
callback->sendFailure(Response::ServerError("Promise was collected"));
}
@ -351,7 +352,7 @@ class InjectedScript::ProtocolPromiseHandler {
WrapMode m_wrapMode;
bool m_replMode;
bool m_throwOnSideEffect;
EvaluateCallback* m_callback;
std::weak_ptr<EvaluateCallback> m_callback;
v8::Global<v8::External> m_wrapper;
v8::Global<v8::Promise> m_evaluationResult;
};
@ -669,21 +670,20 @@ std::unique_ptr<protocol::Runtime::RemoteObject> InjectedScript::wrapTable(
void InjectedScript::addPromiseCallback(
V8InspectorSessionImpl* session, v8::MaybeLocal<v8::Value> value,
const String16& objectGroup, WrapMode wrapMode, bool replMode,
bool throwOnSideEffect, std::unique_ptr<EvaluateCallback> callback) {
bool throwOnSideEffect, std::shared_ptr<EvaluateCallback> callback) {
if (value.IsEmpty()) {
callback->sendFailure(Response::InternalError());
return;
}
EvaluateCallback* cb = callback.release();
m_evaluateCallbacks.insert(cb);
m_evaluateCallbacks.insert(callback);
v8::MicrotasksScope microtasksScope(m_context->isolate(),
v8::MicrotasksScope::kRunMicrotasks);
ProtocolPromiseHandler::add(session, m_context->context(),
value.ToLocalChecked(), m_context->contextId(),
objectGroup, wrapMode, replMode,
throwOnSideEffect, cb);
throwOnSideEffect, callback);
// Do not add any code here! `this` might be invalid.
// `ProtocolPromiseHandler::add` calls into JS which could kill this
// `InjectedScript`.
@ -693,18 +693,15 @@ void InjectedScript::discardEvaluateCallbacks() {
for (auto& callback : m_evaluateCallbacks) {
callback->sendFailure(
Response::ServerError("Execution context was destroyed."));
delete callback;
}
m_evaluateCallbacks.clear();
}
std::unique_ptr<EvaluateCallback> InjectedScript::takeEvaluateCallback(
EvaluateCallback* callback) {
void InjectedScript::deleteEvaluateCallback(
std::shared_ptr<EvaluateCallback> callback) {
auto it = m_evaluateCallbacks.find(callback);
if (it == m_evaluateCallbacks.end()) return nullptr;
std::unique_ptr<EvaluateCallback> value(*it);
CHECK_NE(it, m_evaluateCallbacks.end());
m_evaluateCallbacks.erase(it);
return value;
}
Response InjectedScript::findObject(const RemoteObjectId& objectId,

7
src/inspector/injected-script.h
View File

@ -114,7 +114,7 @@ class InjectedScript final {
v8::MaybeLocal<v8::Value> value,
const String16& objectGroup, WrapMode wrapMode,
bool replMode, bool throwOnSideEffect,
std::unique_ptr<EvaluateCallback> callback);
std::shared_ptr<EvaluateCallback> callback);
Response findObject(const RemoteObjectId&, v8::Local<v8::Value>*) const;
String16 objectGroupName(const RemoteObjectId&) const;
@ -230,8 +230,7 @@ class InjectedScript final {
class ProtocolPromiseHandler;
void discardEvaluateCallbacks();
std::unique_ptr<EvaluateCallback> takeEvaluateCallback(
EvaluateCallback* callback);
void deleteEvaluateCallback(std::shared_ptr<EvaluateCallback> callback);
Response addExceptionToDetails(
v8::Local<v8::Value> exception,
protocol::Runtime::ExceptionDetails* exceptionDetails,
@ -245,7 +244,7 @@ class InjectedScript final {
std::unordered_map<int, v8::Global<v8::Value>> m_idToWrappedObject;
std::unordered_map<int, String16> m_idToObjectGroupName;
std::unordered_map<String16, std::vector<int>> m_nameToObjectGroup;
std::unordered_set<EvaluateCallback*> m_evaluateCallbacks;
std::unordered_set<std::shared_ptr<EvaluateCallback>> m_evaluateCallbacks;
bool m_customPreviewEnabled = false;
};

4
src/inspector/v8-runtime-agent-impl.cc
View File

@ -70,9 +70,9 @@ namespace {
template <typename ProtocolCallback>
class EvaluateCallbackWrapper : public EvaluateCallback {
public:
static std::unique_ptr<EvaluateCallback> wrap(
static std::shared_ptr<EvaluateCallback> wrap(
std::unique_ptr<ProtocolCallback> callback) {
return std::unique_ptr<EvaluateCallback>(
return std::shared_ptr<EvaluateCallback>(
new EvaluateCallbackWrapper(std::move(callback)));
}
void sendSuccess(std::unique_ptr<protocol::Runtime::RemoteObject> result,