[gnome-software] Don't ever call g_object_notify() when using gs_app_subsume()

[Richard Hughes <rhughes src gnome org>]

commit d0b263b230ef4faf8884df5b26151982902e0036
Author: Richard Hughes <richard hughsie com>
Date:   Thu Jan 23 16:41:10 2014 +0000

    Don't ever call g_object_notify() when using gs_app_subsume()
    
    Just define some internal helpers to avoid complexity.

 src/gs-app.c |  207 ++++++++++++++++++++++++++++------------------------------
 1 files changed, 100 insertions(+), 107 deletions(-)
---
diff --git a/src/gs-app.c b/src/gs-app.c
index 9558598..b689793 100644
--- a/src/gs-app.c
+++ b/src/gs-app.c
@@ -89,7 +89,6 @@ struct GsAppPrivate
        GPtrArray               *related; /* of GsApp */
        GPtrArray               *history; /* of GsApp */
        guint64                  install_date;
-       guint                    subsume_idle_id;
 };
 
 enum {
@@ -291,87 +290,6 @@ gs_app_to_string (GsApp *app)
 }
 
 /**
- * gs_app_subsume_notify_cb:
- **/
-static gboolean
-gs_app_subsume_notify_cb (gpointer user_data)
-{
-       GsApp *app = GS_APP (user_data);
-       g_object_thaw_notify (G_OBJECT (app));
-       app->priv->subsume_idle_id = 0;
-       return G_SOURCE_REMOVE;
-}
-
-/**
- * gs_app_subsume:
- *
- * Imports all the useful data from @other into @app.
- *
- * IMPORTANT: This method can be called from a thread, but because of this
- * the g_object_notify() calls are done in an idle callback.
- **/
-void
-gs_app_subsume (GsApp *app, GsApp *other)
-{
-       GList *keys;
-       GList *l;
-       GsAppPrivate *priv = app->priv;
-       GsAppPrivate *priv2 = other->priv;
-       const gchar *tmp;
-
-       /* check we're not doing crazy things */
-       if (app->priv->subsume_idle_id != 0) {
-               g_warning ("Called gs_app_subsume() with idle pending");
-               return;
-       }
-
-       /* wait for all the properties to be set */
-       g_object_freeze_notify (G_OBJECT (app));
-
-       /* an [updatable] installable package is more information than
-        * just the fact that something is installed */
-       if (priv2->state == GS_APP_STATE_UPDATABLE &&
-           priv->state == GS_APP_STATE_INSTALLED) {
-               /* we have to do the little dance to appease the
-                * angry gnome controlling the state-machine */
-               gs_app_set_state (app, GS_APP_STATE_UNKNOWN);
-               gs_app_set_state (app, GS_APP_STATE_UPDATABLE);
-       }
-
-       /* save any properties we already know */
-       if (priv2->sources->len > 0)
-               gs_app_set_sources (app, priv2->sources);
-       if (priv2->project_group != NULL)
-               gs_app_set_project_group (app, priv2->project_group);
-       if (priv2->name != NULL)
-               gs_app_set_name (app, priv2->name);
-       if (priv2->summary != NULL)
-               gs_app_set_summary (app, priv2->summary);
-       if (priv2->description != NULL)
-               gs_app_set_description (app, priv2->description);
-       if (priv2->update_details != NULL)
-               gs_app_set_update_details (app, priv2->update_details);
-       if (priv2->update_version != NULL)
-               gs_app_set_update_version (app, priv2->update_version);
-       if (priv2->pixbuf != NULL)
-               gs_app_set_pixbuf (app, priv2->pixbuf);
-
-       /* also metadata */
-       keys = g_hash_table_get_keys (priv2->metadata);
-       for (l = keys; l != NULL; l = l->next) {
-               tmp = g_hash_table_lookup (priv->metadata, l->data);
-               if (tmp == NULL)
-                       continue;
-               tmp = g_hash_table_lookup (priv2->metadata, l->data);
-               gs_app_set_metadata (other, l->data, tmp);
-       }
-       g_list_free (keys);
-
-       /* now emit all the changed signals */
-       priv->subsume_idle_id = g_idle_add (gs_app_subsume_notify_cb, app);
-}
-
-/**
  * gs_app_get_id:
  **/
 const gchar *
@@ -426,31 +344,16 @@ gs_app_get_state (GsApp *app)
 }
 
 /**
- * gs_app_set_state:
- *
- * This sets the state of the application. The following state diagram explains
- * the typical states. All applications start in state %GS_APP_STATE_UNKNOWN,
- * but the frontend is not supposed to see GsApps with this state, ever.
- * Backend plugins are reponsible for changing the state to one of the other
- * states before the GsApp is passed to the frontend. This is enforced by the
- * #GsPluginLoader.
- *
- * UPDATABLE --> INSTALLING --> INSTALLED
- * UPDATABLE --> REMOVING   --> AVAILABLE
- * INSTALLED --> REMOVING   --> AVAILABLE
- * AVAILABLE --> INSTALLING --> INSTALLED
- * AVAILABLE <--> QUEUED --> INSTALLING --> INSTALLED
- * UNKNOWN   --> UNAVAILABLE
+ * gs_app_set_state_internal:
  */
-void
-gs_app_set_state (GsApp *app, GsAppState state)
+static gboolean
+gs_app_set_state_internal (GsApp *app, GsAppState state)
 {
        gboolean state_change_ok = FALSE;
        GsAppPrivate *priv = app->priv;
 
-       g_return_if_fail (GS_IS_APP (app));
        if (priv->state == state)
-               return;
+               return FALSE;
 
        /* check the state change is allowed */
        switch (priv->state) {
@@ -520,7 +423,7 @@ gs_app_set_state (GsApp *app, GsAppState state)
                           priv->id,
                           gs_app_state_to_string (priv->state),
                           gs_app_state_to_string (state));
-               return;
+               return FALSE;
        }
 
        priv->state = state;
