[glib] GSettings: delay backend subscription

[Ryan Lortie <desrt src gnome org>]

commit 8ff5668a458344da22d30491e3ce726d861b3619
Author: Ryan Lortie <desrt desrt ca>
Date:   Sat Jul 26 17:16:37 2014 +0200

    GSettings: delay backend subscription
    
    GSettings objects begin watching for changes as soon as they are created
    in order that they can emit the "changed" signal.
    
    In the case of dconf, if we want to be able to emit the changed signal,
    we need to go on the bus and add some match rules.  This requires
    creating the dconf helper thread and also requires initialising GDBus
    (which creates another thread).
    
    Some users of GSettings are never interested in the "changed" signal.
    One of these users is the glib-networking code that gets run every time
    a new network connection is created.
    
    Some users are reporting that they are annoyed that simply establishing
    a network connection would spawn two extra threads and create a D-Bus
    connection.
    
    In order to avoid doing unnecessary work for these simple uses, delay
    the subscription until we know that we will actually need to do it.
    
    We do this in a simple way, using a simple argument: in order for the
    user to care that a value changed then they must have:
    
     1) watched for a change signal; and then
     2) actually read a value
    
    If the user didn't actually read a value then they cannot possibly be
    interested in if the value changed or not (since they never knew the old
    value to begin with and therefore would be unable to observe that it
    ever changed, since they have nothing to compare the new value with).
    
    This really is a behaviour change, however, and it does impact at least
    one user: the 'monitor' functionality of the GSettings commandline tool,
    which is interested in reporting changes without ever having known the
    original values.  We add a workaround to the commandline tool in order
    to ensure that it continues to function properly.
    
    It's also possible to argue that it is completely valid to have read a
    value and _then_ established a change signal connection under the
    (correct) assumption that it would not have been possible to miss a
    change signal by virtue of not having returned to the mainloop.
    Although this argument is true, this pattern is extremely non-idiomatic,
    and the problem is easily avoided by doing things in the usual order.
    
    We never really talked about change notification in the overview
    documentation for GSettings, so it seems like now is a good time to add
    some discussion, including the new rules for when one can expect change
    signals to be emitted.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=733791

 gio/gsettings-tool.c |   13 +++++++++++++
 gio/gsettings.c      |   47 +++++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 56 insertions(+), 4 deletions(-)
---
diff --git a/gio/gsettings-tool.c b/gio/gsettings-tool.c
index 61aa863..cf3ef19 100644
--- a/gio/gsettings-tool.c
+++ b/gio/gsettings-tool.c
@@ -408,6 +408,8 @@ value_changed (GSettings   *settings,
 static void
 gsettings_monitor (void)
 {
+  gchar **keys;
+
   if (global_key)
     {
       gchar *name;
@@ -418,6 +420,17 @@ gsettings_monitor (void)
   else
     g_signal_connect (global_settings, "changed", G_CALLBACK (value_changed), NULL);
 
+  /* We have to read a value from GSettings before we start receiving
+   * signals...
+   *
+   * If the schema has zero keys then we won't be displaying any
+   * notifications anyway.
+   */
+  keys = g_settings_list_keys (global_settings);
+  if (keys[0])
+    g_variant_unref (g_settings_get_value (global_settings, keys[0]));
+  g_strfreev (keys);
+
   for (;;)
     g_main_context_iteration (NULL, TRUE);
 }
diff --git a/gio/gsettings.c b/gio/gsettings.c
index fdc3c9d..fb2ce25 100644
--- a/gio/gsettings.c
+++ b/gio/gsettings.c
@@ -83,6 +83,16 @@
  * the names must begin with a lowercase character, must not end
  * with a '-', and must not contain consecutive dashes.
  *
+ * GSettings supports change notification.  The primary mechanism to
+ * watch for changes is to connect to the "changed" signal.  You can
+ * optionally watch for changes on only a single key by using a signal
+ * detail.  Signals are only guaranteed to be emitted for a given key
+ * after you have read the value of that key while a signal handler was
+ * connected for that key.  Signals may or may not be emitted in the
+ * case that the key "changed" to the value that you had previously
+ * read.  Signals may be reported in additional cases as well and the
+ * "changed" signal should really be treated as "may have changed".
+ *
  * Similar to GConf, the default values in GSettings schemas can be
  * localized, but the localized values are stored in gettext catalogs
  * and looked up with the domain that is specified in the
@@ -229,6 +239,8 @@ struct _GSettingsPrivate
   GSettingsSchema *schema;
   gchar *path;
 
+  gboolean is_subscribed;
+
   GDelayedSettingsBackend *delayed;
 };
 
@@ -304,6 +316,26 @@ g_settings_real_writable_change_event (GSettings *settings,
   return FALSE;
 }
 
+static gboolean
+g_settings_has_signal_handlers (GSettings *settings)
+{
+  GSettingsClass *class = G_SETTINGS_GET_CLASS (settings);
+
+  if (class->change_event != g_settings_real_change_event ||
+      class->writable_change_event != g_settings_real_writable_change_event)
+    return TRUE;
+
+  if (g_signal_has_handler_pending (settings, g_settings_signals[SIGNAL_WRITABLE_CHANGE_EVENT], 0, TRUE) ||
+      g_signal_has_handler_pending (settings, g_settings_signals[SIGNAL_WRITABLE_CHANGED], 0, TRUE) ||
+      g_signal_has_handler_pending (settings, g_settings_signals[SIGNAL_CHANGE_EVENT], 0, TRUE) ||
+      g_signal_has_handler_pending (settings, g_settings_signals[SIGNAL_CHANGED], 0, TRUE))
+    return TRUE;
+
+  /* None of that?  Then surely nobody is watching.... */
+  return FALSE;
+}
+
+
 static void
 settings_backend_changed (GObject             *target,
                           GSettingsBackend    *backend,
@@ -570,8 +602,6 @@ g_settings_constructed (GObject *object)
   g_settings_backend_watch (settings->priv->backend,
                             &listener_vtable, G_OBJECT (settings),
                             settings->priv->main_context);
-  g_settings_backend_subscribe (settings->priv->backend,
-                                settings->priv->path);
 }
 
 static void
@@ -579,8 +609,10 @@ g_settings_finalize (GObject *object)
 {
   GSettings *settings = G_SETTINGS (object);
 
-  g_settings_backend_unsubscribe (settings->priv->backend,
-                                  settings->priv->path);
+  if (settings->priv->is_subscribed)
+    g_settings_backend_unsubscribe (settings->priv->backend,
+                                    settings->priv->path);
+
   g_main_context_unref (settings->priv->main_context);
   g_object_unref (settings->priv->backend);
   g_settings_schema_unref (settings->priv->schema);
@@ -1045,6 +1077,13 @@ g_settings_read_from_backend (GSettings          *settings,
   GVariant *fixup;
   gchar *path;
 
+  /* If we are not yet watching for changes, consider doing it now... */
+  if (!settings->priv->is_subscribed && g_settings_has_signal_handlers (settings))
+    {
+      g_settings_backend_subscribe (settings->priv->backend, settings->priv->path);
+      settings->priv->is_subscribed = TRUE;
+    }
+
   path = g_strconcat (settings->priv->path, key->name, NULL);
   if (user_value_only)
     value = g_settings_backend_read_user_value (settings->priv->backend, path, key->type);