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




commit 0155f3e62841ace9d71108a6579c74c7c609988b
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                         | 10 ++++++++++
 installed-tests/js/testGObjectClass.js | 26 ++++++++++++++++++++++++++
 2 files changed, 36 insertions(+)
---
diff --git a/gi/gobject.cpp b/gi/gobject.cpp
index 65ed6638..8164d230 100644
--- a/gi/gobject.cpp
+++ b/gi/gobject.cpp
@@ -58,6 +58,16 @@ 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);
+
+        // Ensure to call any associated setter method
+        if (!JS_SetProperty(cx, object, underscore_name, jsvalue))
+            return false;
+
+        if (g_strcmp0(camel_name.get(), pspec->name) != 0) {
+            if (!JS_SetProperty(cx, object, camel_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..d2be06eb 100644
--- a/installed-tests/js/testGObjectClass.js
+++ b/installed-tests/js/testGObjectClass.js
@@ -770,6 +770,14 @@ 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),
+            'construct-only-setter-called': GObject.ParamSpec.boolean('construct-only-setter-called', 
'Construct only setter was called',
+                'Whether the construct-only property with a setter method was called',
+                GObject.ParamFlags.READABLE,
+                false),
             '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',
@@ -794,6 +802,7 @@ describe('Auto accessor generation', function () {
             this._camelNameSetterCalled = 0;
             this._kebabNameGetterCalled = 0;
             this._kebabNameSetterCalled = 0;
+            this._constructOnlyValue = false;
         }
 
         get snake_name() {
@@ -830,6 +839,19 @@ describe('Auto accessor generation', function () {
         get missing_setter() {
             return 42;
         }
+
+        get construct_only_with_setter() {
+            return this._constructOnlyValue;
+        }
+
+        set constructOnlyWithSetter(value) {
+            this._constructOnlySetterCalled = true;
+            this._constructOnlyValue = value;
+        }
+
+        get constructOnlySetterCalled() {
+            return this._constructOnlySetterCalled;
+        }
     });
 
     let a;
@@ -863,6 +885,7 @@ describe('Auto accessor generation', function () {
             longLongName: 1,
             construct: 1,
             'construct-only': 1,
+            'construct-only-with-setter': 1,
         });
         expect(a.simple).toEqual(1);
         expect(a.long_long_name).toEqual(1);
@@ -872,6 +895,9 @@ 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(1);
+        expect(a.construct_only_with_setter).toEqual(1);
+        expect(a.constructOnlySetterCalled).toEqual(true);
     });
 
     it('notify when the property changes', function () {


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]