> * 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?= <[email protected]>
> +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.

-- 
ubuntu-desktop mailing list
[email protected]
https://lists.ubuntu.com/mailman/listinfo/ubuntu-desktop

Reply via email to