Bug 36490

Summary: TpBaseClient: need a way to tell we are handling a channel
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: trivial    
Priority: medium Keywords: patch
Version: unspecified   
Hardware: Other   
OS: All   
URL: http://cgit.collabora.co.uk/git/user/cassidy/telepathy-glib/log/?h=claim-handle-36490
Whiteboard:
i915 platform: i915 features:

Description Guillaume Desmottes 2011-04-22 06:55:43 UTC 
TpBaseClient is implementing the Handler.HandledChannels property in a transparent way to the user; which is cool.

But that only work when HandleChannels() is called on the client.
There is another way to become the handler of a channel if you are also an Approver: call ChannelDispatchOperation.Claim() and this case is not covered by our API.

We could add a something like tp_base_client_now_handling (TpBaseClient *self, GList *channels) to fix this, but I'm sure loads of clients will forget to call it (I didn't notice this problem myself until I started using _is_handling()).

Another (better?) option could be to jump on bug #28015 and add high level API for the "Claim and handle case". We could then have something like:

void tp_channel_dispatch_operation_claim_and_handle_async (
    TpChannelDispatchOperation *self,
    TpBaseClient *client,
    GAsyncReadyCallback callback,
    gpointer user_data);

or maybe that should be an API on the TpBaseClient?

If we add higher level API (claim_and_close, claim_and_handle) we could even consider deprecating tp_channel_dispatch_operation_claim_async() to enforce user to use the safer APIs.
Comment 1 Guillaume Desmottes 2011-05-04 02:41:51 UTC 
I implemented this in my test branch to test DelegateChannels() with the Shell.
http://cgit.collabora.co.uk/git/user/cassidy/telepathy-glib/log/?h=re-dispatch-34610

That works fine so I think that's the way to go.

One question though. Should this API be used only with Handler or not? If we make it generic, it could replace tp_channel_dispatch_operation_claim_async() ensuring that users always call the right function.
Comment 2 Guillaume Desmottes 2011-05-04 03:20:39 UTC 
(In reply to comment #1)
> One question though. Should this API be used only with Handler or not? If we
> make it generic, it could replace tp_channel_dispatch_operation_claim_async()
> ensuring that users always call the right function.

On a second thought, I think we should have only one function to reduce the risk of mistakes done by user.
Comment 4 Jonny Lamb 2011-05-09 06:25:01 UTC 
(In reply to comment #3)
> Implemented in
> http://cgit.collabora.co.uk/git/user/cassidy/telepathy-glib/log/?h=claim-handle-36490

The commit message "add tp_channel_dispatch_operation_claim_and_handle_async() (fdo #36490)" doesn't really work anymore.

in your test case, in claim_with_cb:

+ test->wait--;
+ if (test->wait <= 0)
+ g_main_loop_quit (test->mainloop);

But I don't see you incrementing test->wait?

+ g_simple_async_result_set_op_res_gpointer (result, g_object_ref (client),
+ g_object_unref);

makes me lol, but oh well!
Comment 5 Guillaume Desmottes 2011-05-10 00:28:35 UTC 
(In reply to comment #4)
> (In reply to comment #3)
> > Implemented in
> > http://cgit.collabora.co.uk/git/user/cassidy/telepathy-glib/log/?h=claim-handle-36490
> 
> The commit message "add tp_channel_dispatch_operation_claim_and_handle_async()
> (fdo #36490)" doesn't really work anymore.

Good catch, I ammended the commit.

> in your test case, in claim_with_cb:
> 
> + test->wait--;
> + if (test->wait <= 0)
> + g_main_loop_quit (test->mainloop);
> 
> But I don't see you incrementing test->wait?

Right, it's only used when waiting for more than one event (as it's init to
0); I always use test->wait that way in all my tests.
But I can explicitely set it to 1 if you prefer

> + g_simple_async_result_set_op_res_gpointer (result, g_object_ref (client),
> + g_object_unref);
> 
> makes me lol, but oh well!

It's much easier than allocating a context struct, etc (thanks C...).
Comment 6 Guillaume Desmottes 2011-05-10 00:49:33 UTC 
Merged to master; will be in 0.15.1

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.