Merge ~3v1n0/ubuntu/+source/nautilus:ubuntu/master-xubuntu-cancel-search into ~ubuntu-desktop/ubuntu/+source/nautilus:ubuntu/master

[Marco Trevisan (Treviño)]

Implement XUbuntuCancel introduced in https://code.launchpad.net/~3v1n0/ubuntu/+source/gnome-shell/+git/gnome-shell/+merge/353825

[Didier Roche-Tolomelli (didrocks)]

This looks mostly good, I have some nitpicks and some questions (see below and inline diff):
* You didn't metnion at all about the ignore_partial_results which leads to 2 different conditions (we want to ignore them or not). I think that ought some mention in the patch description.
* 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?

review: Needs Information

[Marco Trevisan (Treviño) (3v1n0)]

> * 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.

[Didier Roche-Tolomelli (didrocks)]

LGTM, thanks for addressing the issues and answering my questions :)

review: Approve

[Marco Trevisan (Treviño) (3v1n0)]

FYI I've submitted also part of this upstream (as it will be needed anyway for future developments): https://gitlab.gnome.org/GNOME/nautilus/merge_requests/303

diff --git a/debian/changelog b/debian/changelog
index 3e72451..4ceff74 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,11 @@
+nautilus (1:3.26.4-0ubuntu3) cosmic; urgency=medium
+
+  * d/p/ubuntu/shell-search-provider-implement-XUbuntuCancel-to-request-.patch:
+    shell-search-provider: implement XUbuntuCancel to request pending
+    search cancellation from gnome-shell (LP: #1756826)
+
+ -- Marco Trevisan (Treviño) <[e-mail]>  Thu, 30 Aug 2018 18:15:24 -0500
+
 nautilus (1:3.26.4-0ubuntu2) cosmic; urgency=medium
 
   * debian/patches/nautilusgtkplacesview-show-error-if-volume-is-not-mo.patch:
diff --git a/debian/patches/series b/debian/patches/series
index 41a35d1..61d3d3a 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -24,3 +24,4 @@ query-add-recursive-flags-and-use-it-in-search-engines.patch
 appstream-compulsory.patch
 nautilusgtkplacesview-show-error-if-volume-is-not-mo.patch
 file-view-Always-unset-pending_selection-after-freeing-it.patch
+ubuntu/shell-search-provider-implement-XUbuntuCancel-to-request-.patch
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..910d08f
--- /dev/null
+++ b/debian/patches/ubuntu/shell-search-provider-implement-XUbuntuCancel-to-request-.patch
@@ -0,0 +1,228 @@
+From: =?utf-8?b?Ik1hcmNvIFRyZXZpc2FuIChUcmV2acOxbyki?= <[e-mail]>
+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.
+
+To implement this redefine `cancel_current_search` with `ignore_partial_results`
+parameter so that when this is TRUE we won't return any value to the caller.
+This is the case on disposal and when the XUbuntuCancel has been requested,
+to avoid some uneeded traffic.
+
+Keep track of all the pending metadata requests in `metas_requests` and
+use it to cancel the requests if requested.
+
+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           | 101 +++++++++++++++++++++++--
+ 2 files changed, 95 insertions(+), 7 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..535436c 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,7 @@ 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 +566,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++)
+     {
+         meta = g_hash_table_lookup (data->self->metas_cache,
+                                     data->uris[idx]);
+@@ -564,6 +581,45 @@ result_metas_return_from_cache (ResultMetasData *data)
+                                            g_variant_new ("(aa{sv})", &builder));
+ }
+ 
++static void
++result_metas_return_empty (ResultMetasData *data)
++{
++    g_clear_pointer (&data->uris, g_strfreev);
++    result_metas_return_from_cache (data);
++    result_metas_data_free (data);
++}
++
++static void
++cancel_result_meta_requests (NautilusShellSearchProvider *self,
++                             GDBusMethodInvocation       *invocation)
++{
++    g_debug ("*** Cancel Results Meta requests");
++
++    if (invocation == NULL)
++    {
++        g_list_free_full (self->metas_requests,
++                          (GDestroyNotify) result_metas_return_empty);
++        self->metas_requests = NULL;
++    }
++    else
++    {
++        GList *l, *next = NULL;
++
++        for (l = self->metas_requests; l != NULL; l = next)
++        {
++            next = l->next;
++            ResultMetasData *data = l->data;
++
++            if (g_strcmp0 (g_dbus_method_invocation_get_sender (data->invocation),
++                           g_dbus_method_invocation_get_sender (invocation)) == 0)
++            {
++                result_metas_return_empty (data);
++                self->metas_requests = g_list_delete_link (self->metas_requests, l);
++            }
++        }
++    }
++}
++
+ static void
+ result_list_attributes_ready_cb (GList    *file_list,
+                                  gpointer  user_data)
+@@ -639,6 +695,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 +741,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,6 +803,30 @@ handle_launch_search (NautilusShellSearchProvider2  *skeleton,
+     return TRUE;
+ }
+ 
++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)
+ {
+@@ -750,7 +834,8 @@ search_provider_dispose (GObject *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 +858,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