From 31e640e0fab56192c1c9d31c2a8d6188dfc7a40f Mon Sep 17 00:00:00 2001 From: Corey Berla Date: Sat, 10 Jun 2023 17:13:48 -0700 Subject: [PATCH 1/6] window-slot: Don't force reload the starred directory We are force reloading the starred directory every time upon navigation. This is unnecessary and wasteful because even though it's not a native file, we are able to monitor it. --- src/nautilus-window-slot.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/nautilus-window-slot.c b/src/nautilus-window-slot.c index 80227ab46e..48aa61f553 100644 --- a/src/nautilus-window-slot.c +++ b/src/nautilus-window-slot.c @@ -1278,7 +1278,7 @@ check_force_reload (GFile *location, } else { - force_reload = !g_file_is_native (location); + force_reload = !g_file_is_native (location) && !nautilus_file_is_starred_location (file); } /* We need to invalidate file attributes as well due to how mounting works -- GitLab From 4c9a2b455ed1782916c4fb3e475a164f35777d6d Mon Sep 17 00:00:00 2001 From: Corey Berla Date: Sat, 10 Jun 2023 17:16:28 -0700 Subject: [PATCH 2/6] tag-manager: Batch files in ::starred-changed The starred-changed signal passes a GList as its data, but always only sends a single item list. Batch the files and emit the signal a single time. --- src/nautilus-tag-manager.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/nautilus-tag-manager.c b/src/nautilus-tag-manager.c index 2bdde9a208..fd2bf64938 100644 --- a/src/nautilus-tag-manager.c +++ b/src/nautilus-tag-manager.c @@ -44,6 +44,8 @@ struct _NautilusTagManager GHashTable *starred_file_uris; GFile *home; + GList *pending_changed_files; + GCancellable *cancellable; }; @@ -217,7 +219,6 @@ on_get_starred_files_cursor_callback (GObject *object, const gchar *url; gboolean success; NautilusTagManager *self; - GList *changed_files; NautilusFile *file; cursor = TRACKER_SPARQL_CURSOR (object); @@ -233,6 +234,12 @@ on_get_starred_files_cursor_callback (GObject *object, g_warning ("Error on getting all tags cursor callback: %s", error->message); } + if (self->pending_changed_files != NULL) + { + g_signal_emit_by_name (self, "starred-changed", self->pending_changed_files); + g_clear_pointer (&self->pending_changed_files, nautilus_file_list_free); + } + g_clear_object (&cursor); return; } @@ -245,11 +252,7 @@ on_get_starred_files_cursor_callback (GObject *object, if (file) { - changed_files = g_list_prepend (NULL, file); - - g_signal_emit_by_name (self, "starred-changed", changed_files); - - nautilus_file_list_free (changed_files); + self->pending_changed_files = g_list_prepend (self->pending_changed_files, file); } else { @@ -542,6 +545,7 @@ nautilus_tag_manager_finalize (GObject *object) g_clear_object (&self->db); g_clear_object (&self->query_file_is_starred); g_clear_object (&self->query_starred_files); + g_clear_pointer (&self->pending_changed_files, nautilus_file_list_free); g_hash_table_destroy (self->starred_file_uris); g_clear_object (&self->home); -- GitLab From 918b5066d7d3bf6b1ed47761f9077805501f04a1 Mon Sep 17 00:00:00 2001 From: Corey Berla Date: Sat, 10 Jun 2023 21:09:28 -0700 Subject: [PATCH 3/6] tag-manager: Make nautilus_tag_manager_get_starred_files return files It returns uri's which is very confusing not only because of the name of the function, but also because the ::starred-changed returns files. The only consumer is starred-directory, so this is easy. --- src/nautilus-starred-directory.c | 24 ++++++++++-------------- src/nautilus-tag-manager.c | 15 ++++++++------- 2 files changed, 18 insertions(+), 21 deletions(-) diff --git a/src/nautilus-starred-directory.c b/src/nautilus-starred-directory.c index 947ac991d1..4c13afbccf 100644 --- a/src/nautilus-starred-directory.c +++ b/src/nautilus-starred-directory.c @@ -98,14 +98,12 @@ nautilus_starred_directory_update_files (NautilusFavoriteDirectory *self) NautilusTagManager *tag_manager = nautilus_tag_manager_get (); GList *l; GList *tmp_l; - g_autoptr (GList) new_starred_files = NULL; + g_autolist (NautilusFile) new_starred_files = NULL; GList *monitor_list; FavoriteMonitor *monitor; - NautilusFile *file; GHashTable *uri_table; GList *files_added; GList *files_removed; - gchar *uri; files_added = NULL; files_removed = NULL; @@ -124,10 +122,11 @@ nautilus_starred_directory_update_files (NautilusFavoriteDirectory *self) for (l = new_starred_files; l != NULL; l = l->next) { - if (!g_hash_table_contains (uri_table, l->data)) - { - file = nautilus_file_get_by_uri ((gchar *) l->data); + NautilusFile *file = l->data; + g_autofree char *uri = nautilus_file_get_uri (file); + if (!g_hash_table_contains (uri_table, uri)) + { for (monitor_list = self->monitor_list; monitor_list; monitor_list = monitor_list->next) { monitor = monitor_list->data; @@ -138,14 +137,14 @@ nautilus_starred_directory_update_files (NautilusFavoriteDirectory *self) g_signal_connect (file, "changed", G_CALLBACK (file_changed), self); - files_added = g_list_prepend (files_added, file); + files_added = g_list_prepend (files_added, nautilus_file_ref (file)); } } l = self->files; while (l != NULL) { - uri = nautilus_file_get_uri (NAUTILUS_FILE (l->data)); + g_autofree char *uri = nautilus_file_get_uri (NAUTILUS_FILE (l->data)); if (!nautilus_tag_manager_file_is_starred (tag_manager, uri)) { @@ -170,8 +169,6 @@ nautilus_starred_directory_update_files (NautilusFavoriteDirectory *self) { l = l->next; } - - g_free (uri); } if (files_added) @@ -441,8 +438,7 @@ real_get_file_list (NautilusDirectory *directory) static void nautilus_starred_directory_set_files (NautilusFavoriteDirectory *self) { - g_autoptr (GList) starred_files = NULL; - NautilusFile *file; + g_autolist (NautilusFile) starred_files = NULL; GList *l; GList *file_list; FavoriteMonitor *monitor; @@ -454,7 +450,7 @@ nautilus_starred_directory_set_files (NautilusFavoriteDirectory *self) for (l = starred_files; l != NULL; l = l->next) { - file = nautilus_file_get_by_uri ((gchar *) l->data); + NautilusFile *file = l->data; g_signal_connect (file, "changed", G_CALLBACK (file_changed), self); @@ -466,7 +462,7 @@ nautilus_starred_directory_set_files (NautilusFavoriteDirectory *self) nautilus_file_monitor_add (file, monitor, monitor->monitor_attributes); } - file_list = g_list_prepend (file_list, file); + file_list = g_list_prepend (file_list, nautilus_file_ref (file)); } nautilus_directory_emit_files_added (NAUTILUS_DIRECTORY (self), file_list); diff --git a/src/nautilus-tag-manager.c b/src/nautilus-tag-manager.c index fd2bf64938..7378379a11 100644 --- a/src/nautilus-tag-manager.c +++ b/src/nautilus-tag-manager.c @@ -184,29 +184,30 @@ on_update_callback (GObject *object, * nautilus_tag_manager_get_starred_files: * @self: The tag manager singleton * - * Returns: (element-type gchar*) (transfer container): A list of the starred urls. + * Returns: (element-type gchar*) (transfer full): A list of the starred NautilusFile. */ GList * nautilus_tag_manager_get_starred_files (NautilusTagManager *self) { GHashTableIter starred_iter; gchar *starred_uri; - GList *starred_file_uris = NULL; + GList *starred_files = NULL; g_hash_table_iter_init (&starred_iter, self->starred_file_uris); while (g_hash_table_iter_next (&starred_iter, (gpointer *) &starred_uri, NULL)) { - g_autoptr (GFile) file = g_file_new_for_uri (starred_uri); + g_autoptr (GFile) location = g_file_new_for_uri (starred_uri); + NautilusFile *file = nautilus_file_get (location); - /* Skip files ouside $HOME, because we don't support starring these yet. + /* Skip files outside $HOME, because we don't support starring these yet. * See comment on nautilus_tag_manager_can_star_contents() */ - if (g_file_has_prefix (file, self->home)) + if (g_file_has_prefix (location, self->home)) { - starred_file_uris = g_list_prepend (starred_file_uris, starred_uri); + starred_files = g_list_prepend (starred_files, file); } } - return starred_file_uris; + return starred_files; } static void -- GitLab From 3872f1903d474c2cb1be8befa1d52801cde61d07 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ant=C3=B3nio=20Fernandes?= Date: Tue, 11 Jul 2023 18:45:23 +0100 Subject: [PATCH 4/6] tag-manager: Remove duplicate emissions of "starred-changed" We notify the changes we make change the database, then we get the signal from the database about these changes, and we notify them again. The first emission is untimely, because it leaves us in an inconsistent state: the `starred_file_uris` hash table hasn't been updated yet, and any clients calling `nautilus_tag_manager_file_is_starred()` to assess the result of the notified change get the wrong answer. So, let's stop emitting "starred-changed" before we get the database signal about the changes. This also resolves a leak of a files list from update_moved_uris_callback(). --- src/nautilus-tag-manager.c | 24 +----------------------- 1 file changed, 1 insertion(+), 23 deletions(-) diff --git a/src/nautilus-tag-manager.c b/src/nautilus-tag-manager.c index 7378379a11..0d74b9c224 100644 --- a/src/nautilus-tag-manager.c +++ b/src/nautilus-tag-manager.c @@ -148,8 +148,6 @@ on_update_callback (GObject *object, if (error == NULL) { - /* FIXME: make sure data->tag_manager->starred_file_uris is up to date */ - if (!nautilus_file_undo_manager_is_operating ()) { NautilusFileUndoInfo *undo_info; @@ -160,8 +158,6 @@ on_update_callback (GObject *object, g_object_unref (undo_info); } - g_signal_emit_by_name (data->tag_manager, "starred-changed", nautilus_file_list_copy (data->selection)); - g_task_return_boolean (data->task, TRUE); g_object_unref (data->task); } @@ -731,7 +727,6 @@ update_moved_uris_callback (GObject *object, gpointer user_data) { g_autoptr (GError) error = NULL; - g_autoptr (GPtrArray) new_uris = user_data; tracker_sparql_connection_update_finish (TRACKER_SPARQL_CONNECTION (object), result, @@ -741,19 +736,6 @@ update_moved_uris_callback (GObject *object, { g_warning ("Error updating moved uris: %s", error->message); } - else - { - g_autolist (NautilusFile) updated_files = NULL; - - for (guint i = 0; i < new_uris->len; i++) - { - gchar *new_uri = g_ptr_array_index (new_uris, i); - - updated_files = g_list_prepend (updated_files, nautilus_file_get_by_uri (new_uri)); - } - - g_signal_emit_by_name (tag_manager, "starred-changed", updated_files); - } } /** @@ -846,15 +828,11 @@ nautilus_tag_manager_update_moved_uris (NautilusTagManager *self, g_string_append (query, "}"); - /* Forward the new_uris list to later pass in the ::files-changed signal. - * There is no need to pass the old_uris because the file model is updated - * independently; we need only inform the view where to display stars now. - */ tracker_sparql_connection_update_async (self->db, query->str, self->cancellable, update_moved_uris_callback, - g_steal_pointer (&new_uris)); + NULL); } static void -- GitLab From 28da9408c01484d9ebd64225690212518265a128 Mon Sep 17 00:00:00 2001 From: Corey Berla Date: Sat, 10 Jun 2023 17:36:17 -0700 Subject: [PATCH 5/6] starred-directory: Only update changed starred files nautilus_starred_directory_update_files listens to the starred-changed signal, but then processes all starred files. Process only the data passed from the starred-changed signal. This doesn't entail a change in behavior, it's just more efficient. --- src/nautilus-starred-directory.c | 76 ++++++++++---------------------- 1 file changed, 24 insertions(+), 52 deletions(-) diff --git a/src/nautilus-starred-directory.c b/src/nautilus-starred-directory.c index 4c13afbccf..7fc652a645 100644 --- a/src/nautilus-starred-directory.c +++ b/src/nautilus-starred-directory.c @@ -93,39 +93,43 @@ disconnect_and_unmonitor_file (NautilusFile *file, } static void -nautilus_starred_directory_update_files (NautilusFavoriteDirectory *self) +nautilus_starred_directory_update_files (NautilusFavoriteDirectory *self, + GList *changed_files) { NautilusTagManager *tag_manager = nautilus_tag_manager_get (); - GList *l; - GList *tmp_l; - g_autolist (NautilusFile) new_starred_files = NULL; GList *monitor_list; FavoriteMonitor *monitor; - GHashTable *uri_table; - GList *files_added; - GList *files_removed; - - files_added = NULL; - files_removed = NULL; + g_autoptr (GHashTable) uri_table = NULL; + GList *next; + g_autolist (NautilusFile) files_added = NULL; + g_autolist (NautilusFile) files_removed = NULL; uri_table = g_hash_table_new_full (g_str_hash, g_str_equal, (GDestroyNotify) g_free, NULL); - for (l = self->files; l != NULL; l = l->next) + for (GList *l = self->files; l != NULL; l = l->next) { g_hash_table_add (uri_table, nautilus_file_get_uri (NAUTILUS_FILE (l->data))); } - new_starred_files = nautilus_tag_manager_get_starred_files (tag_manager); - - for (l = new_starred_files; l != NULL; l = l->next) + for (GList *l = changed_files; l != NULL; l = next) { NautilusFile *file = l->data; g_autofree char *uri = nautilus_file_get_uri (file); - if (!g_hash_table_contains (uri_table, uri)) + next = l->next; + if (g_hash_table_contains (uri_table, uri) && + !nautilus_tag_manager_file_is_starred (tag_manager, uri)) + { + disconnect_and_unmonitor_file (file, self); + nautilus_file_unref (file); + files_removed = g_list_prepend (files_removed, nautilus_file_ref (file)); + self->files = g_list_remove (self->files, file); + } + else if (!g_hash_table_contains (uri_table, uri) && + nautilus_tag_manager_file_is_starred (tag_manager, uri)) { for (monitor_list = self->monitor_list; monitor_list; monitor_list = monitor_list->next) { @@ -138,57 +142,25 @@ nautilus_starred_directory_update_files (NautilusFavoriteDirectory *self) g_signal_connect (file, "changed", G_CALLBACK (file_changed), self); files_added = g_list_prepend (files_added, nautilus_file_ref (file)); - } - } - - l = self->files; - while (l != NULL) - { - g_autofree char *uri = nautilus_file_get_uri (NAUTILUS_FILE (l->data)); - - if (!nautilus_tag_manager_file_is_starred (tag_manager, uri)) - { - files_removed = g_list_prepend (files_removed, - nautilus_file_ref (NAUTILUS_FILE (l->data))); - - disconnect_and_unmonitor_file (NAUTILUS_FILE (l->data), self); - - if (l == self->files) - { - self->files = g_list_delete_link (self->files, l); - l = self->files; - } - else - { - tmp_l = l->prev; - self->files = g_list_delete_link (self->files, l); - l = tmp_l->next; - } + self->files = g_list_prepend (self->files, nautilus_file_ref (file)); } else { - l = l->next; + /* This may happen if we have moved the file and updated the starred + * database for the new URI. In that case, the NautilusFile instance + * has already updated its own URI. There's no need to re-add it. */ } } if (files_added) { nautilus_directory_emit_files_added (NAUTILUS_DIRECTORY (self), files_added); - - for (l = files_added; l != NULL; l = l->next) - { - self->files = g_list_prepend (self->files, nautilus_file_ref (NAUTILUS_FILE (l->data))); - } } if (files_removed) { nautilus_directory_emit_files_changed (NAUTILUS_DIRECTORY (self), files_removed); } - - nautilus_file_list_free (files_added); - nautilus_file_list_free (files_removed); - g_hash_table_destroy (uri_table); } static void @@ -200,7 +172,7 @@ on_starred_files_changed (NautilusTagManager *tag_manager, self = NAUTILUS_STARRED_DIRECTORY (user_data); - nautilus_starred_directory_update_files (self); + nautilus_starred_directory_update_files (self, changed_files); } static gboolean -- GitLab From ba78d5d16dc843ca42fe8b446d1518269ad72a60 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ant=C3=B3nio=20Fernandes?= Date: Mon, 10 Jul 2023 10:40:02 +0100 Subject: [PATCH 6/6] starred-directory: Don't emit ::files-added when [re]loading On force reload, we clear the list of starred files and repopulate it. But emitting NautilusDirectory::files-added when repopulating means any view displaying this directory, other than the one being reloaded, is going to react to this signal by adding to the view files that were already in view, duplicating them in the view UI. Also populate the list on init() using the same codepath, while nobody is listening to our signals yet. In that case it's safe, but useless. As such, we should stop emiting the signal. Fixes https://gitlab.gnome.org/GNOME/nautilus/-/issues/2998 --- src/nautilus-starred-directory.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/nautilus-starred-directory.c b/src/nautilus-starred-directory.c index 7fc652a645..0a97e554aa 100644 --- a/src/nautilus-starred-directory.c +++ b/src/nautilus-starred-directory.c @@ -437,8 +437,6 @@ nautilus_starred_directory_set_files (NautilusFavoriteDirectory *self) file_list = g_list_prepend (file_list, nautilus_file_ref (file)); } - nautilus_directory_emit_files_added (NAUTILUS_DIRECTORY (self), file_list); - self->files = file_list; } -- GitLab