[Merge] ~3v1n0/ubuntu/+source/nautilus:ubuntu/master-xubuntu-cancel-search into ~ubuntu-desktop/ubuntu/+source/nautilus:ubuntu/master
Marco Trevisan (Treviño)
mail at 3v1n0.net
Thu Aug 30 23:30:43 UTC 2018
> * You didn't metnion at all about the ignore_partial_results. [...]
> I think that ought some mention in the patch description.
Done
> * I'm unsure to understand what the meta_requests are and why they are treated differently (and unconditionnally).
> Is it a queue before them becoming the current requests, and this is why you are cancelling if the invocation caller matches as well?
So, while each search stops the previous one, it might happen that the shell (not really as per how it's implemented, but the API isn't clear here, and so we can't make assumptions), that the shell fetches the metadata on the returned results in different calls, thus nautilus allowed these calls to be performed in parallel if requested.
Creating multiple ResultMetasData instances... However in order to be able to stop these requests, if XUbuntuCancel is called, we need to keep track of these data instances, so that we can use the nautilus handler to stop the metadata fetching and the invocation to return to the dbus caller. Not to mention to free the instance when that happens.
Diff comments:
> diff --git a/debian/patches/ubuntu/shell-search-provider-implement-XUbuntuCancel-to-request-.patch b/debian/patches/ubuntu/shell-search-provider-implement-XUbuntuCancel-to-request-.patch
> new file mode 100644
> index 0000000..ca55544
> --- /dev/null
> +++ b/debian/patches/ubuntu/shell-search-provider-implement-XUbuntuCancel-to-request-.patch
> @@ -0,0 +1,212 @@
> +From: =?utf-8?b?Ik1hcmNvIFRyZXZpc2FuIChUcmV2acOxbyki?= <mail at 3v1n0.net>
> +Date: Tue, 28 Aug 2018 01:44:49 +0200
> +Subject: shell-search-provider: implement XUbuntuCancel to request search cancellation
> +
> +Stop search and Metadata fetching on XUbuntuCancel dbus method call.
> +Only allow this if the caller is the same who triggered the actual event.
> +
> +Bug-GNOME: https://gitlab.gnome.org/GNOME/gnome-shell/issues/183
> +Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/bionic/+source/nautilus/+bug/1756826
> +Forwarded: not-needed
> +---
> + data/shell-search-provider-dbus-interfaces.xml | 1 +
> + src/nautilus-shell-search-provider.c | 96 +++++++++++++++++++++++---
> + 2 files changed, 88 insertions(+), 9 deletions(-)
> +
> +diff --git a/data/shell-search-provider-dbus-interfaces.xml b/data/shell-search-provider-dbus-interfaces.xml
> +index f6840e2..4529c1e 100644
> +--- a/data/shell-search-provider-dbus-interfaces.xml
> ++++ b/data/shell-search-provider-dbus-interfaces.xml
> +@@ -40,5 +40,6 @@
> + <arg type='as' name='Terms' direction='in' />
> + <arg type='u' name='Timestamp' direction='in' />
> + </method>
> ++ <method name = 'XUbuntuCancel' />
> + </interface>
> + </node>
> +diff --git a/src/nautilus-shell-search-provider.c b/src/nautilus-shell-search-provider.c
> +index ffc2b7f..58864d6 100644
> +--- a/src/nautilus-shell-search-provider.c
> ++++ b/src/nautilus-shell-search-provider.c
> +@@ -60,6 +60,7 @@ struct _NautilusShellSearchProvider
> +
> + PendingSearch *current_search;
> +
> ++ GList *metas_requests;
> + GHashTable *metas_cache;
> + };
> +
> +@@ -143,11 +144,25 @@ pending_search_finish (PendingSearch *search,
> + }
> +
> + static void
> +-cancel_current_search (NautilusShellSearchProvider *self)
> ++cancel_current_search (NautilusShellSearchProvider *self,
> ++ gboolean ignore_partial_results)
> + {
> +- if (self->current_search != NULL)
> ++ PendingSearch *search = self->current_search;
> ++
> ++ if (search != NULL)
> + {
> +- nautilus_search_provider_stop (NAUTILUS_SEARCH_PROVIDER (self->current_search->engine));
> ++ g_debug ("*** Cancel current search");
> ++
> ++ nautilus_search_provider_stop (NAUTILUS_SEARCH_PROVIDER (search->engine));
> ++
> ++ if (ignore_partial_results)
> ++ {
> ++ g_signal_handlers_disconnect_by_data (G_OBJECT (search->engine),
> ++ search);
> ++
> ++ pending_search_finish (search, search->invocation,
> ++ g_variant_new ("(as)", NULL));
> ++ }
> + }
> + }
> +
> +@@ -451,7 +466,7 @@ execute_search (NautilusShellSearchProvider *self,
> + NautilusQuery *query;
> + PendingSearch *pending_search;
> +
> +- cancel_current_search (self);
> ++ cancel_current_search (self, FALSE);
> +
> + /* don't attempt searches for a single character */
> + if (g_strv_length (terms) == 1 &&
> +@@ -524,6 +539,7 @@ typedef struct
> + NautilusShellSearchProvider *self;
> +
> + gint64 start_time;
> ++ NautilusFileListHandle *handle;
> + GDBusMethodInvocation *invocation;
> +
> + gchar **uris;
> +@@ -532,6 +548,8 @@ typedef struct
> + static void
> + result_metas_data_free (ResultMetasData *data)
> + {
> ++ g_clear_pointer (&data->handle, nautilus_file_list_cancel_call_when_ready);
> ++
> + g_clear_object (&data->self);
> + g_clear_object (&data->invocation);
> + g_strfreev (data->uris);
> +@@ -549,7 +567,7 @@ result_metas_return_from_cache (ResultMetasData *data)
> +
> + g_variant_builder_init (&builder, G_VARIANT_TYPE ("aa{sv}"));
> +
> +- for (idx = 0; data->uris[idx] != NULL; idx++)
> ++ for (idx = 0; data->uris && data->uris[idx] != NULL; idx++)
Well, yes we could check before the loop for sure, but I wanted to to keep the diff not too big, while such a check isn't really expensive these days.
As per data->uris, no... Once it's initialized for such async, there's nothing that changes it. And the data is alive until the request is alive.
> + {
> + meta = g_hash_table_lookup (data->self->metas_cache,
> + data->uris[idx]);
> +@@ -564,6 +582,37 @@ result_metas_return_from_cache (ResultMetasData *data)
> + g_variant_new ("(aa{sv})", &builder));
> + }
> +
> ++static void
> ++cancel_result_meta_requests (NautilusShellSearchProvider *self,
> ++ GDBusMethodInvocation *invocation)
> ++{
> ++ GList *l;
> ++ GList *to_remove = NULL;
> ++
> ++ g_debug ("*** Cancel Results Meta requests");
> ++
> ++ for (l = self->metas_requests; l; l = l->next)
> ++ {
> ++ ResultMetasData *data = l->data;
> ++
> ++ if (invocation == NULL ||
> ++ g_strcmp0 (g_dbus_method_invocation_get_sender (data->invocation),
> ++ g_dbus_method_invocation_get_sender (invocation)) == 0)
> ++ {
> ++ g_clear_pointer (&data->uris, g_strfreev);
> ++ result_metas_return_from_cache (data);
> ++ to_remove = g_list_prepend (to_remove, data);
> ++ }
> ++ }
> ++
> ++ for (l = to_remove; l; l = l->next)
> ++ {
> ++ self->metas_requests = g_list_remove (self->metas_requests, l->data);
> ++ }
> ++
> ++ g_list_free (to_remove);
> ++}
> ++
> + static void
> + result_list_attributes_ready_cb (GList *file_list,
> + gpointer user_data)
> +@@ -639,6 +688,9 @@ result_list_attributes_ready_cb (GList *file_list,
> + g_free (uri);
> + }
> +
> ++ data->handle = NULL;
> ++ data->self->metas_requests = g_list_remove (data->self->metas_requests, data);
> ++
> + result_metas_return_from_cache (data);
> + result_metas_data_free (data);
> + }
> +@@ -682,9 +734,10 @@ handle_get_result_metas (NautilusShellSearchProvider2 *skeleton,
> +
> + nautilus_file_list_call_when_ready (missing_files,
> + NAUTILUS_FILE_ATTRIBUTES_FOR_ICON,
> +- NULL,
> ++ &data->handle,
> + result_list_attributes_ready_cb,
> + data);
> ++ self->metas_requests = g_list_prepend (self->metas_requests, data);
> + nautilus_file_list_free (missing_files);
> + return TRUE;
> + }
> +@@ -743,14 +796,37 @@ handle_launch_search (NautilusShellSearchProvider2 *skeleton,
> + return TRUE;
> + }
> +
> +-static void
> +-search_provider_dispose (GObject *obj)
> ++static gboolean
> ++handle_xubuntu_cancel (NautilusShellSearchProvider2 *skeleton,
> ++ GDBusMethodInvocation *invocation,
> ++ gpointer user_data)
> + {
> ++ NautilusShellSearchProvider *self = user_data;
> ++ PendingSearch *search = self->current_search;
> ++
> ++ g_debug ("*** XUbuntuCancel called");
> ++
> ++ if (search != NULL &&
> ++ g_strcmp0 (g_dbus_method_invocation_get_sender (search->invocation),
> ++ g_dbus_method_invocation_get_sender (invocation)) == 0)
> ++ {
> ++ cancel_current_search (self, TRUE);
> ++ }
> ++
> ++ cancel_result_meta_requests (self, invocation);
> ++
> ++ nautilus_shell_search_provider2_complete_xubuntu_cancel (skeleton, invocation);
> ++
> ++ return TRUE;
> ++}
> ++
> ++ static void search_provider_dispose(GObject * obj) {
Ops sorry... Blame vscode for that :)
> + NautilusShellSearchProvider *self = NAUTILUS_SHELL_SEARCH_PROVIDER (obj);
> +
> + g_clear_object (&self->skeleton);
> + g_hash_table_destroy (self->metas_cache);
> +- cancel_current_search (self);
> ++ cancel_current_search (self, TRUE);
> ++ cancel_result_meta_requests (self, NULL);
> +
> + G_OBJECT_CLASS (nautilus_shell_search_provider_parent_class)->dispose (obj);
> + }
> +@@ -773,6 +849,8 @@ nautilus_shell_search_provider_init (NautilusShellSearchProvider *self)
> + G_CALLBACK (handle_activate_result), self);
> + g_signal_connect (self->skeleton, "handle-launch-search",
> + G_CALLBACK (handle_launch_search), self);
> ++ g_signal_connect (self->skeleton, "handle-xubuntu-cancel",
> ++ G_CALLBACK (handle_xubuntu_cancel), self);
> + }
> +
> + static void
--
https://code.launchpad.net/~3v1n0/ubuntu/+source/nautilus/+git/nautilus/+merge/353826
Your team Ubuntu Desktop is subscribed to branch ~ubuntu-desktop/ubuntu/+source/nautilus:ubuntu/master.
More information about the ubuntu-desktop
mailing list