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

[Evan Welsh <ewlsh src gnome org>]

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

    gi: Use accessors to dynamically fetch prototype properties
    
    Co-authored-by: Philip Chimento <philip chimento gmail com>

 gi/object.cpp                              | 139 +++++++++++++++++++++++------
 gjs/atoms.h                                |   1 +
 installed-tests/js/testGObjectInterface.js |  77 +++++++++++-----
 3 files changed, 168 insertions(+), 49 deletions(-)
---
diff --git a/gi/object.cpp b/gi/object.cpp
index da5d632b..0effe95f 100644
--- a/gi/object.cpp
+++ b/gi/object.cpp
@@ -645,40 +645,121 @@ 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) {
-    GType gtype = g_base_info_get_type(iface_info);
-    JS::RootedObject prototype(
-        cx, gjs_lookup_object_prototype_from_info(cx, iface_info, gtype));
-    if (!prototype)
-        return false;
+// An object shared by the getter and setter to store the interface' prototype
+// and overrides.
+static constexpr size_t ACCESSOR_SLOT = 0;
+
+static bool interface_getter(JSContext* cx, unsigned argc, JS::Value* vp) {
+    JS::CallArgs args = JS::CallArgsFromVp(argc, vp);
 
-    JS::RootedId method_name_id(cx, gjs_intern_string_to_id(cx, method_name));
+    JS::RootedValue v_accessor(
+        cx, js::GetFunctionNativeReserved(&args.callee(), ACCESSOR_SLOT));
+    g_assert(v_accessor.isObject() && "accessor must be an object");
+    JS::RootedObject accessor(cx, &v_accessor.toObject());
 
-    bool target_has_property;
-    if (!JS_HasPropertyById(cx, target_prototype, method_name_id,
-                            &target_has_property))
+    const GjsAtoms& atoms = GjsContextPrivate::atoms(cx);
+
+    // Check if an override value has been set
+    bool has_override = false;
+    if (!JS_HasPropertyById(cx, accessor, atoms.override(), &has_override))
         return false;
+    if (has_override)
+        return JS_GetPropertyById(cx, accessor, atoms.override(), args.rval());
 
-    // Don't overwrite an existing method implementation...
-    if (target_has_property) {
-        *found = true;
-        return true;
-    }
+    JS::RootedValue v_prototype(cx);
+    if (!JS_GetPropertyById(cx, accessor, atoms.prototype(), &v_prototype))
+        return false;
+    g_assert(v_prototype.isObject() && "prototype must be an object");
+
+    JS::RootedObject prototype(cx, &v_prototype.toObject());
+    JS::RootedId id(cx, JS::PropertyKey::fromNonIntAtom(JS_GetFunctionId(
+                            JS_GetObjectFunction(&args.callee()))));
+    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(), ACCESSOR_SLOT));
+    JS::RootedObject accessor(cx, &v_accessor.toObject());
+
+    const GjsAtoms& atoms = GjsContextPrivate::atoms(cx);
+    return JS_SetPropertyById(cx, accessor, atoms.override(), args[0]);
+}
+
+static bool resolve_on_interface_prototype(JSContext* cx,
+                                           GIObjectInfo* iface_info,
+                                           JS::HandleId identifier,
+                                           JS::HandleObject class_prototype,
+                                           bool* found) {
+    GType gtype = g_base_info_get_type(iface_info);
+    JS::RootedObject interface_prototype(
+        cx, gjs_lookup_object_prototype_from_info(cx, iface_info, gtype));
+    if (!interface_prototype)
+        return false;
 
     JS::Rooted<JS::PropertyDescriptor> desc(cx);
-    if (!JS_GetPropertyDescriptorById(cx, prototype, method_name_id, &desc))
+    if (!JS_GetPropertyDescriptorById(cx, interface_prototype, identifier,
+                                      &desc))
         return false;
 
+    // If the property doesn't exist on the interface prototype, we don't need
+    // to perform this trick.
     if (!desc.object()) {
         *found = false;
         return true;
     }
 
+    // Lazily define a property on the class prototype if a property
+    // of that name is present on an interface prototype that the class
+    // implements.
+    //
+    // Define a property of the same name on the class prototype, with a
+    // getter and setter. This is so that e.g. file.dup() calls the _current_
+    // value of Gio.File.prototype.dup(), not the original, so that it can be
+    // overridden (or monkeypatched).
+    //
+    // The setter (interface_setter() above) marks the property as overridden if
+    // it is set from user code. The getter (interface_getter() above) proxies
+    // the interface prototype's property, unless it was marked as overridden.
+    //
+    // Store the identifier in the getter and setter function's ID slots for
+    // to enable looking up the original value on the interface prototype.
+    JS::RootedObject getter(
+        cx, JS_GetFunctionObject(js::NewFunctionByIdWithReserved(
+                cx, interface_getter, 0, 0, identifier)));
+    if (!getter)
+        return false;
+
+    JS::RootedObject setter(
+        cx, JS_GetFunctionObject(js::NewFunctionByIdWithReserved(
+                cx, interface_setter, 1, 0, identifier)));
+    if (!setter)
+        return false;
+
+    JS::RootedObject accessor(cx, JS_NewPlainObject(cx));
+    if (!accessor)
+        return false;
+
+    js::SetFunctionNativeReserved(setter, ACCESSOR_SLOT,
+                                  JS::ObjectValue(*accessor));
+    js::SetFunctionNativeReserved(getter, ACCESSOR_SLOT,
+                                  JS::ObjectValue(*accessor));
+
+    const GjsAtoms& atoms = GjsContextPrivate::atoms(cx);
+    JS::RootedValue v_prototype(cx, JS::ObjectValue(*interface_prototype));
+    if (!JS_SetPropertyById(cx, accessor, atoms.prototype(), v_prototype))
+        return false;
+
+    // Copy the original descriptor and remove any value, instead
+    // adding our getter and setter.
     JS::Rooted<JS::PropertyDescriptor> target_desc(cx, desc);
-    if (!JS_DefinePropertyById(cx, target_prototype, method_name_id,
-                               target_desc))
+    target_desc.setValue(JS::UndefinedHandleValue);
+    target_desc.setAttributes(JSPROP_SETTER | JSPROP_GETTER);
+    target_desc.setGetterObject(getter);
+    target_desc.setSetterObject(setter);
+
+    if (!JS_DefinePropertyById(cx, class_prototype, identifier, target_desc))
         return false;
 
     *found = true;
@@ -733,8 +814,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...
@@ -747,13 +828,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(
@@ -763,6 +843,9 @@ bool ObjectPrototype::resolve_no_info(JSContext* cx, JS::HandleObject obj,
                                                     name);
             }
         }
