Created attachment 57940 [details] [review]
Don't use tp_channel_call_when_ready, except in its  regression tests

Created attachment 57941 [details] [review]
Use tp_proxy_prepare_async() to prepare connections in  example code

Created attachment 57942 [details] [review]
Deprecate tp_channel_call_when_ready,  tp_connection_call_when_ready

Created attachment 57943 [details] [review]
Deprecate tp_channel_is_ready and  TpChannel:channel-ready

Created attachment 57944 [details] [review]
TpContact: stop using about-to-be-deprecated Connection  API

Created attachment 57945 [details] [review]
Deprecate tp_connection_is_ready,  TpConnection:connection-ready

Created attachment 57946 [details] [review]
inspect-cm example: stop using call_when_ready

(Still to do: actually deprecate tp_connection_manager_call_when_ready())

All these patches so far look good.

(In reply to comment #9)
> All these patches so far look good.

Thanks, merged. More to come...

Created attachment 58024 [details] [review]
tests/dbus/cm: use g_assert_no_error for better  diagnostics

---

I have a branch in progress to do this to the other tests, but this bit seemed likely to conflict.

Created attachment 58025 [details] [review]
Improve coverage of TpConnectionManager test

* test prepare_async in every situation where we previously tested
  call_when_ready
* test is_prepared (CORE) in the same situations
* test get_invalidated in the same situations

This brings us closer to being able to deprecate call_when_ready
and is_ready.

Created attachment 58026 [details] [review]
tp_list_connection_managers: use tp_proxy_prepare_async

We're about to deprecate call_when_ready, so let's not call it.

Created attachment 58027 [details] [review]
Deprecate tp_connection_manager_call_when_ready and  is_ready

Created attachment 58031 [details] [review]
[next] Remove TpChannel-specific readiness, just use TpProxy  preparation

Created attachment 58032 [details] [review]
[next] Remove TpConnection-specific readiness, just use TpProxy  preparation

gitwebs:

http://cgit.freedesktop.org/~smcv/telepathy-glib/log?h=deprecate-when-ready-45842
http://cgit.freedesktop.org/~smcv/telepathy-glib/log?h=next-remove-cwr-45842

Still to do: remove tp_connection_manager_call_when_ready from next (after merging master, with the patches from this bug applied).

Comment on attachment 58026 [details] [review]
tp_list_connection_managers: use tp_proxy_prepare_async

Review of attachment 58026 [details] [review]:
-----------------------------------------------------------------

::: telepathy-glib/connection-manager.c
@@ +1676,5 @@
>    size_t base_len;
>    gsize refcount;
>    gsize cms_to_ready;
>    unsigned getting_names:1;
> +  unsigned had_weak_object:1;

Are we really still using this instead of gboolean in new code?

@@ +1732,4 @@
>        g_ptr_array_add (list_context->arr, NULL);
>        cms = (TpConnectionManager **) list_context->arr->pdata;
>  
> +      if (!list_context->had_weak_object || list_context->weak_object != NULL)

I don't understand this. Call the callback if:

 * there was no weak object (regardless of whether it's been disposed already or not)
 * OR, there is a weak object around right now

So the only case where the callback won't get called is if:

 * had_weak_object is TRUE
 * AND, the weak object is not around.

Okay so this now makes sense: it'll only call the callback if the weak object is still around. Would it be possible to get a comment to explain this a bit more though please?

Every other patch looks good to me. If you could add another comment I'd appreciate it.

(In reply to comment #18)
> >    unsigned getting_names:1;
> > +  unsigned had_weak_object:1;
> 
> Are we really still using this instead of gboolean in new code?

I was going for "if there's already a bitfield extend it, if not, add a gboolean".

> Okay so this now makes sense: it'll only call the callback if the weak object
> is still around. Would it be possible to get a comment to explain this a bit
> more though please?

      /* If we never had a weak object anyway, call the callback.
       * If we had a weak object when we started, only call the callback
       * if it hasn't died yet. */

I regret introducing the weak_object parameter, and look forward to the glorious GIO future, with async results, cancellables where we really really need them, and "shut up and use TpWeakRef as your user_data" for the few cases where the weak_object semantics are actually useful.

Deprecation in 0.17.6, removal in next. Leaving the bug open to remove the CM readiness in next.

Created attachment 58053 [details] [review]
[next] Remove tp_connection_manager_call_when_ready, is_ready

(In reply to comment #20)
> I was going for "if there's already a bitfield extend it, if not, add a
> gboolean".

I think we want to replace all these with gbooleans in next.

>       /* If we never had a weak object anyway, call the callback.
>        * If we had a weak object when we started, only call the callback
>        * if it hasn't died yet. */

Okay, thanks.

Comment on attachment 58053 [details] [review]
[next] Remove tp_connection_manager_call_when_ready, is_ready

Review of attachment 58053 [details] [review]:
-----------------------------------------------------------------

Perfecto!

(In reply to comment #23)
> (In reply to comment #20)
> > I was going for "if there's already a bitfield extend it, if not, add a
> > gboolean".
> 
> I think we want to replace all these with gbooleans in next.

Bug #48544.

(In reply to comment #24)
> Perfecto!

Fixed in next.