[gjs] object: Enter global compartment in custom constructor

[Philip Chimento <pchimento src gnome org>]

commit 2eb640d208e3d1ede654bdc3990ee4e5ed1fdb24
Author: Philip Chimento <philip endlessm com>
Date:   Tue Nov 15 11:32:31 2016 -0800

    object: Enter global compartment in custom constructor
    
    Previously you could construct JS-defined GObjects from inside JS, but
    not from C. That's because when the constructor() vfunc is called from C,
    the JS engine isn't inside the right compartment.
    
    While entering the compartment using JSAutoCompartment, we also change
    the constructor vfunc to use JSAutoRequest, so that it consistently uses
    a more RAII style.
    
    Test case from commit by Juan Pablo Ugarte <juanpablougarte gmail com>
    
    https://bugzilla.gnome.org/show_bug.cgi?id=770244

 gi/object.cpp      |   84 ++++++++++++++++++++++++----------------------------
 test/gjs-tests.cpp |   31 +++++++++++++++++++
 2 files changed, 70 insertions(+), 45 deletions(-)
---
diff --git a/gi/object.cpp b/gi/object.cpp
index a95e8ae..f0c67a1 100644
--- a/gi/object.cpp
+++ b/gi/object.cpp
@@ -2375,8 +2375,6 @@ gjs_object_constructor (GType                  type,
                         guint                  n_construct_properties,
                         GObjectConstructParam *construct_properties)
 {
-    GObject *gobj = NULL;
-
     if (!object_init_list.empty()) {
         GType parent_type = g_type_parent(type);
 
@@ -2386,59 +2384,55 @@ gjs_object_constructor (GType                  type,
         while (G_OBJECT_CLASS(g_type_class_peek(parent_type))->constructor == gjs_object_constructor)
             parent_type = g_type_parent(parent_type);
 
-        gobj = G_OBJECT_CLASS(g_type_class_peek(parent_type))->constructor(type, n_construct_properties, 
construct_properties);
-    } else {
-        GjsContext *gjs_context;
-        JSContext *context;
-        JSObject *object;
-        ObjectInstance *priv;
-
-        /* The object is being constructed from native code (e.g. GtkBuilder):
-         * Construct the JS object from the constructor, then use the GObject
-         * that was associated in gjs_object_custom_init()
-         */
-        gjs_context = gjs_context_get_current();
-        context = (JSContext*) gjs_context_get_native_context(gjs_context);
-
-        JS_BeginRequest(context);
+        return G_OBJECT_CLASS(g_type_class_peek(parent_type))->constructor(type, n_construct_properties, 
construct_properties);
+    }
 
-        JS::RootedObject constructor(context,
-            gjs_lookup_object_constructor_from_info(context, NULL, type));
-        if (!constructor)
-          goto out;
+    GjsContext *gjs_context;
+    JSContext *context;
+    JSObject *object;
+    ObjectInstance *priv;
 
-        if (n_construct_properties) {
-            guint i;
+    /* The object is being constructed from native code (e.g. GtkBuilder):
+     * Construct the JS object from the constructor, then use the GObject
+     * that was associated in gjs_object_custom_init()
+     */
+    gjs_context = gjs_context_get_current();
+    context = (JSContext*) gjs_context_get_native_context(gjs_context);
 
-            JS::RootedObject props_hash(context,
-                JS_NewObject(context, NULL, JS::NullPtr(), JS::NullPtr()));
+    JSAutoRequest ar(context);
+    JSAutoCompartment ac(context, gjs_get_import_global(context));
 
-            for (i = 0; i < n_construct_properties; i++)
-                jsobj_set_gproperty(context, props_hash,
-                                    construct_properties[i].value,
-                                    construct_properties[i].pspec);
+    JS::RootedObject constructor(context,
+        gjs_lookup_object_constructor_from_info(context, NULL, type));
+    if (!constructor)
+        return NULL;
 
-            JS::AutoValueArray<1> args(context);
-            args[0].set(JS::ObjectValue(*props_hash));
-            object = JS_New(context, constructor, args);
-        } else {
-            object = JS_New(context, constructor, JS::HandleValueArray::empty());
-        }
+    if (n_construct_properties) {
+        guint i;
 
-        if (!object)
-          goto out;
+        JS::RootedObject props_hash(context,
+            JS_NewObject(context, NULL, JS::NullPtr(), JS::NullPtr()));
 
-        priv = (ObjectInstance*) JS_GetPrivate(object);
-        /* We only hold a toggle ref at this point, add back a ref that the
-         * native code can own.
-         */
-        gobj = G_OBJECT(g_object_ref(priv->gobj));
+        for (i = 0; i < n_construct_properties; i++)
+            jsobj_set_gproperty(context, props_hash,
+                                construct_properties[i].value,
+                                construct_properties[i].pspec);
 
-out:
-        JS_EndRequest(context);
+        JS::AutoValueArray<1> args(context);
+        args[0].set(JS::ObjectValue(*props_hash));
+        object = JS_New(context, constructor, args);
+    } else {
+        object = JS_New(context, constructor, JS::HandleValueArray::empty());
     }
 
-    return gobj;
+    if (!object)
+        return NULL;
+
+    priv = (ObjectInstance*) JS_GetPrivate(object);
+    /* We only hold a toggle ref at this point, add back a ref that the
+     * native code can own.
+     */
+    return G_OBJECT(g_object_ref(priv->gobj));
 }
 
 static void
diff --git a/test/gjs-tests.cpp b/test/gjs-tests.cpp
index 065ca45..38a2216 100644
--- a/test/gjs-tests.cpp
+++ b/test/gjs-tests.cpp
@@ -59,6 +59,36 @@ gjstest_test_func_gjs_context_construct_eval(void)
     g_object_unref (context);
 }
 