+
+        return resolve_on_interface_prototype(cx, iface_info, id, obj,
+                                              resolved);
     }
 
     *resolved = false;
@@ -951,8 +1034,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
@@ -966,8 +1049,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/gjs/atoms.h b/gjs/atoms.h
index 4d3f0faf..2c6e7c6b 100644
--- a/gjs/atoms.h
+++ b/gjs/atoms.h
@@ -51,6 +51,7 @@ class JSTracer;
     macro(name, "name") \
     macro(new_, "new") \
     macro(new_internal, "_new_internal") \
+    macro(override, "override") \
     macro(overrides, "overrides") \
     macro(param_spec, "ParamSpec") \
     macro(parent_module, "__parentModule__") \
diff --git a/installed-tests/js/testGObjectInterface.js b/installed-tests/js/testGObjectInterface.js
index 94c7f012..cbeaa4f9 100644
--- a/installed-tests/js/testGObjectInterface.js
+++ b/installed-tests/js/testGObjectInterface.js
@@ -302,6 +302,63 @@ 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 file, originalDup;
+
+        beforeAll(function () {
+            file = Gio.File.new_for_path('/');
+            originalDup = Gio.File.prototype.dup;
+        });
+
+        it('overrides are inherited by implementing classes', function () {
+            spyOn(Gio.File.prototype, 'dup');
+
+            expect(file).toBeInstanceOf(Gio.File);
+            expect(file).toBeInstanceOf(Gio._LocalFilePrototype.constructor);
+
+            file.dup();
+            expect(Gio.File.prototype.dup).toHaveBeenCalledOnceWith();
+
+            Gio.File.prototype.dup = originalDup;
+            expect(file.dup).toBe(originalDup);
+        });
+
+        it('unknown properties are inherited by implementing classes', function () {
+            Gio.File.prototype._originalDup = originalDup;
+            expect(file._originalDup).toBe(originalDup);
+
+            Gio.File.prototype._originalDup = 5;
+            expect(file._originalDup).toBe(5);
+
+            delete Gio.File.prototype._originalDup;
+            expect(file._originalDup).not.toBeDefined();
+        });
+
+        it('original property can be shadowed by class prototype property', function () {
+            spyOn(Gio._LocalFilePrototype, 'dup').and.returnValue(5);
+
+            expect(file.dup()).toBe(5);
+            expect(Gio._LocalFilePrototype.dup).toHaveBeenCalled();
+        });
+
+        it('overridden property can be shadowed by class prototype property', function () {
+            spyOn(Gio._LocalFilePrototype, 'dup');
+            spyOn(Gio.File.prototype, 'dup');
+
+            file.dup();
+            expect(Gio._LocalFilePrototype.dup).toHaveBeenCalled();
+            expect(Gio.File.prototype.dup).not.toHaveBeenCalled();
+        });
+
+        it('shadowed property can be restored', function () {
+            Gio._LocalFilePrototype.dup = 5;
+            expect(file.dup).toBe(5);
+
+            delete Gio._LocalFilePrototype.dup;
+            expect(file.dup).toBeInstanceOf(Function);
+        });
+    });
 });
 
 describe('Specific class and interface checks', function () {
@@ -313,23 +370,3 @@ describe('Specific class and interface checks', function () {
         })).toThrow();
     });
 });
-
-describe('Interface prototypes', function () {
-    beforeAll(function () {
-        // Use .dup because we are unlikely to use it in this test.
-        // Unfortunately, overriding interface prototypes is not easily
-        // reversed.
-        spyOn(Gio.File.prototype, 'dup');
-    });
-
-    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(Gio.File.prototype.dup).toHaveBeenCalledOnceWith();
-    });
-});