[calls] manager: Delay UI Call removal and adjust to changes

[Evangelos Ribeiro Tzaras <devrtz src gnome org>]

commit 3fe976505cde413bf5bcaed04360c3ec83d6200c
Author: Evangelos Ribeiro Tzaras <devrtz fortysixandtwo eu>
Date:   Thu Feb 3 07:49:55 2022 +0100

    manager: Delay UI Call removal and adjust to changes
    
    This was handled explicitly in the Call window.
    By changing the logic to delay the emission of "ui-call-removed" we make sure
    that the Call UI and the exported DBus object is consistent.
    
    We also need to change the test cases to use run a GMainLoop because we now have
    to wait until signal comes in.

 src/calls-call-window.c |  65 ++++------------------------
 src/calls-manager.c     |  51 ++++++++++++++++++----
 tests/meson.build       |  11 ++---
 tests/test-manager.c    | 112 ++++++++++++++++++++++++++++++++++++------------
 4 files changed, 141 insertions(+), 98 deletions(-)
---
diff --git a/src/calls-call-window.c b/src/calls-call-window.c
index e88a99e0..6d21055d 100644
--- a/src/calls-call-window.c
+++ b/src/calls-call-window.c
@@ -39,7 +39,6 @@
 #include <glib-object.h>
 #include <handy.h>
 
-#define CALLS_WINDOW_HIDE_DELAY 3
 
 struct _CallsCallWindow
 {
@@ -56,8 +55,6 @@ struct _CallsCallWindow
   GtkFlowBox *call_selector;
 
   guint inhibit_cookie;
-  guint hideout_id;
-
 };
 
 G_DEFINE_TYPE (CallsCallWindow, calls_call_window, GTK_TYPE_APPLICATION_WINDOW);
@@ -86,41 +83,19 @@ session_inhibit (CallsCallWindow *self, gboolean inhibit)
 }
 
 
-static gboolean
-on_delayed_window_hide (gpointer user_data)
-{
-  CallsCallWindow *self = user_data;
-  g_assert (CALLS_IS_CALL_WINDOW (self));
-
-  gtk_widget_set_visible (GTK_WIDGET (self), FALSE);
-
-  gtk_stack_set_visible_child_name (self->main_stack, "calls");
-
-  self->hideout_id = 0;
-
-  return G_SOURCE_REMOVE;
-}
-
 static void
 update_visibility (CallsCallWindow *self)
 {
   guint calls = g_list_model_get_n_items (G_LIST_MODEL (self->calls));
 
-  if (calls == 0) {
-    self->hideout_id =
-      g_timeout_add_seconds (CALLS_WINDOW_HIDE_DELAY,
-                             G_SOURCE_FUNC (on_delayed_window_hide),
-                             self);
-  } else {
-    g_clear_handle_id (&self->hideout_id, g_source_remove);
-
-    gtk_widget_set_visible (GTK_WIDGET (self), TRUE);
-
-    if (calls == 1)
-      gtk_stack_set_visible_child_name (self->main_stack, "active-call");
-  }
+  gtk_widget_set_visible (GTK_WIDGET (self), calls > 0);
   gtk_widget_set_sensitive (GTK_WIDGET (self->show_calls), calls > 1);
 
+  if (calls == 0)
+    gtk_stack_set_visible_child_name (self->main_stack, "calls");
+  else if (calls == 1)
+    gtk_stack_set_visible_child_name (self->main_stack, "active-call");
+
   session_inhibit (self, !!calls);
 }
 
@@ -233,23 +208,6 @@ add_call (CallsCallWindow *self,
                     self);
 }
 
-struct DisplayData
-{
-  GtkStack *call_stack;
-  CuiCallDisplay *display;
-};
-
-static gboolean
-on_remove_delayed (gpointer user_data)
-{
-  struct DisplayData *display_data = user_data;
-
-  gtk_container_remove (GTK_CONTAINER (display_data->call_stack),
-                        GTK_WIDGET (display_data->display));
-
-  g_free (display_data);
-  return G_SOURCE_REMOVE;
-}
 
 static void
 remove_call (CallsCallWindow *self,
@@ -272,15 +230,9 @@ remove_call (CallsCallWindow *self,
       CALLS_UI_CALL_DATA (cui_call_display_get_call (display));
 
     if (display_call_data == ui_call_data) {
-      struct DisplayData *display_data = g_new0 (struct DisplayData, 1);
-
       g_list_store_remove (self->calls, i);
-      display_data->call_stack = self->call_stack;
-      display_data->display = display;
-      g_timeout_add_seconds (CALLS_WINDOW_HIDE_DELAY,
-                             G_SOURCE_FUNC (on_remove_delayed),
-                             display_data);
-
+      gtk_container_remove (GTK_CONTAINER (self->call_stack),
+                            GTK_WIDGET (display));
       break;
     }
   }
@@ -399,4 +351,3 @@ calls_call_window_new (GtkApplication *application)
                        NULL);
 }
 
