While updating Empathy's GOA plugin to the new plugin API in master, I noticed that the "ready" vfunc is basically rubbish. Pedantic plugins that implement it correctly (TestDBusAccountStorage, UOA) have to write a bunch of extra code to queue up events for no good reason; plugins that don't implement it correctly (GOA) might not load a complete set of accounts. I think we should fix MC so plugins don't need to jump through those hoops.
Created attachment 93478 [details] [review] McdAccountManager: ref the objects in McdLoadAccountsData
Created attachment 93479 [details] [review] McdAccountManager: if an account is async-added during setup, wait for it Previously, we'd only wait for accounts added in mcd_storage_load() to become ready.
Created attachment 93480 [details] [review] 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.
Created attachment 93481 [details] [review] update documentation for McpAcccountStorageIface members
Created attachment 93482 [details] [review] [Empathy] GOA plugin: use signal-emission helpers --- Tested by making 3.8's goa-mc-plugin/ identical to master's, then applying this over the top, since I don't have a build environment for current Empathy right now...
Created attachment 93483 [details] [review] [Empathy] GOA plugin: implement both MC 5.16 and MC 5.18 APIs --- When MC 5.17 has had some actual releases, we can delete the 5.16 code paths and depend on it... but this plugin is so small that it's actually easy to support both.
Created attachment 93484 [details] [review] [Empathy] GOA plugin: implement both MC 5.16 and MC 5.18 APIs --- Oops, here is the patch I intended to attach.
Comment on attachment 93478 [details] [review] McdAccountManager: ref the objects in McdLoadAccountsData Review of attachment 93478 [details] [review]: ----------------------------------------------------------------- ++
Comment on attachment 93479 [details] [review] McdAccountManager: if an account is async-added during setup, wait for it Review of attachment 93479 [details] [review]: ----------------------------------------------------------------- ++
Comment on attachment 93480 [details] [review] McdStorage: watch and proxy plugins' change-notification signals Review of attachment 93480 [details] [review]: ----------------------------------------------------------------- ++ nice simplification
Comment on attachment 93481 [details] [review] update documentation for McpAcccountStorageIface members Review of attachment 93481 [details] [review]: ----------------------------------------------------------------- ++
Comment on attachment 93482 [details] [review] [Empathy] GOA plugin: use signal-emission helpers Review of attachment 93482 [details] [review]: ----------------------------------------------------------------- ++
Comment on attachment 93483 [details] [review] [Empathy] GOA plugin: implement both MC 5.16 and MC 5.18 APIs Review of attachment 93483 [details] [review]: ----------------------------------------------------------------- Give me a branch and I'll try building it.
(In reply to comment #13) > Give me a branch and I'll try building it. Here's what I used: ssh://people.freedesktop.org/~smcv/empathy.git smcv/38-goa-mc-517 http://cgit.freedesktop.org/~smcv/empathy/log?h=38-goa-mc-517 The branch to try for master is a cherry-pick of the 2nd and 3rd commits there: ssh://people.freedesktop.org/~smcv/empathy.git smcv/untested-goa-mc-517 http://cgit.freedesktop.org/~smcv/empathy/log/?h=untested-goa-mc-517&i-hate-the-cgit-cache Build against MC master.
Fixed in git for MC 5.17.0; Empathy bits pending, but this isn't really its bug tracker anyway.
(In reply to comment #14) > The branch to try for master is a cherry-pick of the 2nd and 3rd commits > there: Makeses sense and build fine with Empathy master (using older and newer MC) so I merged these 2 patches directly. Thanks!
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.