Bug 52938

I'd like to fix this to simplify Empathy code before porting to next.

My idea was to:
- add TP_ACCOUNT_FEATURE_AVATAR, when preparing this feature connect the 'AvatarChanged' signal and fetch the initial avatar
- Expose the avatar as a TpAccount property
- Fire a property change notification when the avatar is changed

Question: how should we expose the avatar on the TpAccount?
- a (GFile, const gchar *mime_type) as TpContact does (2 properties)? But then we should write the file to disk first.
- a (GArray, const gchar *mime_type) to stay coherent with tp_account_get_avatar_finish()?
- a GVariant packing these 2 values into one?
- something else?

(In reply to comment #1)
> I'd like to fix this to simplify Empathy code before porting to next.

Does Empathy really want to be tracking the avatar (= pull it into memory whenever it changes) *all the time*, or only when the account editor is actually open?

> Question: how should we expose the avatar on the TpAccount?
> - a (GFile, const gchar *mime_type) as TpContact does (2 properties)? But
> then we should write the file to disk first.

I don't think this should happen unless we replace A.I.Avatar with something URI-based at the D-Bus level (Bug #33409). Random UIs copying the avatar to a separate file on disk seems rather pointless.

> - a (GArray, const gchar *mime_type) to stay coherent with
> tp_account_get_avatar_finish()?
> - a GVariant packing these 2 values into one?

Or a GBytes and a string, maybe? But I don't think random UIs keeping our avatar in memory is desirable, if they're not actually actively using it.

Until Bug #33409 is done, I'd be tempted to do the straightforward binding of the current D-Bus API:

    /**
     * TpAccount::avatar-changed:
     *
     * Emitted when the avatar changes. Call tp_account_get_avatar_async()
     * to get the new avatar data.
     */

If you're going to load a GFile from disk, with current GIO best-practices it's going to be an async operation *anyway*... so we might as well use the current D-Bus API?

(In reply to comment #2)
> (In reply to comment #1)
> > I'd like to fix this to simplify Empathy code before porting to next.
> 
> Does Empathy really want to be tracking the avatar (= pull it into memory
> whenever it changes) *all the time*, or only when the account editor is
> actually open?

To be clear, my objection here is that features are non-revocable: once you've started, you can't stop. I'm not sure that's a great fit for monitoring accounts' avatars.

(In reply to comment #2)
> (In reply to comment #1)
> > I'd like to fix this to simplify Empathy code before porting to next.
> 
> Does Empathy really want to be tracking the avatar (= pull it into memory
> whenever it changes) *all the time*, or only when the account editor is
> actually open?

Indeed, only when empathy-accounts (or the equivalent GOA UI) is opened.


>     /**
>      * TpAccount::avatar-changed:
>      *
>      * Emitted when the avatar changes. Call tp_account_get_avatar_async()
>      * to get the new avatar data.
>      */

You're right, that's enough for now, I'll do that.

Created attachment 88031 [details] [review]
simple-account: add test API to change the avatar

Created attachment 88032 [details] [review]
simple-account: claim that we support Avatar

Created attachment 88033 [details] [review]
add TpAccount::avatar-changed signal

++ for those patches.

Merged to master, will be in 0.23.0.