[gjs/ewlsh/fix-closures] WIP: Getting Closure working: fixing inout bugs, fixing signal metadata handling, cleaning up JS API




commit ab0a551428a4eb31bbc7673c9da1b6d43ac0c87b
Author: Evan Welsh <contact evanwelsh com>
Date:   Sun Aug 29 20:37:38 2021 -0700

    WIP: Getting Closure working: fixing inout bugs, fixing signal metadata handling, cleaning up JS API

 gi/arg-cache.cpp                         | 173 ++++++++++++++++++++++++++++++-
 gi/arg-cache.h                           |   5 +-
 gi/arg.cpp                               |  42 ++++++--
 gi/arg.h                                 |   1 +
 gi/boxed.cpp                             |  10 +-
 gi/closure.h                             |  11 +-
 gi/private.cpp                           |  85 +++++++++++++++
 gi/value.cpp                             |  29 ++++--
 installed-tests/js/meson.build           |   1 +
 installed-tests/js/testGIMarshalling.js  |  14 +--
 installed-tests/js/testGObject.js        |   2 +-
 installed-tests/js/testGObjectClosure.js | 141 +++++++++++++++++++++++++
 modules/core/overrides/GObject.js        |  35 +++++++
 modules/core/overrides/Gtk.js            |  12 ++-
 14 files changed, 517 insertions(+), 44 deletions(-)