-#undef CALLS_WINDOW_HIDE_DELAY
diff --git a/src/calls-manager.c b/src/calls-manager.c
index b194f2ef..a39175b4 100644
--- a/src/calls-manager.c
+++ b/src/calls-manager.c
@@ -246,27 +246,62 @@ add_call (CallsManager *self, CallsCall *call, CallsOrigin *origin)
 }
 
 
+struct CallsRemoveData
+{
+  CallsManager *manager;
+  CallsCall    *call;
+};
+
+
+static gboolean
+on_remove_delayed (struct CallsRemoveData *data)
+{
+  CallsUiCallData *call_data;
+
+  g_assert (CALLS_IS_MANAGER (data->manager));
+  g_assert (CALLS_IS_CALL (data->call));
+
+  call_data = g_hash_table_lookup (data->manager->calls, data->call);
+  if (!call_data) {
+    g_warning ("Could not find call %s in UI call hash table",
+               calls_call_get_id (data->call));
+  } else {
+    g_signal_emit (data->manager, signals[UI_CALL_REMOVED], 0, call_data);
+    g_hash_table_remove (data->manager->calls, data->call);
+  }
+
+  g_free (data);
+
+  return G_SOURCE_REMOVE;
+}
+
+
+#define DELAY_CALL_REMOVE_MS 3000
 static void
 remove_call (CallsManager *self, CallsCall *call, gchar *reason, CallsOrigin *origin)
 {
-  CallsUiCallData *call_data;
+  struct CallsRemoveData *data;
 
   g_return_if_fail (CALLS_IS_MANAGER (self));
   g_return_if_fail (CALLS_IS_ORIGIN (origin));
   g_return_if_fail (CALLS_IS_CALL (call));
 
-  call_data = g_hash_table_lookup (self->calls, call);
-  if (!call_data) {
-    g_warning ("Could not remove call %s from hash table", calls_call_get_id (call));
-  } else {
-    g_signal_emit (self, signals[UI_CALL_REMOVED], 0, call_data);
-    g_hash_table_remove (self->calls, call);
-  }
+  data = g_new (struct CallsRemoveData, 1);
+  data->manager = self;
+  data->call = call;
+
+  /** Having the delay centrally managed makes sure our UI
+   *  and the DBus side stays consistent
+   */
+  g_timeout_add (DELAY_CALL_REMOVE_MS,
+                 G_SOURCE_FUNC (on_remove_delayed),
+                 data);
 
   /* TODO get rid of SIGNAL_CALL_REMOVE signal */
   /* We ignore the reason for now, because it doesn't give any usefull information */
   g_signal_emit (self, signals[SIGNAL_CALL_REMOVE], 0, call, origin);
 }
+#undef DELAY_CALL_REMOVE_MS
 
 
 static void
diff --git a/tests/meson.build b/tests/meson.build
index dc45c328..d08c15ec 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -24,6 +24,11 @@ test_link_args = [
   '-fPIC',
 ]
 
