Bug 38524

Summary: TpChannel: priv->handle_type overrided during object construction
Product: Telepathy Reporter: Guillaume Desmottes <guillaume.desmottes>
Component: tp-glibAssignee: Telepathy bugs list <telepathy-bugs>
Status: RESOLVED FIXED QA Contact: Telepathy bugs list <telepathy-bugs>
Severity: normal    
Priority: medium Keywords: patch
Version: unspecified   
Hardware: Other   
OS: All   
URL: http://cgit.collabora.com/git/user/cassidy/telepathy-glib/log/?h=channel-type-38524
Whiteboard:
i915 platform: i915 features:
Attachments: channel-iface: set TP_UNKNOWN_HANDLE_TYPE as default handle type

Description Guillaume Desmottes 2011-06-21 06:39:10 UTC 
I create a TpChannel subclass using:

g_object_new (EMPATHY_TYPE_TP_CHAT,
			     "account", account,
			     "connection", conn,
			     "dbus-daemon", conn_proxy->dbus_daemon,
			     "bus-name", conn_proxy->bus_name,
			     "object-path", object_path,
			     "channel-properties", immutable_properties,
			     NULL);


The props hash table contains the right handle-type (TP_HANDLE_TYPE_ROOM), as expected.

But if I call tp_channel_get_handle() it returns TP_HANDLE_TYPE_NONE as handle type!

After some debugging I noticed that the following happen:
- _tp_channel_maybe_set_handle_type() is called a first time when setting the PROP_CHANNEL_PROPERTIES property; handle-type is correctly defined.
- _tp_channel_maybe_set_handle_type() is called a second time as the default setter for the handle-type property (which was not defined in g_object_new), setting it back to TP_HANDLE_TYPE_NONE


_tp_channel_maybe_set_handle_type() thinks that the handle_type is valid because it's NONE and not UNKNOWN. Setting TP_UNKNOWN_HANDLE_TYPE as default handle-type in channel-iface solves this but I'm really wondering why we didn't hit this issue before, this is very old code.
Comment 1 Guillaume Desmottes 2011-06-21 06:43:42 UTC 
http://cgit.collabora.com/git/user/cassidy/telepathy-glib/log/?h=channel-type-38524

Note that this bug has been discovered in https://bugzilla.gnome.org/show_bug.cgi?id=653003 as Empathy now rely more on tp_channel_get_handle().
Comment 2 Guillaume Desmottes 2011-06-21 06:43:56 UTC 
Created attachment 48228 [details] [review]
channel-iface: set TP_UNKNOWN_HANDLE_TYPE as default handle type

TP_HANDLE_TYPE_NONE is not a good default, if we don't know the handle type we
shouln't try to guess it.
This also confuses TpChannel which think we are setting a valid handle type;
see fdo#38524.
Comment 3 Will Thompson 2011-06-21 07:00:51 UTC 
Review of attachment 48228 [details] [review]:

r+

::: telepathy-glib/channel-iface.c
@@ +97,3 @@
     param_spec = g_param_spec_uint ("handle-type", "Handle type",
         "The TpHandleType of this channel's associated handle.",
+        0, G_MAXUINT32, TP_UNKNOWN_HANDLE_TYPE,

Well, this feels a bit sketchy because TP_UNKNOWN_HANDLE_TYPE is not really a TpHandleType: but the property is already documented to have this value in some cases, as is tp_channel_get_handle(). So this is fine!

Good catch, I would never have thought of looking here.
Comment 4 Guillaume Desmottes 2011-06-21 07:11:11 UTC 
Merged to 0.14 and master. Will be in 0.14.8 and 0.15.2

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.