[gjs: 1/2] GDBus Implementation: fix invalidating properties when emitting changes

[Philip Chimento <pchimento src gnome org>]

commit 6f3219374db3c5211df434a13c4052c83441a686
Author: Andy Holmes <andrew g r holmes gmail com>
Date:   Mon Jul 12 18:52:10 2021 -0700

    GDBus Implementation: fix invalidating properties when emitting changes
    
    The hash table used to batch emit property changes was not able to hold
    NULL values, which is how we determine a property change is an
    invalidation.
    
    Add _g_variant_ref_sink0() and _g_variant_unref0() which can handle
    NULL values safely and use them where appropriate, including the hash
    table's value-free function.
    
    closes #427

 installed-tests/js/testGDBus.js    | 21 +++++++++++++++++++++
 libgjs-private/gjs-gdbus-wrapper.c | 27 +++++++++++++++++++++------
 2 files changed, 42 insertions(+), 6 deletions(-)
---
diff --git a/installed-tests/js/testGDBus.js b/installed-tests/js/testGDBus.js
index ae93cda2..78907e48 100644
--- a/installed-tests/js/testGDBus.js
+++ b/installed-tests/js/testGDBus.js
@@ -142,6 +142,10 @@ class Test {
         return `${a} ${b} ${c} ${d} ${e}`;
     }
 
+    emitPropertyChanged(name, value) {
+        this._impl.emit_property_changed(name, value);
+    }
+
     emitSignal() {
         this._impl.emit_signal('signalFoo', GLib.Variant.new('(s)', ['foobar']));
     }
@@ -661,4 +665,21 @@ describe('Exported DBus object', function () {
 
         expect(proxy.PropReadOnly).toBe(PROP_READ_ONLY_INITIAL_VALUE);
     });
+
+    it('Marking a property as invalidated works', function () {
+        let changedProps = {};
+        let invalidatedProps = [];
+
+        proxy.connect('g-properties-changed', (proxy_, changed, invalidated) => {
+            changedProps = changed.deepUnpack();
+            invalidatedProps = invalidated;
+            loop.quit();
+        });
+
+        test.emitPropertyChanged('PropReadOnly', null);
+        loop.run();
+
+        expect(changedProps).not.toContain('PropReadOnly');
+        expect(invalidatedProps).toContain('PropReadOnly');
+    });
 });
diff --git a/libgjs-private/gjs-gdbus-wrapper.c b/libgjs-private/gjs-gdbus-wrapper.c
index e46fd931..9d405901 100644
--- a/libgjs-private/gjs-gdbus-wrapper.c
+++ b/libgjs-private/gjs-gdbus-wrapper.c
@@ -39,6 +39,18 @@ struct _GjsDBusImplementationPrivate {
 G_DEFINE_TYPE_WITH_PRIVATE(GjsDBusImplementation, gjs_dbus_implementation,
                            G_TYPE_DBUS_INTERFACE_SKELETON);
 
+static inline GVariant* _g_variant_ref_sink0(void* value) {
+    if (value)
+        g_variant_ref_sink(value);
+
+    return value;
+}
+
+static inline void _g_variant_unref0(void* value) {
+    if (value)
+        g_variant_unref(value);
+}
+
 static gboolean gjs_dbus_implementation_check_interface(
     GjsDBusImplementation* self, GDBusConnection* connection,
     const char* object_path, const char* interface_name, GError** error) {
@@ -157,7 +169,10 @@ gjs_dbus_implementation_init(GjsDBusImplementation *self) {
     priv->vtable.get_property = gjs_dbus_implementation_property_get;
     priv->vtable.set_property = gjs_dbus_implementation_property_set;
 
-    priv->outstanding_properties = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, 
(GDestroyNotify)g_variant_unref);
+    priv->outstanding_properties = g_hash_table_new_full(g_str_hash,
+                                                         g_str_equal,
+                                                         g_free,
+                                                         _g_variant_unref0);
 }
 
 static void gjs_dbus_implementation_dispose(GObject* object) {
@@ -361,7 +376,9 @@ gjs_dbus_implementation_emit_property_changed (GjsDBusImplementation *self,
                                                gchar                 *property,
                                                GVariant              *newvalue)
 {
-    g_hash_table_replace (self->priv->outstanding_properties, g_strdup (property), g_variant_ref (newvalue));
+    g_hash_table_replace(self->priv->outstanding_properties,
+                         g_strdup(property),
+                         _g_variant_ref_sink0(newvalue));
 
     if (!self->priv->idle_id)
         self->priv->idle_id = g_idle_add(idle_cb, self);
@@ -385,8 +402,7 @@ gjs_dbus_implementation_emit_signal (GjsDBusImplementation *self,
     GList *connections = g_dbus_interface_skeleton_get_connections(skeleton);
     const char *object_path = g_dbus_interface_skeleton_get_object_path(skeleton);
 
-    if (parameters != NULL)
-        g_variant_ref_sink(parameters);
+    _g_variant_ref_sink0(parameters);
 
     for (const GList *iter = connections; iter; iter = iter->next) {
         g_dbus_connection_emit_signal(G_DBUS_CONNECTION(iter->data),
@@ -399,8 +415,7 @@ gjs_dbus_implementation_emit_signal (GjsDBusImplementation *self,
 
         g_object_unref(iter->data);
     }
-    if (parameters != NULL)
-        g_variant_unref(parameters);
+    _g_variant_unref0(parameters);
 
     g_list_free(connections);
 }