[next] telepathy-glib: TpClientFactory: Simplify TpProtocol code

[Simon McVittie]

Module: telepathy-glib
Branch: next
Commit: 3ec480675c078c44f949e7e1f33b59ce0b5aa3b8
URL:    http://cgit.freedesktop.org/telepathy/telepathy-glib/commit/?id=3ec480675c078c44f949e7e1f33b59ce0b5aa3b8

Author: Xavier Claessens <xavier.claessens at collabora.com>
Date:   Fri Mar 28 13:14:21 2014 -0400

TpClientFactory: Simplify TpProtocol code

---

 telepathy-glib/client-factory.c    |   52 +++++++++---------------------------
 telepathy-glib/protocol-internal.h |    3 +++
 telepathy-glib/protocol.c          |   21 ++++++++++++---
 3 files changed, 32 insertions(+), 44 deletions(-)

diff --git a/telepathy-glib/client-factory.c b/telepathy-glib/client-factory.c
index 4b84536..ed9080a 100644
--- a/telepathy-glib/client-factory.c
+++ b/telepathy-glib/client-factory.c
@@ -141,10 +141,6 @@ struct _TpClientFactoryPrivate
   TpDBusDaemon *dbus;
   /* Owned object-path -> weakref to TpProxy */
   GHashTable *proxy_cache;
-  /* TpProtocol aren't requested using their object path so we concat their CM
-   * and name to identifiy them.
-   * Owned "cm-name protocol-name" -> weakref to TpProtocol */
-  GHashTable *protocols_cache;
   GArray *desired_account_features;
   GArray *desired_connection_features;
   GArray *desired_channel_features;
@@ -342,7 +338,6 @@ tp_client_factory_finalize (GObject *object)
 
   g_clear_object (&self->priv->dbus);
   tp_clear_pointer (&self->priv->proxy_cache, g_hash_table_unref);
-  tp_clear_pointer (&self->priv->protocols_cache, g_hash_table_unref);
   tp_clear_pointer (&self->priv->desired_account_features, g_array_unref);
   tp_clear_pointer (&self->priv->desired_connection_features, g_array_unref);
   tp_clear_pointer (&self->priv->desired_channel_features, g_array_unref);
@@ -363,8 +358,6 @@ tp_client_factory_init (TpClientFactory *self)
       TpClientFactoryPrivate);
 
   self->priv->proxy_cache = g_hash_table_new (g_str_hash, g_str_equal);
-  self->priv->protocols_cache = g_hash_table_new_full (g_str_hash, g_str_equal,
-      g_free, NULL);
 
   self->priv->desired_account_features = g_array_new (TRUE, FALSE,
       sizeof (GQuark));
@@ -1331,27 +1324,6 @@ _tp_client_factory_ensure_channel_dispatch_operation (
   return dispatch;
 }
 
