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

[Philip Chimento <pchimento src gnome org>]

commit 5add9251fff63ad5eef628dcfebfa35b87228158
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  | 23 +++++++++++++++++++++++
 installed-tests/js/testLegacyGObject.js | 27 +++++++++++++++++++++++++++
 3 files changed, 60 insertions(+), 2 deletions(-)
---
diff --git a/gi/object.cpp b/gi/object.cpp
index b5a638b8..2f5b7816 100644
--- a/gi/object.cpp
+++ b/gi/object.cpp
@@ -713,8 +713,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 298559db..27a0c3c3 100644
--- a/installed-tests/js/testGObjectClass.js
+++ b/installed-tests/js/testGObjectClass.js
@@ -377,6 +377,23 @@ 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();
+
+        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({
@@ -388,6 +405,12 @@ describe('GObject class with decorator', function () {
         const obj = new ForgottenAccessors();
         expect(obj.anchors).toBeNull();  // the property's default value
         obj.anchors = 'foo';
+
+        const ForgottenAccessors2 =
+            GObject.registerClass(class ForgottenAccessors2 extends ForgottenAccessors {});
+        const obj2 = new ForgottenAccessors2();
+        expect(obj2.anchors).toBeNull();
+        obj2.anchors = '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..fff0b111 100644
--- a/installed-tests/js/testLegacyGObject.js
+++ b/installed-tests/js/testLegacyGObject.js
@@ -382,6 +382,25 @@ 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();
+
+        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 +414,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';
     });
 });