[gjs/ewlsh/fix-blow-up] Ensure instance constructor values are chained from Gtk.Widget




commit fb7c21727917b48ef7204f5c91becac39447f631
Author: Evan Welsh <contact evanwelsh com>
Date:   Sat Sep 4 12:38:44 2021 -0700

    Ensure instance constructor values are chained from Gtk.Widget
    
    If a wrapper for a GObject has already been created, _init is
    supposed to return the correct wrapper instead. We were not
    handling this correctly for Gtk.Widget
    
    Fixes #50

 gi/object.cpp                  | 21 ++++++++++++---------
 gi/object.h                    |  2 +-
 installed-tests/js/testGtk3.js | 17 +++++++++++++++++
 modules/core/overrides/Gtk.js  | 26 +++++++++++++++-----------
 4 files changed, 45 insertions(+), 21 deletions(-)
---
diff --git a/gi/object.cpp b/gi/object.cpp
index 77acf3f4..cc945d6e 100644
--- a/gi/object.cpp
+++ b/gi/object.cpp
@@ -1568,11 +1568,8 @@ ObjectInstance::disassociate_js_gobject(void)
     m_wrapper_finalized = true;
 }
 
-bool
-ObjectInstance::init_impl(JSContext              *context,
-                          const JS::CallArgs&     args,
-                          JS::MutableHandleObject object)
-{
+bool ObjectInstance::init_impl(JSContext* context, const JS::CallArgs& args,
+                               JS::HandleObject object) {
     g_assert(gtype() != G_TYPE_NONE);
 
     if (args.length() > 1 &&
@@ -1622,8 +1619,14 @@ ObjectInstance::init_impl(JSContext              *context,
     ObjectInstance *other_priv = ObjectInstance::for_gobject(gobj);
     if (other_priv && other_priv->m_wrapper != object.get()) {
         /* g_object_new_with_properties() returned an object that's already
-         * tracked by a JS object. Let's assume this is a singleton like
-         * IBus.IBus and return the existing JS wrapper object.
+         * tracked by a JS object.
+         *
+         * This typically occurs in one of two cases:
+         * - This object is a singleton like IBus.IBus
+         * - This object passed itself to JS before g_object_new_* returned
+         *
+         * In these cases, return the existing JS wrapper object instead
+         * of creating a new one.
          *
          * 'object' has a value that was originally created by
          * JS_NewObjectForConstructor in GJS_NATIVE_CONSTRUCTOR_PRELUDE, but
@@ -1642,7 +1645,7 @@ ObjectInstance::init_impl(JSContext              *context,
             toggle_ref_added = m_uses_toggle_ref;
         }
 
-        object.set(other_priv->m_wrapper);
+        args.rval().setObject(*other_priv->m_wrapper);
 
         if (toggle_ref_added)
             g_clear_object(&gobj); /* We already own a reference */
@@ -2331,7 +2334,7 @@ bool ObjectBase::init_gobject(JSContext* context, unsigned argc,
     std::string dynamicString = priv->format_name() + "._init";
     AutoProfilerLabel label(context, "", dynamicString.c_str());
 
-    return priv->to_instance()->init_impl(context, argv, &obj);
+    return priv->to_instance()->init_impl(context, argv, obj);
 }
 
 // clang-format off
diff --git a/gi/object.h b/gi/object.h
index 0d9d40fe..e988189c 100644
--- a/gi/object.h
+++ b/gi/object.h
@@ -450,7 +450,7 @@ class ObjectInstance : public GIWrapperInstance<ObjectBase, ObjectPrototype,
         JSContext* cx, const JS::CallArgs& args);
     GJS_JSAPI_RETURN_CONVENTION
     bool init_impl(JSContext* cx, const JS::CallArgs& args,
-                   JS::MutableHandleObject obj);
+                   JS::HandleObject obj);
     [[nodiscard]] const char* to_string_kind() const;
 
     GJS_JSAPI_RETURN_CONVENTION
diff --git a/installed-tests/js/testGtk3.js b/installed-tests/js/testGtk3.js
index 73f3ecb4..27450fea 100644
--- a/installed-tests/js/testGtk3.js
+++ b/installed-tests/js/testGtk3.js
@@ -301,4 +301,21 @@ describe('Gtk overrides', function () {
         win.style_get_property('decoration-button-layout', value);
         expect(value.get_string()).not.toEqual('EMPTY');
     });
+
+    it('can pass a parent object to a child at construction', function () {
+        const frame = new Gtk.Frame();
+        let frameChild = null;
+        frame.connect('add', (_widget, child) => {
+            frameChild = child;
+        });
+        const widget = new Gtk.Label({parent: frame});
+
+        expect(widget).toBe(frameChild);
+        expect(widget instanceof Gtk.Label).toBeTruthy();
+        expect(frameChild instanceof Gtk.Label).toBeTruthy();
+
+        expect(frameChild.visible).toBe(false);
+        expect(() => widget.show()).not.toThrow();
+        expect(frameChild.visible).toBe(true);
+    });
 });
diff --git a/modules/core/overrides/Gtk.js b/modules/core/overrides/Gtk.js
index 306f7d3f..77649a73 100644
--- a/modules/core/overrides/Gtk.js
+++ b/modules/core/overrides/Gtk.js
@@ -26,13 +26,15 @@ function _init() {
     }
 
     Gtk.Widget.prototype._init = function (params) {
-        if (this.constructor[Gtk.template]) {
+        let wrapper = this;
+
+        if (wrapper.constructor[Gtk.template]) {
             if (!BuilderScope) {
-                Gtk.Widget.set_connect_func.call(this.constructor,
+                Gtk.Widget.set_connect_func.call(wrapper.constructor,
                     (builder, obj, signalName, handlerName, connectObj, flags) => {
                         const swapped = flags & GObject.ConnectFlags.SWAPPED;
                         const closure = _createClosure(
-                            builder, this, handlerName, swapped, connectObj);
+                            builder, wrapper, handlerName, swapped, connectObj);
 
                         if (flags & GObject.ConnectFlags.AFTER)
                             obj.connect_after(signalName, closure);
@@ -42,21 +44,23 @@ function _init() {
             }
         }
 
-        GObject.Object.prototype._init.call(this, params);
+        wrapper = GObject.Object.prototype._init.call(wrapper, params) ?? wrapper;
 
-        if (this.constructor[Gtk.template]) {
-            let children = this.constructor[Gtk.children] || [];
+        if (wrapper.constructor[Gtk.template]) {
+            let children = wrapper.constructor[Gtk.children] || [];
             for (let child of children) {
-                this[child.replace(/-/g, '_')] =
-                    this.get_template_child(this.constructor, child);
+                wrapper[child.replace(/-/g, '_')] =
+                    wrapper.get_template_child(wrapper.constructor, child);
             }
 
-            let internalChildren = this.constructor[Gtk.internalChildren] || [];
+            let internalChildren = wrapper.constructor[Gtk.internalChildren] || [];
             for (let child of internalChildren) {
-                this[`_${child.replace(/-/g, '_')}`] =
-                    this.get_template_child(this.constructor, child);
+                wrapper[`_${child.replace(/-/g, '_')}`] =
+                    wrapper.get_template_child(wrapper.constructor, child);
             }
         }
+
+        return wrapper;
     };
 
     Gtk.Widget._classInit = function (klass) {


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