@@ -529,7 +432,31 @@ gs_app_set_state (GsApp *app, GsAppState state)
             state == GS_APP_STATE_AVAILABLE)
                app->priv->install_date = 0;
 
-       g_object_notify (G_OBJECT (app), "state");
+       return TRUE;
+}
+
+/**
+ * gs_app_set_state:
+ *
+ * This sets the state of the application. The following state diagram explains
+ * the typical states. All applications start in state %GS_APP_STATE_UNKNOWN,
+ * but the frontend is not supposed to see GsApps with this state, ever.
+ * Backend plugins are reponsible for changing the state to one of the other
+ * states before the GsApp is passed to the frontend. This is enforced by the
+ * #GsPluginLoader.
+ *
+ * UPDATABLE --> INSTALLING --> INSTALLED
+ * UPDATABLE --> REMOVING   --> AVAILABLE
+ * INSTALLED --> REMOVING   --> AVAILABLE
+ * AVAILABLE --> INSTALLING --> INSTALLED
+ * AVAILABLE <--> QUEUED --> INSTALLING --> INSTALLED
+ * UNKNOWN   --> UNAVAILABLE
+ */
+void
+gs_app_set_state (GsApp *app, GsAppState state)
+{
+       if (gs_app_set_state_internal (app, state))
+               g_object_notify (G_OBJECT (app), "state");
 }
 
 /**
@@ -1202,15 +1129,24 @@ gs_app_get_update_version_ui (GsApp *app)
 }
 
 /**
+ * gs_app_set_update_version_internal:
+ */
+static void
+gs_app_set_update_version_internal (GsApp *app, const gchar *update_version)
+{
+       g_free (app->priv->update_version);
+       app->priv->update_version = g_strdup (update_version);
+       gs_app_ui_versions_invalidate (app);
+}
+
+/**
  * gs_app_set_update_version:
  */
 void
 gs_app_set_update_version (GsApp *app, const gchar *update_version)
 {
        g_return_if_fail (GS_IS_APP (app));
-       g_free (app->priv->update_version);
-       app->priv->update_version = g_strdup (update_version);
-       gs_app_ui_versions_invalidate (app);
+       gs_app_set_update_version_internal (app, update_version);
        g_object_notify (G_OBJECT (app), "version");
 }
 
@@ -1468,6 +1404,63 @@ gs_app_set_keywords (GsApp *app, GPtrArray *keywords)
 }
 
 /**
+ * gs_app_subsume:
+ *
+ * Imports all the useful data from @other into @app.
+ *
+ * IMPORTANT: This method can be called from a thread as the notify signals
+ * are not sent.
+ **/
+void
+gs_app_subsume (GsApp *app, GsApp *other)
+{
+       GList *keys;
+       GList *l;
+       GsAppPrivate *priv = app->priv;
+       GsAppPrivate *priv2 = other->priv;
+       const gchar *tmp;
+
+       /* an [updatable] installable package is more information than
+        * just the fact that something is installed */
+       if (priv2->state == GS_APP_STATE_UPDATABLE &&
+           priv->state == GS_APP_STATE_INSTALLED) {
+               /* we have to do the little dance to appease the
+                * angry gnome controlling the state-machine */
+               gs_app_set_state_internal (app, GS_APP_STATE_UNKNOWN);
+               gs_app_set_state_internal (app, GS_APP_STATE_UPDATABLE);
+       }
+
+       /* save any properties we already know */
+       if (priv2->sources->len > 0)
+               gs_app_set_sources (app, priv2->sources);
+       if (priv2->project_group != NULL)
+               gs_app_set_project_group (app, priv2->project_group);
+       if (priv2->name != NULL)
+               gs_app_set_name (app, priv2->name);
+       if (priv2->summary != NULL)
+               gs_app_set_summary (app, priv2->summary);
+       if (priv2->description != NULL)
+               gs_app_set_description (app, priv2->description);
+       if (priv2->update_details != NULL)
+               gs_app_set_update_details (app, priv2->update_details);
+       if (priv2->update_version != NULL)
+               gs_app_set_update_version_internal (app, priv2->update_version);
+       if (priv2->pixbuf != NULL)
+               gs_app_set_pixbuf (app, priv2->pixbuf);
+
+       /* also metadata */
+       keys = g_hash_table_get_keys (priv2->metadata);
+       for (l = keys; l != NULL; l = l->next) {
+               tmp = g_hash_table_lookup (priv->metadata, l->data);
+               if (tmp == NULL)
+                       continue;
+               tmp = g_hash_table_lookup (priv2->metadata, l->data);
+               gs_app_set_metadata (other, l->data, tmp);
+       }
+       g_list_free (keys);
+}
+
+/**
  * gs_app_get_property:
  */
 static void
@@ -1541,7 +1534,7 @@ gs_app_set_property (GObject *object, guint prop_id, const GValue *value, GParam
                gs_app_set_kind (app, g_value_get_uint (value));
                break;
        case PROP_STATE:
-               gs_app_set_state (app, g_value_get_uint (value));
+               gs_app_set_state_internal (app, g_value_get_uint (value));
                break;
        case PROP_INSTALL_DATE:
                gs_app_set_install_date (app, g_value_get_uint64 (value));