[Bug 38248] Get handle identifiers for contacts related to a channel

[bugzilla-daemon at freedesktop.org]

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

--- Comment #23 from Xavier Claessens <xclaesse at gmail.com> 2011-09-29 07:30:24 PDT ---
(In reply to comment #22)
> tp_svc_channel_interface_group_emit_handle_owners_changed_detailed (
>     obj, empty_hash_table, arr_owners_removed, empty_hash_table);
> According to the spec we MAY include the identifier of the removed handles;
> any reason to not to?

Same rational than for contact-ids in MembersChangedDetailed. This makes
message size smaller since we don't need them anyway.

> dup_handle_array(): can't we use g_array_ref? Doesn't dbus-glib play nice with
> GArray now?

ahah! no :(

I've checked dbus-glib code and only GHashTable are safe to ref, arrays are
destroyed.

> dup_owners_table(): you don't check if dup_contact returned NULL.

We actually want to map to NULL in that case, meaning it is channel-specific
but mapping is unknown. If we don't include in the map it would mean it is
globally valid contact. See in tp_channel_group_get_contact_owner() we use
g_hash_table_lookup_extended() to make distinction between not in table and
mapped to NULL.

> contacts_queue_item_set_contacts(): add a "g_assert (item->contacts == NULL);"
> to make sure we don't leak contacts?

can't hurt indeed. done.

> tp_channel_group_get_contact_owner:
> * - if @self does not have flags that include
>  *   %TP_CHANNEL_GROUP_FLAG_PROPERTIES,
>  *   result is undefined;
> Did you mean TP_CHANNEL_GROUP_FLAG_CHANNEL_SPECIFIC_HANDLES?

That's copy/pasted from tp_channel_group_get_handle_owner. I think it is
correct because that flag tells if GetAll works. that's totally old stuff where
group handles were not fetched via dbus properties. In the case we don't have
TP_CHANNEL_GROUP_FLAG_CHANNEL_SPECIFIC_HANDLES, the result is defined since we
know all contacts are globally valid so it will always return @self.

> _tp_channel_contacts_prepare_async
>           "The Connection Manager does not implement the required telepathy "
>           "specification to prepare TP_CHANNEL_FEATURE_CONTACTS");
> Include the spec version number?

right, done.

> +   * TpChannel::group-contacts-changed:
> +   * @added: (type GLib.PtrArray) (element-type TelepathyGLib.Contact):
> +   *  a #GList of #TpContact containing the full members added
> It's not a GList.
> ditto for the other args.
> Say that those contacts have been prepared if possible?

I've added 
   * This is not guaranteed to be emitted until tp_proxy_prepare_async() has
   * finished preparing %TP_CHANNEL_FEATURE_CONTACTS; until then, it may be
   * %NULL.

Since preparing that would catch too old CMs. And the doc of that feature says
it is guaranteed that contacts are always prepared. It's the same text copied
from other properties.

> 
> http://cgit.collabora.com/git/user/xclaesse/telepathy-glib.git/commit/?h=channel-contacts&id=bad617f34e09084ef977fa11fa2f1ae1af5cffbe
> 
> +  g_print ("contact added: %s\n", tp_contact_get_identifier (g_ptr_array_index
> (added, 0)));
> should be removed.

Oops, removed.


Pushed an extra commit to fix review comments, I'll squash if it's OK.

-- 
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.