Attachment 98905 Details for Bug 77882 – base-client: use gdbus-codegen's skeleton to implement Observer

[patch] base-client: use gdbus-codegen's skeleton to implement Observer

0003-base-client-use-gdbus-codegen-s-skeleton-to-implemen.patch (text/plain), 14.09 KB, created by Guillaume Desmottes on 2014-05-12 12:36:25 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Guillaume Desmottes

Created: 2014-05-12 12:36:25 UTC

Size: 14.09 KB

patch

obsolete

>From c59f4b8b39e9b52cc6fdd76ccbdd2d1eb65fe4cd Mon Sep 17 00:00:00 2001
>From: Guillaume Desmottes <guillaume.desmottes@collabora.co.uk>
>Date: Fri, 9 May 2014 16:23:55 +0200
>Subject: [PATCH 03/11] base-client: use gdbus-codegen's skeleton to implement
> Observer
>
>---
> telepathy-glib/Makefile.am   |   2 +
> telepathy-glib/base-client.c | 213 +++++++++++++++++++++++++------------------
> 2 files changed, 126 insertions(+), 89 deletions(-)
>
>diff --git a/telepathy-glib/Makefile.am b/telepathy-glib/Makefile.am
>index 7c93ac2..017ff48 100644
>--- a/telepathy-glib/Makefile.am
>+++ b/telepathy-glib/Makefile.am
>@@ -357,6 +357,8 @@ gdbus_codegen = \
>     _gdbus/Connection_Interface_Requests.h \
>     _gdbus/Client.c \
>     _gdbus/Client.h \
>+    _gdbus/Client_Observer.c \
>+    _gdbus/Client_Observer.h \
>     $(NULL)
> 
> _gdbus/%.c: _gdbus/%-stamp
>diff --git a/telepathy-glib/base-client.c b/telepathy-glib/base-client.c
>index 6439418..5c2a27a 100644
>--- a/telepathy-glib/base-client.c
>+++ b/telepathy-glib/base-client.c
>@@ -194,6 +194,7 @@
> #include <telepathy-glib/value-array.h>
> 
> #include <telepathy-glib/_gdbus/Client.h>
>+#include <telepathy-glib/_gdbus/Client_Observer.h>
> 
> #define DEBUG_FLAG TP_DEBUG_CLIENT
> #include "telepathy-glib/connection-internal.h"
>@@ -202,14 +203,13 @@
> #include "telepathy-glib/util-internal.h"
> #include "telepathy-glib/variant-util.h"
> 
>-static void observer_iface_init (gpointer, gpointer);
>+static void observer_skeleton_init (TpBaseClient *self);
> static void approver_iface_init (gpointer, gpointer);
> static void handler_iface_init (gpointer, gpointer);
> static void requests_iface_init (gpointer, gpointer);
> 
> G_DEFINE_ABSTRACT_TYPE_WITH_CODE(TpBaseClient, tp_base_client,
>     G_TYPE_DBUS_OBJECT_SKELETON,
>-    G_IMPLEMENT_INTERFACE(TP_TYPE_SVC_CLIENT_OBSERVER, observer_iface_init);
>     G_IMPLEMENT_INTERFACE(TP_TYPE_SVC_CLIENT_APPROVER, approver_iface_init);
>     G_IMPLEMENT_INTERFACE(TP_TYPE_SVC_CLIENT_HANDLER, handler_iface_init);
>     G_IMPLEMENT_INTERFACE(TP_TYPE_SVC_CLIENT_INTERFACE_REQUESTS,
>@@ -250,8 +250,9 @@ struct _TpBaseClientPrivate
> 
>   gboolean registered;
>   ClientFlags flags;
>-  /* array of TP_HASH_TYPE_CHANNEL_CLASS */
>-  GPtrArray *observer_filters;
>+  /* builders are 'protected' by the registered boolean ensuring they won't be
>+   * modified after g_variant_builder_end() has been called. */
>+  GVariantBuilder *observer_filters;
>   /* array of TP_HASH_TYPE_CHANNEL_CLASS */
>   GPtrArray *approver_filters;
>   /* array of TP_HASH_TYPE_CHANNEL_CLASS */
>@@ -274,6 +275,7 @@ struct _TpBaseClientPrivate
>   GDestroyNotify delegated_channels_destroy;
> 
>   _TpGDBusClient *client_skeleton;
>+  _TpGDBusClientObserver *observer_skeleton;
> };
> 
> /*
>@@ -423,10 +425,8 @@ tp_base_client_add_observer_filter_variant (TpBaseClient *self,
>   g_return_if_fail (cls->observe_channel != NULL);
>   g_return_if_fail (g_variant_is_of_type (filter, G_VARIANT_TYPE_VARDICT));
> 
>-  g_variant_ref_sink (filter);
>   self->priv->flags |= CLIENT_IS_OBSERVER;
>-  g_ptr_array_add (self->priv->observer_filters, tp_asv_from_vardict (filter));
>-  g_variant_unref (filter);
>+  g_variant_builder_add_value (self->priv->observer_filters, filter);
> }
> 
> /**
>@@ -917,7 +917,11 @@ update_interfaces (TpBaseClient *self)
>   GPtrArray *arr = g_ptr_array_new ();
> 
>   if (self->priv->flags & CLIENT_IS_OBSERVER)
>-    g_ptr_array_add (arr, TP_IFACE_CLIENT_OBSERVER);
>+    {
>+      g_ptr_array_add (arr, TP_IFACE_CLIENT_OBSERVER);
>+      observer_skeleton_init (self);
>+     }
>+
>   if (self->priv->flags & CLIENT_IS_APPROVER)
>     g_ptr_array_add (arr, TP_IFACE_CLIENT_APPROVER);
>   if (self->priv->flags & CLIENT_IS_HANDLER)
>@@ -1073,8 +1077,8 @@ tp_base_client_init (TpBaseClient *self)
>       TpBaseClientPrivate);
> 
>   /* wild guess: most clients won't need more than one of each filter */
>-  self->priv->observer_filters = g_ptr_array_new_with_free_func (
>-      (GDestroyNotify) g_hash_table_unref);
>+  self->priv->observer_filters = g_variant_builder_new (
>+      G_VARIANT_TYPE ("aa{sv}"));
>   self->priv->approver_filters = g_ptr_array_new_with_free_func (
>       (GDestroyNotify) g_hash_table_unref);
>   self->priv->handler_filters = g_ptr_array_new_with_free_func (
>@@ -1119,6 +1123,7 @@ tp_base_client_dispose (GObject *object)
>     }
> 
>   g_clear_object (&self->priv->client_skeleton);
>+  g_clear_object (&self->priv->observer_skeleton);
> 
>   if (dispose != NULL)
>     dispose (object);
>@@ -1133,7 +1138,7 @@ tp_base_client_finalize (GObject *object)
> 
>   g_free (self->priv->name);
> 
>-  g_ptr_array_unref (self->priv->observer_filters);
>+  g_variant_builder_unref (self->priv->observer_filters);
>   g_ptr_array_unref (self->priv->approver_filters);
>   g_ptr_array_unref (self->priv->handler_filters);
>   g_ptr_array_unref (self->priv->handler_caps);
>@@ -1244,7 +1249,6 @@ tp_base_client_constructed (GObject *object)
>   g_dbus_object_skeleton_add_interface (skel,
>       G_DBUS_INTERFACE_SKELETON (self->priv->client_skeleton));
> 
>-  object_skeleton_take_svc_interface (skel, TP_TYPE_SVC_CLIENT_OBSERVER);
>   object_skeleton_take_svc_interface (skel, TP_TYPE_SVC_CLIENT_APPROVER);
>   object_skeleton_take_svc_interface (skel, TP_TYPE_SVC_CLIENT_HANDLER);
>   object_skeleton_take_svc_interface (skel,
>@@ -1291,9 +1295,6 @@ typedef enum {
>     DP_BYPASS_APPROVAL,
>     DP_CAPABILITIES,
>     DP_HANDLED_CHANNELS,
>-    DP_OBSERVER_CHANNEL_FILTER,
>-    DP_OBSERVER_RECOVER,
>-    DP_OBSERVER_DELAY_APPROVERS,
> } ClientDBusProp;
> 
> static void
>@@ -1308,10 +1309,6 @@ tp_base_client_get_dbus_properties (GObject *object,
> 
>   switch (which)
>     {
>-    case DP_OBSERVER_CHANNEL_FILTER:
>-      g_value_set_boxed (value, self->priv->observer_filters);
>-      break;
>-
>     case DP_APPROVER_CHANNEL_FILTER:
>       g_value_set_boxed (value, self->priv->approver_filters);
>       break;
>@@ -1345,16 +1342,6 @@ tp_base_client_get_dbus_properties (GObject *object,
>         }
>       break;
> 
>-    case DP_OBSERVER_RECOVER:
>-      g_value_set_boolean (value,
>-          (self->priv->flags & CLIENT_OBSERVER_RECOVER) != 0);
>-      break;
>-
>-    case DP_OBSERVER_DELAY_APPROVERS:
>-      g_value_set_boolean (value,
>-          (self->priv->flags & CLIENT_OBSERVER_DELAYS_APPROVERS) != 0);
>-      break;
>-
>     default:
>       g_assert_not_reached ();
>     }
>@@ -1380,15 +1367,6 @@ tp_base_client_class_init (TpBaseClientClass *cls)
>           GINT_TO_POINTER (DP_APPROVER_CHANNEL_FILTER) },
>         { NULL }
>   };
>-  static TpDBusPropertiesMixinPropImpl observer_properties[] = {
>-        { "ObserverChannelFilter",
>-          GINT_TO_POINTER (DP_OBSERVER_CHANNEL_FILTER) },
>-        { "Recover",
>-          GINT_TO_POINTER (DP_OBSERVER_RECOVER) },
>-        { "DelayApprovers",
>-          GINT_TO_POINTER (DP_OBSERVER_DELAY_APPROVERS) },
>-        { NULL }
>-  };
>   GObjectClass *object_class = G_OBJECT_CLASS (cls);
> 
>   g_type_class_add_private (cls, sizeof (TpBaseClientPrivate));
>@@ -1511,9 +1489,6 @@ tp_base_client_class_init (TpBaseClientClass *cls)
> 
>   tp_dbus_properties_mixin_class_init (object_class, 0);
>   tp_dbus_properties_mixin_implement_interface (object_class,
>-        TP_IFACE_QUARK_CLIENT_OBSERVER, tp_base_client_get_dbus_properties,
>-        NULL, observer_properties);
>-  tp_dbus_properties_mixin_implement_interface (object_class,
>         TP_IFACE_QUARK_CLIENT_APPROVER, tp_base_client_get_dbus_properties,
>         NULL, approver_properties);
>   tp_dbus_properties_mixin_implement_interface (object_class,
>@@ -1664,18 +1639,56 @@ error:
>   return FALSE;
> }
> 
>-static void
>-_tp_base_client_observe_channel (TpSvcClientObserver *iface,
>+static gboolean
>+create_channel_request_array (TpBaseClient *self,
>+    GVariant *requests_hash,
>+    GPtrArray **requests,
>+    GError **error)
>+{
>+  GVariantIter iter;
>+  const gchar *key;
>+  GVariant *value;
>+
>+  *requests = g_ptr_array_new_full (g_variant_n_children (requests_hash),
>+      g_object_unref);
>+
>+  g_variant_iter_init (&iter, requests_hash);
>+  while (g_variant_iter_next (&iter, "{o@a{sv}}", &key, &value))
>+    {
>+      const gchar *req_path = key;
>+      GHashTable *props = tp_asv_from_vardict (value);
>+      TpChannelRequest *request;
>+
>+      request = _tp_client_factory_ensure_channel_request (
>+          self->priv->factory, req_path, props, error);
>+      g_hash_table_unref (props);
>+      g_variant_unref (value);
>+
>+      if (request == NULL)
>+        {
>+          DEBUG ("Failed to create TpChannelRequest: %s", (*error)->message);
>+          g_ptr_array_unref (*requests);
>+          return FALSE;
>+        }
>+
>+      g_ptr_array_add (*requests, request);
>+    }
>+
>+  return TRUE;
>+}
>+
>+static gboolean
>+_tp_base_client_observe_channel (_TpGDBusClientObserverSkeleton *skeleton,
>+    GDBusMethodInvocation *context,
>     const gchar *account_path,
>     const gchar *connection_path,
>     const gchar *channel_path,
>-    GHashTable *channel_props,
>+    GVariant *channel_props,
>     const gchar *dispatch_operation_path,
>-    GHashTable *requests_hash,
>-    GHashTable *observer_info,
>-    GDBusMethodInvocation *context)
>+    GVariant *requests_hash,
>+    GVariant *observer_info,
>+    TpBaseClient *self)
> {
>-  TpBaseClient *self = TP_BASE_CLIENT (iface);
>   TpObserveChannelContext *ctx;
>   TpBaseClientClass *cls = TP_BASE_CLIENT_GET_CLASS (self);
>   GError *error = NULL;
>@@ -1687,16 +1700,7 @@ _tp_base_client_observe_channel (TpSvcClientObserver *iface,
>   GArray *account_features;
>   GArray *connection_features;
>   GArray *channel_features;
>-  GHashTableIter iter;
>-  gpointer k, v;
>-
>-  if (!(self->priv->flags & CLIENT_IS_OBSERVER))
>-    {
>-      /* Pretend that the method is not implemented if we are not supposed to
>-       * be an Observer. */
>-      tp_dbus_g_method_return_not_implemented (context);
>-      return;
>-    }
>+  GHashTable *asv = NULL;
> 
>   if (cls->observe_channel == NULL)
>     {
>@@ -1704,11 +1708,12 @@ _tp_base_client_observe_channel (TpSvcClientObserver *iface,
>           G_OBJECT_TYPE_NAME (self));
> 
>       tp_dbus_g_method_return_not_implemented (context);
>-      return;
>+      return TRUE;
>     }
> 
>+  asv = tp_asv_from_vardict (channel_props);
>   if (!ensure_account_connection_channel (self, account_path,
>-      connection_path, channel_path, channel_props, &account, &connection,
>+      connection_path, channel_path, asv, &account, &connection,
>       &channel, &error))
>     goto out;
> 
>@@ -1729,30 +1734,11 @@ _tp_base_client_observe_channel (TpSvcClientObserver *iface,
>         }
>     }
> 
>-  requests = g_ptr_array_new_full (g_hash_table_size (requests_hash),
>-      g_object_unref);
>-
>-  g_hash_table_iter_init (&iter, requests_hash);
>-  while (g_hash_table_iter_next (&iter, &k, &v))
>-    {
>-      const gchar *req_path = k;
>-      GHashTable *props = v;
>-      TpChannelRequest *request;
>-
>-      request = _tp_client_factory_ensure_channel_request (
>-          self->priv->factory, req_path, props, &error);
>-      if (request == NULL)
>-        {
>-          DEBUG ("Failed to create TpChannelRequest: %s", error->message);
>-          goto out;
>-        }
>-
>-      g_ptr_array_add (requests, request);
>-    }
>+  if (!create_channel_request_array (self, requests_hash, &requests, &error))
>+    goto out;
> 
>   ctx = _tp_observe_channel_context_new (account, connection, channel,
>-      dispatch_operation, requests, tp_asv_to_vardict (observer_info),
>-      context);
>+      dispatch_operation, requests, observer_info, context);
> 
>   account_features = dup_features_for_account (self, account);
>   connection_features = dup_features_for_connection (self, connection);
>@@ -1773,6 +1759,7 @@ out:
>   g_clear_object (&account);
>   g_clear_object (&connection);
>   g_clear_object (&channel);
>+  g_clear_pointer (&asv, g_hash_table_unref);
> 
>   if (dispatch_operation != NULL)
>     g_object_unref (dispatch_operation);
>@@ -1781,19 +1768,42 @@ out:
>     g_ptr_array_unref (requests);
> 
>   if (error == NULL)
>-    return;
>+    return TRUE;
> 
>   g_dbus_method_invocation_take_error (context, error);
>+  return TRUE;
> }
> 
> static void
>-observer_iface_init (gpointer g_iface,
>-    gpointer unused G_GNUC_UNUSED)
>+observer_skeleton_init (TpBaseClient *self)
> {
>-#define IMPLEMENT(x) tp_svc_client_observer_implement_##x (\
>-  g_iface, _tp_base_client_##x)
>-  IMPLEMENT (observe_channel);
>-#undef IMPLEMENT
>+  GDBusObjectSkeleton *skel = G_DBUS_OBJECT_SKELETON (self);
>+  gboolean delay_approvers, recover;
>+
>+  self->priv->observer_skeleton = _tp_gdbus_client_observer_skeleton_new ();
>+
>+  g_dbus_object_skeleton_add_interface (skel,
>+      G_DBUS_INTERFACE_SKELETON (self->priv->observer_skeleton));
>+
>+  /* Properties */
>+  delay_approvers = ((self->priv->flags & CLIENT_OBSERVER_DELAYS_APPROVERS)
>+      != 0);
>+  _tp_gdbus_client_observer_set_delay_approvers (self->priv->observer_skeleton,
>+      delay_approvers);
>+
>+  recover = ((self->priv->flags & CLIENT_OBSERVER_RECOVER)
>+      != 0);
>+  _tp_gdbus_client_observer_set_recover (self->priv->observer_skeleton,
>+      recover);
>+
>+  _tp_gdbus_client_observer_set_observer_channel_filter (
>+      self->priv->observer_skeleton,
>+      g_variant_builder_end (self->priv->observer_filters));
>+
>+  /* Method */
>+  g_signal_connect_object (self->priv->observer_skeleton,
>+      "handle-observe-channel", G_CALLBACK (_tp_base_client_observe_channel),
>+      self, 0);
> }
> 
> static void
>@@ -2589,6 +2599,20 @@ tp_base_client_implement_handle_channel (TpBaseClientClass *cls,
>   cls->handle_channel = impl;
> }
> 
>+static void
>+reinitialize_channel_filter_builder (GVariantBuilder *builder,
>+    GVariant *arr)
>+{
>+  GVariantIter iter;
>+  GVariant *child;
>+
>+  g_variant_builder_init (builder, G_VARIANT_TYPE ("aa{sv}"));
>+
>+  g_variant_iter_init (&iter, arr);
>+  while ((child = g_variant_iter_next_value (&iter)))
>+    g_variant_builder_add_value (builder, child);
>+}
>+
> /**
>  * tp_base_client_unregister:
>  * @self: a client, which may already have been registered with
>@@ -2640,6 +2664,17 @@ tp_base_client_unregister (TpBaseClient *self)
>         g_hash_table_remove (clients, self->priv->object_path);
>     }
> 
>+  /* Re-initialize the filter builders as they may be modified again */
>+  if (self->priv->observer_skeleton != NULL)
>+    {
>+      GVariant *arr;
>+
>+      arr = _tp_gdbus_client_observer_get_observer_channel_filter (
>+          self->priv->observer_skeleton);
>+
>+      reinitialize_channel_filter_builder (self->priv->observer_filters, arr);
>+    }
>+
>   self->priv->registered = FALSE;
> }
> 
>-- 
>1.9.0
>

Actions: View | Splinter Review

Attachments on bug 77882: 98903 | 98904 | 98905 | 98906 | 98907 | 98908 | 98909 | 98910 | 98911 | 98912 | 98913 | 98919 | 99023

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.