---
diff --git a/gi/arg-cache.cpp b/gi/arg-cache.cpp
index f4d8136e..3f362ced 100644
--- a/gi/arg-cache.cpp
+++ b/gi/arg-cache.cpp
@@ -206,6 +206,9 @@ static bool gjs_marshal_generic_inout_in(JSContext* cx, GjsArgumentCache* self,
         return false;
 
     int ix = self->arg_pos;
+    // FIXME If it isn't caller allocated, ignore releasing it.
+    if (!(self->flags & GjsArgumentFlags::CALLER_ALLOCATES))
+        state->ignore_release.insert(arg);
     state->out_cvalue(ix) = state->inout_original_cvalue(ix) = *arg;
     gjs_arg_set(arg, &state->out_cvalue(ix));
     return true;
@@ -462,6 +465,14 @@ bool GjsArgumentCache::handle_nullable(JSContext* cx, GIArgument* arg) {
     return true;
 }
 
+bool GjsArgumentCache::handle_nullable_or_optional(JSContext* cx,
+                                                   GIArgument* arg) {
+    if (!(flags & GjsArgumentFlags::OPTIONAL))
+        return handle_nullable(cx, arg);
+    gjs_arg_unset<void*>(arg);
+    return true;
+}
+
 GJS_JSAPI_RETURN_CONVENTION
 static bool gjs_marshal_string_in_in(JSContext* cx, GjsArgumentCache* self,
                                      GjsFunctionCallState*, GIArgument* arg,
@@ -581,6 +592,45 @@ static bool gjs_marshal_gvalue_in_in(JSContext* cx, GjsArgumentCache*,
     return true;
 }
 
+GJS_JSAPI_RETURN_CONVENTION
+static bool gjs_marshal_gvalue_inout_in(JSContext* cx, GjsArgumentCache* self,
+                                        GjsFunctionCallState* state,
+                                        GIArgument* arg,
+                                        JS::HandleValue value) {
+    int ix = self->arg_pos;
+
+    if (value.isNull()) {
+        if (self->handle_nullable_or_optional(cx, arg)) {
+            state->out_cvalue(ix) = state->inout_original_cvalue(ix) =
+                state->in_cvalue(ix) = *arg;
+            gjs_arg_set(arg, &state->out_cvalue(ix));
+            return true;
+        }
+        return false;
+    }
+
+    if (value.isObject()) {
+        JS::RootedObject obj(cx, &value.toObject());
+        GType gtype;
+
+        if (!gjs_gtype_get_actual_gtype(cx, obj, &gtype))
+            return false;
+
+        if (gtype == G_TYPE_VALUE) {
+            gjs_arg_set(arg, BoxedBase::to_c_ptr<GValue>(cx, obj));
+
+            state->out_cvalue(ix) = state->inout_original_cvalue(ix) = *arg;
+            state->ignore_release.insert(arg);
+            return true;
+        }
+    }
+
+    gjs_throw(cx,
+              "JavaScript values are not supported for inout GValues, use "
+              "GObject.Value instead.");
+    return false;
+}
+
 GJS_JSAPI_RETURN_CONVENTION
 static bool gjs_marshal_boxed_in_in(JSContext* cx, GjsArgumentCache* self,
                                     GjsFunctionCallState*, GIArgument* arg,
@@ -628,11 +678,25 @@ static bool gjs_marshal_union_in_in(JSContext* cx, GjsArgumentCache* self,
 
 GJS_JSAPI_RETURN_CONVENTION
 static bool gjs_marshal_gclosure_in_in(JSContext* cx, GjsArgumentCache* self,
-                                       GjsFunctionCallState*, GIArgument* arg,
-                                       JS::HandleValue value) {
+                                       GjsFunctionCallState* state,
+                                       GIArgument* arg, JS::HandleValue value) {
     if (value.isNull())
         return self->handle_nullable(cx, arg);
 
+    if (value.isObject()) {
+        JS::RootedObject obj(cx, &value.toObject());
+        GType gtype;
+
+        if (!gjs_gtype_get_actual_gtype(cx, obj, &gtype))
+            return false;
+
+        if (gtype == G_TYPE_CLOSURE) {
+            gjs_arg_set(arg, BoxedBase::to_c_ptr<Gjs::Closure>(cx, obj));
+            state->ignore_release.insert(arg);
+            return true;
+        }
+    }
+
     if (!(JS_TypeOfValue(cx, value) == JSTYPE_FUNCTION))
         return report_typeof_mismatch(cx, self->arg_name, value,
                                       ExpectedType::FUNCTION);
@@ -846,6 +910,17 @@ static bool gjs_marshal_generic_out_release(JSContext* cx,
                                   out_arg);
 }
 
+GJS_JSAPI_RETURN_CONVENTION
+static bool gjs_marshal_generic_out_maybe_release(
+    JSContext* cx, GjsArgumentCache* self, GjsFunctionCallState* state,
+    GIArgument* in_arg [[maybe_unused]], GIArgument* out_arg) {
+    if (state->ignore_release.erase(out_arg))
+        return true;
+
+    return gjs_g_argument_release(cx, self->transfer, &self->type_info,
+                                  out_arg);
+}
+
 GJS_JSAPI_RETURN_CONVENTION
 static bool gjs_marshal_generic_inout_release(JSContext* cx,
                                               GjsArgumentCache* self,
@@ -857,11 +932,13 @@ static bool gjs_marshal_generic_inout_release(JSContext* cx,
     // freeing it.
 
     GIArgument* original_out_arg = &state->inout_original_cvalue(self->arg_pos);
-    if (!gjs_g_argument_release_in_arg(cx, GI_TRANSFER_NOTHING,
+    if (!state->ignore_release.erase(in_arg) &&
+        !gjs_g_argument_release_in_arg(cx, GI_TRANSFER_NOTHING,
                                        &self->type_info, original_out_arg))
         return false;
 
-    return gjs_marshal_generic_out_release(cx, self, state, in_arg, out_arg);
+    return gjs_marshal_generic_out_maybe_release(cx, self, state, in_arg,
+                                                 out_arg);
 }
 
 GJS_JSAPI_RETURN_CONVENTION
@@ -1090,6 +1167,12 @@ static const GjsArgumentMarshallers gvalue_in_marshallers = {
     gjs_arg_cache_interface_free,  // free
 };
 
+static const GjsArgumentMarshallers gvalue_inout_marshallers = {
+    gjs_marshal_gvalue_inout_in,        // in
+    gjs_marshal_generic_out_out,        // out
+    gjs_marshal_generic_inout_release,  // release
+};
+
 static const GjsArgumentMarshallers gvalue_in_transfer_none_marshallers = {
     gjs_marshal_gvalue_in_in,             // in
     gjs_marshal_skipped_out,              // out
@@ -1590,6 +1673,84 @@ static void gjs_arg_cache_build_normal_in_arg(GjsArgumentCache* self,
     }
 }
 
+static void gjs_arg_cache_build_interface_inout_arg(
+    GjsArgumentCache* self, GIBaseInfo* interface_info) {
+    GIInfoType interface_type = g_base_info_get_type(interface_info);
+
+    switch (interface_type) {
+        case GI_INFO_TYPE_ENUM:
+        case GI_INFO_TYPE_FLAGS:
+            self->marshallers = &fallback_inout_marshallers;
+            return;
+        case GI_INFO_TYPE_STRUCT:
+            if (g_struct_info_is_foreign(interface_info)) {
+                // FIXME: Falling back to the generic marshaller
+                self->marshallers = &fallback_inout_marshallers;
+                return;
+            }
+            [[fallthrough]];
+        case GI_INFO_TYPE_BOXED:
+        case GI_INFO_TYPE_OBJECT:
+        case GI_INFO_TYPE_INTERFACE:
+        case GI_INFO_TYPE_UNION: {
+            GType gtype = g_registered_type_info_get_g_type(interface_info);
+
+            if (interface_type == GI_INFO_TYPE_STRUCT && gtype == G_TYPE_NONE &&
+                !g_struct_info_is_gtype_struct(interface_info)) {
+                // This covers cases such as GTypeInstance
+                self->marshallers = &fallback_inout_marshallers;
+                return;
+            }
+
+            self->contents.object.gtype = gtype;
+            self->contents.object.info = g_base_info_ref(interface_info);
+
+            if (gtype == G_TYPE_VALUE) {
+                self->marshallers = &gvalue_inout_marshallers;
+                return;
+            }
+
+            self->marshallers = &fallback_inout_marshallers;
+            return;
+        } break;
+
+        case GI_INFO_TYPE_INVALID:
+        case GI_INFO_TYPE_FUNCTION:
+        case GI_INFO_TYPE_CALLBACK:
+        case GI_INFO_TYPE_CONSTANT:
+        case GI_INFO_TYPE_INVALID_0:
+        case GI_INFO_TYPE_VALUE:
+        case GI_INFO_TYPE_SIGNAL:
+        case GI_INFO_TYPE_VFUNC:
+        case GI_INFO_TYPE_PROPERTY:
+        case GI_INFO_TYPE_FIELD:
+        case GI_INFO_TYPE_ARG:
+        case GI_INFO_TYPE_TYPE:
+        case GI_INFO_TYPE_UNRESOLVED:
+        default:
+            // Don't know how to handle this interface type (should not happen
+            // in practice, for typelibs emitted by g-ir-compiler)
+            self->marshallers = &invalid_in_marshallers;
+            self->contents.reason = UNSUPPORTED_TYPE;
+    }
+}
+
+static void gjs_arg_cache_build_inout_arg(GjsArgumentCache* self,
+                                          GITypeTag tag) {
+    switch (tag) {
+        case GI_TYPE_TAG_INTERFACE: {
+            GjsAutoBaseInfo interface_info =
+                g_type_info_get_interface(&self->type_info);
+
+            gjs_arg_cache_build_interface_inout_arg(self, interface_info);
+            return;
+        }
+        default:
+            // FIXME: Falling back to the generic marshaller
+            self->marshallers = &fallback_inout_marshallers;
+    }
+}
+
 void gjs_arg_cache_build_instance(GjsArgumentCache* self,
                                   GICallableInfo* callable) {
     GIBaseInfo* interface_info = g_base_info_get_container(callable);  // !owned
@@ -1635,6 +1796,8 @@ void gjs_arg_cache_build_arg(GjsArgumentCache* self,
     GjsArgumentFlags flags = GjsArgumentFlags::NONE;
     if (g_arg_info_may_be_null(arg))
         flags |= GjsArgumentFlags::MAY_BE_NULL;
+    if (g_arg_info_is_optional(arg))
+        flags |= GjsArgumentFlags::OPTIONAL;
     if (g_arg_info_is_caller_allocates(arg))
         flags |= GjsArgumentFlags::CALLER_ALLOCATES;
 
@@ -1772,7 +1935,7 @@ void gjs_arg_cache_build_arg(GjsArgumentCache* self,
     if (direction == GI_DIRECTION_IN)
         gjs_arg_cache_build_normal_in_arg(self, type_tag);
     else if (direction == GI_DIRECTION_INOUT)
-        self->marshallers = &fallback_inout_marshallers;
+        gjs_arg_cache_build_inout_arg(self, type_tag);
     else
         self->marshallers = &fallback_out_marshallers;
 }
diff --git a/gi/arg-cache.h b/gi/arg-cache.h
index 562dcd3d..f7a2fffa 100644
--- a/gi/arg-cache.h
+++ b/gi/arg-cache.h
@@ -56,7 +56,7 @@ struct GjsArgumentCache {
 
     uint8_t arg_pos;
     GITransfer transfer : 2;
-    GjsArgumentFlags flags : 5;
+    GjsArgumentFlags flags : 6;
 
     union {
         // for explicit array only
@@ -101,6 +101,9 @@ struct GjsArgumentCache {
     GJS_JSAPI_RETURN_CONVENTION
     bool handle_nullable(JSContext* cx, GIArgument* arg);
 
+    GJS_JSAPI_RETURN_CONVENTION
+    bool handle_nullable_or_optional(JSContext* cx, GIArgument* arg);
+
     // Introspected functions can have up to 253 arguments. 255 is a placeholder
     // for the return value and 254 for the instance parameter. The callback
     // closure or destroy notify parameter may have a value of 255 to indicate
diff --git a/gi/arg.cpp b/gi/arg.cpp
index cff40e10..67ae706d 100644
--- a/gi/arg.cpp
+++ b/gi/arg.cpp
@@ -1223,6 +1223,21 @@ static bool value_to_interface_gi_argument(
             return true;
         }
 
+        if (value.isObject()) {
+            JS::RootedObject obj(cx, &value.toObject());
+            GType gtype;
+
+            if (!gjs_gtype_get_actual_gtype(cx, obj, &gtype))
+                return false;
+
+            if (gtype == G_TYPE_VALUE) {
+                GValue* value = BoxedBase::to_c_ptr<GValue>(cx, obj);
+                gjs_arg_set(arg, value);
+
+                return true;
+            }
+        }
+
         Gjs::AutoGValue gvalue;
         if (!gjs_value_to_g_value(cx, value, &gvalue)) {
             gjs_arg_unset<void*>(arg);
@@ -1344,17 +1359,23 @@ static bool value_to_interface_gi_argument(
 
             } else if (g_type_is_a(gtype, G_TYPE_BOXED)) {
                 if (g_type_is_a(gtype, G_TYPE_CLOSURE)) {
-                    GClosure* closure = Gjs::Closure::create_marshaled(
-                        cx, JS_GetObjectFunction(obj), "boxed");
-                    // GI doesn't know about floating GClosure references. We
-                    // guess that if this is a return value going from JS::Value
-                    // to GArgument, it's intended to be passed to a C API that
-                    // will consume the floating reference.
-                    if (arg_type != GJS_ARGUMENT_RETURN_VALUE) {
-                        g_closure_ref(closure);
-                        g_closure_sink(closure);
+                    if (JS_ObjectIsFunction(obj)) {
+                        GClosure* closure = Gjs::Closure::create_marshaled(
+                            cx, JS_GetObjectFunction(obj), "boxed");
+                        // GI doesn't know about floating GClosure references.
+                        // We guess that if this is a return value going from
+                        // JS::Value to GArgument, it's intended to be passed to
+                        // a C API that will consume the floating reference.
+                        if (arg_type != GJS_ARGUMENT_RETURN_VALUE) {
+                            g_closure_ref(closure);
+                            g_closure_sink(closure);
+                        }
+
+                        gjs_arg_set(arg, closure);
+                        return true;
                     }
-                    gjs_arg_set(arg, closure);
+
+                    gjs_arg_set(arg, BoxedBase::to_c_ptr<GClosure>(cx, obj));
                     return true;
                 }
 
@@ -1555,7 +1576,6 @@ bool gjs_value_to_g_argument(JSContext* context, JS::HandleValue value,
             return false;
         }
         break;
-
     case GI_TYPE_TAG_GTYPE:
         if (value.isObjectOrNull()) {
             GType gtype;
diff --git a/gi/arg.h b/gi/arg.h
index c81c2979..eaa1c6d9 100644
--- a/gi/arg.h
+++ b/gi/arg.h
@@ -38,6 +38,7 @@ enum class GjsArgumentFlags : uint8_t {
     SKIP_ALL = SKIP_IN | SKIP_OUT,
     FILENAME = 1 << 4,  //  Sharing the bit with UNSIGNED, used only for strings
     UNSIGNED = 1 << 4,  //  Sharing the bit with FILENAME, used only for enums
+    OPTIONAL = 1 << 5,
 };
 
 [[nodiscard]] char* gjs_argument_display_name(const char* arg_name,
diff --git a/gi/boxed.cpp b/gi/boxed.cpp
index 7327c317..8ca0f74b 100644
--- a/gi/boxed.cpp
+++ b/gi/boxed.cpp
@@ -322,18 +322,18 @@ bool BoxedInstance::constructor_impl(JSContext* context, JS::HandleObject obj,
         }
     }
 
-    if (gtype() == G_TYPE_VARIANT) {
+    if (gtype() == G_TYPE_VARIANT || gtype() == G_TYPE_CLOSURE) {
         /* Short-circuit construction for GVariants by calling into the JS packing
            function */
         const GjsAtoms& atoms = GjsContextPrivate::atoms(context);
         if (!boxed_invoke_constructor(context, obj, atoms.new_internal(), args))
             return false;
 
-        // The return value of GLib.Variant.new_internal() gets its own
-        // BoxedInstance, and the one we're setting up in this constructor is
-        // discarded.
+        // The return values of GLib.Variant.new_internal() and
+        // GObject.Closure.new_internal() gets their own BoxedInstance,
+        // and the one we're setting up in this constructor is discarded.
         debug_lifecycle(
-            "Boxed construction delegated to GVariant constructor, "
+            "Boxed construction delegated to JavaScript constructor, "
             "boxed object discarded");
 
         return true;
diff --git a/gi/closure.h b/gi/closure.h
index a9110fc5..23007402 100644
--- a/gi/closure.h
+++ b/gi/closure.h
@@ -27,6 +27,10 @@ class HandleValueArray;
 
 namespace Gjs {
 
+struct SignalClosureMeta {
+    uint32_t signal_id;
+};
+
 class Closure : public GClosure {
  protected:
     Closure(JSContext*, JSFunction*, bool root, const char* description);
@@ -79,11 +83,12 @@ class Closure : public GClosure {
     [[nodiscard]] static Closure* create_for_signal(JSContext* cx,
                                                     JSFunction* callable,
                                                     const char* description,
-                                                    int signal_id) {
+                                                    int32_t 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 a6ecc723..534fc502 100644
--- a/gi/private.cpp
+++ b/gi/private.cpp
@@ -19,6 +19,7 @@
 #include <js/Utility.h>  // for UniqueChars
 #include <jsapi.h>       // for JS_GetElement
 
+#include "gi/boxed.h"
 #include "gi/gobject.h"
 #include "gi/gtype.h"
 #include "gi/interface.h"
@@ -239,6 +240,87 @@ static inline void gjs_add_interface(GType instance_type,
                                 &interface_vtable);
 }
 
+GJS_JSAPI_RETURN_CONVENTION
+static bool gjs_value_from_closure(JSContext* cx, Gjs::Closure* closure,
+                                   JS::MutableHandleValue value) {
+    GjsAutoStructInfo info =
+        g_irepository_find_by_gtype(nullptr, G_TYPE_CLOSURE);
+    g_assert(info);
+
+    JS::RootedObject boxed(cx, BoxedInstance::new_for_c_struct(
+                                   cx, info, closure, BoxedInstance::NoCopy()));
+    if (!boxed)
+        return false;
+
+    value.setObject(*boxed);
+    return true;
+}
+
+GJS_JSAPI_RETURN_CONVENTION
+static bool gjs_create_closure(JSContext* cx, unsigned argc, JS::Value* vp) {
+    JS::CallArgs args = JS::CallArgsFromVp(argc, vp);
+
+    JS::RootedObject callable(cx);
+
+    if (!gjs_parse_call_args(cx, "create_closure", args, "o", "callable",
+                             &callable))
+        return false;
+
+    if (!JS_ObjectIsFunction(callable)) {
+        gjs_throw(cx, "create_closure() expects a callable function");
+        return false;
+    }
+
+    JS::RootedFunction func(cx, JS_GetObjectFunction(callable));
+
+    Gjs::Closure* closure =
+        Gjs::Closure::create_marshaled(cx, func, "custom callback");
+    if (closure == nullptr)
+        return false;
+
+    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_register_type(JSContext* cx, unsigned argc, JS::Value* vp) {
     JS::CallArgs argv = JS::CallArgsFromVp(argc, vp);
@@ -409,6 +491,9 @@ static JSFunctionSpec private_module_funcs[] = {
           GJS_MODULE_PROP_FLAGS),
     JS_FN("register_type", gjs_register_type, 4, GJS_MODULE_PROP_FLAGS),
     JS_FN("signal_new", gjs_signal_new, 6, GJS_MODULE_PROP_FLAGS),
+    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_FS_END,
 };
 
diff --git a/gi/value.cpp b/gi/value.cpp
index cc85b654..a28d3ff2 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);
+        uint32_t 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);
 
@@ -764,11 +778,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/installed-tests/js/meson.build b/installed-tests/js/meson.build
index b42f3b20..c3df3ed5 100644
--- a/installed-tests/js/meson.build
+++ b/installed-tests/js/meson.build
@@ -123,6 +123,7 @@ jasmine_tests = [
     'GLib',
     'GObject',
     'GObjectClass',
+    'GObjectClosure',
     'GObjectInterface',
     'GObjectValue',
     'GTypeClass',
diff --git a/installed-tests/js/testGIMarshalling.js b/installed-tests/js/testGIMarshalling.js
index 7d524919..06ee19e5 100644
--- a/installed-tests/js/testGIMarshalling.js
+++ b/installed-tests/js/testGIMarshalling.js
@@ -79,9 +79,6 @@ function testTransferMarshalling(root, value, inoutValue, options = {}) {
             in: {
                 omit: true,  // this case is not in the test suite
             },
-            inout: {
-                skip: 'https://gitlab.gnome.org/GNOME/gobject-introspection/issues/192',
-            },
         };
         Object.assign(fullOptions, options.full);
         testSimpleMarshalling(`${root}_full`, value, inoutValue, fullOptions);
@@ -896,12 +893,11 @@ describe('Callback', function () {
     describe('GClosure', function () {
         testInParameter('gclosure', () => 42);
 
-        xit('marshals a GClosure as a return value', function () {
-            // Currently a GObject.Closure instance is returned, upon which it's
-            // not possible to call invoke() because that method takes a bare
-            // pointer as an argument.
-            expect(GIMarshallingTests.gclosure_return()()).toEqual(42);
-        }).pend('https://gitlab.gnome.org/GNOME/gjs/issues/80');
+        it('marshals a GClosure as a return value', function () {
+            const closure = GIMarshallingTests.gclosure_return();
+            expect(closure instanceof GObject.Closure).toBeTruthy();
+            expect(closure.invoke(GObject.TYPE_INT, [])).toEqual(42);
+        });
     });
 
     it('marshals a return value', function () {
diff --git a/installed-tests/js/testGObject.js b/installed-tests/js/testGObject.js
index 885a93fb..99687973 100644
--- a/installed-tests/js/testGObject.js
+++ b/installed-tests/js/testGObject.js
@@ -62,7 +62,7 @@ describe('GObject overrides', function () {
 });
 
 describe('GObject should', function () {
-    const types = ['gpointer', 'GBoxed', 'GParam', 'GInterface', 'GObject', 'GVariant'];
+    const types = ['gpointer', 'GBoxed', 'GParam', 'GInterface', 'GObject', 'GVariant', 'GClosure'];
 
     types.forEach(type => {
         it(`be able to create a GType object for ${type}`, function () {
diff --git a/installed-tests/js/testGObjectClosure.js b/installed-tests/js/testGObjectClosure.js
new file mode 100644
index 00000000..7660a882
--- /dev/null
+++ b/installed-tests/js/testGObjectClosure.js
@@ -0,0 +1,141 @@
+// SPDX-License-Identifier: MIT OR LGPL-2.0-or-later
+// SPDX-FileCopyrightText: 2020 Marco Trevisan <marco trevisan canonical com>
+
+const {GLib, GObject, GIMarshallingTests, Regress} = imports.gi;
+
+const SIGNED_TYPES = ['schar', 'int', 'int64', 'long'];
+const UNSIGNED_TYPES = ['char', 'uchar', 'uint', 'uint64', 'ulong'];
+const FLOATING_TYPES = ['double', 'float'];
+const NUMERIC_TYPES = [...SIGNED_TYPES, ...UNSIGNED_TYPES, ...FLOATING_TYPES];
+const SPECIFIC_TYPES = ['gtype', 'boolean', 'string', 'param', 'variant', 'boxed', 'gvalue'];
+const INSTANCED_TYPES = ['object', 'instance'];
+const ALL_TYPES = [...NUMERIC_TYPES, ...SPECIFIC_TYPES, ...INSTANCED_TYPES];
+
+describe('GObject closure (GClosure)', function () {
+    let spyFn;
+    let closure;
+    beforeEach(function () {
+        spyFn = jasmine.createSpy();
+        closure = new GObject.Closure(spyFn);
+    });
+
+    function getDefaultContentByType(type) {
+        if (SIGNED_TYPES.includes(type))
+            return -((Math.random() * 100 | 0) + 1);
+        if (UNSIGNED_TYPES.includes(type))
+            return -getDefaultContentByType('int') + 2;
+        if (FLOATING_TYPES.includes(type))
+            return getDefaultContentByType('uint') + 0.5;
+        if (type === 'string')
+            return `Hello GValue! ${getDefaultContentByType('uint')}`;
+        if (type === 'boolean')
+            return !!(getDefaultContentByType('int') % 2);
+        if (type === 'gtype')
+            return getGType(ALL_TYPES[Math.random() * ALL_TYPES.length | 0]);
+
+        if (type === 'boxed' || type === 'boxed-struct') {
+            return new GIMarshallingTests.BoxedStruct({
+                long_: getDefaultContentByType('long'),
+                // string_: getDefaultContentByType('string'), not supported
+            });
+        }
+        if (type === 'object') {
+            const wasCreatingObject = this.creatingObject;
+            this.creatingObject = true;
+            const props = ALL_TYPES.filter(e =>
+                (e !== 'object' || !wasCreatingObject) &&
+                e !== 'boxed' &&
+                e !== 'gtype' &&
+                e !== 'instance' &&
+                e !== 'param' &&
+                // Include string when gobject-introspection!268 is merged
+                e !== 'string' &&
+                e !== 'schar').concat([
+                'boxed-struct',
+            ]).reduce((ac, a) => ({
+                ...ac, [`some-${a}`]: getDefaultContentByType(a),
+            }), {});
+            delete this.creatingObject;
+            return new GIMarshallingTests.PropertiesObject(props);
+        }
+        if (type === 'param') {
+            return GObject.ParamSpec.string('test-param', '', getDefaultContentByType('string'),
+                GObject.ParamFlags.READABLE, '');
+        }
+        if (type === 'variant') {
+            return new GLib.Variant('a{sv}', {
+                pasta: new GLib.Variant('s', 'Carbonara (con guanciale)'),
+                pizza: new GLib.Variant('s', 'Verace'),
+                randomString: new GLib.Variant('s', getDefaultContentByType('string')),
+            });
+        }
+        if (type === 'gvalue') {
+            const value = new GObject.Value();
+            const valueType = NUMERIC_TYPES[Math.random() * NUMERIC_TYPES.length | 0];
+            value.init(getGType(valueType));
+            return value;
+        }
+        if (type === 'instance')
+            return new Regress.TestFundamentalSubObject(getDefaultContentByType('string'));
+
+
+        throw new Error(`No default content set for type ${type}`);
+    }
+
+    function getGType(type) {
+        if (type === 'schar')
+            return GObject.TYPE_CHAR;
+
+        if (type === 'boxed' || type === 'gvalue' || type === 'instance')
+            return getDefaultContentByType(type).constructor.$gtype;
+
+        return GObject[`TYPE_${type.toUpperCase()}`];
+    }
+
+    function skipUnsupported(type) {
+        if (type === 'boxed')
+            pending('https://gitlab.gnome.org/GNOME/gjs/-/issues/402');
+
+        if (type === 'gvalue')
+            pending('https://gitlab.gnome.org/GNOME/gjs/-/issues/272');
+    }
+
+    it('is an instanceof GObject.Closure', function () {
+        expect(closure instanceof GObject.Closure).toBeTruthy();
+    });
+
+    ALL_TYPES.forEach(type => {
+        const gtype = getGType(type);
+
+        it(`can return ${type}`, function () {
+            let randomContent = getDefaultContentByType(type);
+
+            skipUnsupported(type);
+            spyFn.and.returnValue(randomContent);
+            expect(closure.invoke(gtype, [])).toEqual(randomContent);
+        });
+    });
+
+    it('can be invalidated', function () {
+        log(GObject.TYPE_INT);
+
+        spyFn.and.returnValue(13);
+        expect(closure.invoke(GObject.TYPE_INT, [])).toBe(13);
+        closure.invalidate();
+        expect(closure.invoke(null, [])).toBe(null);
+    });
+
+    it('can be called with parameters', function () {
+        log(GObject.TYPE_INT);
+        const plusClosure = new GObject.Closure((a, b) => {
+            return a + b;
+        });
+
+        expect(plusClosure.invoke(GObject.TYPE_INT, [5, 6])).toBe(11);
+        expect(plusClosure.invoke(GObject.TYPE_STRING, ['hello', ', world'])).toBe('hello, world');
+    });
+
+    afterEach(function () {
+        closure = null;
+    });
+});
diff --git a/modules/core/overrides/GObject.js b/modules/core/overrides/GObject.js
index 6bfaf144..7063765f 100644
--- a/modules/core/overrides/GObject.js
+++ b/modules/core/overrides/GObject.js
@@ -753,4 +753,39 @@ function _init() {
         throw new Error('GObject.signal_handlers_disconnect_by_data() is not \
 introspectable. Use GObject.signal_handlers_disconnect_by_func() instead.');
     };
+
+    GObject.Closure._new_internal = function (callable) {
+        'sensitive';
+
+        return Gi.create_closure(callable);
+    };
+
+    GObject.Closure.new_simple = function () {
+        throw new Error('GObject.Closure.new_simple() is not introspectable. \
+Use new GObject.Closure() instead.');
+    };
+
+    GObject.Closure.new_object = function () {
+        throw new Error('GObject.Closure.new_object() is not introspectable. \
+Use new GObject.Closure() instead.');
+    };
+
+    const invoke = GObject.Closure.prototype.invoke;
+    /**
+     * @param {GType | null} return_type the GType of the return value or null if the closure returns void
+     * @param {any[]} parameters a list of values to pass to the closure
+     * @returns {any}
+     */
+    GObject.Closure.prototype.invoke = function (return_type, parameters) {
+        'hide source';
+
+        if (return_type === null)
+            return invoke.call(this, null, parameters, null);
+
+
+        const return_value = new GObject.Value();
+        return_value.init(return_type);
+
+        return invoke.call(this, return_value, parameters, null);
+    };
 }
diff --git a/modules/core/overrides/Gtk.js b/modules/core/overrides/Gtk.js
index 306f7d3f..c2156935 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;
@@ -120,14 +121,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;
 


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]