[Pkg-telepathy-commits] [telepathy-mission-control-6] 37/280: McdAccount: go back to using GetKnownAvatarTokens

Simon McVittie smcv at debian.org
Thu Mar 27 20:07:02 UTC 2014


This is an automated email from the git hooks/post-receive script.

smcv pushed a commit to branch debian
in repository telepathy-mission-control-6.

commit 4f4ed243e50fa2ff1f273827ca3f9a578297834f
Author: Simon McVittie <simon.mcvittie at collabora.co.uk>
Date:   Tue Oct 1 15:38:53 2013 +0100

    McdAccount: go back to using GetKnownAvatarTokens
    
    Sadly, contact attributes aren't enough to distinguish between
    "this protocol doesn't store avatars and you haven't re-sent your
    avatar since you last connected", "this protocol stores avatars but
    the CM hasn't checked for your current avatar yet", and "this protocol
    stores avatars, but there is no avatar on the server for you".
    
    GetKnownAvatarTokens specifically excludes the middle option (blocking
    on a server round-trip if it needs to), and uses "avatar token missing"
    for the first and "avatar token empty" for the last.
    
    Bug: https://bugs.freedesktop.org/show_bug.cgi?id=69885
    Reviewed-by: Guillaume Desmottes <guillaume.desmottes at collabora.co.uk>
---
 src/mcd-account.c                               | 123 ++++++++++++++++++++----
 tests/twisted/account-manager/avatar-refresh.py |  23 -----
 2 files changed, 102 insertions(+), 44 deletions(-)

diff --git a/src/mcd-account.c b/src/mcd-account.c
index fb03638..f9e874e 100644
--- a/src/mcd-account.c
+++ b/src/mcd-account.c
@@ -172,6 +172,7 @@ struct _McdAccountPrivate
     gboolean always_on;
     gboolean changing_presence;
     gboolean setting_avatar;
+    gboolean waiting_for_initial_avatar;
     gboolean waiting_for_connectivity;
 
     gboolean hidden;
@@ -1415,6 +1416,13 @@ mcd_account_self_contact_notify_avatar_file_cb (McdAccount *self,
       return;
     }
 
+  if (self->priv->waiting_for_initial_avatar)
+    {
+      DEBUG ("Ignoring avatar change notification: we are waiting for the "
+          "initial value");
+      return;
+    }
+
   prev_token = _mcd_account_get_avatar_token (self);
   changed = tp_strdiff (prev_token, token);
   g_free (prev_token);
@@ -4813,28 +4821,49 @@ _mcd_account_get_old_avatar_filename (McdAccount *account,
 }
 
 static void
