[gjs/wip/ptomato/mozjs38: 1/17] function: Two wrappers in GjsCallbackTrampoline

[Philip Chimento <pchimento src gnome org>]

commit 382d8ba491dfcbbe4ae6268b6d32204dded2d060
Author: Philip Chimento <philip chimento gmail com>
Date:   Wed Jan 11 23:34:40 2017 -0800

    function: Two wrappers in GjsCallbackTrampoline
    
    The previous situation was that the JS::Value holding the a trampoline's
    function was stored in a JS::Heap wrapper. If the function was owned by
    the trampoline (the normal case), then it was rooted. If the function was
    a vfunc, in which case it was owned by the GObject class prototype and
    the trampoline was essentially leaked, then it remained a weak pointer.
    
    In SpiderMonkey 38, JS::AddValueRoot() and friends are going away, in
    favour of JS::PersistentRooted. However, JS::PersistentRooted does not
    have the option of maintaining a weak pointer to the value. Therefore, we
    use a JS::Heap to store a vfunc value and JS::PersistentRooted* otherwise.
    We maintain the invariant that one of the two must be null and provide a
    function to get the function value from the appropriate wrapper.
    
    This will still need to change further, since weak pointers must be
    updated when they are moved by the GC.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=776966

 gi/function.cpp |   34 +++++++++++++++++++++-------------
 gi/function.h   |    5 +++++
 2 files changed, 26 insertions(+), 13 deletions(-)
---
diff --git a/gi/function.cpp b/gi/function.cpp
index b7a0661..ae46d69 100644
--- a/gi/function.cpp
+++ b/gi/function.cpp
@@ -83,14 +83,7 @@ gjs_callback_trampoline_unref(GjsCallbackTrampoline *trampoline)
 
     trampoline->ref_count--;
     if (trampoline->ref_count == 0) {
-        JSContext *context = trampoline->context;
-
-        if (!trampoline->is_vfunc) {
-            JS_BeginRequest(context);
-            JS::RemoveValueRoot(context, &trampoline->js_function);
-            JS_EndRequest(context);
-        }
-
+        delete trampoline->js_rooted_function;
         g_callable_info_free_closure(trampoline->info, trampoline->closure);
         g_base_info_unref( (GIBaseInfo*) trampoline->info);
         g_free (trampoline->param_types);
@@ -99,6 +92,18 @@ gjs_callback_trampoline_unref(GjsCallbackTrampoline *trampoline)
     }
 }
 
+static JS::Value
+gjs_callback_trampoline_get_function(GjsCallbackTrampoline *trampoline)
+{
+    g_assert(trampoline->js_function.isUndefined() ||
+             trampoline->js_rooted_function == NULL);
+
+    if (trampoline->is_vfunc)
+        return trampoline->js_function;
+    else
+        return trampoline->js_rooted_function->get();
+}
+
 static void
 set_return_ffi_arg_from_giargument (GITypeInfo  *ret_type,
                                     void        *result,
@@ -209,7 +214,7 @@ gjs_callback_closure(ffi_cif *cif,
     }
 
     JS_BeginRequest(context);
-    func_obj = &trampoline->js_function.toObject();
+    func_obj = &gjs_callback_trampoline_get_function(trampoline).toObject();
     JSAutoCompartment ac(context, func_obj);
 
     n_args = g_callable_info_get_n_args(trampoline->info);
@@ -220,7 +225,8 @@ gjs_callback_closure(ffi_cif *cif,
     JS::AutoValueVector jsargs(context);
     jsargs.reserve(n_args);
     JS::RootedValue rval(context);
-    JS::RootedValue rooted_function(context, trampoline->js_function);
+    JS::RootedValue rooted_function(context,
+        gjs_callback_trampoline_get_function(trampoline));
     JS::RootedObject this_object(context);
 
     for (i = 0, n_jsargs = 0; i < n_args; i++) {
@@ -456,9 +462,11 @@ gjs_callback_trampoline_new(JSContext      *context,
     trampoline->context = context;
     trampoline->info = callable_info;
     g_base_info_ref((GIBaseInfo*)trampoline->info);
-    trampoline->js_function = function;
-    if (!is_vfunc)
-        JS::AddValueRoot(context, &trampoline->js_function);
+    if (is_vfunc)
+        trampoline->js_function = function;
+    else
+        trampoline->js_rooted_function =
+            new JS::PersistentRootedValue(context, function);
 
     /* Analyze param types and directions, similarly to init_cached_function_data */
     n_args = g_callable_info_get_n_args(trampoline->info);
diff --git a/gi/function.h b/gi/function.h
index 8bc9cb1..046abbd 100644
--- a/gi/function.h
+++ b/gi/function.h
@@ -45,7 +45,12 @@ struct GjsCallbackTrampoline {
     gint ref_count;
     JSContext *context;
     GICallableInfo *info;
+
+    /* One of these two must be null, depending on whether the function
+     * is owned by the trampoline (normal) or not (vfunc) */
     JS::Heap<JS::Value> js_function;
+    JS::PersistentRootedValue *js_rooted_function;
+
     ffi_cif cif;
     ffi_closure *closure;
     GIScopeType scope;