Wait for the test-application startup using an atspi listener

This should be more robust than just waiting and sleeping.

* test-application now claims a different DBus name for each run, to disambiguate.

* atk_bridge_adaptor_cleanup() - release the dbus name if there was one

7 files changed, 191 insertions, 60 deletions

diff --git a/atk-adaptor/bridge.c b/atk-adaptor/bridge.c
index a2b1de83..462ec0a3 100644
--- a/atk-adaptor/bridge.c
+++ b/atk-adaptor/bridge.c
@@ -686,7 +686,7 @@ new_connection_cb (DBusServer *server, DBusConnection *con, void *data)
   spi_global_app_data->direct_connections = g_list_append (spi_global_app_data->direct_connections, con);
 }
 
-gchar *atspi_dbus_name = NULL;
+static gchar *atspi_dbus_name = NULL;
 static gboolean atspi_no_register = FALSE;
 
 static GOptionEntry atspi_option_entries[] = {
@@ -1222,6 +1222,26 @@ atk_bridge_adaptor_cleanup (void)
     {
       dbus_connection_remove_filter (spi_global_app_data->bus, signal_filter, NULL);
       droute_context_unregister (spi_global_app_data->droute, spi_global_app_data->bus);
+
+      if (atspi_dbus_name != NULL)
+        {
+          DBusError error;
+          int result;
+
+          dbus_error_init (&error);
+          result = dbus_bus_release_name (spi_global_app_data->bus, atspi_dbus_name, &error);
+          if (result == -1)
+            {
+              g_warning ("atk-bridge: could not release dbus name: %s", error.message);
+            }
+          else
+            {
+              g_print ("bridge: released name, result %d\n", result);
+            }
+
+          dbus_error_free (&error);
+        }
+
       dbus_connection_close (spi_global_app_data->bus);
       dbus_connection_unref (spi_global_app_data->bus);
       spi_global_app_data->bus = NULL;
diff --git a/atk/atkutil.c b/atk/atkutil.c
index db957c7a..cf5eb3b5 100644
--- a/atk/atkutil.c
+++ b/atk/atkutil.c
@@ -488,6 +488,7 @@ atk_get_root (void)
 {
   AtkUtilClass *klass = g_type_class_ref (ATK_TYPE_UTIL);
   AtkObject *retval;
+
   if (klass->get_root)
     {
       retval = klass->get_root ();
diff --git a/atspi/atspi-misc.c b/atspi/atspi-misc.c
index 9e68049d..01bd1634 100644
--- a/atspi/atspi-misc.c
+++ b/atspi/atspi-misc.c
@@ -246,6 +246,8 @@ get_application (const char *bus_name)
   DBusMessage *message;
   DBusPendingCall *pending = NULL;
 
+  g_print ("get_application %s\n", bus_name);
+
   if (!app_hash)
     {
       app_hash = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, (GDestroyNotify) g_object_unref);
diff --git a/tests/at-spi2-atk/atk_suite.c b/tests/at-spi2-atk/atk_suite.c
index 02f85917..7132c148 100644
--- a/tests/at-spi2-atk/atk_suite.c
+++ b/tests/at-spi2-atk/atk_suite.c
@@ -53,9 +53,25 @@ atk_suite_build (void)
 int
 main (int argc, char **argv)
 {
+  int init_result;
+
   g_test_init (&argc, &argv, NULL);
 
+  init_result = atspi_init ();
+  if (init_result != 0)
+    {
+      g_error ("Could not initialize atspi, code %d", init_result);
+    }
+
+  fixture_listener_init ();
+
   atk_suite_build ();
 
-  return g_test_run ();
+  int result = g_test_run ();
+  g_assert_cmpint (result, ==, 0);
+
+  int leaked = atspi_exit ();
+  g_assert_cmpint (leaked, ==, 0);
+
+  return 0;
 }
diff --git a/tests/at-spi2-atk/atk_test_util.c b/tests/at-spi2-atk/atk_test_util.c
index 57f7098f..2af00135 100644
--- a/tests/at-spi2-atk/atk_test_util.c
+++ b/tests/at-spi2-atk/atk_test_util.c
@@ -23,8 +23,11 @@
 #include "atk_test_util.h"
 #include <signal.h>
 
+static AtspiEventListener *fixture_listener = NULL;
+static TestAppFixture *current_fixture = NULL;
+
 static pid_t
-run_app (const char *file_name)
+run_app (const char *file_name, const char *name_to_claim)
 {
   pid_t child_pid = fork ();
   if (child_pid == 0)
@@ -34,12 +37,10 @@ run_app (const char *file_name)
               "--test-data-file",
               file_name,
               "--atspi-dbus-name",
-              "org.a11y.Atspi2Atk.TestApplication",
+              name_to_claim,
               NULL);
       _exit (EXIT_SUCCESS);
     }
-  if (child_pid)
-    fprintf (stderr, "child_pid %d\n", child_pid);
 
   return child_pid;
 }
@@ -47,113 +48,185 @@ run_app (const char *file_name)
 static AtspiAccessible *
 try_get_root_obj (AtspiAccessible *obj)
 {
+  GError *error = NULL;
   gchar *name;
   int i;
 
-  gint child_count = atspi_accessible_get_child_count (obj, NULL);
-  if (child_count < 1)
+  gint child_count = atspi_accessible_get_child_count (obj, &error);
+  if (child_count < 0)
     {
+      if (error)
+        {
+          g_print ("  get_child_count: %s\n", error->message);
+          g_error_free (error);
+        }
+      else
+        {
+          g_print ("  get_child_count=%d with no error\n", child_count);
+        }
+      return NULL;
+    }
+  else if (child_count < 1)
+    {
+      g_print ("  child_count == %d, bailing out\n", child_count);
       return NULL;
     }
 
   for (i = 0; i < child_count; i++)
     {
-      AtspiAccessible *child = atspi_accessible_get_child_at_index (obj, i, NULL);
+      AtspiAccessible *child = atspi_accessible_get_child_at_index (obj, i, &error);
       if (!child)
-        continue;
-      if ((name = atspi_accessible_get_name (child, NULL)) != NULL)
+        {
+          if (error)
+            {
+              g_print ("  getting child_at_index: %s\n", error->message);
+              g_error_free (error);
+            }
+          else
+            {
+              g_print ("  getting child_at_index returned NULL child with no error\n");
+            }
+          continue;
+        }
+      if ((name = atspi_accessible_get_name (child, &error)) != NULL)
         {
           if (!strcmp (name, "root_object"))
             {
               g_free (name);
               return child;
             }
+          g_print ("  name=%s\n", name);
           g_free (name);
         }
+      else
+        {
+          if (error)
+            {
+              g_print ("try_get_root_obj getting child name: %s\n", error->message);
+              g_error_free (error);
+            }
+          else
+            {
+              g_print ("  get_name returned NULL name with no error\n");
+            }
+        }
       g_object_unref (child);
     }
 
   return NULL;
 }
 
+/* Callback from AtspiEventListener.  We monitor children-changed on the root, so we can know
+ * when the helper test-application has launched and registered.
+ */
 static void
-get_root_obj (const char *file_name, AtspiAccessible **out_root_obj, pid_t *out_child_pid)
+listener_event_cb (AtspiEvent *event, void *user_data)
 {
-  int tries = 0;
-  AtspiAccessible *child;
-  struct timespec timeout = { .tv_sec = 0, .tv_nsec = 10 * 1000000 };
-  AtspiAccessible *obj = NULL;
-  pid_t child_pid;
+  TestAppFixture *fixture = current_fixture;
 
-  fprintf (stderr, "run_app: %s\n", file_name);
-  child_pid = run_app (file_name);
-  *out_child_pid = child_pid;
+  if (atspi_accessible_get_role (event->source, NULL) == ATSPI_ROLE_DESKTOP_FRAME && strstr (event->type, "add"))
+    {
+      AtspiAccessible *obj = atspi_get_desktop (0);
 
-  obj = atspi_get_desktop (0);
+      fixture->root_obj = try_get_root_obj (obj);
 
-  /* Wait for application to start, up to 100 times 10ms.  */
-  while (++tries <= 100)
-    {
-      child = try_get_root_obj (obj);
-      if (child)
+      if (fixture->root_obj)
         {
-          *out_root_obj = child;
-          return;
+          fixture->state = FIXTURE_STATE_CHILD_ACQUIRED;
+          atspi_event_quit ();
         }
-
-      nanosleep (&timeout, NULL);
     }
+}
+
+/* Sets up the atspi event listener for the test-application helpers.
+ *
+ * We get notified when the test-application registers its root object by listening
+ * to the children-changed signal.
+ */
+void
+fixture_listener_init (void)
+{
+  GError *error = NULL;
 
-  if (atspi_accessible_get_child_count (obj, NULL) < 1)
+  fixture_listener = atspi_event_listener_new (listener_event_cb, NULL, NULL);
+  if (!atspi_event_listener_register (fixture_listener, "object:children-changed", &error))
     {
-      g_test_message ("Fail, test application not found\n");
+      g_error ("Could not register event listener for children-changed: %s\n", error->message);
     }
-  else
+}
+
+void
+fixture_listener_destroy (void)
+{
+  GError *error = NULL;
+
+  if (!atspi_event_listener_deregister (fixture_listener, "object:children-changed", &error))
     {
-      g_test_message ("test object not found\n");
+      g_error ("Could not deregister event listener: %s", error->message);
     }
-  g_test_fail ();
-  kill (child_pid, SIGTERM);
-  *out_root_obj = NULL;
+
+  g_object_unref (fixture_listener);
+  fixture_listener = NULL;
+}
+
+static gboolean
+wait_for_test_app_timeout_cb (gpointer user_data)
+{
+  TestAppFixture *fixture = user_data;
+
+  fixture->test_app_timed_out = TRUE;
+  atspi_event_quit ();
+
+  return FALSE;
 }
 
+/* Each of the helper programs with the test fixtures claims a different DBus name,
+ * to make them non-ambiguous when they get restarted all the time.  This is the serial
+ * number that gets appended to each name.
+ */
+static guint fixture_serial = 0;
+
 void
 fixture_setup (TestAppFixture *fixture, gconstpointer user_data)
 {
   const char *file_name = user_data;
-  pid_t child_pid;
-  AtspiAccessible *root_obj;
 
-  get_root_obj (file_name, &root_obj, &child_pid);
-  g_assert (root_obj != NULL);
+  fixture->state = FIXTURE_STATE_WAITING_FOR_CHILD;
+  fixture->name_to_claim = g_strdup_printf ("org.a11y.Atspi2Atk.TestApplication_%u", fixture_serial);
+  fixture_serial += 1;
+
+  fixture->child_pid = run_app (file_name, fixture->name_to_claim);
+
+  fixture->test_app_timed_out = FALSE;
+  fixture->wait_for_test_app_timeout = g_timeout_add (500, wait_for_test_app_timeout_cb, fixture); /* 500 msec */
+
+  current_fixture = fixture;
+  atspi_event_main ();
 
-  fixture->child_pid = child_pid;
-  fixture->root_obj = root_obj;
+  g_source_remove (fixture->wait_for_test_app_timeout);
+  fixture->wait_for_test_app_timeout = 0;
+
+  if (fixture->test_app_timed_out)
+    {
+      g_print ("test app timed out before registering its root object");
+      g_test_fail ();
+    }
 }
 
 void
 fixture_teardown (TestAppFixture *fixture, gconstpointer user_data)
 {
-  int tries = 0;
-
-  AtspiAccessible *child;
-  struct timespec timeout = { .tv_sec = 0, .tv_nsec = 10 * 1000000 };
-  AtspiAccessible *obj = NULL;
+  current_fixture = NULL;
 
   kill (fixture->child_pid, SIGTERM);
+  fixture->child_pid = -1;
 
-  obj = atspi_get_desktop (0);
-
-  /* Wait for application to stop, up to 100 times 10ms.  */
-  while (++tries <= 100)
+  if (fixture->root_obj)
     {
-      child = try_get_root_obj (obj);
-      if (child == NULL)
-        return;
-
-      nanosleep (&timeout, NULL);
+      g_object_unref (fixture->root_obj);
+      fixture->root_obj = NULL;
     }
 
-  g_test_message ("Fail, test application still running\n");
-  g_test_fail ();
+  g_free (fixture->name_to_claim);
+  fixture->name_to_claim = NULL;
 }
diff --git a/tests/at-spi2-atk/atk_test_util.h b/tests/at-spi2-atk/atk_test_util.h
index 6f4f2ab6..c664f777 100644
--- a/tests/at-spi2-atk/atk_test_util.h
+++ b/tests/at-spi2-atk/atk_test_util.h
@@ -33,8 +33,20 @@
 #include <sys/wait.h>
 #include <unistd.h>
 
+typedef enum
+{
+  FIXTURE_STATE_WAITING_FOR_CHILD,
+  FIXTURE_STATE_CHILD_ACQUIRED,
+} FixtureState;
+
 typedef struct
 {
+  FixtureState state;
+
+  char *name_to_claim;
+
+  guint wait_for_test_app_timeout;
+  gboolean test_app_timed_out;
   pid_t child_pid;
 
   AtspiAccessible *root_obj;
@@ -42,8 +54,9 @@ typedef struct
 
 extern pid_t child_pid;
 
+void fixture_listener_init (void);
+void fixture_listener_destroy (void);
 void fixture_setup (TestAppFixture *fixture, gconstpointer user_data);
 void fixture_teardown (TestAppFixture *fixture, gconstpointer user_data);
-void clean_exit_on_fail ();
 
 #endif /* _ATK_TEST_UTIL_H */
diff --git a/tests/at-spi2-atk/test-application.c b/tests/at-spi2-atk/test-application.c
index 56ce4a18..b8a8da01 100644
--- a/tests/at-spi2-atk/test-application.c
+++ b/tests/at-spi2-atk/test-application.c
@@ -100,6 +100,7 @@ static gboolean
 sigterm_received_cb (gpointer user_data)
 {
   GMainLoop *mainloop = user_data;
+  g_print ("test application received SIGTERM\n");
   g_main_loop_quit (mainloop);
   return G_SOURCE_REMOVE;
 }
@@ -125,5 +126,10 @@ main (int argc, char *argv[])
   g_unix_signal_add (SIGTERM, sigterm_received_cb, mainloop);
   g_main_loop_run (mainloop);
 
+  g_print ("test application exited main loop; terminating after cleanup\n");
+
+  atk_bridge_adaptor_cleanup ();
+
+  g_print ("test application %d exiting!\n", getpid ());
   return 0;
 }