I noticed that whenever a chat state changes in an Empathy conversation window, Empathy (well, tp-glib on behalf of Empathy) would re-request Location attributes. For instance, here's me typing a single character into a conversation window: > method call sender=:1.657 -> dest=:1.452 serial=427 > path=/org/freedesktop/Telepathy/Connection/gabble/jabber/will_40example_2fb756e715/ImChannel11; > interface=org.freedesktop.Telepathy.Channel.Interface.ChatState; > member=SetChatState > uint32 3 > signal sender=:1.452 -> dest=(null destination) serial=10051 > path=/org/freedesktop/Telepathy/Connection/gabble/jabber/will_40example_2fb756e715/ImChannel11; > interface=org.freedesktop.Telepathy.Channel.Interface.ChatState; > member=ChatStateChanged > uint32 1 > uint32 3 > method return sender=:1.452 -> dest=:1.657 reply_serial=427 > method call sender=:1.657 -> dest=:1.452 serial=428 > path=/org/freedesktop/Telepathy/Connection/gabble/jabber/will_40example_2fb756e715; > interface=org.freedesktop.Telepathy.Connection.Interface.Contacts; > member=GetContactAttributes > array [ > uint32 1 > ] > array [ > string "org.freedesktop.Telepathy.Connection.Interface.Location" > ] > boolean true > method return sender=:1.452 -> dest=:1.657 reply_serial=428 > array [ > dict entry( > uint32 1 > array [ > dict entry( > string "org.freedesktop.Telepathy.Connection/contact-id" > variant string "will@example" > ) > ] > ) > ] This happens because the internal CONTACT_FEATURE_FLAG_LOCATION flag is only set on a TpContact when a location is discovered for them (specifically, by contact_maybe_set_location(), which is called from the LocationUpdated signal and from the GetContactAttributes return callback. (And, indirectly, from the inexplicable GetLocations() fallback path, but we'll ignore that.) contact_maybe_set_location() looks like this: static void contact_maybe_set_location (TpContact *self, GHashTable *location) { if (self == NULL || location == NULL) return; if (self->priv->location != NULL) g_hash_table_unref (self->priv->location); self->priv->has_features |= CONTACT_FEATURE_FLAG_LOCATION; self->priv->location = g_hash_table_ref (location); g_object_notify ((GObject *) self, "location"); } But the ".../location" attribute is defined to be omitted entirely if the contact has no location. <http://telepathy.freedesktop.org/spec/Connection_Interface_Location.html#Contact-Attribute:location> TpContact should be able to tell that it requested location information, and hence that the absence of the attribute indicates that the contact has no published location information. (Relatedly, I noticed that Empathy will crash if ::notify(location) is emitted by TpContact and tp_contact_get_location() subsequently returns NULL. So I guess we have to represent “I know this contact has no published location information” by an empty hash table. Joy!) Some other features may have similar issues. I haven't really checked. This is the one that bothers me whenever I look at dbus-monitor. :)
I tweaked test_by_handle_again() to see which other features have this issue. - TpContactFeature feature = TP_CONTACT_FEATURE_ALIAS; + TpContactFeature features[] = { + TP_CONTACT_FEATURE_ALIAS, + // TP_CONTACT_FEATURE_AVATAR_TOKEN, + TP_CONTACT_FEATURE_PRESENCE, + // TP_CONTACT_FEATURE_LOCATION, + // TP_CONTACT_FEATURE_CAPABILITIES, + // TP_CONTACT_FEATURE_AVATAR_DATA, + // TP_CONTACT_FEATURE_CONTACT_INFO, + TP_CONTACT_FEATURE_CLIENT_TYPES, + TP_CONTACT_FEATURE_SUBSCRIPTION_STATES, + TP_CONTACT_FEATURE_CONTACT_GROUPS + }; Uncommenting any one of the commented features above makes the test fail. In some cases this could be due to misimplementation of the test CM—I haven't checked.
Here's a branch which fixes this for Location on the fast path, and removes the slow path for Location. I haven't checked out the other ones yet. It also includes a hack to disable the test timeouts under gdb, and some crucial localization changes.
Branch looks good. Epic hats off to you for writing a nice tp-glib test; they're such a pain in the ass. Just one point: + g_warning ("%s supports Location but not Contacts! Where did you find " Use the WARNING macro? but ++++++++++++++++++++++
(In reply to comment #3) > Branch looks good. Epic hats off to you for writing a nice tp-glib test; > they're such a pain in the ass. Oh there is *way* more where this came from. Fancy reviewing http://cgit.collabora.com/git/user/wjt/telepathy-glib.git/log/?h=fd.o-39377-repeated-location-requests-moar ? > Just one point: > > + g_warning ("%s supports Location but not Contacts! Where did you find " > > Use the WARNING macro? Good point. Haven't fixed this yet.
#define TRACER_T "\nTracerPid:\t" // http://www.youtube.com/watch?v=SXmv8quf_xM I would change that to: /* http://www.youtube.com/watch?v=SXmv8quf_xM */ #define TRACER_T "\nTracerPid:\t" Otherwise branch looks good.
Rebased onto the freshly-merged xclaesse/contacts-refactor (yum, conflicts), checked that that branch does indeed fix the one remaining feature that was broken (AVATAR_DATA), amended the test accordingly, fixed the review feedback, and merged the lot. That felt good.
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.