refactor: make ReplyChannel inherit from gin::Wrappable (#49339)

* refactor: make ReplyChannel inherit from gin::Wrappable

* chore: add kElectronReplyChannel to chore_add_electron_objects_to_wrappablepointertag.patch

* fix: use gin::PerIsolateData::DisposeObserver

* fix: lifetime issues

* chore: rm perisolatedata hook in favor of prefinalizer

---------

Co-authored-by: deepak1556 <hop2deep@gmail.com>
This commit is contained in:
Charles Kerr
2026-01-14 18:35:01 -06:00
committed by GitHub
parent 5d80264944
commit ddeb970f18
3 changed files with 66 additions and 39 deletions

View File

@@ -8,10 +8,10 @@ electron objects that extend gin::Wrappable and gets
allocated on the cpp heap allocated on the cpp heap
diff --git a/gin/public/wrappable_pointer_tags.h b/gin/public/wrappable_pointer_tags.h diff --git a/gin/public/wrappable_pointer_tags.h b/gin/public/wrappable_pointer_tags.h
index 573bcb2e56068a2ade6d8ab28964b077487874fd..0321ca6d3c7e1ed541cc1beffb20b1db3d03a0c8 100644 index 573bcb2e56068a2ade6d8ab28964b077487874fd..42add73062b723b03fc15ddcce905e4d5061c384 100644
--- a/gin/public/wrappable_pointer_tags.h --- a/gin/public/wrappable_pointer_tags.h
+++ b/gin/public/wrappable_pointer_tags.h +++ b/gin/public/wrappable_pointer_tags.h
@@ -74,7 +74,14 @@ enum WrappablePointerTag : uint16_t { @@ -74,7 +74,15 @@ enum WrappablePointerTag : uint16_t {
kTextInputControllerBindings, // content::TextInputControllerBindings kTextInputControllerBindings, // content::TextInputControllerBindings
kWebAXObjectProxy, // content::WebAXObjectProxy kWebAXObjectProxy, // content::WebAXObjectProxy
kWrappedExceptionHandler, // extensions::WrappedExceptionHandler kWrappedExceptionHandler, // extensions::WrappedExceptionHandler
@@ -21,6 +21,7 @@ index 573bcb2e56068a2ade6d8ab28964b077487874fd..0321ca6d3c7e1ed541cc1beffb20b1db
+ kElectronEvent, // gin_helper::internal::Event + kElectronEvent, // gin_helper::internal::Event
+ kElectronMenu, // electron::api::Menu + kElectronMenu, // electron::api::Menu
+ kElectronNetLog, // electron::api::NetLog + kElectronNetLog, // electron::api::NetLog
+ kElectronReplyChannel, // gin_helper::internal::ReplyChannel
+ kElectronSession, // electron::api::Session + kElectronSession, // electron::api::Session
+ kElectronWebRequest, // electron::api::WebRequest + kElectronWebRequest, // electron::api::WebRequest
+ kLastPointerTag = kElectronWebRequest, + kLastPointerTag = kElectronWebRequest,

View File

@@ -4,42 +4,24 @@
#include "shell/common/gin_helper/reply_channel.h" #include "shell/common/gin_helper/reply_channel.h"
#include "base/debug/stack_trace.h"
#include "gin/data_object_builder.h" #include "gin/data_object_builder.h"
#include "gin/object_template_builder.h" #include "gin/object_template_builder.h"
#include "shell/browser/javascript_environment.h" #include "shell/browser/javascript_environment.h"
#include "shell/common/gin_converters/blink_converter.h" #include "shell/common/gin_converters/blink_converter.h"
#include "shell/common/gin_helper/handle.h" #include "shell/common/gin_helper/handle.h"
#include "v8/include/cppgc/allocation.h"
#include "v8/include/v8-cppgc.h"
namespace gin_helper::internal { namespace gin_helper::internal {
// static const gin::WrapperInfo ReplyChannel::kWrapperInfo = {
using InvokeCallback = electron::mojom::ElectronApiIPC::InvokeCallback; {gin::kEmbedderNativeGin},
gin_helper::Handle<ReplyChannel> ReplyChannel::Create(v8::Isolate* isolate, gin::kElectronReplyChannel};
InvokeCallback callback) {
return gin_helper::CreateHandle(isolate,
new ReplyChannel(std::move(callback)));
}
gin::ObjectTemplateBuilder ReplyChannel::GetObjectTemplateBuilder( ReplyChannel::ReplyChannel(v8::Isolate* isolate, InvokeCallback callback)
v8::Isolate* isolate) { : callback_{std::move(callback)} {}
return gin_helper::DeprecatedWrappable<
ReplyChannel>::GetObjectTemplateBuilder(isolate)
.SetMethod("sendReply", &ReplyChannel::SendReply);
}
const char* ReplyChannel::GetTypeName() { ReplyChannel::~ReplyChannel() = default;
return "ReplyChannel";
}
ReplyChannel::ReplyChannel(InvokeCallback callback)
: callback_(std::move(callback)) {}
ReplyChannel::~ReplyChannel() {
if (callback_)
SendError(electron::JavascriptEnvironment::GetIsolate(),
std::move(callback_), "reply was never sent");
}
// static // static
void ReplyChannel::SendError(v8::Isolate* isolate, void ReplyChannel::SendError(v8::Isolate* isolate,
@@ -73,11 +55,43 @@ bool ReplyChannel::SendReplyImpl(v8::Isolate* isolate,
return true; return true;
} }
// static
ReplyChannel* ReplyChannel::Create(v8::Isolate* isolate,
InvokeCallback callback) {
return cppgc::MakeGarbageCollected<ReplyChannel>(
isolate->GetCppHeap()->GetAllocationHandle(), isolate,
std::move(callback));
}
const gin::WrapperInfo* ReplyChannel::wrapper_info() const {
return &kWrapperInfo;
}
const char* ReplyChannel::GetHumanReadableName() const {
return "Electron / ReplyChannel";
}
gin::ObjectTemplateBuilder ReplyChannel::GetObjectTemplateBuilder(
v8::Isolate* isolate) {
return gin::Wrappable<ReplyChannel>::GetObjectTemplateBuilder(isolate)
.SetMethod("sendReply", &ReplyChannel::SendReply);
}
bool ReplyChannel::SendReply(v8::Isolate* isolate, v8::Local<v8::Value> arg) { bool ReplyChannel::SendReply(v8::Isolate* isolate, v8::Local<v8::Value> arg) {
return SendReplyImpl(isolate, std::move(callback_), std::move(arg)); return SendReplyImpl(isolate, std::move(callback_), std::move(arg));
} }
gin::DeprecatedWrapperInfo ReplyChannel::kWrapperInfo = { void ReplyChannel::EnsureReplySent() {
gin::kEmbedderNativeGin}; v8::Isolate* isolate = v8::Isolate::GetCurrent();
if (callback_) {
DCHECK(isolate);
// Create a task since we cannot allocate on V8 heap during GC
// which is needed by ReplyChannel::SendError implementation.
base::SingleThreadTaskRunner::GetCurrentDefault()->PostTask(
FROM_HERE,
base::BindOnce(&ReplyChannel::SendError, isolate, std::move(callback_),
"reply was never sent"));
}
}
} // namespace gin_helper::internal } // namespace gin_helper::internal

View File

@@ -7,8 +7,9 @@
#include <string_view> #include <string_view>
#include "gin/wrappable.h"
#include "shell/common/api/api.mojom.h" #include "shell/common/api/api.mojom.h"
#include "shell/common/gin_helper/wrappable.h" #include "v8/include/cppgc/prefinalizer.h"
namespace gin_helper { namespace gin_helper {
template <typename T> template <typename T>
@@ -28,27 +29,38 @@ namespace gin_helper::internal {
// This object wraps the InvokeCallback so that if it gets GC'd by V8, we can // This object wraps the InvokeCallback so that if it gets GC'd by V8, we can
// still call the callback and send an error. Not doing so causes a Mojo DCHECK, // still call the callback and send an error. Not doing so causes a Mojo DCHECK,
// since Mojo requires callbacks to be called before they are destroyed. // since Mojo requires callbacks to be called before they are destroyed.
class ReplyChannel : public gin_helper::DeprecatedWrappable<ReplyChannel> { class ReplyChannel : public gin::Wrappable<ReplyChannel> {
CPPGC_USING_PRE_FINALIZER(ReplyChannel, EnsureReplySent);
public: public:
using InvokeCallback = electron::mojom::ElectronApiIPC::InvokeCallback; using InvokeCallback = electron::mojom::ElectronApiIPC::InvokeCallback;
static gin_helper::Handle<ReplyChannel> Create(v8::Isolate* isolate,
InvokeCallback callback); static ReplyChannel* Create(v8::Isolate* isolate, InvokeCallback callback);
// Constructor is public only to satisfy cppgc::MakeGarbageCollected.
// Callers should use Create() instead.
explicit ReplyChannel(v8::Isolate* isolate, InvokeCallback callback);
~ReplyChannel() override;
// disable copy
ReplyChannel(const ReplyChannel&) = delete;
ReplyChannel& operator=(const ReplyChannel&) = delete;
// gin_helper::Wrappable // gin_helper::Wrappable
static gin::DeprecatedWrapperInfo kWrapperInfo; static const gin::WrapperInfo kWrapperInfo;
const gin::WrapperInfo* wrapper_info() const override;
const char* GetHumanReadableName() const override;
gin::ObjectTemplateBuilder GetObjectTemplateBuilder( gin::ObjectTemplateBuilder GetObjectTemplateBuilder(
v8::Isolate* isolate) override; v8::Isolate* isolate) override;
const char* GetTypeName() override;
// Invokes `callback` (if it's non-null) with `errmsg` as an arg. // Invokes `callback` (if it's non-null) with `errmsg` as an arg.
static void SendError(v8::Isolate* isolate, static void SendError(v8::Isolate* isolate,
InvokeCallback callback, InvokeCallback callback,
std::string_view errmsg); std::string_view errmsg);
private: void EnsureReplySent();
explicit ReplyChannel(InvokeCallback callback);
~ReplyChannel() override;
private:
static bool SendReplyImpl(v8::Isolate* isolate, static bool SendReplyImpl(v8::Isolate* isolate,
InvokeCallback callback, InvokeCallback callback,
v8::Local<v8::Value> arg); v8::Local<v8::Value> arg);