GNOME Bugzilla – Bug 655799
Use new client factory
Last modified: 2011-08-18 10:30:23 UTC
tp-glib master introduced a new more generic kind of factory. We should use it once it's released.
Not sure if that's a merge blocker but we should be able to share objects with folks: bug #655815.
Created attachment 193078 [details] [review] Port to new tp-glib client factory - EmpathyChannelFactory has been changed to EmpathyClientFactory and inherit from TpAutomaticClientFactory. - We now always use the _with_am variant of TpSimple* constructors - We always use empathy_account_manager_dup() instead of tp_account_manager_dup() - Replace empathy_get_account_for_connection() by tp_connection_get_account() - The factory is passed to EmpathyTpChat and TpyCallChannel This should ensure that all the TpProxy and TpContact objects created in Empathy are shared and use EmpathyClientFactory.
(In reply to comment #1) > Not sure if that's a merge blocker but we should be able to share objects with > folks: bug #655815. I would say that's blocker, it would duplicate all TpContact objects, right? Well if not merge blocker, at least 3.2 blocker.
Review of attachment 193078 [details] [review]: ::: libempathy/empathy-chatroom-manager.c @@ +581,3 @@ } + am = empathy_account_manager_dup (); you can use priv->account_manager initialized just above ::: libempathy/empathy-client-factory.c @@ +173,3 @@ + TP_SIMPLE_CLIENT_FACTORY (factory)); + g_object_unref (factory); + note that you could cache the manager ptr in static here instead of calling ensure_account_manager() all the time, because that one would lookup into a GHashTable containing all TpProxy subclasses. Or that optimization could be done in TpSimpleClientFactory itself tbh.
(In reply to comment #4) > Review of attachment 193078 [details] [review]: > > ::: libempathy/empathy-chatroom-manager.c > @@ +581,3 @@ > } > > + am = empathy_account_manager_dup (); > > you can use priv->account_manager initialized just above done. > ::: libempathy/empathy-client-factory.c > @@ +173,3 @@ > + TP_SIMPLE_CLIENT_FACTORY (factory)); > + g_object_unref (factory); > + > > note that you could cache the manager ptr in static here instead of calling > ensure_account_manager() all the time, because that one would lookup into a > GHashTable containing all TpProxy subclasses. Or that optimization could be > done in TpSimpleClientFactory itself tbh. done.
Created attachment 193083 [details] [review] Port to new tp-glib client factory - EmpathyChannelFactory has been changed to EmpathyClientFactory and inherit from TpAutomaticClientFactory. - We now always use the _with_am variant of TpSimple* constructors - We always use empathy_account_manager_dup() instead of tp_account_manager_dup() - Replace empathy_get_account_for_connection() by tp_connection_get_account() - The factory is passed to EmpathyTpChat and TpyCallChannel This should ensure that all the TpProxy and TpContact objects created in Empathy are shared and use EmpathyClientFactory.
Patch looks good, IMO it is ready to be merged once we sorted out bug #655815.
Created attachment 194112 [details] [review] Depend on telepathy-glib 0.15.5 Needed for new factory API.
Created attachment 194113 [details] [review] Port to new tp-glib client factory - EmpathyChannelFactory has been changed to EmpathyClientFactory and inherit from TpAutomaticClientFactory. - We now always use the _with_am variant of TpSimple* constructors - We define our own factory as default. - Replace empathy_get_account_for_connection() by tp_connection_get_account() - The factory is passed to EmpathyTpChat and TpyCallChannel - Use tp_simple_client_factory_ensure_account() instead of tp_account_manager_ensure_account(). - Rely on the factory to prepare connection features. This should ensure that all the TpProxy and TpContact objects created in Empathy are shared and use EmpathyClientFactory.
Review of attachment 194113 [details] [review]: missing the new factory files ::: libempathy/empathy-chatroom-manager.c @@ -605,3 @@ - tp_base_client_add_connection_features_varargs (priv->observer, - TP_CONNECTION_FEATURE_CAPABILITIES, NULL); that feature is not set on the new factory. ::: tests/empathy-chatroom-manager-test.c @@ +110,3 @@ { "name2", "room2", FALSE, TRUE }}; + account_manager = tp_account_manager_dup_singleton (); does that build? s/_singleton// afaik.
(In reply to comment #10) > Review of attachment 194113 [details] [review]: > > missing the new factory files Oooops; added. > ::: libempathy/empathy-chatroom-manager.c > @@ -605,3 @@ > > - tp_base_client_add_connection_features_varargs (priv->observer, > - TP_CONNECTION_FEATURE_CAPABILITIES, NULL); > > that feature is not set on the new factory. it is in the EmpathyClientFactory. > ::: tests/empathy-chatroom-manager-test.c > @@ +110,3 @@ > { "name2", "room2", FALSE, TRUE }}; > > + account_manager = tp_account_manager_dup_singleton (); > > does that build? s/_singleton// afaik. Those disables have been disabled, so yes. Anyway, fixed.
Created attachment 194114 [details] [review] Port to new tp-glib client factory - EmpathyChannelFactory has been changed to EmpathyClientFactory and inherit from TpAutomaticClientFactory. - We now always use the _with_am variant of TpSimple* constructors - We define our own factory as default. - Replace empathy_get_account_for_connection() by tp_connection_get_account() - The factory is passed to EmpathyTpChat and TpyCallChannel - Use tp_simple_client_factory_ensure_account() instead of tp_account_manager_ensure_account(). - Rely on the factory to prepare connection features. This should ensure that all the TpProxy and TpContact objects created in Empathy are shared and use EmpathyClientFactory.
(In reply to comment #11) > (In reply to comment #10) > > ::: libempathy/empathy-chatroom-manager.c > > @@ -605,3 @@ > > > > - tp_base_client_add_connection_features_varargs (priv->observer, > > - TP_CONNECTION_FEATURE_CAPABILITIES, NULL); > > > > that feature is not set on the new factory. > > it is in the EmpathyClientFactory. ah, ok. Other features (contacts alias, etc) are still prepared manually I guess? Branch looks good to me, thanks.
Attachment 194112 [details] pushed as 7fbf3e0 - Depend on telepathy-glib 0.15.5 Attachment 194114 [details] pushed as 9ddd25f - Port to new tp-glib client factory