[telepathy-glib/master] Don't ping Group channels with GetInterfaces()

Will Thompson will.thompson at collabora.co.uk
Mon Jul 27 01:32:10 PDT 2009


If we're about to introspect the Group interface anyway, there's no need
to call GetInterfaces() to check if the channel's alive: the Group
introspection will abort if the channel's dead or broken.

This saves an unnecessary roundtrip in the case of group channels for
which we have immutable properties; on the happy path, we can introspect
a group channel with just one call to GetAll("Chan.Iface.Group").
---
 telepathy-glib/channel.c        |   13 ++++-
 tests/dbus/channel-introspect.c |   77 ++++++++++++++++++++++++++++-
 tests/lib/textchan-null.c       |  103 ++++++++++++++++++++++++++++++++++++++-
 tests/lib/textchan-null.h       |   38 ++++++++++++++-
 4 files changed, 223 insertions(+), 8 deletions(-)

diff --git a/telepathy-glib/channel.c b/telepathy-glib/channel.c
index de565bb..540ec26 100644
--- a/telepathy-glib/channel.c
+++ b/telepathy-glib/channel.c
@@ -576,10 +576,17 @@ _tp_channel_get_interfaces (TpChannel *self)
 {
   DEBUG ("%p", self);
 
-  if (self->priv->exists &&
-      tp_asv_lookup (self->priv->channel_properties,
-          TP_IFACE_CHANNEL ".Interfaces") != NULL)
+  if (tp_asv_lookup (self->priv->channel_properties,
+          TP_IFACE_CHANNEL ".Interfaces") != NULL &&
+      (self->priv->exists ||
+       tp_proxy_has_interface_by_id (self,
+          TP_IFACE_QUARK_CHANNEL_INTERFACE_GROUP)))
     {
+      /* If we already know the channel's interfaces, and either have already
+       * successfully called a method on the channel (so know it's alive) or
+       * are going to call one on it when we introspect the Group properties,
+       * then we don't need to do anything here.
+       */
       _tp_channel_continue_introspection (self);
     }
   else
diff --git a/tests/dbus/channel-introspect.c b/tests/dbus/channel-introspect.c
index 4cbc7d0..b637ea2 100644
--- a/tests/dbus/channel-introspect.c
+++ b/tests/dbus/channel-introspect.c
@@ -93,7 +93,9 @@ main (int argc,
   TpHandleRepoIface *contact_repo;
   TestTextChannelNull *service_chan;
   TestTextChannelNull *service_props_chan;
+  TestTextChannelNull *service_props_group_chan;
   TestPropsTextChannel *service_props_chan_;
+  TestPropsTextChannel *service_props_group_chan_;
   TpDBusDaemon *dbus;
   TpConnection *conn;
   TpChannel *chan;
@@ -102,6 +104,7 @@ main (int argc,
   gchar *conn_path;
   gchar *chan_path;
   gchar *props_chan_path;
+  gchar *props_group_chan_path;
   gchar *bad_chan_path;
   TpHandle handle;
   gboolean was_ready;
@@ -159,6 +162,17 @@ main (int argc,
         NULL));
   service_props_chan_ = TEST_PROPS_TEXT_CHANNEL (service_props_chan);
 
+  props_group_chan_path = g_strdup_printf ("%s/PropsGroupChannel", conn_path);
+
+  service_props_group_chan = TEST_TEXT_CHANNEL_NULL (g_object_new (
+        TEST_TYPE_PROPS_GROUP_TEXT_CHANNEL,
+        "connection", service_conn,
+        "object-path", props_group_chan_path,
+        "handle", handle,
+        NULL));
+  service_props_group_chan_ =
+      TEST_PROPS_TEXT_CHANNEL (service_props_group_chan);
+
   mainloop = g_main_loop_new (NULL, FALSE);
 
   g_message ("Channel becomes invalid while we wait");
@@ -251,7 +265,8 @@ main (int argc,
   service_props_chan->get_interfaces_called = 0;
   service_props_chan->get_channel_type_called = 0;
 
-  service_props_chan_->dbus_property_retrieved = 0;
+  g_hash_table_remove_all (
+      service_props_chan_->dbus_property_interfaces_retrieved);
 
   asv = tp_asv_new (
       TP_IFACE_CHANNEL ".ChannelType", G_TYPE_STRING,
@@ -273,7 +288,8 @@ main (int argc,
 
   MYASSERT (tp_channel_run_until_ready (chan, &error, NULL), "");
   test_assert_no_error (error);
-  MYASSERT_SAME_UINT (service_props_chan_->dbus_property_retrieved, 0);
+  MYASSERT_SAME_UINT (g_hash_table_size (
+      service_props_chan_->dbus_property_interfaces_retrieved), 0);
   MYASSERT_SAME_UINT (service_props_chan->get_handle_called, 0);
   MYASSERT_SAME_UINT (service_props_chan->get_channel_type_called, 0);
   /* FIXME: with an improved fast-path we could avoid this one too maybe? */
@@ -284,6 +300,61 @@ main (int argc,
   g_object_unref (chan);
   chan = NULL;
 
+  g_message ("Group channel becomes ready while we wait (preloading immutable "
+      "properties)");
+
+  test_connection_run_until_dbus_queue_processed (conn);
+
+  service_props_group_chan->get_handle_called = 0;
+  service_props_group_chan->get_interfaces_called = 0;
+  service_props_group_chan->get_channel_type_called = 0;
+
+  g_hash_table_remove_all (
+      service_props_group_chan_->dbus_property_interfaces_retrieved);
+
+  {
+    const gchar *interfaces[] = {
+        TP_IFACE_CHANNEL_INTERFACE_GROUP,
+        NULL
+    };
+
+    asv = tp_asv_new (
+        TP_IFACE_CHANNEL ".ChannelType", G_TYPE_STRING,
+            TP_IFACE_CHANNEL_TYPE_TEXT,
+        TP_IFACE_CHANNEL ".TargetHandleType", G_TYPE_UINT,
+            TP_HANDLE_TYPE_CONTACT,
+        TP_IFACE_CHANNEL ".TargetHandle", G_TYPE_UINT, handle,
+        TP_IFACE_CHANNEL ".TargetID", G_TYPE_STRING, IDENTIFIER,
+        TP_IFACE_CHANNEL ".InitiatorHandle", G_TYPE_UINT, handle,
+        TP_IFACE_CHANNEL ".InitiatorID", G_TYPE_STRING, IDENTIFIER,
+        TP_IFACE_CHANNEL ".Interfaces", G_TYPE_STRV, interfaces,
+        TP_IFACE_CHANNEL ".Requested", G_TYPE_BOOLEAN, FALSE,
+        NULL);
+  }
+
+  chan = tp_channel_new_from_properties (conn, props_group_chan_path, asv, &error);
+  test_assert_no_error (error);
+
+  g_hash_table_destroy (asv);
+  asv = NULL;
+
+  MYASSERT (tp_channel_run_until_ready (chan, &error, NULL), "");
+  test_assert_no_error (error);
+  MYASSERT_SAME_UINT (service_props_group_chan->get_handle_called, 0);
+  MYASSERT_SAME_UINT (service_props_group_chan->get_channel_type_called, 0);
+  MYASSERT_SAME_UINT (service_props_group_chan->get_interfaces_called, 0);
+  MYASSERT_SAME_UINT (g_hash_table_size (
+      service_props_group_chan_->dbus_property_interfaces_retrieved), 1);
+  MYASSERT (g_hash_table_lookup (
+      service_props_group_chan_->dbus_property_interfaces_retrieved,
+      GUINT_TO_POINTER (TP_IFACE_QUARK_CHANNEL_INTERFACE_GROUP)) != NULL,
+      "Only Chan.I.Group's properties should have been retrieved");
+
+  assert_chan_sane (chan, handle);
+
+  g_object_unref (chan);
+  chan = NULL;
+
   g_message ("Channel becomes ready while we wait (in the case where we "
       "have to discover the channel type)");
 
@@ -522,6 +593,7 @@ main (int argc,
   g_object_unref (conn);
   g_object_unref (service_chan);
   g_object_unref (service_props_chan);
+  g_object_unref (service_props_group_chan);
 
   service_conn_as_base = NULL;
   g_object_unref (service_conn);
@@ -530,6 +602,7 @@ main (int argc,
   g_free (conn_path);
   g_free (chan_path);
   g_free (props_chan_path);
+  g_free (props_group_chan_path);
 
   return 0;
 }
diff --git a/tests/lib/textchan-null.c b/tests/lib/textchan-null.c
index 3713b38..507cd86 100644
--- a/tests/lib/textchan-null.c
+++ b/tests/lib/textchan-null.c
@@ -35,6 +35,12 @@ G_DEFINE_TYPE_WITH_CODE (TestPropsTextChannel,
     G_IMPLEMENT_INTERFACE (TP_TYPE_SVC_DBUS_PROPERTIES,
       tp_dbus_properties_mixin_iface_init))
 
+G_DEFINE_TYPE_WITH_CODE (TestPropsGroupTextChannel,
+    test_props_group_text_channel,
+    TEST_TYPE_PROPS_TEXT_CHANNEL,
+    G_IMPLEMENT_INTERFACE (TP_TYPE_SVC_CHANNEL_INTERFACE_GROUP,
+        tp_group_mixin_iface_init))
+
 static const char *test_text_channel_null_interfaces[] = { NULL };
 
 /* type definition stuff */
@@ -74,6 +80,7 @@ test_text_channel_null_init (TestTextChannelNull *self)
 static void
 test_props_text_channel_init (TestPropsTextChannel *self)
 {
+  self->dbus_property_interfaces_retrieved = g_hash_table_new (NULL, NULL);
 }
 
 static GObject *
@@ -305,15 +312,27 @@ test_props_text_channel_getter_gobject_properties (GObject *object,
 {
   TestPropsTextChannel *self = TEST_PROPS_TEXT_CHANNEL (object);
 
-  self->dbus_property_retrieved = 1;
+  g_hash_table_insert (self->dbus_property_interfaces_retrieved,
+      GUINT_TO_POINTER (interface), GUINT_TO_POINTER (interface));
 
   tp_dbus_properties_mixin_getter_gobject_properties (object, interface, name,
       value, getter_data);
 }
 
 static void
+props_finalize (GObject *object)
+{
+  TestPropsTextChannel *self = TEST_PROPS_TEXT_CHANNEL (object);
+
+  g_hash_table_unref (self->dbus_property_interfaces_retrieved);
+
+  ((GObjectClass *) test_text_channel_null_parent_class)->finalize (object);
+}
+
+static void
 test_props_text_channel_class_init (TestPropsTextChannelClass *klass)
 {
+  GObjectClass *object_class = (GObjectClass *) klass;
   static TpDBusPropertiesMixinPropImpl channel_props[] = {
       { "TargetHandleType", "handle-type", NULL },
       { "TargetHandle", "handle", NULL },
@@ -334,12 +353,92 @@ test_props_text_channel_class_init (TestPropsTextChannelClass *klass)
       { NULL }
   };
 
+  object_class->finalize = props_finalize;
+
   klass->dbus_properties_class.interfaces = prop_interfaces;
-  tp_dbus_properties_mixin_class_init ((GObjectClass *) klass,
+  tp_dbus_properties_mixin_class_init (object_class,
       G_STRUCT_OFFSET (TestPropsTextChannelClass, dbus_properties_class));
 }
 
 static void
+test_props_group_text_channel_init (TestPropsGroupTextChannel *self)
+{
+}
+
+static void
+group_constructed (GObject *self)
+{
+  TpBaseConnection *conn = TEST_TEXT_CHANNEL_NULL (self)->priv->conn;
+  void (*chain_up) (GObject *) =
+    ((GObjectClass *) test_props_group_text_channel_parent_class)->constructed;
+
+  if (chain_up != NULL)
+    chain_up (self);
+
+  tp_group_mixin_init (self,
+      G_STRUCT_OFFSET (TestPropsGroupTextChannel, group),
+      tp_base_connection_get_handles (conn, TP_HANDLE_TYPE_CONTACT),
+      tp_base_connection_get_self_handle (conn));
+  tp_group_mixin_change_flags (self, TP_CHANNEL_GROUP_FLAG_PROPERTIES, 0);
+}
+
+static void
+group_finalize (GObject *self)
+{
+  tp_group_mixin_finalize (self);
+}
+
+static gboolean
+dummy_add_remove_member (GObject *obj,
+    TpHandle handle,
+    const gchar *message,
+    GError **error)
+{
+  return TRUE;
+}
+
+static void
+group_iface_props_getter (GObject *object,
+    GQuark interface,
+    GQuark name,
+    GValue *value,
+    gpointer getter_data)
+{
+  TestPropsTextChannel *self = TEST_PROPS_TEXT_CHANNEL (object);
+
+  g_hash_table_insert (self->dbus_property_interfaces_retrieved,
+      GUINT_TO_POINTER (interface), GUINT_TO_POINTER (interface));
+
+  tp_group_mixin_get_dbus_property (object, interface, name, value, getter_data);
+}
+
+static void
+test_props_group_text_channel_class_init (TestPropsGroupTextChannelClass *klass)
+{
+  GObjectClass *object_class = (GObjectClass *) klass;
+  static TpDBusPropertiesMixinPropImpl group_props[] = {
+      { "GroupFlags", NULL, NULL },
+      { "HandleOwners", NULL, NULL },
+      { "LocalPendingMembers", NULL, NULL },
+      { "Members", NULL, NULL },
+      { "RemotePendingMembers", NULL, NULL },
+      { "SelfHandle", NULL, NULL },
+      { NULL }
+  };
+
+  object_class->constructed = group_constructed;
+  object_class->finalize = group_finalize;
+
+  tp_group_mixin_class_init (object_class,
+      G_STRUCT_OFFSET (TestPropsGroupTextChannelClass, group_class),
+      dummy_add_remove_member,
+      dummy_add_remove_member);
+  tp_dbus_properties_mixin_implement_interface (object_class,
+      TP_IFACE_QUARK_CHANNEL_INTERFACE_GROUP, group_iface_props_getter, NULL,
+      group_props);
+}
+
+static void
 channel_close (TpSvcChannel *iface,
                DBusGMethodInvocation *context)
 {
diff --git a/tests/lib/textchan-null.h b/tests/lib/textchan-null.h
index 2c51b0c..58bf908 100644
--- a/tests/lib/textchan-null.h
+++ b/tests/lib/textchan-null.h
@@ -15,6 +15,7 @@
 #include <glib-object.h>
 #include <telepathy-glib/base-connection.h>
 #include <telepathy-glib/text-mixin.h>
+#include <telepathy-glib/group-mixin.h>
 
 G_BEGIN_DECLS
 
@@ -65,7 +66,7 @@ typedef struct _TestPropsTextChannelClass TestPropsTextChannelClass;
 struct _TestPropsTextChannel {
     TestTextChannelNull parent;
 
-    guint dbus_property_retrieved;
+    GHashTable *dbus_property_interfaces_retrieved;
 };
 
 struct _TestPropsTextChannelClass {
@@ -92,6 +93,41 @@ GType test_props_text_channel_get_type (void);
   (G_TYPE_INSTANCE_GET_CLASS ((obj), TEST_TYPE_PROPS_TEXT_CHANNEL, \
                               TestPropsTextChannelClass))
 
+/* Subclass with D-Bus properties and Group */
+
+typedef struct _TestPropsGroupTextChannel TestPropsGroupTextChannel;
+typedef struct _TestPropsGroupTextChannelClass TestPropsGroupTextChannelClass;
+
+struct _TestPropsGroupTextChannel {
+    TestPropsTextChannel parent;
+
+    TpGroupMixin group;
+};
+
+struct _TestPropsGroupTextChannelClass {
+    TestPropsTextChannelClass parent;
+
+    TpGroupMixinClass group_class;
+};
+
+GType test_props_group_text_channel_get_type (void);
+
+#define TEST_TYPE_PROPS_GROUP_TEXT_CHANNEL \
+  (test_props_group_text_channel_get_type ())
+#define TEST_PROPS_GROUP_TEXT_CHANNEL(obj) \
+  (G_TYPE_CHECK_INSTANCE_CAST ((obj), TEST_TYPE_PROPS_GROUP_TEXT_CHANNEL, \
+                               TestPropsGroupTextChannel))
+#define TEST_PROPS_GROUP_TEXT_CHANNEL_CLASS(klass) \
+  (G_TYPE_CHECK_CLASS_CAST ((klass), TEST_TYPE_PROPS_GROUP_TEXT_CHANNEL, \
+                            TestPropsGroupTextChannelClass))
+#define TEST_IS_PROPS_GROUP_TEXT_CHANNEL(obj) \
+  (G_TYPE_CHECK_INSTANCE_TYPE ((obj), TEST_TYPE_PROPS_GROUP_TEXT_CHANNEL))
+#define TEST_IS_PROPS_GROUP_TEXT_CHANNEL_CLASS(klass) \
+  (G_TYPE_CHECK_CLASS_TYPE ((klass), TEST_TYPE_PROPS_GROUP_TEXT_CHANNEL))
+#define TEST_PROPS_GROUP_TEXT_CHANNEL_GET_CLASS(obj) \
+  (G_TYPE_INSTANCE_GET_CLASS ((obj), TEST_TYPE_PROPS_GROUP_TEXT_CHANNEL, \
+                              TestPropsGroupTextChannelClass))
+
 G_END_DECLS
 
 #endif /* #ifndef __TEST_TEXT_CHANNEL_NULL_H__ */
-- 
1.5.6.5




More information about the telepathy-commits mailing list