[gjs/ewlsh/fix-closures: 11/11] Use GObject.Closure to marshal signal IDs

[Evan Welsh <ewlsh src gnome org>]

commit 82593553799749ad85924891289fa14a9e171ae6
Author: Evan Welsh <contact evanwelsh com>
Date:   Sat Jan 15 12:26:53 2022 -0800

    Use GObject.Closure to marshal signal IDs
    
    Fixes #421

 gi/closure.h                  | 13 ++++++++-----
 gi/private.cpp                | 42 ++++++++++++++++++++++++++++++++++++++++++
 gi/value.cpp                  | 29 ++++++++++++++++++++++-------
 modules/core/overrides/Gtk.js | 12 ++++++++++--
 4 files changed, 82 insertions(+), 14 deletions(-)
---
diff --git a/gi/closure.h b/gi/closure.h
index a9110fc5..551939d5 100644
--- a/gi/closure.h
+++ b/gi/closure.h
@@ -12,10 +12,8 @@
 #include <glib-object.h>
 #include <stddef.h>
 
-#include <js/RootingAPI.h>
 #include <js/TypeDecls.h>
 
-#include "gi/utils-inl.h"
 #include "gjs/jsapi-util-root.h"
 #include "gjs/jsapi-util.h"
 #include "gjs/macros.h"
