GNOME Bugzilla – Bug 599743
Should use MC5 to dispatch tube
Last modified: 2010-01-14 13:53:41 UTC
We plan to remove the old style tube dispatching during this cycle (bug #599159) so Vinagre should use MC5 to dispatch his tube.
Created attachment 148429 [details] [review] Port to MC5 using proposed TpHandler class http://git.collabora.co.uk/?p=user/danni/vinagre.git;a=shortlog;h=refs/heads/tphandler data/Makefile.am | 12 +- data/Vinagre.client | 7 + ...freedesktop.Telepathy.Client.Vinagre.service.in | 3 + ....gnome.Empathy.StreamTubeHandler.rfb.service.in | 3 - vinagre/Makefile.am | 6 - vinagre/tube-handler.xml | 57 ------ vinagre/vinagre-tubes-manager.c | 199 ++++++-------------- vinagre/vinagre-tubes-manager.h | 9 +- 8 files changed, 86 insertions(+), 210 deletions(-) This patch ports Vinagre away from the Empathy Tube dispatcher to MC5. It utilises the proposed TpHandler class, available here: http://git.collabora.co.uk/?p=user/danni/telepathy-glib.git;a=shortlog;h=refs/heads/tphandler
Cool; I'm happy to see that the port didn't requiere a lot of refactoring (I tried to design the existing code with MC5 in mind). I didn't properly review your patch but have 2 comments after a very quick reading of it: - Vinagre should only handle stream tube channel having Requested=False (incoming tubes). Outgoing tubes will be dispatched by Vino. - Having the callback as a GObject property seems strange. Any reason to not have an abstract method in TpHandler implemented by the subclass?
(1) Ok. I wasn't sure, so left it out. This sounds like we'll need to port Vino to MC5 too? (2) The callback is a property, rather than a a vcall method so that you can instantiate a TpHandler without subclassing (e.g. handler = TpHandler (filter, FALSE, callback_func);). I subclassed here because there was already a class for the Handler, so it was a simpler port.
(In reply to comment #3) > (1) Ok. I wasn't sure, so left it out. This sounds like we'll need to port Vino > to MC5 too? Indeed. I just opened bug #602917 for that. Note that this is less urgent than the Vinagre port as that doesn't block bug #599159. > (2) The callback is a property, rather than a a vcall method so that you can > instantiate a TpHandler without subclassing (e.g. handler = TpHandler (filter, > FALSE, callback_func);). I subclassed here because there was already a class > for the Handler, so it was a simpler port. Fair enough. I'll let our tp-glib guru decide of the API anyway :)
Danielle: I'd really like to see this fixed for 2.30 as we plan to drop old style tube dispatching (bug #599159). It's unlikely we'll have an Approver in telepathy-glib soonish so how do you feel about copying your TpHandler implementation into Vinagre's tree and use this one instead?
I rebased your branch on top of master and imported TpHandler: http://git.collabora.co.uk/?p=user/cassidy/vinagre;a=shortlog;h=refs/heads/tp-handler
Looks good to me.
Thanks. Vinagre dev: Are you ok with this branch? I'd like to get it merged asap so Empathy can drop old style tube dispatching.
Please, go ahead.
Merged to master. Thanks a lot to both of you. This problem has been fixed in the development version. The fix will be available in the next major software release. Thank you for your bug report.