-mcd_account_process_initial_avatar_token (McdAccount *self)
+mcd_account_process_initial_avatar_token (McdAccount *self,
+    const gchar *token)
 {
+  GArray *avatar = NULL;
+  gchar *mime_type = NULL;
   gchar *prev_token;
-  const gchar *token;
 
   g_assert (self->priv->self_contact != NULL);
 
   prev_token = _mcd_account_get_avatar_token (self);
-  token = tp_contact_get_avatar_token (self->priv->self_contact);
+
+  DEBUG ("%s", self->priv->unique_name);
+
+  if (prev_token == NULL)
+    DEBUG ("no previous local avatar token");
+  else
+    DEBUG ("previous local avatar token: '%s'", prev_token);
+
+  if (avatar == NULL)
+    DEBUG ("no previous local avatar");
+  else
+    DEBUG ("previous local avatar: %u bytes, MIME type '%s'", avatar->len,
+        (mime_type != NULL ? mime_type : "(null)"));
+
+  if (token == NULL)
+    DEBUG ("no remote avatar token");
+  else
+    DEBUG ("remote avatar token: '%s'", token);
+
+  _mcd_account_get_avatar (self, &avatar, &mime_type);
 
   /* If we have a stored avatar but no avatar token, we must have
    * changed it locally; set it.
    *
-   * Meanwhile, if the self-contact's avatar token is empty, this is a
-   * protocol like link-local XMPP where avatars don't persist.
-   * Upload ours, if any. */
-  if (tp_str_empty (prev_token) || tp_str_empty (token))
+   * Meanwhile, if the self-contact's avatar token is missing, this is
+   * a protocol like link-local XMPP where avatars don't persist.
+   * We can distinguish between this case (token is missing, so token = NULL)
+   * and the case where there is no avatar on an XMPP server (token is
+   * present and empty), although it's ridiculously subtle.
+   *
+   * Either way, upload our avatar, if any. */
+  if (tp_str_empty (prev_token) || token == NULL)
     {
-      GArray *avatar = NULL;
-      gchar *mime_type = NULL;
-
-      _mcd_account_get_avatar (self, &avatar, &mime_type);
 
       if (avatar != NULL)
         {
@@ -4844,19 +4873,19 @@ mcd_account_process_initial_avatar_token (McdAccount *self)
             DEBUG ("We have an avatar and the server doesn't");
 
           mcd_account_send_avatar_to_connection (self, avatar, mime_type);
-          g_array_unref (avatar);
-          g_free (mime_type);
-          return;
+          goto out;
         }
-
-      tp_clear_pointer (&avatar, g_array_unref);
-      g_free (mime_type);
     }
 
   /* Otherwise, if the self-contact's avatar token
    * differs from ours, one of our "other selves" must have changed
    * it remotely. Behave the same as if it changes remotely
-   * mid-session - i.e. download it and use it as our new avatar. */
+   * mid-session - i.e. download it and use it as our new avatar.
+   *
+   * In particular, this includes the case where we had
+   * a non-empty avatar last time we signed in, but another client
+   * has deleted it from the server since then (prev_token nonempty,
+   * token = ""). */
   if (tp_strdiff (token, prev_token))
     {
       GFile *file = tp_contact_get_avatar_file (self->priv->self_contact);
@@ -4873,15 +4902,52 @@ mcd_account_process_initial_avatar_token (McdAccount *self)
        * notify::avatar-file will go off. */
     }
 
+out:
   g_free (prev_token);
+  tp_clear_pointer (&avatar, g_array_unref);
+  g_free (mime_type);
 }
 
