[gjs/wip/carlosg/setter-methods-on-construct-only] GObject: Ensure to call setter methods for construct-only properties

[Carlos Garnacho <carlosg src gnome org>]

commit 829627a747ea9c368a9287bfb5d449123dca6c59
Author: Carlos Garnacho <carlosg gnome org>
Date:   Fri Apr 30 19:33:30 2021 +0200

    GObject: Ensure to call setter methods for construct-only properties
    
    Construct-only properties on JS-defined GObject subclasses takes some
    shortcuts to define these JS properties as readonly ones. However, this
    hardcodes the given G/JSValue without giving an opportunity to any
    defined setter methods to be called. These setter methods could do
    more things than setting the value (e.g. initialize related data,
    ensure the given object is in an specific state, ...) and are thus
    not trivial to bypass.
    
    To fix this, ensure to set these properties, before locking them as
    readonly.
    
    Related: https://gitlab.gnome.org/GNOME/gnome-shell/-/issues/3926

 gi/gobject.cpp                         | 25 +++++++++++++++++++++++++
 installed-tests/js/testGObjectClass.js | 19 +++++++++++++++++++
 2 files changed, 44 insertions(+)
---
diff --git a/gi/gobject.cpp b/gi/gobject.cpp
index 65ed6638..1de4b469 100644
--- a/gi/gobject.cpp
+++ b/gi/gobject.cpp
@@ -58,6 +58,31 @@ static bool jsobj_set_gproperty(JSContext* cx, JS::HandleObject object,
     if (pspec->flags & G_PARAM_CONSTRUCT_ONLY) {
         unsigned flags = GJS_MODULE_PROP_FLAGS | JSPROP_READONLY;
         GjsAutoChar camel_name = gjs_hyphen_to_camel(pspec->name);
+        JS::Rooted<JS::PropertyDescriptor> jsprop(cx);
+
+        // Ensure to call any associated setter method
+        if (!g_str_equal(underscore_name.get(), pspec->name)) {
+            if (!JS_GetPropertyDescriptor(cx, object, underscore_name, &jsprop))
+                return false;
+            if (jsprop.setter() &&
+                !JS_SetProperty(cx, object, underscore_name, jsvalue))
+                return false;
+        }
+
+        if (!g_str_equal(camel_name.get(), pspec->name)) {
+            if (!JS_GetPropertyDescriptor(cx, object, camel_name, &jsprop))
+                return false;
+            if (jsprop.setter() &&
+                !JS_SetProperty(cx, object, camel_name, jsvalue))
+                return false;
+        }
+
+        if (!JS_GetPropertyDescriptor(cx, object, pspec->name, &jsprop))
+            return false;
+        if (jsprop.setter() &&
+            !JS_SetProperty(cx, object, pspec->name, jsvalue))
+            return false;
+
         return JS_DefineProperty(cx, object, underscore_name, jsvalue, flags) &&
                JS_DefineProperty(cx, object, camel_name, jsvalue, flags) &&
                JS_DefineProperty(cx, object, pspec->name, jsvalue, flags);
diff --git a/installed-tests/js/testGObjectClass.js b/installed-tests/js/testGObjectClass.js
index 7073ccba..5512dc0e 100644
--- a/installed-tests/js/testGObjectClass.js
+++ b/installed-tests/js/testGObjectClass.js
@@ -770,6 +770,10 @@ describe('Auto accessor generation', function () {
                 'Construct-only property',
                 GObject.ParamFlags.READWRITE | GObject.ParamFlags.CONSTRUCT_ONLY,
                 0, 100, 80),
+            'construct-only-with-setter': GObject.ParamSpec.int('construct-only-with-setter', 'Construct 
only with setter',
+                'Construct-only property with a setter method',
+                GObject.ParamFlags.READWRITE | GObject.ParamFlags.CONSTRUCT_ONLY,
+                0, 100, 80),
             'snake-name': GObject.ParamSpec.int('snake-name', 'Snake name',
                 'Snake-cased property', GObject.ParamFlags.READWRITE, 0, 100, 36),
             'camel-name': GObject.ParamSpec.int('camel-name', 'Camel name',
@@ -787,6 +791,7 @@ describe('Auto accessor generation', function () {
         },
     }, class AutoAccessors extends GObject.Object {
         _init(props = {}) {
+            this._constructOnlySetterCalled = 0;
             super._init(props);
             this._snakeNameGetterCalled = 0;
             this._snakeNameSetterCalled = 0;
@@ -830,6 +835,15 @@ describe('Auto accessor generation', function () {
         get missing_setter() {
             return 42;
         }
+
+        get construct_only_with_setter() {
+            return this._constructOnlyValue;
+        }
+
+        set constructOnlyWithSetter(value) {
+            this._constructOnlySetterCalled++;
+            this._constructOnlyValue = value;
+        }
     });
 
     let a;
@@ -863,6 +877,7 @@ describe('Auto accessor generation', function () {
             longLongName: 1,
             construct: 1,
             'construct-only': 1,
+            'construct-only-with-setter': 2,
         });
         expect(a.simple).toEqual(1);
         expect(a.long_long_name).toEqual(1);
@@ -872,6 +887,10 @@ describe('Auto accessor generation', function () {
         expect(a.construct_only).toEqual(1);
         expect(a.constructOnly).toEqual(1);
         expect(a['construct-only']).toEqual(1);
+        expect(a.constructOnlyWithSetter).toEqual(2);
+        expect(a.construct_only_with_setter).toEqual(2);
+        expect(a['construct-only-with-setter']).toEqual(2);
+        expect(a._constructOnlySetterCalled).toEqual(1);
     });
 
     it('notify when the property changes', function () {