+mock_link_args = [ test_link_args,
+                   '-Wl,--wrap=calls_contacts_provider_new',
+                 ]
+
+
 tests = [
   [ 'provider', []                       ],
   [ 'origin',   [ 'provider' ]           ],
@@ -63,7 +68,7 @@ test_sources = [ 'test-manager.c' ]
 
 t = executable('manager', test_sources,
                  c_args : test_cflags,
-                 link_args: test_link_args,
+                 link_args: mock_link_args,
                  pie: true,
                  link_with : [calls_vala, libcalls],
                  dependencies: calls_deps,
@@ -115,10 +120,6 @@ t = executable('util', test_sources,
                )
 test('util', t, env: test_env)
 
-mock_link_args = [ test_link_args,
-                   '-Wl,--wrap=calls_contacts_provider_new',
-                 ]
-
 test_sources = [ 'test-ui-call.c', 'mock-call.c', 'mock-call.h' ]
 t = executable('ui-call', test_sources,
                c_args : test_cflags,
diff --git a/tests/test-manager.c b/tests/test-manager.c
index b705fc44..0f122560 100644
--- a/tests/test-manager.c
+++ b/tests/test-manager.c
@@ -5,24 +5,76 @@
  */
 
 #include "calls-manager.h"
+#include "mock-contacts-provider.h"
 
 #include <cui-call.h>
 #include <gtk/gtk.h>
 #include <libpeas/peas.h>
 
-CuiCall *test_call = NULL;
+CallsContactsProvider *
+__wrap_calls_contacts_provider_new (CallsSettings *settings)
+{
+  return NULL;
+}
+
+struct TestData {
+  GMainLoop *loop;
+  CallsManager *manager;
+  GListModel *origins;
+  GListModel *origins_tel;
+  CallsOrigin *origin;
+  CuiCall *call;
+};
 
 static void
-call_add_cb (CallsManager *manager, CuiCall *call)
+call_add_cb (CallsManager    *manager,
+             CuiCall         *call,
+             struct TestData *data)
 {
-  test_call = call;
+  static guint phase = 0;
+
+  data->call = call;
+  switch (phase++) {
+  case 0:
+    cui_call_hang_up (call);
+    break;
+
+  case 1:
+    /* Unload the provider */
+    calls_manager_remove_provider (data->manager, "dummy");
+    g_assert_false (calls_manager_has_provider (data->manager, "dummy"));
+    g_assert_false (calls_manager_has_any_provider (data->manager));
+
+    break;
+
+  default:
+    g_assert_not_reached ();
+  }
 }
 
 static void
-call_remove_cb (CallsManager *manager, CuiCall *call)
+call_remove_cb (CallsManager    *manager,
+                CuiCall         *call,
+                struct TestData *data)
 {
-  g_assert_true (call == test_call);
-  test_call = NULL;
+  static guint phase = 0;
+
+  g_assert_true (call == data->call);
+  data->call = NULL;
+  switch (phase++) {
+  case 0:
+    /* Add new call do check if we remove it when we unload the provider */
+    calls_origin_dial (data->origin, "+393422342");
+    break;
+
+  case 1:
+    g_main_loop_quit (data->loop);
+    break;
+
+  default:
+    g_assert_not_reached ();
+  }
+
 }
 
 static void
@@ -55,16 +107,21 @@ test_calls_manager_without_provider (void)
 static void
 test_calls_manager_dummy_provider (void)
 {
-  g_autoptr (CallsManager) manager = calls_manager_new ();
+  CallsManager *manager;
   GListModel *origins;
   GListModel *origins_tel;
   guint position;
-  g_autoptr (CallsOrigin) origin = NULL;
+  struct TestData *test_data;
 
+  test_data = g_new0 (struct TestData, 1);
+  test_data->manager = calls_manager_new ();
+  manager = test_data->manager;
   g_assert_true (CALLS_IS_MANAGER (manager));
   g_assert_cmpuint (calls_manager_get_state_flags (manager), ==, CALLS_MANAGER_FLAGS_UNKNOWN);
 
-  origins = calls_manager_get_origins (manager);
+
+  test_data->origins = calls_manager_get_origins (manager);
+  origins = test_data->origins;
   g_assert_true (origins);
   g_assert_cmpuint (g_list_model_get_n_items (origins), ==, 0);
 
@@ -79,36 +136,35 @@ test_calls_manager_dummy_provider (void)
   g_assert_cmpuint (g_list_model_get_n_items (origins), >, 0);
   g_assert_null (calls_manager_get_calls (manager));
 
-  test_call = NULL;
-  g_signal_connect (manager, "ui-call-added", G_CALLBACK (call_add_cb), NULL);
-  g_signal_connect (manager, "ui-call-removed", G_CALLBACK (call_remove_cb), NULL);
+  test_data->origin = g_list_model_get_item (origins, 0);
+  g_assert_true (CALLS_IS_ORIGIN (test_data->origin));
 
-  origin = g_list_model_get_item (origins, 0);
-  g_assert_true (CALLS_IS_ORIGIN (origin));
-
-  origins_tel = calls_manager_get_suitable_origins (manager, "tel:+393422342");
+  test_data->origins_tel = calls_manager_get_suitable_origins (manager, "tel:+393422342");
+  origins_tel = test_data->origins_tel;
   g_assert_true (G_IS_LIST_MODEL (origins_tel));
   g_assert_true (G_IS_LIST_STORE (origins_tel));
-  g_assert_true (g_list_store_find (G_LIST_STORE (origins_tel), origin, &position));
+  g_assert_true (g_list_store_find (G_LIST_STORE (origins_tel), test_data->origin, &position));
 
-  calls_origin_dial (origin, "+393422342");
-  g_assert_true (CUI_IS_CALL (test_call));
-  cui_call_hang_up (test_call);
-  g_assert_null (test_call);
+  g_signal_connect (manager, "ui-call-added", G_CALLBACK (call_add_cb), test_data);
+  g_signal_connect (manager, "ui-call-removed", G_CALLBACK (call_remove_cb), test_data);
 
-  /* Add new call do check if we remove it when we unload the provider */
-  calls_origin_dial (origin, "+393422342");
+  calls_origin_dial (test_data->origin, "+393422343");
+  g_assert_true (CUI_IS_CALL (test_data->call));
 
-  /* Unload the provider */
-  calls_manager_remove_provider (manager, "dummy");
-  g_assert_false (calls_manager_has_provider (manager, "dummy"));
-  g_assert_false (calls_manager_has_any_provider (manager));
+  test_data->loop = g_main_loop_new (NULL, FALSE);
+  g_main_loop_run (test_data->loop);
+
+  g_main_loop_unref (test_data->loop);
 
-  g_assert_null (test_call);
   g_assert_cmpuint (g_list_model_get_n_items (origins), ==, 0);
   g_assert_cmpuint (g_list_model_get_n_items (origins_tel), ==, 0);
 
   g_assert_cmpuint (calls_manager_get_state_flags (manager), ==, CALLS_MANAGER_FLAGS_UNKNOWN);
+
+  g_assert_finalize_object (test_data->origin);
+  g_assert_finalize_object (test_data->manager);
+
+  g_free (test_data);
 }
 
 static void