[gjs/wip/xclaesse/57-should-use-g-i-annotations-for-signal] object: Use GISignalInfo to marshal signal arguments

[Philip Chimento <pchimento src gnome org>]

commit 5ed3dd4c6f0a1c604997da365d8a703193d16885
Author: Xavier Claessens <xclaesse gmail com>
Date:   Thu Aug 25 18:09:00 2011 +0200

    object: Use GISignalInfo to marshal signal arguments
    
    Closes #57.

 gi/function.cpp  | 75 ++++++++++++++++++++++++++++++++++++++++----------------
 gi/function.h    | 19 ++++++++------
 gi/object.cpp    | 26 ++++++++++++++++++--
 gi/value.cpp     | 15 ------------
 gi/value.h       |  5 ----
 gjs/jsapi-util.h |  1 +
 6 files changed, 90 insertions(+), 51 deletions(-)
---
diff --git a/gi/function.cpp b/gi/function.cpp
index 2673eaf5..dd17ddd7 100644
--- a/gi/function.cpp
+++ b/gi/function.cpp
@@ -190,11 +190,12 @@ gjs_callback_closure(ffi_cif *cif,
 {
     JSContext *context;
     GjsCallbackTrampoline *trampoline;
-    int i, n_args, n_jsargs, n_outargs, c_args_offset = 0;
+    int i, n_args, n_jsargs = 0, n_outargs, c_args_offset = 0;
     GITypeInfo ret_type;
     bool success = false;
     bool ret_type_is_void;
     auto args = reinterpret_cast<GIArgument **>(ffi_args);
+    GSignalQuery signal_query = {0};
 
     trampoline = (GjsCallbackTrampoline *) data;
     g_assert(trampoline);
@@ -229,7 +230,10 @@ gjs_callback_closure(ffi_cif *cif,
     JSAutoCompartment ac(context,
                          gjs_closure_get_callable(trampoline->js_function));
 
+    /* If this callable is a signal, there is an extra arg for self */
+
     bool can_throw_gerror = g_callable_info_can_throw_gerror(trampoline->info);
+    bool is_signal = GI_IS_SIGNAL_INFO(trampoline->info);
     n_args = g_callable_info_get_n_args(trampoline->info);
 
     g_assert(n_args >= 0);
@@ -247,19 +251,39 @@ gjs_callback_closure(ffi_cif *cif,
     n_outargs = 0;
     JS::AutoValueVector jsargs(context);
 
-    if (!jsargs.reserve(n_args))
+    if (!jsargs.reserve(is_signal ? n_args + 1 : n_args))
         g_error("Unable to reserve space for vector");
 
+    if (is_signal) {
+        if (!jsargs.growBy(1))
+            g_error("Unable to grow vector");
+
+        auto* this_gobject = static_cast<GObject*>(args[0]->v_pointer);
+        JSObject* obj = gjs_object_from_g_object(context, this_gobject);
+        jsargs[n_jsargs++].setObject(*obj);
+        args++;
+
+        /* Query more info about this signal */
+        g_assert(trampoline->signal_id != 0);
+        g_signal_query(trampoline->signal_id, &signal_query);
+        g_assert(signal_query.n_params == unsigned(n_args));
+    }
+
     JS::RootedValue rval(context);
 
-    for (i = 0, n_jsargs = 0; i < n_args; i++) {
+    for (i = 0; i < n_args; i++) {
         GIArgInfo arg_info;
         GITypeInfo type_info;
         GjsParamType param_type;
+        bool should_copy = false;
 
         g_callable_info_load_arg(trampoline->info, i, &arg_info);
         g_arg_info_load_type(&arg_info, &type_info);
 
+        if (is_signal)
+            should_copy =
+                (signal_query.param_types[i] & G_SIGNAL_TYPE_STATIC_SCOPE) == 0;
+
         /* Skip void * arguments */
         if (g_type_info_get_tag(&type_info) == GI_TYPE_TAG_VOID)
             continue;
@@ -304,10 +328,9 @@ gjs_callback_closure(ffi_cif *cif,
                 if (!jsargs.growBy(1))
                     g_error("Unable to grow vector");
 
-                if (!gjs_value_from_g_argument(context, jsargs[n_jsargs++],
-                                               &type_info,
-                                               args[i + c_args_offset],
-                                               false))
+                if (!gjs_value_from_g_argument(
+                        context, jsargs[n_jsargs++], &type_info,
+                        args[i + c_args_offset], should_copy))
                     goto out;
                 break;
             case PARAM_CALLBACK:
@@ -490,14 +513,10 @@ gjs_destroy_notify_callback(gpointer data)
     gjs_callback_trampoline_unref(trampoline);
 }
 
-GjsCallbackTrampoline*
-gjs_callback_trampoline_new(JSContext       *context,
-                            JS::HandleValue  function,
-                            GICallableInfo  *callable_info,
-                            GIScopeType      scope,
-                            JS::HandleObject scope_object,
-                            bool             is_vfunc)
-{
+GjsCallbackTrampoline* gjs_callback_trampoline_new(
+    JSContext* context, JS::HandleValue function, GICallableInfo* callable_info,
+    GIScopeType scope, JS::HandleObject scope_object, bool is_vfunc,
+    unsigned signal_id) {
     GjsCallbackTrampoline *trampoline;
     int n_args, i;
 
@@ -583,11 +602,28 @@ gjs_callback_trampoline_new(JSContext       *context,
                                                           gjs_callback_closure, trampoline);
 
     trampoline->scope = scope;
+    trampoline->signal_id = signal_id;
     trampoline->is_vfunc = is_vfunc;
 
     return trampoline;
 }
 
+GClosure* gjs_signal_closure_new(JSContext* cx, JS::HandleValue function,
+                                 GISignalInfo* signal_info,
+                                 unsigned signal_id) {
+    GjsCallbackTrampoline* trampoline = gjs_callback_trampoline_new(
+        cx, function, signal_info, GI_SCOPE_TYPE_CALL, nullptr, false,
+        signal_id);
+    if (!trampoline)
+        return nullptr;
+
+    return g_cclosure_new(reinterpret_cast<GCallback>(&trampoline->closure),
+                          trampoline, [](void* data, GClosure*) {
+                              gjs_callback_trampoline_unref(
+                                  static_cast<GjsCallbackTrampoline*>(data));
+                          });
+}
+
 /* an helper function to retrieve array lengths from a GArgument
    (letting the compiler generate good instructions in case of
    big endian machines) */
@@ -960,12 +996,9 @@ gjs_invoke_c_function(JSContext                             *context,
                     }
 
                     callable_info = (GICallableInfo*) g_type_info_get_interface(&ainfo);
-                    trampoline = gjs_callback_trampoline_new(context,
-                                                             current_arg,
-                                                             callable_info,
-                                                             scope,
-                                                             is_object_method ? obj : nullptr,
-                                                             false);
+                    trampoline = gjs_callback_trampoline_new(
+                        context, current_arg, callable_info, scope,
+                        is_object_method ? obj : nullptr, false, 0);
                     closure = trampoline->closure;
                     g_base_info_unref(callable_info);
                 }
diff --git a/gi/function.h b/gi/function.h
index 30f473c7..66433359 100644
--- a/gi/function.h
+++ b/gi/function.h
@@ -51,18 +51,11 @@ struct GjsCallbackTrampoline {
     ffi_cif cif;
     ffi_closure *closure;
     GIScopeType scope;
+    unsigned signal_id;
     bool is_vfunc;
     GjsParamType *param_types;
 };
 
-GJS_JSAPI_RETURN_CONVENTION
-GjsCallbackTrampoline* gjs_callback_trampoline_new(JSContext       *context,
-                                                   JS::HandleValue  function,
-                                                   GICallableInfo  *callable_info,
-                                                   GIScopeType      scope,
-                                                   JS::HandleObject scope_object,
-                                                   bool             is_vfunc);
-
 void gjs_callback_trampoline_unref(GjsCallbackTrampoline *trampoline);
 void gjs_callback_trampoline_ref(GjsCallbackTrampoline *trampoline);
 
@@ -86,6 +79,16 @@ bool gjs_invoke_constructor_from_c(JSContext                  *context,
                                    const JS::HandleValueArray& args,
                                    GIArgument                 *rvalue);
 
+GJS_JSAPI_RETURN_CONVENTION
+GClosure* gjs_signal_closure_new(JSContext* cx, JS::HandleValue function,
+                                 GISignalInfo* signal_info, unsigned signal_id);
+
 G_END_DECLS
 
+GJS_JSAPI_RETURN_CONVENTION
+GjsCallbackTrampoline* gjs_callback_trampoline_new(
+    JSContext* cx, JS::HandleValue function, GICallableInfo* callable_info,
+    GIScopeType scope, JS::HandleObject scope_object, bool is_vfunc,
+    unsigned signal_id = 0);
+
 #endif  /* __GJS_FUNCTION_H__ */
diff --git a/gi/object.cpp b/gi/object.cpp
index 9d19672e..c37ad378 100644
--- a/gi/object.cpp
+++ b/gi/object.cpp
@@ -1875,6 +1875,22 @@ bool ObjectBase::connect_after(JSContext* cx, unsigned argc, JS::Value* vp) {
     return priv->to_instance()->connect_impl(cx, args, true);
 }
 
+/* Return value must be freed */
+GJS_USE
+static GISignalInfo* lookup_signal(GIObjectInfo* info,
+                                   const char* signal_name) {
+    GISignalInfo* signal_info = g_object_info_find_signal(info, signal_name);
+
+    if (!signal_info) {
+        /* Not found, recurse on parent object info */
+        GjsAutoObjectInfo parent = g_object_info_get_parent(info);
+        if (parent)
+            signal_info = lookup_signal(parent, signal_name);
+    }
+
+    return signal_info;
+}
+
 bool
 ObjectInstance::connect_impl(JSContext          *context,
                              const JS::CallArgs& args,
@@ -1909,9 +1925,15 @@ ObjectInstance::connect_impl(JSContext          *context,
         return false;
     }
 
-    closure = gjs_closure_new_for_signal(context, callback, "signal callback", signal_id);
-    if (closure == NULL)
+    GjsAutoSignalInfo signal_info =
+        lookup_signal(info(), g_signal_name(signal_id));
+    if (!signal_info) {
+        gjs_throw(context, "No introspection information for signal %s",
+                  g_signal_name(signal_id));
         return false;
+    }
+
+    closure = gjs_signal_closure_new(context, args[1], signal_info, signal_id);
     associate_closure(context, closure);
 
     id = g_signal_connect_closure_by_id(m_gobj,
diff --git a/gi/value.cpp b/gi/value.cpp
index fabecca2..d1d980c3 100644
--- a/gi/value.cpp
+++ b/gi/value.cpp
@@ -292,21 +292,6 @@ closure_marshal(GClosure        *closure,
     }
 }
 
-GClosure*
-gjs_closure_new_for_signal(JSContext  *context,
-                           JSObject   *callable,
-                           const char *description,
-                           guint       signal_id)
-{
-    GClosure *closure;
-
-    closure = gjs_closure_new(context, callable, description, false);
-
-    g_closure_set_meta_marshal(closure, GUINT_TO_POINTER(signal_id), closure_marshal);
-
-    return closure;
-}
-
 GClosure*
 gjs_closure_new_marshaled (JSContext    *context,
                            JSObject     *callable,
diff --git a/gi/value.h b/gi/value.h
index af61b8d8..67936cc8 100644
--- a/gi/value.h
+++ b/gi/value.h
@@ -50,11 +50,6 @@ GJS_USE
 GClosure*  gjs_closure_new_marshaled    (JSContext    *context,
                                          JSObject     *callable,
                                          const char   *description);
-GJS_USE
-GClosure*  gjs_closure_new_for_signal   (JSContext    *context,
-                                         JSObject     *callable,
-                                         const char   *description,
-                                         guint         signal_id);
 
 G_END_DECLS
 
diff --git a/gjs/jsapi-util.h b/gjs/jsapi-util.h
index ee9b73c5..f7d6a434 100644
--- a/gjs/jsapi-util.h
+++ b/gjs/jsapi-util.h
@@ -131,6 +131,7 @@ using GjsAutoFunctionInfo = GjsAutoInfo<GI_INFO_TYPE_FUNCTION>;
 using GjsAutoInterfaceInfo = GjsAutoInfo<GI_INFO_TYPE_INTERFACE>;
 using GjsAutoObjectInfo = GjsAutoInfo<GI_INFO_TYPE_OBJECT>;
 using GjsAutoPropertyInfo = GjsAutoInfo<GI_INFO_TYPE_PROPERTY>;
+using GjsAutoSignalInfo = GjsAutoInfo<GI_INFO_TYPE_SIGNAL>;
 using GjsAutoStructInfo = GjsAutoInfo<GI_INFO_TYPE_STRUCT>;
 using GjsAutoTypeInfo = GjsAutoInfo<GI_INFO_TYPE_TYPE>;
 using GjsAutoVFuncInfo = GjsAutoInfo<GI_INFO_TYPE_VFUNC>;