Do domain-specific errors via GDBus, not dbus-glib

We can get rid of TP_DBUS_ERROR_UNKNOWN_REMOTE_ERROR, because
G_IO_ERROR_DBUS_ERROR doesn't have the same weird abuse of GError
semantics that DBUS_GERROR_REMOTE_EXCEPTION did.

I moved TP_DBUS_ERROR_INCONSISTENT from last to 0'th TP_DBUS_ERROR,
to avoid having to renumber all of them.

We can probably get rid of a lot of these errors in favour of
G_IO_ERROR or G_DBUS_ERROR members, but for now, let's keep them.

10 files changed, 67 insertions, 231 deletions

diff --git a/docs/reference/telepathy-glib/telepathy-glib-sections.txt b/docs/reference/telepathy-glib/telepathy-glib-sections.txt
index cd52b7b4c..4e483ba71 100644
--- a/docs/reference/telepathy-glib/telepathy-glib-sections.txt
+++ b/docs/reference/telepathy-glib/telepathy-glib-sections.txt
@@ -3007,7 +3007,6 @@ tp_proxy_get_interface_by_id
 tp_proxy_check_interface_by_id
 tp_proxy_invalidate
 TpProxyInterfaceAddedCb
-tp_proxy_subclass_add_error_mapping
 <SUBSECTION>
 TpProxyInvokeFunc
 tp_proxy_pending_call_v0_new