+static void
+account_conn_get_known_avatar_tokens_cb (TpConnection *conn,
+    GHashTable *tokens,
+    const GError *error,
+    gpointer user_data,
+    GObject *weak_object)
+{
+  McdAccount *self = g_object_ref (weak_object);
+
+  self->priv->waiting_for_initial_avatar = FALSE;
+
+  if (error != NULL)
+    {
+      DEBUG ("%s: GetKnownAvatarTokens raised %s #%d: %s",
+          self->priv->unique_name, g_quark_to_string (error->domain),
+          error->code, error->message);
+    }
+  else if (self->priv->self_contact == user_data)
+    {
+      TpHandle handle = tp_contact_get_handle (self->priv->self_contact);
+
+      mcd_account_process_initial_avatar_token (self,
+          g_hash_table_lookup (tokens, GUINT_TO_POINTER (handle)));
+    }
+  else
+    {
+      DEBUG ("%s: GetKnownAvatarTokens for outdated self-contact '%s', "
+          "ignoring",
+          self->priv->unique_name, tp_contact_get_identifier (user_data));
+    }
+
+  g_object_unref (self);
+}
 
 static void
 mcd_account_self_contact_upgraded_cb (GObject *source_object,
     GAsyncResult *res,
     gpointer user_data)
 {
+  TpConnection *conn = TP_CONNECTION (source_object);
   McdAccount *self = tp_weak_ref_dup_object (user_data);
   GPtrArray *contacts = NULL;
   GError *error = NULL;
@@ -4891,8 +4957,7 @@ mcd_account_self_contact_upgraded_cb (GObject *source_object,
 
   g_return_if_fail (MCD_IS_ACCOUNT (self));
 
-  if (tp_connection_upgrade_contacts_finish (TP_CONNECTION (source_object),
-          res, &contacts, &error))
+  if (tp_connection_upgrade_contacts_finish (conn, res, &contacts, &error))
     {
       TpContact *self_contact;
 
@@ -4911,7 +4976,6 @@ mcd_account_self_contact_upgraded_cb (GObject *source_object,
           tp_g_signal_connect_object (self_contact, "notify::avatar-file",
               G_CALLBACK (mcd_account_self_contact_notify_avatar_file_cb),
               self, G_CONNECT_SWAPPED);
-          mcd_account_process_initial_avatar_token (self);
 
           tp_g_signal_connect_object (self_contact, "presence-changed",
               G_CALLBACK (mcd_account_update_self_presence),
@@ -4924,6 +4988,22 @@ mcd_account_self_contact_upgraded_cb (GObject *source_object,
               tp_contact_get_presence_status (self_contact),
               tp_contact_get_presence_message (self_contact),
               self_contact);
+
+          /* We have to use GetKnownAvatarTokens() because of its special
+           * case for CMs that don't always download an up-to-date
+           * avatar token before signalling CONNECTED. */
+          if (tp_proxy_has_interface_by_id (conn,
+              TP_IFACE_QUARK_CONNECTION_INTERFACE_AVATARS))
+            {
+              guint self_handle = tp_contact_get_handle (self_contact);
+              GArray *arr = g_array_new (FALSE, FALSE, sizeof (guint));
+
+              g_array_append_val (arr, self_handle);
+              tp_cli_connection_interface_avatars_call_get_known_avatar_tokens (
+                  conn, -1, arr, account_conn_get_known_avatar_tokens_cb,
+                  g_object_ref (self_contact), g_object_unref,
+                  (GObject *) self);
+            }
         }
       else if (self->priv->self_contact == NULL)
         {
@@ -5073,6 +5153,7 @@ _mcd_account_set_connection (McdAccount *account, McdConnection *connection)
     tp_clear_object (&priv->tp_connection);
 
     priv->connection = connection;
+    priv->waiting_for_initial_avatar = TRUE;
 
     if (connection)
     {
diff --git a/tests/twisted/account-manager/avatar-refresh.py b/tests/twisted/account-manager/avatar-refresh.py
index 03a02b9..e1c7051 100644
--- a/tests/twisted/account-manager/avatar-refresh.py
+++ b/tests/twisted/account-manager/avatar-refresh.py
@@ -148,29 +148,6 @@ class Account(object):
             # Nobody has an avatar - nothing should happen
             self.winner = None
 
-        # Hack around bugs... ideally, the tests would pass without these.
-        if server_delays and local_avatar == 'old' and remote_avatar:
-            # What we *should* do is wait for GetKnownAvatarTokens
-            # (because GetContactAttributes isn't guaranteed to fetch
-            # our own up-to-date avatar token from the server), then
-            # download the remote avatar. We currently don't.
-            self.winner = 'MC'
-        elif server_delays and remote_avatar and not local_avatar:
-            # What we *should* do is wait for GetKnownAvatarTokens
-            # (because GetContactAttributes isn't guaranteed to fetch
-            # our own up-to-date avatar token from the server), then
-            # download the remote avatar. At the moment we never actually
-            # download it at all.
-            self.winner = None
-        elif avatars_persist and local_avatar == 'old' and not remote_avatar:
-            # What we *should* do is work out that the avatar on the
-            # server has been deleted since we last signed in,
-            # and delete our local avatar to match. (telepathy-spec
-            # does provide a way to distinguish between this and
-            # "the protocol doesn't store avatars", but it's
-            # really subtle; it's hardly surprising if this is wrong.)
-            self.winner = 'MC'
-
     def test(self, q, bus, mc):
         expected_params = {
                 'account': self.id,

-- 
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-telepathy/telepathy-mission-control-6.git



More information about the Pkg-telepathy-commits mailing list