-static void
-protocol_destroyed_cb (gpointer data,
-    GObject *protocol)
-{
-  /* We can't compute the key as @protocol has been destroyed so look for it
-   * in the hash table. */
-  TpClientFactory *self = data;
-  GHashTableIter iter;
-  gpointer p;
-
-  g_hash_table_iter_init (&iter, self->priv->protocols_cache);
-  while (g_hash_table_iter_next (&iter, NULL, &p))
-    {
-      if (p == protocol)
-        {
-          g_hash_table_iter_remove (&iter);
-          return;
-        }
-    }
-}
-
 /**
  * tp_client_factory_ensure_protocol:
  * @self: a #TpClientFactory
@@ -1386,7 +1358,7 @@ tp_client_factory_ensure_protocol (TpClientFactory *self,
     GError **error)
 {
   TpProtocol *protocol;
-  gchar *key;
+  gchar *object_path;
 
   g_return_val_if_fail (TP_IS_CLIENT_FACTORY (self), NULL);
 
@@ -1395,27 +1367,27 @@ tp_client_factory_ensure_protocol (TpClientFactory *self,
 
   g_variant_ref_sink (immutable_properties);
 
-  key = g_strdup_printf ("%s %s", cm_name, protocol_name);
-
-  protocol = g_hash_table_lookup (self->priv->protocols_cache, key);
+  object_path = _tp_protocol_build_object_path (cm_name, protocol_name);
+  protocol = lookup_proxy (self, object_path);
   if (protocol != NULL)
     {
       g_object_ref (protocol);
-
-      g_free (key);
     }
   else
     {
-      protocol = TP_CLIENT_FACTORY_GET_CLASS (self)->create_protocol (
-          self, cm_name, protocol_name, immutable_properties, error);
+      protocol = TP_CLIENT_FACTORY_GET_CLASS (self)->create_protocol (self,
+          cm_name, protocol_name, immutable_properties, error);
 
-      g_object_weak_ref (G_OBJECT (protocol), protocol_destroyed_cb, self);
-
-      /* pass ownership of 'key' to the hash table */
-      g_hash_table_insert (self->priv->protocols_cache, key, protocol);
+      if (protocol != NULL)
+        {
+          g_assert (g_str_equal (tp_proxy_get_object_path (protocol),
+              object_path));
+          insert_proxy (self, protocol);
+        }
     }
 
   g_variant_unref (immutable_properties);
+  g_free (object_path);
 
   return protocol;
 }
diff --git a/telepathy-glib/protocol-internal.h b/telepathy-glib/protocol-internal.h
index d677ed9..f897af5 100644
--- a/telepathy-glib/protocol-internal.h
+++ b/telepathy-glib/protocol-internal.h
@@ -34,6 +34,9 @@ struct _TpConnectionManagerParam
   guint flags;
 };
 
+gchar *_tp_protocol_build_object_path (const gchar *cm_name,
+    const gchar *protocol_name);
+
 TpProtocol * _tp_protocol_new (TpDBusDaemon *dbus,
     TpClientFactory *factory,
     const gchar *cm_name,
diff --git a/telepathy-glib/protocol.c b/telepathy-glib/protocol.c
index 5714901..7ee9e71 100644
--- a/telepathy-glib/protocol.c
+++ b/telepathy-glib/protocol.c
@@ -47,6 +47,7 @@
 #define DEBUG_FLAG TP_DEBUG_PARAMS
 #include "telepathy-glib/capabilities-internal.h"
 #include "telepathy-glib/cli-misc.h"
+#include "telepathy-glib/client-factory-internal.h"
 #include "telepathy-glib/debug-internal.h"
 #include "telepathy-glib/proxy-internal.h"
 #include "telepathy-glib/util-internal.h"
@@ -887,6 +888,21 @@ tp_protocol_init (TpProtocol *self)
       TpProtocolPrivate);
 }
 
+gchar *
+_tp_protocol_build_object_path (const gchar *cm_name,
+    const gchar *protocol_name)
+{
+  gchar *object_path;
+
+  object_path = g_strdup_printf ("%s%s/%s", TP_CM_OBJECT_PATH_BASE, cm_name,
+      protocol_name);
+
+  /* e.g. local-xmpp -> local_xmpp */
+  g_strdelimit (object_path, "-", '_');
+
+  return object_path;
+}
+
 /*
  * _tp_protocol_new:
  * @dbus: proxy for the D-Bus daemon; may not be %NULL
@@ -938,10 +954,7 @@ _tp_protocol_new (TpDBusDaemon *dbus,
     goto finally;
 
   bus_name = g_strdup_printf ("%s%s", TP_CM_BUS_NAME_BASE, cm_name);
-  object_path = g_strdup_printf ("%s%s/%s", TP_CM_OBJECT_PATH_BASE, cm_name,
-      protocol_name);
-  /* e.g. local-xmpp -> local_xmpp */
-  g_strdelimit (object_path, "-", '_');
+  object_path = _tp_protocol_build_object_path (cm_name, protocol_name);
 
   ret = TP_PROTOCOL (g_object_new (TP_TYPE_PROTOCOL,
         "dbus-daemon", dbus,