[gjs/ewlsh/interface-resolution] gi: Use accessors to dynamically fetch prototype properties

[Evan Welsh <ewlsh src gnome org>]

commit b931a704ea90b48a3fef40d8f488a2b96887d950
Author: Evan Welsh <contact evanwelsh com>
Date:   Thu Aug 26 21:18:46 2021 -0700

    gi: Use accessors to dynamically fetch prototype properties

 gi/object.cpp                              | 133 +++++++++++++++++++++++++----
 installed-tests/js/testGObjectInterface.js |  62 ++++++++------
 2 files changed, 153 insertions(+), 42 deletions(-)
---
diff --git a/gi/object.cpp b/gi/object.cpp
index 5932d12b..7be05e18 100644
--- a/gi/object.cpp
+++ b/gi/object.cpp
@@ -641,10 +641,62 @@ bool ObjectPrototype::lazy_define_gobject_property(JSContext* cx,
     return true;
 }
 
-static bool copy_interface_method_to_prototype(JSContext* cx, GIObjectInfo* iface_info,
-                                  const char* method_name,
-                                  JS::HandleObject target_prototype,
-                                  bool* found) {
+enum class InterfaceAccessorPrivateSlot : size_t {
+    // The identifier to get on the interface' prototype.
+    IDENTIFIER = 0,
+    // An object shared by the getter and setter to store the interface'
+    // prototype and
+    // overrides.
+    ACCESSOR = 1,
+};
+
+static bool interface_getter(JSContext* cx, unsigned argc, JS::Value* vp) {
+    JS::CallArgs args = JS::CallArgsFromVp(argc, vp);
+    JS::RootedValue v_method_id(
+        cx, js::GetFunctionNativeReserved(
+                &args.callee(),
+                static_cast<size_t>(InterfaceAccessorPrivateSlot::IDENTIFIER)));
+    JS::RootedId id(cx);
+    if (!JS_ValueToId(cx, v_method_id, &id))
+        return false;
+    JS::RootedValue v_accessor(
+        cx, js::GetFunctionNativeReserved(
+                &args.callee(),
+                static_cast<size_t>(InterfaceAccessorPrivateSlot::ACCESSOR)));
+    JS::RootedObject accessor(cx, &v_accessor.toObject());
+    JS::RootedValue v_prototype(cx);
+
+    const GjsAtoms& atoms = GjsContextPrivate::atoms(cx);
+    bool has_override = false;
+    if (!JS_HasPropertyById(cx, accessor, atoms.overrides(), &has_override))
+        return false;
+    if (has_override)
+        return JS_GetPropertyById(cx, accessor, atoms.overrides(), args.rval());
+    if (!JS_GetPropertyById(cx, accessor, atoms.prototype(), &v_prototype))
+        return false;
+
+    JS::RootedObject prototype(cx, &v_prototype.toObject());
+
+    return JS_GetPropertyById(cx, prototype, id, args.rval());
+}
+
+static bool interface_setter(JSContext* cx, unsigned argc, JS::Value* vp) {
+    JS::CallArgs args = JS::CallArgsFromVp(argc, vp);
+    JS::RootedValue v_accessor(
+        cx, js::GetFunctionNativeReserved(
+                &args.callee(),
+                static_cast<size_t>(InterfaceAccessorPrivateSlot::ACCESSOR)));
+    JS::RootedObject accessor(cx, &v_accessor.toObject());
+
+    const GjsAtoms& atoms = GjsContextPrivate::atoms(cx);
+    return JS_SetPropertyById(cx, accessor, atoms.overrides(), args[0]);
+}
+
+static bool resolve_on_interface_prototype(JSContext* cx,
+                                           GIObjectInfo* iface_info,
+                                           JS::HandleId identifier,
+                                           JS::HandleObject target_prototype,
+                                           bool* found) {
     GType gtype = g_base_info_get_type(iface_info);
     JS::RootedObject constructor(
         cx, gjs_lookup_object_constructor_from_info(cx, iface_info, gtype));
@@ -664,10 +716,9 @@ static bool copy_interface_method_to_prototype(JSContext* cx, GIObjectInfo* ifac
     }
 
     JS::RootedObject prototype(cx, &v_prototype.toObject());
-    JS::RootedId method_name_id(cx, gjs_intern_string_to_id(cx, method_name));
 
     bool target_has_property;
-    if (!JS_HasPropertyById(cx, target_prototype, method_name_id,
+    if (!JS_HasPropertyById(cx, target_prototype, identifier,
                             &target_has_property))
         return false;
 
@@ -676,17 +727,57 @@ static bool copy_interface_method_to_prototype(JSContext* cx, GIObjectInfo* ifac
         return true;
 
     JS::Rooted<JS::PropertyDescriptor> desc(cx);
-    if (!JS_GetPropertyDescriptorById(cx, prototype, method_name_id, &desc))
+    if (!JS_GetPropertyDescriptorById(cx, prototype, identifier, &desc))
         return false;
 
+    // Check if the property exists on the prototype...
     if (!desc.object()) {
        *found = false;
        return true;
     }
 
     JS::Rooted<JS::PropertyDescriptor> target_desc(cx, desc);
-    if (!JS_DefinePropertyById(cx, target_prototype, method_name_id,
-                            target_desc))
+    JS::RootedObject getter(
+        cx, JS_GetFunctionObject(js::NewFunctionWithReserved(
+                cx, interface_getter, 0, 0, "interface_prototype_getter")));
+    if (!getter)
+        return false;
+
+    JS::RootedObject setter(
+        cx, JS_GetFunctionObject(js::NewFunctionWithReserved(
+                cx, interface_setter, 1, 0, "interface_prototype_setter")));
+    if (!setter)
+        return false;
+
+    JS::RootedValue v_id(cx);
+    if (!JS_IdToValue(cx, identifier, &v_id))
+        return false;
+
+    // Store the property identifier...
+    js::SetFunctionNativeReserved(
+        setter, static_cast<size_t>(InterfaceAccessorPrivateSlot::IDENTIFIER),
+        v_id);
+    js::SetFunctionNativeReserved(
+        getter, static_cast<size_t>(InterfaceAccessorPrivateSlot::IDENTIFIER),
+        v_id);
+
+    JS::RootedObject accessor(cx, JS_NewPlainObject(cx));
+    js::SetFunctionNativeReserved(
+        setter, static_cast<size_t>(InterfaceAccessorPrivateSlot::ACCESSOR),
+        JS::ObjectValue(*accessor));
+    js::SetFunctionNativeReserved(
+        getter, static_cast<size_t>(InterfaceAccessorPrivateSlot::ACCESSOR),
+        JS::ObjectValue(*accessor));
+
+    if (!JS_SetPropertyById(cx, accessor, atoms.prototype(), v_prototype))
+        return false;
+
+    target_desc.setValue(JS::UndefinedHandleValue);
+    target_desc.setAttributes(JSPROP_SETTER | JSPROP_GETTER);
+    target_desc.setGetterObject(getter);
+    target_desc.setSetterObject(setter);
+
+    if (!JS_DefinePropertyById(cx, target_prototype, identifier, target_desc))
         return false;
 
     *found = true;
@@ -741,8 +832,8 @@ bool ObjectPrototype::resolve_no_info(JSContext* cx, JS::HandleObject obj,
         if (method_info) {
             if (g_function_info_get_flags (method_info) & GI_FUNCTION_IS_METHOD) {
                 bool found = false;
-                if (!copy_interface_method_to_prototype(cx, iface_info, method_info.name(),
-                                           obj, &found))
+                if (!resolve_on_interface_prototype(cx, iface_info, id, obj,
+                                                    &found))
                     return false;
 
                 // Fallback to defining the function from type info...
@@ -754,13 +845,12 @@ bool ObjectPrototype::resolve_no_info(JSContext* cx, JS::HandleObject obj,
             }
         }
 
-        if (!canonical_name)
-            continue;
 
         /* 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)) {
+        if (canonical_name &&
+            is_ginterface_property_name(iface_info, canonical_name)) {
             GjsAutoTypeClass<GObjectClass> oclass(m_gtype);
             // unowned
             GParamSpec* pspec = g_object_class_find_property(
@@ -770,6 +860,15 @@ bool ObjectPrototype::resolve_no_info(JSContext* cx, JS::HandleObject obj,
                                                     name);
             }
         }
+
+        bool found = false;
+        if (!resolve_on_interface_prototype(cx, iface_info, id, obj, &found))
+            return false;
+
+        if (found) {
+            *resolved = true;
+            return true;
+        }
     }
 
     *resolved = false;
@@ -958,8 +1057,8 @@ bool ObjectPrototype::uncached_resolve(JSContext* context, JS::HandleObject obj,
                   method_info.name(), type_name(), ns(), this->name());
         if (GI_IS_INTERFACE_INFO(implementor_info)) {
             bool found = false;
-            if (!copy_interface_method_to_prototype(context, implementor_info,
-                                       method_info.name(), obj, &found))
+            if (!resolve_on_interface_prototype(context, implementor_info, id,
+                                                obj, &found))
                 return false;
 
             // If the method was not found fallback to defining the function
@@ -972,8 +1071,6 @@ bool ObjectPrototype::uncached_resolve(JSContext* context, JS::HandleObject obj,
         }
 
         *resolved = true; /* we defined the prop in obj */
-    } else {
-        *resolved = false;
     }
 
     return true;
diff --git a/installed-tests/js/testGObjectInterface.js b/installed-tests/js/testGObjectInterface.js
index 34236c53..f6f61e7a 100644
--- a/installed-tests/js/testGObjectInterface.js
+++ b/installed-tests/js/testGObjectInterface.js
@@ -302,6 +302,44 @@ describe('GObject interface', function () {
         expect(new GObjectImplementingGObjectInterface().toString()).toMatch(
             /\[object instance wrapper GType:Gjs_GObjectImplementingGObjectInterface jsobj@0x[a-f0-9]+ 
native@0x[a-f0-9]+\]/);
     });
+
+    describe('prototype', function () {
+        let originalDup;
+
+        beforeAll(function () {
+            originalDup =  Gio.File.prototype.dup;
+        });
+
+        it('overrides are inherited by implementing classes', function () {
+            let dupSpy = jasmine.createSpy();
+            Gio.File.prototype.dup = dupSpy;
+
+            const file = Gio.File.new_for_path('/');
+
+            expect(file).toBeInstanceOf(Gio.File);
+            expect(file).toBeInstanceOf(Gio._LocalFilePrototype.constructor);
+
+            file.dup();
+
+            expect(dupSpy).toHaveBeenCalledOnceWith();
+
+            Gio.File.prototype.dup = originalDup;
+
+            expect(file.dup).toBe(originalDup);
+        });
+
+        it('unknown properties are inherited by implementing classes', function () {
+            const file = Gio.File.new_for_path('/');
+
+            Gio.File.prototype._originalDup = originalDup;
+
+            expect(file._originalDup).toBe(originalDup);
+
+            Gio.File.prototype._originalDup = 5;
+
+            expect(file._originalDup).toBe(5);
+        });
+    });
 });
 
 describe('Specific class and interface checks', function () {
@@ -313,27 +351,3 @@ describe('Specific class and interface checks', function () {
         })).toThrow();
     });
 });
-
-describe('Interface prototypes', function () {
-    let dupSpy = jasmine.createSpy();
-
-    beforeAll(function () {
-        // Use .dup because we are unlikely to
-        // use it in this test.
-        //
-        // Unfortunately, overriding interface
-        // prototypes is not easily reversed.
-        Gio.File.prototype.dup = dupSpy;
-    });
-
-    it('overrides are inherited by implementing classes', function () {
-        const file = Gio.File.new_for_path('/');
-
-        expect(file).toBeInstanceOf(Gio.File);
-        expect(file).toBeInstanceOf(Gio._LocalFilePrototype.constructor);
-
-        file.dup();
-
-        expect(dupSpy).toHaveBeenCalledOnceWith();
-    });
-});