+#define JS_CLASS "\
+const Lang    = imports.lang; \
+const GObject = imports.gi.GObject; \
+\
+const FooBar = new Lang.Class({ \
+    Name: 'FooBar', \
+    Extends: GObject.Object, \
+}); \
+"
+
+static void
+gjstest_test_func_gjs_gobject_js_defined_type(void)
+{
+    GjsContext *context = gjs_context_new();
+    GError *error = NULL;
+    int status;
+    bool ok = gjs_context_eval(context, JS_CLASS, -1, "<input>", &status, &error);
+    g_assert_no_error(error);
+    g_assert_true(ok);
+
+    GType foo_type = g_type_from_name("Gjs_FooBar");
+    g_assert_cmpuint(foo_type, !=, G_TYPE_INVALID);
+
+    gpointer foo = g_object_new(foo_type, NULL);
+    g_assert(G_IS_OBJECT(foo));
+
+    g_object_unref(foo);
+    g_object_unref(context);
+}
+
 static void
 gjstest_test_func_gjs_jsapi_util_string_js_string_utf8(GjsUnitTestFixture *fx,
                                                        gconstpointer       unused)
@@ -224,6 +254,7 @@ main(int    argc,
 
     g_test_add_func("/gjs/context/construct/destroy", gjstest_test_func_gjs_context_construct_destroy);
     g_test_add_func("/gjs/context/construct/eval", gjstest_test_func_gjs_context_construct_eval);
+    g_test_add_func("/gjs/gobject/js_defined_type", gjstest_test_func_gjs_gobject_js_defined_type);
     g_test_add_func("/gjs/jsutil/strip_shebang/no_shebang", 
gjstest_test_strip_shebang_no_advance_for_no_shebang);
     g_test_add_func("/gjs/jsutil/strip_shebang/have_shebang", 
gjstest_test_strip_shebang_advance_for_shebang);
     g_test_add_func("/gjs/jsutil/strip_shebang/only_shebang", 
gjstest_test_strip_shebang_return_null_for_just_shebang);