[Bug 30617] TpChannel: high level API to close/leave channel

[bugzilla-daemon at freedesktop.org]

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

Simon McVittie <simon.mcvittie at collabora.co.uk> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                URL|http://git.collabora.co.uk/ |http://git.collabora.co.uk/
                   |?p=user/cassidy/telepathy-g |?p=user/smcv/telepathy-glib
                   |lib;a=shortlog;h=refs/heads |-smcv.git;a=shortlog;h=refs
                   |/channel-close-30617        |/heads/leave-the-area

--- Comment #9 from Simon McVittie <simon.mcvittie at collabora.co.uk> 2010-12-09 09:43:30 PST ---
(In reply to comment #2)
> Also, if the channel is invalidated before we've finished and the method call
> fails, we should DEBUG() the failure reason, and succeed anyway - again, the
> important thing is that we left. The only way "leave" can fail should be if
> Close() failed and the channel remained open (e.g. non-empty Group).

I implemented this for the Close() case too.

(In reply to comment #6)
> > > +  g_simple_async_result_complete (result);
> > 
> > This should be in an idle: TpProxy doesn't guarantee delayed callback calls,
> > sadly.
> 
> How so? Both _complete are in a D-Bus method callback so it should be fine,
> no?

Unfortunately, tp_cli_*_call_* calls the callback immediately (before
returning) if the proxy has already been invalidated, or if it doesn't have the
interface. I'll fix that for 1.0, but I think it'd be a subtle API break to fix
it now.

> > I think it'd be useful if tp_channel_leave_async() supported a NULL callback?
> 
> Doesn't GSimpleAsyncResult already do that?

I thought it did, but you're right, it doesn't. This doesn't give us the
optimization of ignoring the reply at the D-Bus level, though.

I implemented that optimization for close_async, because it's trivial, but not
for leave_async, because it's harder there.

> > > +  /* FIXME: This is crack a chan_room_service is actually not a
> > > +   * TpTestsTextChannelNull */
> > > +  props = tp_tests_text_channel_get_props (
> > > +      (TpTestsTextChannelNull *) test->chan_room_service);
> > 
> > Er, yes, that...
> 
> Oh I forgot that. From a quick look, tests doesn't seem to rely on it leaving
> in the past so I'll port it to TpBaseChannel.

You forgot to fix this bit, but I've done that now.

(In reply to comment #8)
> Perhaps it'd be better to have close_async() always be Close(),
> leave_group_async() always be removal of the self-handle, and leave it at
> that...

I'm still not sure exactly what the semantics should be. :-(

I've made close_async() always call Close: I'm pretty sure that's correct -
when we implement proxy mode in Idle, we want it to close channels, but not
leave them.

At the moment, leave_async() calls RemoveMembersWithReason with failover to
Close. I think that's *probably* right - "leaving" a non-Group channel is
fairly meaningless, but it seems harmless to implement that as Close (leaving
is "more destructive than" closing, even in the proposed proxy mode).

Another possibility would be to make leave_async g_return_if_fail that it is
(already known to be) a Group, rename it to group_leave_async, and remove the
fallbacks to Close.

Sjoerd, any thoughts?

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.