Attachment 93480 Details for Bug 74581 – McdStorage: watch and proxy plugins' change-notification signals

[patch] McdStorage: watch and proxy plugins' change-notification signals

0005-McdStorage-watch-and-proxy-plugins-change-notificati.patch (text/plain), 32.10 KB, created by Simon McVittie on 2014-02-05 18:54:38 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Simon McVittie

Created: 2014-02-05 18:54:38 UTC

Size: 32.10 KB

patch

obsolete

>From fe935854d0ad4c3fb99b418c841bc3703cd374dd Mon Sep 17 00:00:00 2001
>From: Simon McVittie <simon.mcvittie@collabora.co.uk>
>Date: Wed, 5 Feb 2014 13:24:45 +0000
>Subject: [PATCH 5/6] McdStorage: watch and proxy plugins' change-notification
> signals
>
>This lets us get rid of the "ready" vfunc on plugins: we now
>connect to each plugin's signals only after we have called
>mcp_account_storage_list(), so we won't get double-notification for
>accounts that are both present in the initial list and signalled.
>
>This means we can remove a queue of delayed signal emissions from the
>test D-Bus plugin (and when it's ported to this API, from Empathy's
>libaccounts/UOA plugin). As far as I can see, list() and ready() happen
>within the same main-loop iteration anyway, so I don't think it was
>even possible to receive notification of a new account in that window.
>
>Empathy's GNOME Online Accounts plugin never really implemented this:
>in theory, it was incorrect, since any account that happened to be
>added between list() and ready() would be lost altogether. However,
>list() and ready() seem to happen in the same main-loop iteration,
>so this might never have been a practical concern.
>
>Rather than "fixing" Empathy's GOA plugin, it seems better to remove
>the difficult case altogether.
>---
> mission-control-plugins/account-storage.c |  60 +--------
> mission-control-plugins/account-storage.h |   6 -
> src/mcd-account-manager.c                 | 134 +++++++-------------
> src/mcd-storage.c                         | 191 ++++++++++++++++++++++++----
> tests/twisted/dbus-account-plugin.c       | 204 ++++++------------------------
> 5 files changed, 246 insertions(+), 349 deletions(-)
>
>diff --git a/mission-control-plugins/account-storage.c b/mission-control-plugins/account-storage.c
>index b04e3bd..51d3e6d 100644
>--- a/mission-control-plugins/account-storage.c
>+++ b/mission-control-plugins/account-storage.c
>@@ -61,7 +61,6 @@
>  *   iface->delete_finish = foo_plugin_delete_finish;
>  *   iface->commit = foo_plugin_commit;
>  *   iface->list = foo_plugin_list;
>- *   iface->ready = foo_plugin_ready;
>  *   iface->get_identifier = foo_plugin_get_identifier;
>  *   iface->get_additional_info = foo_plugin_get_additional_info;
>  *   iface->get_restrictions = foo_plugin_get_restrictions;
>@@ -166,13 +165,6 @@ default_create (McpAccountStorage *storage,
> }
> 
> static void
>-default_ready (McpAccountStorage *storage,
>-    McpAccountManager *am)
>-{
>-  /* do nothing */
>-}
>-
>-static void
> default_get_identifier (McpAccountStorage *storage,
>     const gchar *account,
>     GValue *identifier)
>@@ -244,7 +236,6 @@ class_init (gpointer klass,
>   iface->delete_async = default_delete_async;
>   iface->delete_finish = default_delete_finish;
>   iface->commit = default_commit;
>-  iface->ready = default_ready;
>   iface->get_identifier = default_get_identifier;
>   iface->get_additional_info = default_get_additional_info;
>   iface->get_restrictions = default_get_restrictions;
>@@ -265,8 +256,10 @@ class_init (gpointer klass,
>    * Emitted if an external entity creates an account
>    * in the backend the emitting plugin handles.
>    *
>-   * Should not be fired until mcp_account_storage_ready() has been called
>-   *
>+   * This signal does not need to be emitted before mcp_account_storage_list()
>+   * returns (if it is, it will be ignored). All accounts that exist
>+   * at the time that mcp_account_storage_list() returns must be included
>+   * in its result, even if they were also signalled via this signal.
>    */
>   signals[CREATED] = g_signal_new ("created",
>       type, G_SIGNAL_RUN_LAST, 0, NULL, NULL,
>@@ -291,8 +284,6 @@ class_init (gpointer klass,
>    * mcp_account_storage_list_typed_parameters() and
>    * mcp_account_storage_set_parameter() do not use the
>    * "param-" prefix, but this signal does.
>-   *
>-   * Should not be fired until mcp_account_storage_ready() has been called
>    */
>   signals[ALTERED_ONE] = g_signal_new ("altered-one",
>       type, G_SIGNAL_RUN_LAST, 0, NULL, NULL,
>@@ -306,9 +297,6 @@ class_init (gpointer klass,
>    *
>    * Emitted if an external entity deletes an account
>    * in the backend the emitting plugin handles.
>-   *
>-   * Should not be fired until mcp_account_storage_ready() has been called
>-   *
>    */
>   signals[DELETED] = g_signal_new ("deleted",
>       type, G_SIGNAL_RUN_LAST, 0, NULL, NULL,
>@@ -328,9 +316,6 @@ class_init (gpointer klass,
>    * Before emitting this signal, the plugin must update its
>    * internal cache (if any) so that mcp_account_storage_get_attribute()
>    * will return the new value for Enabled when queried.
>-   *
>-   * Should not be fired until mcp_account_storage_ready() has been called
>-   *
>    */
>   signals[TOGGLED] = g_signal_new ("toggled",
>       type, G_SIGNAL_RUN_LAST, 0, NULL, NULL,
>@@ -343,8 +328,6 @@ class_init (gpointer klass,
>    *
>    * emitted if an external entity modified important parameters of the
>    * account and a reconnection is required in order to apply them.
>-   *
>-   * Should not be fired until mcp_account_storage_ready() has been called
>    **/
>   signals[RECONNECT] = g_signal_new ("reconnect",
>       type, G_SIGNAL_RUN_LAST, 0, NULL, NULL,
>@@ -402,7 +385,6 @@ mcp_account_storage_get_type (void)
>  * @delete: implementation of mcp_account_storage_delete()
>  * @commit: implementation of mcp_account_storage_commit()
>  * @list: implementation of mcp_account_storage_list()
>- * @ready: implementation of mcp_account_storage_ready()
>  * @get_identifier: implementation of mcp_account_storage_get_identifier()
>  * @get_additional_info: implementation of
>  *  mcp_account_storage_get_additional_info()
>@@ -939,40 +921,6 @@ mcp_account_storage_list (McpAccountStorage *storage,
> }
> 
> /**
>- * McpAccountStorageReadyFunc:
>- * @storage: an #McpAccountStorage instance
>- * @am: an #McpAccountManager instance
>- *
>- * An implementation of mcp_account_storage_ready().
>- */
>-
>-/**
>- * mcp_account_storage_ready:
>- * @storage: an #McpAccountStorage instance
>- * @am: an #McpAccountManager instance
>- *
>- * Informs the plugin that it is now permitted to create new accounts,
>- * ie it can now fire its "created", "altered-one", "toggled" and "deleted"
>- * signals.
>- *
>- * The default implementation does nothing. It should be overridden by
>- * any plugin that will emit "created", "altered-one", "toggled" and/or
>- * "deleted".
>- */
>-void
>-mcp_account_storage_ready (McpAccountStorage *storage,
>-    McpAccountManager *am)
>-{
>-  McpAccountStorageIface *iface = MCP_ACCOUNT_STORAGE_GET_IFACE (storage);
>-
>-  SDEBUG (storage, "");
>-  g_return_if_fail (iface != NULL);
>-  g_return_if_fail (iface->ready != NULL);
>-
>-  iface->ready (storage, am);
>-}
>-
>-/**
>  * McpAccountStorageGetIdentifierFunc:
>  * @storage: an #McpAccountStorage instance
>  * @account: the unique name of the account
>diff --git a/mission-control-plugins/account-storage.h b/mission-control-plugins/account-storage.h
>index 95ffcc1..b801521 100644
>--- a/mission-control-plugins/account-storage.h
>+++ b/mission-control-plugins/account-storage.h
>@@ -116,9 +116,6 @@ struct _McpAccountStorageIface
>   GList * (*list) (McpAccountStorage *storage,
>     McpAccountManager *am);
> 
>-  void (*ready) (McpAccountStorage *storage,
>-    McpAccountManager *am);
>-
>   void (*get_identifier) (McpAccountStorage *storage,
>     const gchar *account,
>     GValue *identifier);
>@@ -193,9 +190,6 @@ gboolean mcp_account_storage_delete_finish (McpAccountStorage *storage,
>     GAsyncResult *result,
>     GError **error);
> 
>-void mcp_account_storage_ready (McpAccountStorage *storage,
>-    McpAccountManager *am);
>-
> gboolean
> mcp_account_storage_commit (McpAccountStorage *storage,
>     McpAccountManager *am,
>diff --git a/src/mcd-account-manager.c b/src/mcd-account-manager.c
>index 489de45..a221910 100644
>--- a/src/mcd-account-manager.c
>+++ b/src/mcd-account-manager.c
>@@ -174,7 +174,8 @@ async_altered_one_manager_cb (McdManager *cm,
> 
> 
> static void
>-altered_one_cb (McpAccountStorage *storage,
>+altered_one_cb (McdStorage *storage,
>+                McpAccountStorage *plugin,
>                 const gchar *account_name,
>                 const gchar *key,
>                 gpointer data)
>@@ -184,7 +185,6 @@ altered_one_cb (McpAccountStorage *storage,
>     McdAccount *account = NULL;
>     McdManager *cm = NULL;
>     const gchar *cm_name = NULL;
>-    McpAccountStorage *its_plugin;
> 
>     account = mcd_account_manager_lookup_account (am, account_name);
> 
>@@ -194,18 +194,6 @@ altered_one_cb (McpAccountStorage *storage,
>         return;
>     }
> 
>-    its_plugin = mcd_account_get_storage_plugin (account);
>-
>-    if (storage != its_plugin)
>-    {
>-        DEBUG ("Ignoring altered-one from plugin %s because account %s "
>-            "belongs to %s",
>-            mcp_account_storage_name (storage),
>-            account_name,
>-            mcp_account_storage_name (its_plugin));
>-        return;
>-    }
>-
>     /* in theory, the CM is already ready by this point, but make sure: */
>     cm_name = mcd_account_get_manager_name (account);
> 
>@@ -271,7 +259,8 @@ async_created_manager_cb (McdManager *cm, const GError *error, gpointer data)
>  * to fetch the named account explicitly at this point (ie it's a read, not *
>  * not a write, from the plugin's POV:                                      */
> static void
>-created_cb (GObject *storage_plugin_obj,
>+created_cb (McdStorage *storage,
>+    GObject *storage_plugin_obj,
>     const gchar *name,
>     gpointer data)
> {
>@@ -280,41 +269,25 @@ created_cb (GObject *storage_plugin_obj,
>     McdAccountManagerPrivate *priv = MCD_ACCOUNT_MANAGER_PRIV (am);
>     McdLoadAccountsData *lad = NULL;
>     McdAccount *account = NULL;
>-    McdStorage *storage = priv->storage;
>     McdMaster *master = mcd_master_get_default ();
>     McdManager *cm = NULL;
>     const gchar *cm_name = NULL;
>-    GError *error = NULL;
> 
>-    /* actually fetch the data into our cache from the plugin: */
>-    if (mcd_storage_add_account_from_plugin (storage, plugin, name, &error))
>-    {
>-        account = mcd_account_new (am, name, priv->minotaur, plugin);
>-        g_assert (MCD_IS_ACCOUNT (account));
>+    g_return_if_fail (storage == priv->storage);
> 
>-        lad = g_slice_new (McdLoadAccountsData);
>-        lad->account_manager = g_object_ref (am);
>-        lad->storage_plugin = g_object_ref (plugin);
>-        lad->account_lock = 1; /* released at the end of this function */
>-        lad->account = g_object_ref (account);
>+    account = mcd_account_new (am, name, priv->minotaur, plugin);
>+    g_assert (MCD_IS_ACCOUNT (account));
> 
>-        if (self->priv->setup_lock > 0)
>-        {
>-            lad->holds_setup_lock = TRUE;
>-            self->priv->setup_lock++;
>-        }
>-    }
>-    else
>-    {
>-        WARNING ("%s", error->message);
>-        g_clear_error (&error);
>-        goto finish;
>-    }
>+    lad = g_slice_new (McdLoadAccountsData);
>+    lad->account_manager = g_object_ref (am);
>+    lad->storage_plugin = g_object_ref (plugin);
>+    lad->account_lock = 1; /* released at the end of this function */
>+    lad->account = g_object_ref (account);
> 
>-    if (G_UNLIKELY (!account))
>+    if (am->priv->setup_lock > 0)
>     {
>-        g_warning ("%s: account %s failed to instantiate", G_STRFUNC, name);
>-        goto finish;
>+        lad->holds_setup_lock = TRUE;
>+        am->priv->setup_lock++;
>     }
> 
>     cm_name = mcd_account_get_manager_name (account);
>@@ -336,19 +309,20 @@ created_cb (GObject *storage_plugin_obj,
>         g_object_unref (account);
>     }
> 
>-finish:
>-    if (lad != NULL)
>-        release_load_accounts_lock (lad);
>+    release_load_accounts_lock (lad);
> }
> 
> static void
>-toggled_cb (GObject *plugin, const gchar *name, gboolean on, gpointer data)
>+toggled_cb (McdStorage *storage,
>+    GObject *plugin,
>+    const gchar *name,
>+    gboolean on,
>+    gpointer data)
> {
>   McpAccountStorage *storage_plugin = MCP_ACCOUNT_STORAGE (plugin);
>   McdAccountManager *manager = MCD_ACCOUNT_MANAGER (data);
>   McdAccount *account = NULL;
>   GError *error = NULL;
>-  McpAccountStorage *its_plugin;
> 
>   account = mcd_account_manager_lookup_account (manager, name);
> 
>@@ -362,18 +336,6 @@ toggled_cb (GObject *plugin, const gchar *name, gboolean on, gpointer data)
>       return;
>     }
> 
>-  its_plugin = mcd_account_get_storage_plugin (account);
>-
>-  if (storage_plugin != its_plugin)
>-    {
>-      DEBUG ("Ignoring toggled signal from plugin %s because account %s "
>-          "belongs to %s",
>-          mcp_account_storage_name (storage_plugin),
>-          name,
>-          mcp_account_storage_name (its_plugin));
>-      return;
>-    }
>-
>   _mcd_account_set_enabled (account, on, FALSE,
>                             MCD_DBUS_PROP_SET_FLAG_ALREADY_IN_STORAGE, &error);
> 
>@@ -385,7 +347,10 @@ toggled_cb (GObject *plugin, const gchar *name, gboolean on, gpointer data)
> }
> 
> static void
>-reconnect_cb (GObject *plugin, const gchar *name, gpointer data)
>+reconnect_cb (McdStorage *storage,
>+    GObject *plugin,
>+    const gchar *name,
>+    gpointer data)
> {
>   McpAccountStorage *storage_plugin = MCP_ACCOUNT_STORAGE (plugin);
>   McdAccountManager *manager = MCD_ACCOUNT_MANAGER (data);
>@@ -434,7 +399,10 @@ mcd_account_delete_debug_cb (GObject *source,
> 
> /* a backend plugin notified us that an account was vaporised: remove it */
> static void
>-deleted_cb (GObject *plugin, const gchar *name, gpointer data)
>+deleted_cb (McdStorage *storage,
>+    GObject *plugin,
>+    const gchar *name,
>+    gpointer data)
> {
>     McpAccountStorage *storage_plugin = MCP_ACCOUNT_STORAGE (plugin);
>     McdAccountManager *manager = MCD_ACCOUNT_MANAGER (data);
>@@ -448,18 +416,6 @@ deleted_cb (GObject *plugin, const gchar *name, gpointer data)
>     if (account != NULL)
>     {
>         const gchar * object_path = mcd_account_get_object_path (account);
>-        McpAccountStorage *its_plugin = mcd_account_get_storage_plugin (
>-            account);
>-
>-        if (storage_plugin != its_plugin)
>-        {
>-            DEBUG ("Ignoring deleted signal from plugin %s because account %s "
>-                "belongs to %s",
>-                mcp_account_storage_name (storage_plugin),
>-                name,
>-                mcp_account_storage_name (its_plugin));
>-            return;
>-        }
> 
>         g_object_ref (account);
>         /* this unhooks the account's signal handlers */
>@@ -1443,6 +1399,20 @@ _mcd_account_manager_setup (McdAccountManager *account_manager)
>                               (GObject *)account_manager);
> 
>     accounts = mcd_storage_get_accounts (storage);
>+
>+    /* as soon as we've listed the initial set, connect to signals
>+     * for any subsequently-added accounts */
>+    g_signal_connect_object (priv->storage, "altered-one",
>+        G_CALLBACK (altered_one_cb), account_manager, 0);
>+    g_signal_connect_object (priv->storage, "created",
>+        G_CALLBACK (created_cb), account_manager, 0);
>+    g_signal_connect_object (priv->storage, "toggled",
>+        G_CALLBACK (toggled_cb), account_manager, 0);
>+    g_signal_connect_object (priv->storage, "deleted",
>+        G_CALLBACK (deleted_cb), account_manager, 0);
>+    g_signal_connect_object (priv->storage, "reconnect",
>+        G_CALLBACK (reconnect_cb), account_manager, 0);
>+
>     g_hash_table_iter_init (&iter, accounts);
> 
>     while (g_hash_table_iter_next (&iter, &k, &v))
>@@ -1506,9 +1476,6 @@ _mcd_account_manager_setup (McdAccountManager *account_manager)
>         mcd_storage_commit (storage, k);
>     }
> 
>-    /* uncork signals from storage plugins */
>-    mcd_storage_ready (priv->storage);
>-
>     migrate_accounts (account_manager);
> 
>     release_setup_lock (account_manager);
>@@ -1676,14 +1643,6 @@ _mcd_account_manager_constructed (GObject *obj)
> {
>     McdAccountManager *account_manager = MCD_ACCOUNT_MANAGER (obj);
>     McdAccountManagerPrivate *priv = account_manager->priv;
>-    guint i = 0;
>-    static struct { const gchar *name; GCallback handler; } sig[] =
>-      { { "created", G_CALLBACK (created_cb) },
>-        { "toggled", G_CALLBACK (toggled_cb) },
>-        { "deleted", G_CALLBACK (deleted_cb) },
>-        { "altered-one", G_CALLBACK (altered_one_cb) },
>-        { "reconnect", G_CALLBACK (reconnect_cb) },
>-        { NULL, NULL } };
> 
>     DEBUG ("");
> 
>@@ -1701,13 +1660,6 @@ _mcd_account_manager_constructed (GObject *obj)
>     DEBUG ("loading plugins");
>     mcd_storage_load (priv->storage);
> 
>-    /* hook up all the storage plugin signals to their handlers: */
>-    for (i = 0; sig[i].name != NULL; i++)
>-    {
>-        mcd_storage_connect_signal (sig[i].name, sig[i].handler,
>-                                    account_manager);
>-    }
>-
>     /* initializes the interfaces */
>     mcd_dbus_init_interfaces_instances (account_manager);
> }
>diff --git a/src/mcd-storage.c b/src/mcd-storage.c
>index 90f6247..b2d57dc 100644
>--- a/src/mcd-storage.c
>+++ b/src/mcd-storage.c
>@@ -50,6 +50,17 @@ enum {
>   PROP_DBUS_DAEMON = 1,
> };
> 
>+enum {
>+  SIGNAL_CREATED,
>+  SIGNAL_TOGGLED,
>+  SIGNAL_DELETED,
>+  SIGNAL_ALTERED_ONE,
>+  SIGNAL_RECONNECT,
>+  N_SIGNALS
>+};
>+
>+static guint signals[N_SIGNALS] = { 0 };
>+
> struct _McdStorageClass {
>     GObjectClass parent;
> };
>@@ -147,6 +158,31 @@ mcd_storage_class_init (McdStorageClass *cls)
>   object_class->finalize = storage_finalize;
> 
>   g_object_class_install_property (object_class, PROP_DBUS_DAEMON, spec);
>+
>+  signals[SIGNAL_CREATED] = g_signal_new ("created",
>+      MCD_TYPE_STORAGE, G_SIGNAL_RUN_LAST, 0, NULL, NULL,
>+      NULL, G_TYPE_NONE,
>+      2, MCP_TYPE_ACCOUNT_STORAGE, G_TYPE_STRING);
>+
>+  signals[SIGNAL_ALTERED_ONE] = g_signal_new ("altered-one",
>+      MCD_TYPE_STORAGE, G_SIGNAL_RUN_LAST, 0, NULL, NULL,
>+      NULL, G_TYPE_NONE,
>+      3, MCP_TYPE_ACCOUNT_STORAGE, G_TYPE_STRING, G_TYPE_STRING);
>+
>+  signals[SIGNAL_DELETED] = g_signal_new ("deleted",
>+      MCD_TYPE_STORAGE, G_SIGNAL_RUN_LAST, 0, NULL, NULL,
>+      NULL, G_TYPE_NONE,
>+      2, MCP_TYPE_ACCOUNT_STORAGE, G_TYPE_STRING);
>+
>+  signals[SIGNAL_TOGGLED] = g_signal_new ("toggled",
>+      MCD_TYPE_STORAGE, G_SIGNAL_RUN_LAST, 0, NULL, NULL,
>+      NULL, G_TYPE_NONE,
>+      3, MCP_TYPE_ACCOUNT_STORAGE, G_TYPE_STRING, G_TYPE_BOOLEAN);
>+
>+  signals[SIGNAL_RECONNECT] = g_signal_new ("reconnect",
>+      MCD_TYPE_STORAGE, G_SIGNAL_RUN_LAST, 0, NULL, NULL,
>+      NULL, G_TYPE_NONE,
>+      2, MCP_TYPE_ACCOUNT_STORAGE, G_TYPE_STRING);
> }
> 
> McdStorage *
>@@ -463,21 +499,119 @@ sort_and_cache_plugins ()
>     plugins_cached = TRUE;
> }
> 
>-void
>-mcd_storage_connect_signal (const gchar *signame,
>-    GCallback func,
>-    gpointer user_data)
>+static void
>+created_cb (McpAccountStorage *plugin,
>+    const gchar *account_name,
>+    McdStorage *self)
> {
>-  GList *p;
>+  GError *error = NULL;
> 
>-  for (p = stores; p != NULL; p = g_list_next (p))
>+  g_return_if_fail (MCP_IS_ACCOUNT_STORAGE (plugin));
>+  g_return_if_fail (MCD_IS_STORAGE (self));
>+
>+  if (mcd_storage_add_account_from_plugin (self, plugin, account_name, &error))
>     {
>-      McpAccountStorage *plugin = p->data;
>+      DEBUG ("%s", account_name);
>+      g_signal_emit (self, signals[SIGNAL_CREATED], 0, plugin, account_name);
>+    }
>+  else
>+    {
>+      WARNING ("%s", error->message);
>+      g_error_free (error);
>+    }
>+}
> 
>-      DEBUG ("connecting handler to %s plugin signal %s ",
>-          mcp_account_storage_name (plugin), signame);
>-      g_signal_connect (plugin, signame, func, user_data);
>+static gboolean
>+check_is_responsible (McdStorage *self,
>+    McpAccountStorage *plugin,
>+    const gchar *account_name,
>+    const gchar *changing,
>+    GError **error)
>+{
>+  McpAccountStorage *other = g_hash_table_lookup (self->accounts,
>+      account_name);
>+
>+  if (other == NULL)
>+    {
>+      WARNING ("account %s does not exist, preventing plugin '%s' from "
>+          "%s it", account_name, mcp_account_storage_name (plugin), changing);
>+      return FALSE;
>+    }
>+
>+  if (other != plugin)
>+    {
>+      WARNING ("account %s is in plugin '%s', preventing plugin '%s' from "
>+          "%s it", account_name, mcp_account_storage_name (other),
>+          mcp_account_storage_name (plugin), changing);
>+      return FALSE;
>     }
>+
>+  return TRUE;
>+}
>+
>+static void
>+toggled_cb (McpAccountStorage *plugin,
>+    const gchar *account_name,
>+    gboolean on,
>+    McdStorage *self)
>+{
>+  GError *error = NULL;
>+
>+  g_return_if_fail (MCP_IS_ACCOUNT_STORAGE (plugin));
>+  g_return_if_fail (MCD_IS_STORAGE (self));
>+
>+  if (check_is_responsible (self, plugin, account_name, "toggling", &error))
>+    g_signal_emit (self, signals[SIGNAL_TOGGLED], 0, plugin,
>+        account_name, on);
>+}
>+
>+static void
>+deleted_cb (McpAccountStorage *plugin,
>+    const gchar *account_name,
>+    McdStorage *self)
>+{
>+  GError *error = NULL;
>+
>+  g_return_if_fail (MCP_IS_ACCOUNT_STORAGE (plugin));
>+  g_return_if_fail (MCD_IS_STORAGE (self));
>+
>+  if (check_is_responsible (self, plugin, account_name, "deleting",
>+        &error))
>+    g_signal_emit (self, signals[SIGNAL_DELETED], 0, plugin,
>+        account_name);
>+}
>+
>+static void
>+altered_one_cb (McpAccountStorage *plugin,
>+    const gchar *account_name,
>+    const gchar *key,
>+    McdStorage *self)
>+{
>+  GError *error = NULL;
>+
>+  g_return_if_fail (MCP_IS_ACCOUNT_STORAGE (plugin));
>+  g_return_if_fail (MCD_IS_STORAGE (self));
>+
>+  if (check_is_responsible (self, plugin, account_name, "altering",
>+        &error))
>+    g_signal_emit (self, signals[SIGNAL_ALTERED_ONE], 0, plugin,
>+        account_name, key);
>+}
>+
>+static void
>+reconnect_cb (McpAccountStorage *plugin,
>+    const gchar *account_name,
>+    McdStorage *self)
>+{
>+  GError *error = NULL;
>+
>+  g_return_if_fail (MCP_IS_ACCOUNT_STORAGE (plugin));
>+  g_return_if_fail (MCD_IS_STORAGE (self));
>+
>+  if (check_is_responsible (self, plugin, account_name, "reconnecting",
>+        &error))
>+    g_signal_emit (self, signals[SIGNAL_RECONNECT], 0, plugin,
>+        account_name);
> }
> 
> /*
>@@ -504,11 +638,28 @@ mcd_storage_load (McdStorage *self)
>     {
>       GList *account;
>       McpAccountStorage *plugin = store->data;
>-      GList *stored = mcp_account_storage_list (plugin, ma);
>+      GList *stored;
>       const gchar *pname = mcp_account_storage_name (plugin);
>       const gint prio = mcp_account_storage_priority (plugin);
> 
>-      DEBUG ("listing from plugin %s [prio: %d]", pname, prio);
>+      DEBUG ("listing initial accounts from plugin %s [prio: %d]", pname, prio);
>+      stored = mcp_account_storage_list (plugin, ma);
>+
>+      /* Connect to signals for non-initial accounts. We only do this
>+       * after we have called list(), to make sure the plugins don't need
>+       * to queue up change-notification signals until after we've
>+       * called the old "ready" vfunc. */
>+      g_signal_connect_object (plugin, "created", G_CALLBACK (created_cb),
>+          self, 0);
>+      g_signal_connect_object (plugin, "toggled", G_CALLBACK (toggled_cb),
>+          self, 0);
>+      g_signal_connect_object (plugin, "deleted", G_CALLBACK (deleted_cb),
>+          self, 0);
>+      g_signal_connect_object (plugin, "altered-one",
>+          G_CALLBACK (altered_one_cb), self, 0);
>+      g_signal_connect_object (plugin, "reconnect", G_CALLBACK (reconnect_cb),
>+          self, 0);
>+
>       for (account = stored; account != NULL; account = g_list_next (account))
>         {
>           GError *error = NULL;
>@@ -1928,22 +2079,6 @@ mcd_storage_set_strv (McdStorage *storage,
>   return ret;
> }
> 
>-void
>-mcd_storage_ready (McdStorage *self)
>-{
>-  GList *store;
>-  McpAccountManager *ma = MCP_ACCOUNT_MANAGER (self);
>-
>-  for (store = stores; store != NULL; store = g_list_next (store))
>-    {
>-      McpAccountStorage *plugin = store->data;
>-      const gchar *plugin_name = mcp_account_storage_name (plugin);
>-
>-      DEBUG ("Unblocking async account ops by %s", plugin_name);
>-      mcp_account_storage_ready (plugin, ma);
>-    }
>-}
>-
> static GVariant *
> mcpa_unescape_variant_from_keyfile (const McpAccountManager *mcpa,
>     const gchar *escaped,
>diff --git a/tests/twisted/dbus-account-plugin.c b/tests/twisted/dbus-account-plugin.c
>index 487d35d..79464ed 100644
>--- a/tests/twisted/dbus-account-plugin.c
>+++ b/tests/twisted/dbus-account-plugin.c
>@@ -122,29 +122,6 @@ async_data_free (AsyncData *ad)
>   g_slice_free (AsyncData, ad);
> }
> 
>-typedef enum {
>-    EVENT_PARAMS,
>-    EVENT_ATTRS,
>-    EVENT_CREATION,
>-    EVENT_DELETION
>-} EventType;
>-
>-typedef struct {
>-    EventType type;
>-    GVariant *args;
>-} Event;
>-
>-static Event *
>-event_new (EventType type,
>-    GVariant *args)
>-{
>-  Event *e = g_slice_new0 (Event);
>-
>-  e->type = type;
>-  e->args = g_variant_ref_sink (args);
>-  return e;
>-}
>-
> static Account *
> lookup_account (TestDBusAccountPlugin *self,
>     const gchar *account_name)
>@@ -311,9 +288,15 @@ test_dbus_account_plugin_add_account (TestDBusAccountPlugin *self,
> }
> 
> static void
>-test_dbus_account_plugin_process_account_creation (TestDBusAccountPlugin *self,
>-    GVariant *args)
>+account_created_cb (GDBusConnection *bus,
>+    const gchar *sender_name,
>+    const gchar *object_path,
>+    const gchar *iface_name,
>+    const gchar *signal_name,
>+    GVariant *tuple,
>+    gpointer user_data)
> {
>+  TestDBusAccountPlugin *self = TEST_DBUS_ACCOUNT_PLUGIN (user_data);
>   const gchar *account_name;
>   Account *account;
>   GVariant *attrs;
>@@ -323,7 +306,7 @@ test_dbus_account_plugin_process_account_creation (TestDBusAccountPlugin *self,
>   GVariant *param_flags;
>   guint32 restrictions;
> 
>-  g_variant_get (args, "(&s@a{sv}@a{su}@a{sv}@a{ss}@a{su}u)",
>+  g_variant_get (tuple, "(&s@a{sv}@a{su}@a{sv}@a{ss}@a{su}u)",
>       &account_name, &attrs, &attr_flags,
>       &params, &untyped_params, &param_flags,
>       &restrictions);
>@@ -361,13 +344,19 @@ test_dbus_account_plugin_process_account_creation (TestDBusAccountPlugin *self,
> }
> 
> static void
>-test_dbus_account_plugin_process_account_deletion (TestDBusAccountPlugin *self,
>-    GVariant *args)
>+account_deleted_cb (GDBusConnection *bus,
>+    const gchar *sender_name,
>+    const gchar *object_path,
>+    const gchar *iface_name,
>+    const gchar *signal_name,
>+    GVariant *tuple,
>+    gpointer user_data)
> {
>+  TestDBusAccountPlugin *self = TEST_DBUS_ACCOUNT_PLUGIN (user_data);
>   const gchar *account_name;
>   Account *account;
> 
>-  g_variant_get (args, "(&s)", &account_name);
>+  g_variant_get (tuple, "(&s)", &account_name);
>   DEBUG ("%s", account_name);
>   account = lookup_account (self, account_name);
> 
>@@ -395,16 +384,22 @@ test_dbus_account_plugin_process_account_deletion (TestDBusAccountPlugin *self,
> }
> 
> static void
>-test_dbus_account_plugin_process_attributes (TestDBusAccountPlugin *self,
>-    GVariant *args)
>+attributes_changed_cb (GDBusConnection *bus,
>+    const gchar *sender_name,
>+    const gchar *object_path,
>+    const gchar *iface_name,
>+    const gchar *signal_name,
>+    GVariant *tuple,
>+    gpointer user_data)
> {
>+  TestDBusAccountPlugin *self = TEST_DBUS_ACCOUNT_PLUGIN (user_data);
>   const gchar *account_name;
>   Account *account;
>   GVariant *attrs;
>   GVariant *attr_flags;
>   GVariant *deleted;
> 
>-  g_variant_get (args, "(&s@a{sv}@a{su}@as)",
>+  g_variant_get (tuple, "(&s@a{sv}@a{su}@as)",
>       &account_name, &attrs, &attr_flags, &deleted);
>   DEBUG ("%s", account_name);
>   account = lookup_account (self, account_name);
>@@ -501,9 +496,15 @@ test_dbus_account_plugin_process_attributes (TestDBusAccountPlugin *self,
> }
> 
> static void
>-test_dbus_account_plugin_process_parameters (TestDBusAccountPlugin *self,
>-    GVariant *args)
>+parameters_changed_cb (GDBusConnection *bus,
>+    const gchar *sender_name,
>+    const gchar *object_path,
>+    const gchar *iface_name,
>+    const gchar *signal_name,
>+    GVariant *tuple,
>+    gpointer user_data)
> {
>+  TestDBusAccountPlugin *self = TEST_DBUS_ACCOUNT_PLUGIN (user_data);
>   const gchar *account_name;
>   Account *account;
>   GVariant *params;
>@@ -511,7 +512,7 @@ test_dbus_account_plugin_process_parameters (TestDBusAccountPlugin *self,
>   GVariant *param_flags;
>   GVariant *deleted;
> 
>-  g_variant_get (args, "(&s@a{sv}@a{ss}@a{su}@as)",
>+  g_variant_get (tuple, "(&s@a{sv}@a{ss}@a{su}@as)",
>       &account_name, &params, &untyped_params, &param_flags, &deleted);
>   DEBUG ("%s", account_name);
>   account = lookup_account (self, account_name);
>@@ -630,123 +631,6 @@ test_dbus_account_plugin_process_parameters (TestDBusAccountPlugin *self,
>   g_variant_unref (deleted);
> }
> 
>-static void
>-test_dbus_account_plugin_process_events (TestDBusAccountPlugin *self)
>-{
>-  Event *event;
>-
>-  if (self->feedback == NULL)
>-    return;
>-
>-  while ((event = g_queue_pop_head (&self->events)) != NULL)
>-    {
>-      switch (event->type)
>-        {
>-          case EVENT_CREATION:
>-            test_dbus_account_plugin_process_account_creation (self,
>-                event->args);
>-            break;
>-
>-          case EVENT_DELETION:
>-            test_dbus_account_plugin_process_account_deletion (self,
>-                event->args);
>-            break;
>-
>-          case EVENT_ATTRS:
>-            test_dbus_account_plugin_process_attributes (self,
>-                event->args);
>-            break;
>-
>-          case EVENT_PARAMS:
>-            test_dbus_account_plugin_process_parameters (self,
>-                event->args);
>-            break;
>-        }
>-
>-      g_variant_unref (event->args);
>-      g_slice_free (Event, event);
>-    }
>-}
>-
>-static void
>-account_created_cb (GDBusConnection *bus,
>-    const gchar *sender_name,
>-    const gchar *object_path,
>-    const gchar *iface_name,
>-    const gchar *signal_name,
>-    GVariant *tuple,
>-    gpointer user_data)
>-{
>-  TestDBusAccountPlugin *self = TEST_DBUS_ACCOUNT_PLUGIN (user_data);
>-  const gchar *account_name;
>-
>-  g_variant_get (tuple, "(&s@a{sv}@a{su}@a{sv}@a{ss}@a{su}u)",
>-      &account_name, NULL, NULL, NULL, NULL, NULL, NULL);
>-  DEBUG ("%s", account_name);
>-
>-  g_queue_push_tail (&self->events, event_new (EVENT_CREATION, tuple));
>-  test_dbus_account_plugin_process_events (self);
>-}
>-
>-static void
>-account_deleted_cb (GDBusConnection *bus,
>-    const gchar *sender_name,
>-    const gchar *object_path,
>-    const gchar *iface_name,
>-    const gchar *signal_name,
>-    GVariant *tuple,
>-    gpointer user_data)
>-{
>-  TestDBusAccountPlugin *self = TEST_DBUS_ACCOUNT_PLUGIN (user_data);
>-  const gchar *account_name;
>-
>-  g_variant_get (tuple, "(&s)", &account_name);
>-  DEBUG ("%s", account_name);
>-
>-  g_queue_push_tail (&self->events, event_new (EVENT_DELETION, tuple));
>-  test_dbus_account_plugin_process_events (self);
>-}
>-
>-static void
>-attributes_changed_cb (GDBusConnection *bus,
>-    const gchar *sender_name,
>-    const gchar *object_path,
>-    const gchar *iface_name,
>-    const gchar *signal_name,
>-    GVariant *tuple,
>-    gpointer user_data)
>-{
>-  TestDBusAccountPlugin *self = TEST_DBUS_ACCOUNT_PLUGIN (user_data);
>-  const gchar *account_name;
>-
>-  g_variant_get (tuple, "(&s@a{sv}@a{su}@as)", &account_name,
>-      NULL, NULL, NULL);
>-  DEBUG ("%s", account_name);
>-
>-  g_queue_push_tail (&self->events, event_new (EVENT_ATTRS, tuple));
>-  test_dbus_account_plugin_process_events (self);
>-}
>-
>-static void
>-parameters_changed_cb (GDBusConnection *bus,
>-    const gchar *sender_name,
>-    const gchar *object_path,
>-    const gchar *iface_name,
>-    const gchar *signal_name,
>-    GVariant *tuple,
>-    gpointer user_data)
>-{
>-  TestDBusAccountPlugin *self = TEST_DBUS_ACCOUNT_PLUGIN (user_data);
>-  const gchar *account_name;
>-
>-  g_variant_get (tuple, "(&s@a{sv}@a{ss}@a{su}@as)", &account_name,
>-      NULL, NULL, NULL, NULL);
>-  DEBUG ("%s", account_name);
>-
>-  g_queue_push_tail (&self->events, event_new (EVENT_PARAMS, tuple));
>-  test_dbus_account_plugin_process_events (self);
>-}
>-
> static GList *
> test_dbus_account_plugin_list (McpAccountStorage *storage,
>     McpAccountManager *am)
>@@ -857,21 +741,6 @@ test_dbus_account_plugin_list (McpAccountStorage *storage,
>   return ret;
> }
> 
>-static void
>-test_dbus_account_plugin_ready (McpAccountStorage *storage,
>-    McpAccountManager *am)
>-{
>-  TestDBusAccountPlugin *self = TEST_DBUS_ACCOUNT_PLUGIN (storage);
>-
>-  DEBUG ("called");
>-  g_dbus_connection_emit_signal (self->bus, NULL,
>-      TEST_DBUS_ACCOUNT_PLUGIN_PATH, TEST_DBUS_ACCOUNT_PLUGIN_IFACE,
>-      "Ready", NULL, NULL);
>-  self->feedback = MCP_ACCOUNT_MANAGER (am);
>-
>-  test_dbus_account_plugin_process_events (self);
>-}
>-
> static gchar *
> test_dbus_account_plugin_create (McpAccountStorage *storage,
>     McpAccountManager *am,
>@@ -1568,7 +1437,6 @@ account_storage_iface_init (McpAccountStorageIface *iface)
>   iface->set_attribute = test_dbus_account_plugin_set_attribute;
>   iface->set_parameter = test_dbus_account_plugin_set_parameter;
>   iface->list = test_dbus_account_plugin_list;
>-  iface->ready = test_dbus_account_plugin_ready;
>   iface->delete_async = test_dbus_account_plugin_delete_async;
>   iface->delete_finish = test_dbus_account_plugin_delete_finish;
>   iface->commit = test_dbus_account_plugin_commit;
>-- 
>1.9.rc1
>

Actions: View | Splinter Review

Attachments on bug 74581: 93478 | 93479 | 93480 | 93481 | 93482 | 93483 | 93484

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.