@@ -27,6 +25,10 @@ class HandleValueArray;
 
 namespace Gjs {
 
+struct SignalClosureMeta {
+    unsigned signal_id;
+};
+
 class Closure : public GClosure {
  protected:
     Closure(JSContext*, JSFunction*, bool root, const char* description);
@@ -79,11 +81,12 @@ class Closure : public GClosure {
     [[nodiscard]] static Closure* create_for_signal(JSContext* cx,
                                                     JSFunction* callable,
                                                     const char* description,
-                                                    int signal_id) {
+                                                    unsigned signal_id) {
         auto* self = new Closure(cx, callable, false /* root */, description);
         self->add_finalize_notifier<Closure>();
-        g_closure_set_meta_marshal(self, gjs_int_to_pointer(signal_id),
-                                   marshal_cb);
+        SignalClosureMeta* meta = new SignalClosureMeta();
+        meta->signal_id = signal_id;
+        g_closure_set_meta_marshal(self, meta, marshal_cb);
         return self;
     }
 
diff --git a/gi/private.cpp b/gi/private.cpp
index 175d9f99..1fc13935 100644
--- a/gi/private.cpp
+++ b/gi/private.cpp
@@ -285,6 +285,46 @@ static bool gjs_create_closure(JSContext* cx, unsigned argc, JS::Value* vp) {
     return gjs_value_from_closure(cx, closure, args.rval());
 }
 
+GJS_JSAPI_RETURN_CONVENTION
+static bool gjs_create_signal_closure(JSContext* cx, unsigned argc,
+                                      JS::Value* vp) {
+    JS::CallArgs args = JS::CallArgsFromVp(argc, vp);
+
+    JS::RootedObject owner(cx), callable(cx);
+    uint32_t signal_id = 0;
+
+    if (!gjs_parse_call_args(cx, "create_signal_closure", args, "oou", "owner",
+                             &owner, "callable", &callable, "signal_id",
+                             &signal_id))
+        return false;
+
+    if (!JS_ObjectIsFunction(callable)) {
+        gjs_throw(cx, "create_signal_closure() expects a callable function");
+        return false;
+    }
+
+    ObjectBase* base;
+    if (!ObjectInstance::for_js_typecheck(cx, owner, &base))
+        return false;
+
+    if (!base->check_is_instance(cx, "signal hookup"))
+        return false;
+
+    ObjectInstance* instance = base->to_instance();
+
+    JS::RootedFunction func(cx, JS_GetObjectFunction(callable));
+
+    Gjs::Closure* closure = Gjs::Closure::create_for_signal(
+        cx, func, "custom signal callback", signal_id);
+
+    if (closure == nullptr)
+        return false;
+    if (!instance->associate_closure(cx, closure))
+        return false;
+
+    return gjs_value_from_closure(cx, closure, args.rval());
+}
+
 GJS_JSAPI_RETURN_CONVENTION
 static bool gjs_invoke_closure(JSContext* cx, unsigned argc, JS::Value* vp) {
     JS::CallArgs args = JS::CallArgsFromVp(argc, vp);
@@ -550,6 +590,8 @@ static JSFunctionSpec private_module_funcs[] = {
     JS_FN("signal_new", gjs_signal_new, 6, GJS_MODULE_PROP_FLAGS),
     JS_FN("lookupConstructor", gjs_lookup_constructor, 1, 0),
     JS_FN("create_closure", gjs_create_closure, 1, GJS_MODULE_PROP_FLAGS),
+    JS_FN("create_signal_closure", gjs_create_signal_closure, 3,
+          GJS_MODULE_PROP_FLAGS),
     JS_FN("invoke_closure", gjs_invoke_closure, 3, GJS_MODULE_PROP_FLAGS),
     JS_FS_END,
 };
diff --git a/gi/value.cpp b/gi/value.cpp
index af1f6702..d6e52686 100644
--- a/gi/value.cpp
+++ b/gi/value.cpp
@@ -160,9 +160,23 @@ void Gjs::Closure::marshal(GValue* return_value, unsigned n_param_values,
 
     if (marshal_data) {
         /* we are used for a signal handler */
-        guint signal_id;
+        SignalClosureMeta* signal_meta =
+            static_cast<SignalClosureMeta*>(marshal_data);
+        unsigned signal_id = signal_meta->signal_id;
+
+        if (signal_id == 0) {
+            GSignalInvocationHint* hint =
+                static_cast<GSignalInvocationHint*>(invocation_hint);
+
+            if (!hint) {
+                gjs_debug(GJS_DEBUG_GCLOSURE,
+                          "Closure is not a signal handler but is being "
+                          "handled like one.");
+                return;
+            }
 
-        signal_id = GPOINTER_TO_UINT(marshal_data);
+            signal_id = hint->signal_id;
+        }
 
         g_signal_query(signal_id, &signal_query);
 
@@ -731,11 +745,12 @@ gjs_value_to_g_value_internal(JSContext      *context,
         if (!FundamentalBase::to_gvalue(context, fundamental_object, gvalue))
             return false;
     } else {
-        gjs_debug(GJS_DEBUG_GCLOSURE, "JS::Value is number %d gtype fundamental %d transformable to int %d 
from int %d",
-                  value.isNumber(),
-                  G_TYPE_IS_FUNDAMENTAL(gtype),
-                  g_value_type_transformable(gtype, G_TYPE_INT),
-                  g_value_type_transformable(G_TYPE_INT, gtype));
+        gjs_debug(GJS_DEBUG_GCLOSURE,
+                  "JS::Value is number %d\ngtype fundamental %d\ntransformable "
+                  "to int %s\ntransformable from int %s",
+                  value.isNumber(), G_TYPE_IS_FUNDAMENTAL(gtype),
+                  g_value_type_transformable(gtype, G_TYPE_INT) ? "yes" : "no",
+                  g_value_type_transformable(G_TYPE_INT, gtype) ? "yes" : "no");
 
         gjs_throw(context,
                   "Don't know how to convert JavaScript object to GType %s",
diff --git a/modules/core/overrides/Gtk.js b/modules/core/overrides/Gtk.js
index 77649a73..d974bed5 100644
--- a/modules/core/overrides/Gtk.js
+++ b/modules/core/overrides/Gtk.js
@@ -3,6 +3,7 @@
 // SPDX-FileCopyrightText: 2013 Giovanni Campagna
 
 const Legacy = imports._legacy;
+const Gi = imports._gi;
 const {Gio, GjsPrivate, GObject} = imports.gi;
 
 let Gtk;
@@ -124,14 +125,21 @@ function _init() {
         }, class extends GObject.Object {
             vfunc_create_closure(builder, handlerName, flags, connectObject) {
                 const swapped = flags & Gtk.BuilderClosureFlags.SWAPPED;
-                return _createClosure(
+                return _wrapInSignalMeta(connectObject, _createClosure(
                     builder, builder.get_current_object(),
-                    handlerName, swapped, connectObject);
+                    handlerName, swapped, connectObject));
             }
         });
     }
 }
 
+function _wrapInSignalMeta(connectObject, callable) {
+    if (connectObject instanceof GObject.Object)
+        return Gi.create_signal_closure(connectObject, callable, 0);
+    else
+        return callable;
+}
+
 function _createClosure(builder, thisArg, handlerName, swapped, connectObject) {
     connectObject = connectObject || thisArg;