Bug 40933

This branch fixes the problem by distinguishing when a contact appears in our contact list because it's already there when we connect (so we group the contacts and emit the signal with actor=0) and when contacts appear later because they accepted our subscription request.

Another possible solution is to not emit the signal at all when connecting. I looked into that, but I think that the fix looks more like a hack with little gain (emitting one MembersChanged and one MembersChangedDetailed signal is not a problem).

> + tp_group_mixin_change_members (sub_chan, "", sub, NULL, NULL, NULL, 100,
> + TP_CHANNEL_GROUP_CHANGE_REASON_NONE);

s/100/0/ (the actor)

> + if (interface == TP_IFACE_QUARK_CHANNEL_INTERFACE_GROUP &&
> + msg_type == DBUS_MESSAGE_TYPE_SIGNAL &&
> + g_strcmp0 (member, "MembersChanged") == 0)

dbus_message_is_signal() is ideal for low-level filters like this

Other than that, looks good to me.

+  tp_group_mixin_change_members (sub_chan, "", sub, NULL, NULL, NULL, 100,

Actor 100? :O

The rest of this is just nit-picking

+  closure = g_slice_new0 (MembersChangedClosure);

You don't need to slice-allocate the struct, you could just stick it on the stack. I don't really care though.

+      g_assert (lookup_contact (self, members[i]) == NULL);

g_assert_cmpstr for future reference. :)

+  /* Now put it online and wait for contact list state move to success */

“wait for the contact list state to move”

100 was an interesting typo :-O

> +      g_assert (lookup_contact (self, members[i]) == NULL);
> 
> g_assert_cmpstr for future reference. :)

Er... it's not a string :)

I fixed the things you suggested to change and pushed to master.