diff --git a/telepathy-glib/channel-group.c b/telepathy-glib/channel-group.c
index ddb9ee89d..462976324 100644
--- a/telepathy-glib/channel-group.c
+++ b/telepathy-glib/channel-group.c
@@ -590,17 +590,21 @@ members_changed_prepared_cb (GObject *object,
 
           if (error_detail != NULL)
             {
+              DEBUG ("detailed error: %s", error_detail);
+
               /* CM specified a D-Bus error name */
               tp_proxy_dbus_error_to_gerror (self, error_detail,
-                  debug_message == NULL || debug_message[0] == '\0'
-                      ? error_detail
-                      : debug_message,
-                  &self->priv->group_remove_error);
+                  debug_message, &self->priv->group_remove_error);
+
+              DEBUG ("-> %s #%d: %s",
+                  g_quark_to_string (self->priv->group_remove_error->domain),
+                  self->priv->group_remove_error->code,
+                  self->priv->group_remove_error->message);
 
               /* ... but if we don't know anything about that D-Bus error
                * name, we can still do better by using RemovedFromGroup */
               if (g_error_matches (self->priv->group_remove_error,
-                    TP_DBUS_ERRORS, TP_DBUS_ERROR_UNKNOWN_REMOTE_ERROR))
+                    G_IO_ERROR, G_IO_ERROR_DBUS_ERROR))
                 {
                   self->priv->group_remove_error->domain =
                     TP_ERRORS_REMOVED_FROM_GROUP;
@@ -611,6 +615,8 @@ members_changed_prepared_cb (GObject *object,
             }
           else
             {
+              DEBUG ("no detailed error");
+
               /* Use our separate error domain */
               g_set_error_literal (&self->priv->group_remove_error,
                   TP_ERRORS_REMOVED_FROM_GROUP, reason, debug_message);
diff --git a/telepathy-glib/connection.c b/telepathy-glib/connection.c
index 8666f1d7d..b81ca7dfc 100644
--- a/telepathy-glib/connection.c
+++ b/telepathy-glib/connection.c
@@ -1005,8 +1005,7 @@ tp_connection_status_changed_cb (TpConnection *self,
           /* ... but if we don't know anything about that D-Bus error
            * name, we can still be more helpful by deriving an error code from
            * TpConnectionStatusReason */
-          if (g_error_matches (error, TP_DBUS_ERRORS,
-                TP_DBUS_ERROR_UNKNOWN_REMOTE_ERROR))
+          if (g_error_matches (error, G_IO_ERROR, G_IO_ERROR_DBUS_ERROR))
             {
               GError *from_csr = NULL;
 
@@ -2551,10 +2550,9 @@ tp_connection_get_detailed_error (TpConnection *self,
               break;
 
             case TP_DBUS_ERROR_OBJECT_REMOVED:
-            case TP_DBUS_ERROR_UNKNOWN_REMOTE_ERROR:
             case TP_DBUS_ERROR_INCONSISTENT:
             /* ... and all other cases up to and including
-             * TP_DBUS_ERROR_INCONSISTENT don't make sense in this context, so
+             * TP_DBUS_ERROR_CANCELLED don't make sense in this context, so
              * just use the generic one for them too */
             default:
               return TP_ERROR_STR_DISCONNECTED;
diff --git a/telepathy-glib/errors.c b/telepathy-glib/errors.c
index 978e61d87..ae3039660 100644
--- a/telepathy-glib/errors.c
+++ b/telepathy-glib/errors.c
@@ -44,8 +44,8 @@
  * enumeration.
  *
  * This macro expands to a call to a function returning the Telepathy error
- * domain. Since 0.7.17, this function automatically registers the domain with
- * dbus-glib for server-side use (using dbus_g_error_domain_register()) when
+ * domain. Since 0.UNRELEASED, this function automatically registers the
+ * domain with GIO (using g_dbus_error_register_error()) when
  * called.
  *
  * This used to be called %TP_ERRORS.
@@ -318,11 +318,31 @@ tp_error_quark (void)
 
   if (g_once_init_enter (&quark))
     {
-      GQuark domain = g_quark_from_static_string ("tp-error-quark");
+      gsize domain = 0;
+      GEnumClass *cls;
+      guint i;
+      GDBusErrorEntry *entries;
+
+      cls = g_type_class_ref (TP_TYPE_ERROR);
+      entries = g_new0 (GDBusErrorEntry, cls->n_values);
+
+      for (i = 0; i < cls->n_values; i++)
+        {
+          entries[i].error_code = cls->values[i].value;
+          entries[i].dbus_error_name = g_strdup_printf ("%s.%s",
+              TP_ERROR_PREFIX, cls->values[i].value_nick);
+        }
+
+      g_dbus_error_register_error_domain ("tp-error-quark", &domain,
+          entries, cls->n_values);
+
+      for (i = 0; i < cls->n_values; i++)
+        g_free ((gchar *) entries[i].dbus_error_name);
+
+      g_free (entries);
 
-      dbus_g_error_domain_register (domain, TP_ERROR_PREFIX,
-          TP_TYPE_ERROR);
       g_once_init_leave (&quark, domain);
+      g_type_class_unref (cls);
     }
 
   return (GQuark) quark;
diff --git a/telepathy-glib/proxy-internal.h b/telepathy-glib/proxy-internal.h
index cae4d8ed9..ba65ddef1 100644
--- a/telepathy-glib/proxy-internal.h
+++ b/telepathy-glib/proxy-internal.h
@@ -114,9 +114,6 @@ void _tp_proxy_signal_connection_take_results (TpProxySignalConnection *sc,
  */
 void tp_private_proxy_set_implementation (TpProxyImplementation *impl);
 
-GError *_tp_proxy_take_and_remap_error (TpProxy *self, GError *error)
-  G_GNUC_WARN_UNUSED_RESULT;
-
 typedef void (*TpProxyProc) (TpProxy *);
 
 gboolean _tp_proxy_is_preparing (gpointer self,
diff --git a/telepathy-glib/proxy.c b/telepathy-glib/proxy.c
index f696d13e6..b4aa420f4 100644
--- a/telepathy-glib/proxy.c
+++ b/telepathy-glib/proxy.c
@@ -67,8 +67,9 @@ tp_dbus_errors_quark (void)
 
 /**
  * TpDBusError:
- * @TP_DBUS_ERROR_UNKNOWN_REMOTE_ERROR: Raised if the error raised by
- *  a remote D-Bus object is not recognised
+ * @TP_DBUS_ERROR_INCONSISTENT: Raised if information received from a remote
+ *  object is inconsistent or otherwise obviously wrong.
+ *  See also %TP_ERROR_CONFUSED.
  * @TP_DBUS_ERROR_PROXY_UNREFERENCED: Emitted in #TpProxy::invalidated
  *  when the #TpProxy has lost its last reference
  * @TP_DBUS_ERROR_NO_INTERFACE: Raised by #TpProxy methods if the remote
@@ -91,9 +92,6 @@ tp_dbus_errors_quark (void)
  *  is available.
  * @TP_DBUS_ERROR_CANCELLED: Raised from calls that re-enter the main
  *  loop (*_run_*) if they are cancelled
- * @TP_DBUS_ERROR_INCONSISTENT: Raised if information received from a remote
- *  object is inconsistent or otherwise obviously wrong (added in 0.7.17).
- *  See also %TP_ERROR_CONFUSED.
  *
  * #GError codes for use with the %TP_DBUS_ERRORS domain.
  *
@@ -229,15 +227,6 @@ tp_dbus_errors_quark (void)
  * Since: 0.11.3
  */
 
-typedef struct _TpProxyErrorMappingLink TpProxyErrorMappingLink;
-
-struct _TpProxyErrorMappingLink {
-    const gchar *prefix;
-    GQuark domain;
-    GEnumClass *code_enum_class;
-    TpProxyErrorMappingLink *next;
-};
-
 struct _TpProxyFeaturePrivate
 {
   gpointer unused;
@@ -716,19 +705,6 @@ tp_proxy_add_interfaces (TpProxy *self,
     }
 }
 
-static GQuark
-error_mapping_quark (void)
-{
-  static GQuark q = 0;
-
-  if (G_UNLIKELY (q == 0))
-    {
-      q = g_quark_from_static_string ("TpProxyErrorMappingCb_0.7.1");
-    }
-
-  return q;
-}
-
 /**
  * tp_proxy_dbus_error_to_gerror:
  * @self: a #TpProxy or subclass
@@ -750,77 +726,27 @@ tp_proxy_dbus_error_to_gerror (gpointer self,
                                const char *debug_message,
                                GError **error)
 {
-  GType proxy_type = TP_TYPE_PROXY;
-  GType type;
-
   g_return_if_fail (TP_IS_PROXY (self));
+  g_return_if_fail (dbus_error != NULL);
 
-  if (error == NULL)
-    return;
-
-  g_return_if_fail (*error == NULL);
-
-  if (!tp_dbus_check_valid_interface_name (dbus_error, error))
+  if (tp_str_empty (debug_message))
     {
-      return;
+      /* best we can do */
+      debug_message = dbus_error;
     }
 
-  if (debug_message == NULL)
-    debug_message = "";
-
-  for (type = G_TYPE_FROM_INSTANCE (self);
-       type != proxy_type;
-       type = g_type_parent (type))
+  if (error != NULL)
     {
-      TpProxyErrorMappingLink *iter;
+      /* make sure the error domain is registered */
+      TP_ERROR;
 
-      for (iter = g_type_get_qdata (type, error_mapping_quark ());
-           iter != NULL;
-           iter = iter->next)
-        {
-          size_t prefix_len = strlen (iter->prefix);
+      *error = g_dbus_error_new_for_dbus_error (dbus_error, debug_message);
 
-          if (!strncmp (dbus_error, iter->prefix, prefix_len)
-              && dbus_error[prefix_len] == '.')
-            {
-              GEnumValue *code =
-                g_enum_get_value_by_nick (iter->code_enum_class,
-                    dbus_error + prefix_len + 1);
-
-              if (code != NULL)
-                {
-                  g_set_error (error, iter->domain, code->value,
-                      "%s", debug_message);
-                  return;
-                }
-            }
-        }
-    }
-
-  /* we don't have an error mapping - so let's just paste the
-   * error name and message into TP_DBUS_ERROR_UNKNOWN_REMOTE_ERROR */
-  g_set_error (error, TP_DBUS_ERRORS,
-      TP_DBUS_ERROR_UNKNOWN_REMOTE_ERROR, "%s: %s", dbus_error, debug_message);
-}
-
-GError *
-_tp_proxy_take_and_remap_error (TpProxy *self,
-                                GError *error)
-{
-  if (error == NULL ||
-      error->domain != DBUS_GERROR ||
-      error->code != DBUS_GERROR_REMOTE_EXCEPTION)
-    {
-      return error;
-    }
-  else
-    {
-      GError *replacement = NULL;
-      const gchar *dbus = dbus_g_error_get_name (error);
-
-      tp_proxy_dbus_error_to_gerror (self, dbus, error->message, &replacement);
-      g_error_free (error);
-      return replacement;
+      /* Our old behaviour was that we only put the detailed D-Bus error
+       * in the message if we raised TP_DBUS_ERROR_UNKNOWN_REMOTE_ERROR.
+       * Be consistent with that. */
+      if (!g_error_matches (*error, G_IO_ERROR, G_IO_ERROR_DBUS_ERROR))
+        g_dbus_error_strip_remote_error (*error);
     }
 }
 
@@ -1150,67 +1076,6 @@ tp_proxy_finalize (GObject *object)
   G_OBJECT_CLASS (tp_proxy_parent_class)->finalize (object);
 }
 
-/**
- * tp_proxy_subclass_add_error_mapping:
- * @proxy_subclass: The #GType of a subclass of #TpProxy (which must not be
- *  #TpProxy itself)
- * @static_prefix: A prefix for D-Bus error names, not including the trailing
- *  dot (which must remain valid forever, and should usually be in static
- *  storage)
- * @domain: A quark representing the corresponding #GError domain
- * @code_enum_type: The type of a subclass of #GEnumClass
- *
- * Register a mapping from D-Bus errors received from the given proxy
- * subclass to #GError instances.
- *
- * When a D-Bus error is received, the #TpProxy code checks for error
- * mappings registered for the class of the proxy receiving the error,
- * then for all of its parent classes.
- *
- * If there is an error mapping for which the D-Bus error name
- * starts with the mapping's @static_prefix, the proxy will check the
- * corresponding @code_enum_type for a value whose @value_nick is
- * the rest of the D-Bus error name (with the leading dot removed). If there
- * isn't such a value, it will continue to try other error mappings.
- *
- * If a suitable error mapping and code are found, the #GError that is raised
- * will have its error domain set to the @domain from the error mapping,
- * and its error code taken from the enum represented by the @code_enum_type.
- *
- * If no suitable error mapping or code is found, the #GError will have
- * error domain %TP_DBUS_ERRORS and error code
- * %TP_DBUS_ERROR_UNKNOWN_REMOTE_ERROR.
- *
- * Since: 0.7.1
- */
-void
-tp_proxy_subclass_add_error_mapping (GType proxy_subclass,
-                                     const gchar *static_prefix,
-                                     GQuark domain,
-                                     GType code_enum_type)
-{
-  GQuark q = error_mapping_quark ();
-  TpProxyErrorMappingLink *old_link = g_type_get_qdata (proxy_subclass, q);
-  TpProxyErrorMappingLink *new_link;
-  GType tp_type_proxy = TP_TYPE_PROXY;
-
-  g_return_if_fail (proxy_subclass != tp_type_proxy);
-  g_return_if_fail (g_type_is_a (proxy_subclass, tp_type_proxy));
-  g_return_if_fail (static_prefix != NULL);
-  g_return_if_fail (domain != 0);
-  g_return_if_fail (code_enum_type != G_TYPE_INVALID);
-
-  new_link = g_slice_new0 (TpProxyErrorMappingLink);
-  new_link->prefix = static_prefix;
-  new_link->domain = domain;
-  /* We never unref the enum type - intentional one-per-process leak.
-   * See "tp_proxy_subclass_add_error_mapping refs the enum" in our valgrind
-   * suppressions file */
-  new_link->code_enum_class = g_type_class_ref (code_enum_type);
-  new_link->next = old_link;    /* may be NULL */
-  g_type_set_qdata (proxy_subclass, q, new_link);
-}
-
 static void tp_proxy_once (void);
 
 static void
@@ -1219,6 +1084,9 @@ tp_proxy_class_init (TpProxyClass *klass)
   GParamSpec *param_spec;
   GObjectClass *object_class = G_OBJECT_CLASS (klass);
 
+  /* Ensure that remote errors will be mapped to the TP_ERROR domain */
+  TP_ERROR;
+
   tp_proxy_once ();
 
   g_type_class_add_private (klass, sizeof (TpProxyPrivate));
diff --git a/telepathy-glib/proxy.h b/telepathy-glib/proxy.h
index 3b4c68f16..b0727a6cc 100644
--- a/telepathy-glib/proxy.h
+++ b/telepathy-glib/proxy.h
@@ -47,7 +47,7 @@ typedef struct _TpProxy TpProxy;
 GQuark tp_dbus_errors_quark (void);
 
 typedef enum {
-    TP_DBUS_ERROR_UNKNOWN_REMOTE_ERROR = 0,
+    TP_DBUS_ERROR_INCONSISTENT = 0,
     TP_DBUS_ERROR_PROXY_UNREFERENCED = 1,
     TP_DBUS_ERROR_NO_INTERFACE = 2,
     TP_DBUS_ERROR_NAME_OWNER_LOST = 3,
@@ -57,9 +57,8 @@ typedef enum {
     TP_DBUS_ERROR_INVALID_MEMBER_NAME = 7,
     TP_DBUS_ERROR_OBJECT_REMOVED = 8,
     TP_DBUS_ERROR_CANCELLED = 9,
-    TP_DBUS_ERROR_INCONSISTENT = 10,
 } TpDBusError;
-#define TP_NUM_DBUS_ERRORS (TP_DBUS_ERROR_INCONSISTENT + 1)
+#define TP_NUM_DBUS_ERRORS (TP_DBUS_ERROR_CANCELLED + 1)
 
 struct _TpProxy {
     /*<private>*/
diff --git a/tests/dbus/connection-error.c b/tests/dbus/connection-error.c
index b9c5cc046..16fd4ce9c 100644
--- a/tests/dbus/connection-error.c
+++ b/tests/dbus/connection-error.c
@@ -53,30 +53,6 @@ typedef enum
   DOMAIN_SPECIFIC_ERROR = 0,
 } ExampleError;
 
-/* example_com_error_get_type relies on this */
-G_STATIC_ASSERT (sizeof (GType) <= sizeof (gsize));
-
-static GType
-example_com_error_get_type (void)
-{
-  static gsize type = 0;
-
-  if (g_once_init_enter (&type))
-    {
-      static const GEnumValue values[] = {
-            { DOMAIN_SPECIFIC_ERROR, "DOMAIN_SPECIFIC_ERROR",
-              "DomainSpecificError" },
-            { 0 }
-      };
-      GType gtype;
-
-      gtype = g_enum_register_static ("ExampleError", values);
-      g_once_init_leave (&type, gtype);
-    }
-
-  return (GType) type;
-}
-
 static GQuark
 example_com_error_quark (void)
 {
@@ -88,8 +64,10 @@ example_com_error_quark (void)
 
       g_assert (sizeof (GQuark) <= sizeof (gsize));
 
-      dbus_g_error_domain_register (domain, "com.example",
-          example_com_error_get_type ());
+      if (!g_dbus_error_register_error (domain, DOMAIN_SPECIFIC_ERROR,
+            "com.example.DomainSpecificError"))
+        g_assert_not_reached ();
+
       g_once_init_leave (&quark, domain);
     }
 
@@ -104,26 +82,10 @@ typedef struct {
 } Test;
 
 static void
-global_setup (void)
-{
-  static gboolean done = FALSE;
-
-  if (done)
-    return;
-
-  done = TRUE;
-
-  tp_debug_set_flags ("all");
-
-  tp_proxy_subclass_add_error_mapping (TP_TYPE_CONNECTION,
-      "com.example", example_com_error_quark (), example_com_error_get_type ());
-}
-
-static void
 setup (Test *test,
     gconstpointer nil G_GNUC_UNUSED)
 {
-  global_setup ();
+  tp_debug_set_flags ("all");
 
   test->mainloop = g_main_loop_new (NULL, FALSE);
 
@@ -169,6 +131,9 @@ test_registered_error (Test *test,
   tp_cli_connection_connect_to_status_changed (test->conn, on_status_changed,
       test->mainloop, NULL, NULL, NULL);
 
+  /* evaluate the error quark for its side-effect of registering the domain */
+  g_assert_cmpuint (example_com_error_quark (), !=, 0);
+
   tp_base_connection_disconnect_with_dbus_error (
       test->service_conn_as_base, "com.example.DomainSpecificError", NULL,
       TP_CONNECTION_STATUS_REASON_NETWORK_ERROR);
diff --git a/tests/suppressions/tpl.supp b/tests/suppressions/tpl.supp
index 284da40a6..573d33962 100644
--- a/tests/suppressions/tpl.supp
+++ b/tests/suppressions/tpl.supp
@@ -258,14 +258,6 @@
 }
 
 {
-   tp_proxy_subclass_add_error_mapping refs the enum
-   Memcheck:Leak
-   ...
-   fun:g_type_class_ref
-   fun:tp_proxy_subclass_add_error_mapping
-}
-
-{
    tp_dbus_daemon_constructor filter not freed til we fall off the bus
    Memcheck:Leak
    ...
diff --git a/tools/telepathy-glib.supp b/tools/telepathy-glib.supp
index 81d1c513a..bd6708ec4 100644
--- a/tools/telepathy-glib.supp
+++ b/tools/telepathy-glib.supp
@@ -309,14 +309,6 @@
 }
 
 {
-   tp_proxy_subclass_add_error_mapping refs the enum
-   Memcheck:Leak
-   ...
-   fun:g_type_class_ref
-   fun:tp_proxy_subclass_add_error_mapping
-}
-
-{
    tp_dbus_daemon_constructor filter not freed til we fall off the bus
    Memcheck:Leak
    ...