From d01f063c39a96c87b8afee6d942e0e0b410e5aa9 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Thu, 15 Oct 2020 15:09:11 +0100 Subject: [PATCH 1/5] libmalcontent-ui: Add g_autolist() to simplify memory management This introduces no functional changes. Signed-off-by: Philip Withnall --- libmalcontent-ui/restrict-applications-selector.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/libmalcontent-ui/restrict-applications-selector.c b/libmalcontent-ui/restrict-applications-selector.c index b27418f..ef55d9f 100644 --- a/libmalcontent-ui/restrict-applications-selector.c +++ b/libmalcontent-ui/restrict-applications-selector.c @@ -389,7 +389,8 @@ app_compare_id_length_cb (gconstpointer a, static void reload_apps (MctRestrictApplicationsSelector *self) { - GList *iter, *apps; + g_autolist(GAppInfo) apps = NULL; + GList *iter; g_autoptr(GHashTable) seen_flatpak_ids = NULL; g_autoptr(GHashTable) seen_executables = NULL; @@ -485,8 +486,6 @@ reload_apps (MctRestrictApplicationsSelector *self) compare_app_info_cb, self); } - - g_list_free_full (apps, g_object_unref); } static void From 1d5da7c094ae4d9ed4e827be7ddae1ef4e3fa967 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Thu, 15 Oct 2020 16:39:34 +0100 Subject: [PATCH 2/5] libmalcontent-ui: Adjust the app list sort to be more stable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This doesn’t achieve anything by itself, but makes some upcoming commits to diff the new/old app lists work properly. Signed-off-by: Philip Withnall Helps: #18, #28 --- libmalcontent-ui/restrict-applications-selector.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/libmalcontent-ui/restrict-applications-selector.c b/libmalcontent-ui/restrict-applications-selector.c index ef55d9f..4be7c07 100644 --- a/libmalcontent-ui/restrict-applications-selector.c +++ b/libmalcontent-ui/restrict-applications-selector.c @@ -372,6 +372,7 @@ app_compare_id_length_cb (gconstpointer a, { GAppInfo *info_a = (GAppInfo *) a, *info_b = (GAppInfo *) b; const gchar *id_a, *id_b; + gsize id_a_len, id_b_len; id_a = g_app_info_get_id (info_a); id_b = g_app_info_get_id (info_b); @@ -383,7 +384,12 @@ app_compare_id_length_cb (gconstpointer a, else if (id_b == NULL) return 1; - return strlen (id_a) - strlen (id_b); + id_a_len = strlen (id_a); + id_b_len = strlen (id_b); + if (id_a_len == id_b_len) + return strcmp (id_a, id_b); + else + return id_a_len - id_b_len; } static void From 9d4639cf49f361ec227cb5c0ea5742d456dbe2a2 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Thu, 15 Oct 2020 16:42:10 +0100 Subject: [PATCH 3/5] libmalcontent-ui: Cache and diff the app list when rebuilding it Instead of removing all the entries in the list store when the set of installed apps is updated, diff the old and new lists so that removed apps can be selectively removed from the list store, and added apps can be selectively added. This means that we can (in subsequent commits) reload the app list less conservatively, as doing so will no longer remove in-progress user modifications to the set of blocked apps. Modifications are still only saved when the restrict applications dialogue is closed. Signed-off-by: Philip Withnall Helps: #18, #28 --- .../restrict-applications-selector.c | 105 ++++++++++++++++-- 1 file changed, 95 insertions(+), 10 deletions(-) diff --git a/libmalcontent-ui/restrict-applications-selector.c b/libmalcontent-ui/restrict-applications-selector.c index 4be7c07..15455bd 100644 --- a/libmalcontent-ui/restrict-applications-selector.c +++ b/libmalcontent-ui/restrict-applications-selector.c @@ -61,6 +61,7 @@ struct _MctRestrictApplicationsSelector GtkListBox *listbox; + GList *cached_apps; /* (nullable) (owned) (element-type GAppInfo) */ GListStore *apps; /* (owned) */ GAppInfoMonitor *app_info_monitor; /* (owned) */ gulong app_info_monitor_changed_id; @@ -151,6 +152,7 @@ mct_restrict_applications_selector_dispose (GObject *object) g_clear_pointer (&self->blocklisted_apps, g_hash_table_unref); g_clear_object (&self->apps); + g_clear_list (&self->cached_apps, g_object_unref); if (self->app_info_monitor != NULL && self->app_info_monitor_changed_id != 0) { @@ -225,6 +227,7 @@ mct_restrict_applications_selector_init (MctRestrictApplicationsSelector *self) gtk_widget_init_template (GTK_WIDGET (self)); self->apps = g_list_store_new (G_TYPE_APP_INFO); + self->cached_apps = NULL; self->app_info_monitor = g_app_info_monitor_get (); self->app_info_monitor_changed_id = @@ -392,15 +395,68 @@ app_compare_id_length_cb (gconstpointer a, return id_a_len - id_b_len; } +/* Elements in @added_out and @removed_out are valid as long as @old_apps and + * @new_apps are valid. + * + * Both lists have to be sorted the same before calling this function. */ +static void +diff_app_lists (GList *old_apps, + GList *new_apps, + GPtrArray **added_out, + GPtrArray **removed_out) +{ + g_autoptr(GPtrArray) added = g_ptr_array_new_with_free_func (NULL); + g_autoptr(GPtrArray) removed = g_ptr_array_new_with_free_func (NULL); + GList *o, *n; + + g_return_if_fail (added_out != NULL); + g_return_if_fail (removed_out != NULL); + + for (o = old_apps, n = new_apps; o != NULL || n != NULL;) + { + int comparison; + + if (o == NULL) + comparison = 1; + else if (n == NULL) + comparison = -1; + else + comparison = app_compare_id_length_cb (o->data, n->data); + + if (comparison < 0) + { + g_ptr_array_add (removed, o->data); + o = o->next; + } + else if (comparison > 0) + { + g_ptr_array_add (added, n->data); + n = n->next; + } + else + { + o = o->next; + n = n->next; + } + } + + *added_out = g_steal_pointer (&added); + *removed_out = g_steal_pointer (&removed); +} + +/* This is quite expensive to call, as there’s no way to avoid calling + * g_app_info_get_all() to see if anything’s changed; and that’s quite expensive. */ static void reload_apps (MctRestrictApplicationsSelector *self) { - g_autolist(GAppInfo) apps = NULL; - GList *iter; + g_autolist(GAppInfo) old_apps = NULL; + g_autolist(GAppInfo) new_apps = NULL; + g_autoptr(GPtrArray) added_apps = NULL, removed_apps = NULL; g_autoptr(GHashTable) seen_flatpak_ids = NULL; g_autoptr(GHashTable) seen_executables = NULL; - apps = g_app_info_get_all (); + old_apps = g_steal_pointer (&self->cached_apps); + new_apps = g_app_info_get_all (); /* Sort the apps by increasing length of #GAppInfo ID. When coupled with the * deduplication of flatpak IDs and executable paths, below, this should ensure that we @@ -409,20 +465,46 @@ reload_apps (MctRestrictApplicationsSelector *self) * * This is designed to avoid listing all the components of LibreOffice for example, * which all share an app ID and hence have the same entry in the parental controls - * app filter. */ - apps = g_list_sort (apps, app_compare_id_length_cb); + * app filter. + * + * Then diff the old and new lists so that the code below doesn’t end up + * removing more rows than are necessary, and hence potentially losing + * in-progress user input. */ + new_apps = g_list_sort (new_apps, app_compare_id_length_cb); + diff_app_lists (old_apps, new_apps, &added_apps, &removed_apps); + + g_debug ("%s: Diffed old and new app lists: %u apps added, %u apps removed", + G_STRFUNC, added_apps->len, removed_apps->len); + seen_flatpak_ids = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, NULL); seen_executables = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, NULL); - g_list_store_remove_all (self->apps); - - for (iter = apps; iter; iter = iter->next) + /* Remove items first. */ + for (guint i = 0; i < removed_apps->len; i++) { - GAppInfo *app; + GAppInfo *app = removed_apps->pdata[i]; + guint pos; + gboolean found; + + found = g_list_store_find_with_equal_func (self->apps, app, + (GEqualFunc) g_app_info_equal, &pos); + + /* The app being removed may have not passed the condition checks below + * to have been added to self->apps. */ + if (!found) + continue; + + g_debug ("Removing app ‘%s’", g_app_info_get_id (app)); + g_list_store_remove (self->apps, pos); + } + + /* Now add the new items. */ + for (guint i = 0; i < added_apps->len; i++) + { + GAppInfo *app = added_apps->pdata[i]; const gchar *app_name; const gchar * const *supported_types; - app = iter->data; app_name = g_app_info_get_name (app); supported_types = g_app_info_get_supported_types (app); @@ -492,6 +574,9 @@ reload_apps (MctRestrictApplicationsSelector *self) compare_app_info_cb, self); } + + /* Update the cache for next time. */ + self->cached_apps = g_steal_pointer (&new_apps); } static void From e1b5bcd324c3306d36d4908ded79bc09f5fe2827 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Thu, 15 Oct 2020 16:43:58 +0100 Subject: [PATCH 4/5] libmalcontent-ui: Load the app list when constructing the apps selector Rather than waiting until an app filter is set on the application selector, load the app list immediately at construction time. This is now possible because it can be diffed easily when the app filter is set. Signed-off-by: Philip Withnall Fixes: #28 --- libmalcontent-ui/restrict-applications-selector.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/libmalcontent-ui/restrict-applications-selector.c b/libmalcontent-ui/restrict-applications-selector.c index 15455bd..3955fe1 100644 --- a/libmalcontent-ui/restrict-applications-selector.c +++ b/libmalcontent-ui/restrict-applications-selector.c @@ -104,6 +104,9 @@ mct_restrict_applications_selector_constructed (GObject *obj) g_assert (self->app_filter != NULL); + /* Load the apps. */ + reload_apps (self); + G_OBJECT_CLASS (mct_restrict_applications_selector_parent_class)->constructed (obj); } From 93b88af0c1821228a116d1da107827795472a0d6 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Thu, 15 Oct 2020 16:44:55 +0100 Subject: [PATCH 5/5] libmalcontent-ui: Reload the app list when it changes on the system This will no longer destroy in-progress user input. Signed-off-by: Philip Withnall Fixes: #18 --- libmalcontent-ui/restrict-applications-selector.c | 5 ----- 1 file changed, 5 deletions(-) diff --git a/libmalcontent-ui/restrict-applications-selector.c b/libmalcontent-ui/restrict-applications-selector.c index 3955fe1..b595793 100644 --- a/libmalcontent-ui/restrict-applications-selector.c +++ b/libmalcontent-ui/restrict-applications-selector.c @@ -586,14 +586,9 @@ static void app_info_changed_cb (GAppInfoMonitor *monitor, gpointer user_data) { - /* FIXME: We should update the list of apps here, but we can’t call - * reload_apps() because that will dump and reload the entire list, losing - * any changes the user has already made to the set of switches. We need - * something more fine-grained. MctRestrictApplicationsSelector *self = MCT_RESTRICT_APPLICATIONS_SELECTOR (user_data); reload_apps (self); - */ } /* Will return %NULL if @flatpak_id is not installed. */