[Bug 33410] Do on-disk avatar cache in CM

bugzilla-daemon at freedesktop.org bugzilla-daemon at freedesktop.org
Thu Jun 7 17:42:43 CEST 2012


https://bugs.freedesktop.org/show_bug.cgi?id=33410

--- Comment #14 from Jonny Lamb <jonny.lamb at collabora.co.uk> 2012-06-07 08:42:43 PDT ---
Here are some comments about your tp-glib branch:

 * the docs at the top of TpAvatarMixin are confusing. You start off with
   "To use the avatars mixin" then a section "Implementing Avatars". Try and
   consolidate the two.

+ /* TpHandle -> AvatarData

Owned AvatarData? Please make it explicit.

+ * <informalexample><programlisting>
+ * tp_avatars_mixin_init ((GObject *) self,
+ * G_STRUCT_OFFSET (SomeObject, avatars_mixin));
+ * </programlisting></informalexample>

tp_avatars_mixin_init has loads more parameters than that.

+ static GQuark q[NUM_MIXIN_DBUS_PROPERTIES] = { 0, };
+
+ if (G_UNLIKELY (q[0] == 0))
+ {

I don't like this. Why don't you just use the getter data in
TpDBusPropertiesMixinPropImpl? So something like this:

static TpDBusPropertiesMixinPropImpl known_avatars_props[] = {
  { "AvatarPersists", GUINT_TO_POINTER (MIXIN_DP_AVATAR_PERSISTS), NULL },
...

then you can just check the GPOINTER_TO_UINT-ed value in the getter function?

Include the standard TP_BASE_CONNECTION_ERROR_IF_NOT_CONNECTED in the
RequestAvatars implementation.

g_file_get_contents is always bad. avatar_cache_lookup is only called by
tp_avatars_mixin_avatar_changed so we could make it use the async version, no?
Same for _set_contents?

(I didn't look at your other patches, updating TpContact and the unit tests.)

(In reply to comment #13)
>  * why is there a AvatarsNeedRequest signal? (that name sounds bad too, btw)
>    Why doesn't the CM just request the avatar and save it in the cache
>    immediately?

Actually I can kind of see how if a client doesn't care about avatars it won't
want this unnecessary network traffic. I propose a rw boolean property instead
(which is also a D-Bus connection parameter) which determines whether avatars
will be automatically requested or not. Y/N

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA Contact for the bug.
You are the assignee for the bug.



More information about the telepathy-bugs mailing list