From 3fe976505cde413bf5bcaed04360c3ec83d6200c Mon Sep 17 00:00:00 2001 From: Evangelos Ribeiro Tzaras Date: Thu, 3 Feb 2022 07:49:55 +0100 Subject: [PATCH] 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 e88a99e..6d21055 100644 --- a/src/calls-call-window.c +++ b/src/calls-call-window.c @@ -39,7 +39,6 @@ #include #include -#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 b194f2e..a39175b 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 dc45c32..d08c15e 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 b705fc4..0f12256 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 #include #include -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