Bug 38524 - TpChannel: priv->handle_type overrided during object construction
Summary: TpChannel: priv->handle_type overrided during object construction
Status: RESOLVED FIXED
Alias: None
Product: Telepathy
Classification: Unclassified
Component: tp-glib (show other bugs)
Version: unspecified
Hardware: Other All
: medium normal
Assignee: Telepathy bugs list
QA Contact: Telepathy bugs list
URL: http://cgit.collabora.com/git/user/ca...
Whiteboard:
Keywords: patch
Depends on:
Blocks:
 
Reported: 2011-06-21 06:39 UTC by Guillaume Desmottes
Modified: 2011-06-21 07:11 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments
channel-iface: set TP_UNKNOWN_HANDLE_TYPE as default handle type (1.35 KB, patch)
2011-06-21 06:43 UTC, Guillaume Desmottes
Details | Splinter Review

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.