[gjs/306-too-much-recursion: 42/42] object: Only define interface overridden properties on the owner type

[Philip Chimento <pchimento src gnome org>]

commit c334a8eb05fa79642848db19bded804b5264f939
Author: Philip Chimento <philip chimento gmail com>
Date:   Sat Mar 21 23:22:15 2020 -0700

    object: Only define interface overridden properties on the owner type
    
    The previous code did not check if overridden interface properties were
    actually overridden by the type whose ObjectPrototype it was defining
    them on. So, a subclass of a type that implemented an interface would
    get the interface's properties defined on its prototype. In most cases
    that didn't matter, but in the case where the implementing type was
    defined in JS, then the subclass wouldn't get the implementing type's
    property accessors.
    
    Closes: #306

 gi/object.cpp                           | 12 ++++++++++--
 installed-tests/js/testGObjectClass.js  | 25 +++++++++++++++++++++++++
 installed-tests/js/testLegacyGObject.js | 28 ++++++++++++++++++++++++++++
 3 files changed, 63 insertions(+), 2 deletions(-)
---
diff --git a/gi/object.cpp b/gi/object.cpp
index 6a7872b4..e0480602 100644
--- a/gi/object.cpp
+++ b/gi/object.cpp
@@ -718,8 +718,16 @@ bool ObjectPrototype::resolve_no_info(JSContext* cx, JS::HandleObject obj,
         /* If the name refers to a GObject property, lazily define the property
          * in JS as we do below in the real resolve hook. We ignore fields here
          * because I don't think interfaces can have fields */
-        if (is_ginterface_property_name(iface_info, canonical_name))
-            return lazy_define_gobject_property(cx, obj, id, resolved, name);
+        if (is_ginterface_property_name(iface_info, canonical_name)) {
+            GjsAutoTypeClass<GObjectClass> oclass(m_gtype);
+            // unowned
+            GParamSpec* pspec = g_object_class_find_property(
+                oclass, canonical_name);  // unowned
+            if (pspec && pspec->owner_type == m_gtype) {
+                return lazy_define_gobject_property(cx, obj, id, resolved,
+                                                    name);
+            }
+        }
     }
 
     *resolved = false;
diff --git a/installed-tests/js/testGObjectClass.js b/installed-tests/js/testGObjectClass.js
index 1aa5d364..cd89a9f5 100644
--- a/installed-tests/js/testGObjectClass.js
+++ b/installed-tests/js/testGObjectClass.js
@@ -377,6 +377,24 @@ describe('GObject class with decorator', function () {
         }, class BadOverride extends GObject.Object {})).toThrow();
     });
 
+    it('handles gracefully forgetting to override a C property', function () {
+        GLib.test_expect_message('GLib-GObject', GLib.LogLevelFlags.LEVEL_CRITICAL,
+            "*Object class Gjs_ForgottenOverride doesn't implement property " +
+            "'anchors' from interface 'GTlsFileDatabase'*");
+
+        // This is a random interface in Gio with a read-write property
+        const ForgottenOverride = GObject.registerClass({
+            Implements: [Gio.TlsFileDatabase],
+        }, class ForgottenOverride extends Gio.TlsDatabase {});
+        const obj = new ForgottenOverride();
+        expect(obj.anchors).not.toBeDefined();
+        expect(() => (obj.anchors = 'foo')).not.toThrow();
+        expect(obj.anchors).toEqual('foo');
+
+        GLib.test_assert_expected_messages_internal('Gjs', 'testGObjectClass.js', 0,
+            'testGObjectClassForgottenOverride');
+    });
+
     it('handles gracefully overriding a C property but forgetting the accessors', function () {
         // This is a random interface in Gio with a read-write property
         const ForgottenAccessors = GObject.registerClass({
@@ -389,6 +407,13 @@ describe('GObject class with decorator', function () {
         expect(obj.anchors).toBeNull();  // the property's default value
         obj.anchors = 'foo';
         expect(obj.anchors).toEqual('foo');
+
+        const ForgottenAccessors2 =
+            GObject.registerClass(class ForgottenAccessors2 extends ForgottenAccessors {});
+        const obj2 = new ForgottenAccessors2();
+        expect(obj2.anchors).toBeNull();
+        obj2.anchors = 'foo';
+        expect(obj2.anchors).toEqual('foo');
     });
 
     it('does not pollute the wrong prototype with GObject properties', function () {
diff --git a/installed-tests/js/testLegacyGObject.js b/installed-tests/js/testLegacyGObject.js
index 64c164fb..562c7f18 100644
--- a/installed-tests/js/testLegacyGObject.js
+++ b/installed-tests/js/testLegacyGObject.js
@@ -382,6 +382,26 @@ describe('GObject class', function () {
         })).toThrow();
     });
 
+    it('handles gracefully forgetting to override a C property', function () {
+        GLib.test_expect_message('GLib-GObject', GLib.LogLevelFlags.LEVEL_CRITICAL,
+            "*Object class Gjs_ForgottenOverride doesn't implement property " +
+            "'anchors' from interface 'GTlsFileDatabase'*");
+
+        // This is a random interface in Gio with a read-write property
+        const ForgottenOverride = new Lang.Class({
+            Name: 'ForgottenOverride',
+            Extends: Gio.TlsDatabase,
+            Implements: [Gio.TlsFileDatabase],
+        });
+        const obj = new ForgottenOverride();
+        expect(obj.anchors).not.toBeDefined();
+        expect(() => (obj.anchors = 'foo')).not.toThrow();
+        expect(obj.anchors).toEqual('foo');
+
+        GLib.test_assert_expected_messages_internal('Gjs', 'testGObjectClass.js', 0,
+            'testGObjectClassForgottenOverride');
+    });
+
     it('handles gracefully overriding a C property but forgetting the accessors', function () {
         // This is a random interface in Gio with a read-write property
         const ForgottenAccessors = new Lang.Class({
@@ -395,6 +415,14 @@ describe('GObject class', function () {
         const obj = new ForgottenAccessors();
         expect(obj.anchors).toBeNull();
         obj.anchors = 'foo';
+
+        const ForgottenAccessors2 = new Lang.Class({
+            Name: 'ForgottenAccessors2',
+            Extends: ForgottenAccessors,
+        });
+        const obj2 = new ForgottenAccessors2();
+        expect(obj2.anchors).toBeNull();
+        obj2.anchors = 'foo';
     });
 });