[nautilus] search: done loading signal only if not restarting engine



commit ff6ccbaf40afe9cde56746990888246ac3fdd177
Author: Carlos Soriano <csoriano gnome org>
Date:   Wed Jul 22 00:05:53 2015 +0200

    search: done loading signal only if not restarting engine
    
    It was working as:
    - Search directory starts a new search
    - Search engine starts all search providers, each one in its
    own thread.
    - User changes query
    - Search engine stops all search providers
    - Searchs providers, since they are in its own thread, cancel
    in a unknown time.
    - Search directory starts a new search, even before all providers
    are finished.
    - Search engine is marked as need to restart.
    - Search providers finished.
    - Search engine emits finished signal, since all search providers
    now are stopped.
    - Clients doesn't have a way to know if the engine
    actually finished searching the current search, or the previous
    search that the client asked to stop. That might confuse clients
    if they ask for results.
    - Search engine restart the search providers without noticing the
    client, that thinks that the latest search it started was finished
    already.
    
    So to fix this confusion, only report that the engine actually finished
    if the engine is not going to restart the search providers.
    In this way a client can start a batch of consecutive searches without
    the risk of getting search finished signals from previous searches.
    Clients now will always get the search-finished signal of the
    latest search they started.

 configure.ac                                       |    1 +
 libnautilus-private/Makefile.am                    |   18 +++++++++
 .../nautilus-private-enum-types.c.template         |   40 ++++++++++++++++++++
 .../nautilus-private-enum-types.h.template         |   27 +++++++++++++
 libnautilus-private/nautilus-search-directory.c    |   22 +++++++++-
 libnautilus-private/nautilus-search-engine-model.c |    3 +-
 .../nautilus-search-engine-simple.c                |    3 +-
 .../nautilus-search-engine-tracker.c               |    3 +-
 libnautilus-private/nautilus-search-engine.c       |    9 +++-
 libnautilus-private/nautilus-search-provider.c     |   11 +++--
 libnautilus-private/nautilus-search-provider.h     |   36 +++++++++++++++++-
 test/test-nautilus-search-engine.c                 |    3 +-
 12 files changed, 160 insertions(+), 16 deletions(-)
---
diff --git a/configure.ac b/configure.ac
index 8efaf87..286f61d 100644
--- a/configure.ac
+++ b/configure.ac
@@ -283,6 +283,7 @@ m4_ifdef([GOBJECT_INTROSPECTION_CHECK],
 dnl ==========================================================================
 
 AC_PATH_PROG(UPDATE_MIME_DATABASE, update-mime-database, no)
+AC_PATH_PROG(GLIB_MKENUMS, glib-mkenums)
 
 AC_ARG_ENABLE(update-mimedb,
    AS_HELP_STRING([--disable-update-mimedb],
diff --git a/libnautilus-private/Makefile.am b/libnautilus-private/Makefile.am
index c0753e5..92c6014 100644
--- a/libnautilus-private/Makefile.am
+++ b/libnautilus-private/Makefile.am
@@ -17,12 +17,28 @@ AM_CPPFLAGS =                                               \
        -DNAUTILUS_EXTENSIONDIR=\""$(libdir)/nautilus/extensions-3.0"\" \
        $(NULL)
 
+# Basic sanity checks
+$(if $(GLIB_MKENUMS),,$(error Need to define GLIB_MKENUMS))
+
 if ENABLE_TRACKER
 nautilus_tracker_engine_sources = \
        nautilus-search-engine-tracker.c \
        nautilus-search-engine-tracker.h
 endif
 
+BUILT_SOURCES =                                        \
+       nautilus-private-enum-types.h                   \
+       nautilus-private-enum-types.c                   \
+       $(NULL)
+
+ENUM_TYPES = nautilus-search-provider.h
+
+nautilus-private-enum-types.h: nautilus-private-enum-types.h.template $(ENUM_TYPES) $(GLIB_MKENUMS)
+       $(AM_V_GEN) (cd $(srcdir) && $(GLIB_MKENUMS) --template nautilus-private-enum-types.h.template 
$(ENUM_TYPES)) > $@
+
+nautilus-private-enum-types.c: nautilus-private-enum-types.c.template $(ENUM_TYPES) $(GLIB_MKENUMS)
+       $(AM_V_GEN) (cd $(srcdir) && $(GLIB_MKENUMS) --template nautilus-private-enum-types.c.template 
$(ENUM_TYPES)) > $@
+
 libnautilus_private_la_LDFLAGS =       \
        -no-undefined                   \
        $(NULL)
@@ -186,6 +202,8 @@ EXTRA_DIST =                                \
        nautilus.convert                \
        $(gsettingsschema_in_files)     \
        $(TRACKER_SOURCES)              \
+       nautilus-private-enum-types.h.template  \
+       nautilus-private-enum-types.c.template  \
        $(NULL)
 
 CLEANFILES = \
diff --git a/libnautilus-private/nautilus-private-enum-types.c.template 
b/libnautilus-private/nautilus-private-enum-types.c.template
new file mode 100644
index 0000000..4acb2d8
--- /dev/null
+++ b/libnautilus-private/nautilus-private-enum-types.c.template
@@ -0,0 +1,40 @@
+/*** BEGIN file-header ***/
+#include "nautilus-private-enum-types.h"
+
+/*** END file-header ***/
+
+/*** BEGIN file-production ***/
+/* enumerations from "@filename@" */
+#include "@filename@"
+
+/*** END file-production ***/
+
+/*** BEGIN value-header ***/
+GType
+ enum_name@_get_type (void)
+{
+       static GType the_type = 0;
+
+       if (the_type == 0)
+       {
+               static const G Type@Value values[] = {
+/*** END value-header ***/
+
+/*** BEGIN value-production ***/
+                       { @VALUENAME@,
+                         "@VALUENAME@",
+                         "@valuenick@" },
+/*** END value-production ***/
+
+/*** BEGIN value-tail ***/
+                       { 0, NULL, NULL }
+               };
+               the_type = g_ type@_register_static (
+                               g_intern_static_string ("@EnumName@"),
+                               values);
+       }
+       return the_type;
+}
+
+/*** END value-tail ***/
+
diff --git a/libnautilus-private/nautilus-private-enum-types.h.template 
b/libnautilus-private/nautilus-private-enum-types.h.template
new file mode 100644
index 0000000..c4dbbe7
--- /dev/null
+++ b/libnautilus-private/nautilus-private-enum-types.h.template
@@ -0,0 +1,27 @@
+/*** BEGIN file-header ***/
+#ifndef __NAUTILUS_PRIVATE_ENUM_TYPES_H__
+#define __NAUTILUS_PRIVATE_ENUM_TYPES_H__
+
+#include <glib-object.h>
+
+G_BEGIN_DECLS
+
+/*** END file-header ***/
+
+/*** BEGIN file-production ***/
+/* Enumerations from "@filename@" */
+
+/*** END file-production ***/
+
+/*** BEGIN enumeration-production ***/
+#define @ENUMPREFIX _TYPE_@ENUMSHORT@  (@enum_name _get_type())
+GType @enum_name _get_type     (void) G_GNUC_CONST;
+
+/*** END enumeration-production ***/
+
+/*** BEGIN file-tail ***/
+G_END_DECLS
+
+#endif /* __NAUTILUS_PRIVATE_ENUM_TYPES_H__ */
+/*** END file-tail ***/
+
diff --git a/libnautilus-private/nautilus-search-directory.c b/libnautilus-private/nautilus-search-directory.c
index 836b0d3..c0141b6 100644
--- a/libnautilus-private/nautilus-search-directory.c
+++ b/libnautilus-private/nautilus-search-directory.c
@@ -588,10 +588,26 @@ search_engine_error (NautilusSearchEngine *engine, const char *error_message, Na
 }
 
 static void
-search_engine_finished (NautilusSearchEngine *engine, NautilusSearchDirectory *search)
-{
-       search_directory_ensure_loaded (search);
-       nautilus_directory_emit_done_loading (NAUTILUS_DIRECTORY (search));
+search_engine_finished (NautilusSearchEngine         *engine,
+                        NautilusSearchProviderStatus  status,
+                        NautilusSearchDirectory      *search)
+{
+        /* If the search engine is going to restart means it finished an old search
+         * that was stopped or cancelled.
+         * Don't emit the done loading signal in this case, since this means the search
+         * directory tried to start a new search before all the search providers were finished
+         * in the search engine.
+         * If we emit the done-loading signal in this situation the client will think
+         * that it finished the current search, not an old one like it's actually
+         * happening. */
+        if (status == NAUTILUS_SEARCH_PROVIDER_STATUS_NORMAL) {
+                search_directory_ensure_loaded (search);
+               nautilus_directory_emit_done_loading (NAUTILUS_DIRECTORY (search));
+        } else if (status == NAUTILUS_SEARCH_PROVIDER_STATUS_RESTARTING) {
+                /* Remove file monitors of the files from an old search that just
+                 * actually finished */
+                reset_file_list (search);
+        }
 }
 
 static void
diff --git a/libnautilus-private/nautilus-search-engine-model.c 
b/libnautilus-private/nautilus-search-engine-model.c
index 22a644e..8648cf5 100644
--- a/libnautilus-private/nautilus-search-engine-model.c
+++ b/libnautilus-private/nautilus-search-engine-model.c
@@ -88,8 +88,9 @@ search_finished (NautilusSearchEngineModel *model)
        }
 
        model->details->query_pending = FALSE;
-       nautilus_search_provider_finished (NAUTILUS_SEARCH_PROVIDER (model));
        DEBUG ("Model engine finished");
+       nautilus_search_provider_finished (NAUTILUS_SEARCH_PROVIDER (model),
+                                           NAUTILUS_SEARCH_PROVIDER_STATUS_NORMAL);
        g_object_unref (model);
 
        return FALSE;
diff --git a/libnautilus-private/nautilus-search-engine-simple.c 
b/libnautilus-private/nautilus-search-engine-simple.c
index 5bcea49..33180fb 100644
--- a/libnautilus-private/nautilus-search-engine-simple.c
+++ b/libnautilus-private/nautilus-search-engine-simple.c
@@ -144,7 +144,8 @@ search_thread_done_idle (gpointer user_data)
                DEBUG ("Simple engine finished");
         }
        engine->details->active_search = NULL;
-       nautilus_search_provider_finished (NAUTILUS_SEARCH_PROVIDER (engine));
+       nautilus_search_provider_finished (NAUTILUS_SEARCH_PROVIDER (engine),
+                                           NAUTILUS_SEARCH_PROVIDER_STATUS_NORMAL);
 
        search_thread_data_free (data);
 
diff --git a/libnautilus-private/nautilus-search-engine-tracker.c 
b/libnautilus-private/nautilus-search-engine-tracker.c
index 85e4122..6120448 100644
--- a/libnautilus-private/nautilus-search-engine-tracker.c
+++ b/libnautilus-private/nautilus-search-engine-tracker.c
@@ -115,7 +115,8 @@ search_finished (NautilusSearchEngineTracker *tracker,
                DEBUG ("Tracker engine error %s", error->message);
                nautilus_search_provider_error (NAUTILUS_SEARCH_PROVIDER (tracker), error->message);
        } else {
-               nautilus_search_provider_finished (NAUTILUS_SEARCH_PROVIDER (tracker));
+               nautilus_search_provider_finished (NAUTILUS_SEARCH_PROVIDER (tracker),
+                                                   NAUTILUS_SEARCH_PROVIDER_STATUS_NORMAL);
                 if (g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CANCELLED)) {
                        DEBUG ("Tracker engine finished and cancelled");
                 } else {
diff --git a/libnautilus-private/nautilus-search-engine.c b/libnautilus-private/nautilus-search-engine.c
index a48b61a..0ae369a 100644
--- a/libnautilus-private/nautilus-search-engine.c
+++ b/libnautilus-private/nautilus-search-engine.c
@@ -188,12 +188,14 @@ check_providers_status (NautilusSearchEngine *engine)
                nautilus_search_provider_error (NAUTILUS_SEARCH_PROVIDER (engine),
                                                _("Unable to complete the requested search"));
        } else {
-               nautilus_search_provider_finished (NAUTILUS_SEARCH_PROVIDER (engine));
                 if (engine->details->restart) {
                        DEBUG ("Search engine finished and restarting");
                 } else {
                        DEBUG ("Search engine finished");
                 }
+               nautilus_search_provider_finished (NAUTILUS_SEARCH_PROVIDER (engine),
+                                                   engine->details->restart ? 
NAUTILUS_SEARCH_PROVIDER_STATUS_RESTARTING :
+                                                                              
NAUTILUS_SEARCH_PROVIDER_STATUS_NORMAL);
        }
 
        engine->details->running = FALSE;
@@ -219,8 +221,9 @@ search_provider_error (NautilusSearchProvider *provider,
 }
 
 static void
-search_provider_finished (NautilusSearchProvider *provider,
-                         NautilusSearchEngine   *engine)
+search_provider_finished (NautilusSearchProvider       *provider,
+                          NautilusSearchProviderStatus  status,
+                          NautilusSearchEngine         *engine)
 
 {
        DEBUG ("Search provider finished");
diff --git a/libnautilus-private/nautilus-search-provider.c b/libnautilus-private/nautilus-search-provider.c
index a1c5809..10aca20 100644
--- a/libnautilus-private/nautilus-search-provider.c
+++ b/libnautilus-private/nautilus-search-provider.c
@@ -19,6 +19,7 @@
 
 #include <config.h>
 #include "nautilus-search-provider.h"
+#include "nautilus-private-enum-types.c"
 
 #include <glib-object.h>
 
@@ -85,8 +86,9 @@ nautilus_search_provider_base_init (gpointer g_iface)
                                          G_SIGNAL_RUN_LAST,
                                          G_STRUCT_OFFSET (NautilusSearchProviderIface, finished),
                                          NULL, NULL,
-                                         g_cclosure_marshal_VOID__VOID,
-                                         G_TYPE_NONE, 0);
+                                         g_cclosure_marshal_VOID__ENUM,
+                                         G_TYPE_NONE, 1,
+                                          NAUTILUS_TYPE_SEARCH_PROVIDER_STATUS);
 
        signals[ERROR] = g_signal_new ("error",
                                       NAUTILUS_TYPE_SEARCH_PROVIDER,
@@ -137,11 +139,12 @@ nautilus_search_provider_hits_added (NautilusSearchProvider *provider, GList *hi
 }
 
 void
-nautilus_search_provider_finished (NautilusSearchProvider *provider)
+nautilus_search_provider_finished (NautilusSearchProvider       *provider,
+                                   NautilusSearchProviderStatus  status)
 {
        g_return_if_fail (NAUTILUS_IS_SEARCH_PROVIDER (provider));
 
-       g_signal_emit (provider, signals[FINISHED], 0);
+       g_signal_emit (provider, signals[FINISHED], 0, status);
 }
 
 void
diff --git a/libnautilus-private/nautilus-search-provider.h b/libnautilus-private/nautilus-search-provider.h
index 38ee687..b3be651 100644
--- a/libnautilus-private/nautilus-search-provider.h
+++ b/libnautilus-private/nautilus-search-provider.h
@@ -24,6 +24,11 @@
 
 G_BEGIN_DECLS
 
+typedef enum {
+  NAUTILUS_SEARCH_PROVIDER_STATUS_NORMAL,
+  NAUTILUS_SEARCH_PROVIDER_STATUS_RESTARTING
+} NautilusSearchProviderStatus;
+
 #define NAUTILUS_TYPE_SEARCH_PROVIDER           (nautilus_search_provider_get_type ())
 #define NAUTILUS_SEARCH_PROVIDER(obj)           (G_TYPE_CHECK_INSTANCE_CAST ((obj), 
NAUTILUS_TYPE_SEARCH_PROVIDER, NautilusSearchProvider))
 #define NAUTILUS_IS_SEARCH_PROVIDER(obj)        (G_TYPE_CHECK_INSTANCE_TYPE ((obj), 
NAUTILUS_TYPE_SEARCH_PROVIDER))
@@ -42,7 +47,33 @@ struct _NautilusSearchProviderIface {
 
         /* Signals */
         void (*hits_added) (NautilusSearchProvider *provider, GList *hits);
-        void (*finished) (NautilusSearchProvider *provider);
+        /* This signal has a status parameter because it's necesary to discern
+         * when the search engine finished normally or wheter it finished in a
+         * different situation that will cause the engine to do some action after
+         * finishing.
+         *
+         * For example, the search engine restarts itself if the client starts a
+         * new search before all the search providers finished its current ongoing search.
+         *
+         * A real use case of this is when the user change quickly the query of the search,
+         * the search engine stops all the search providers, but given that each search
+         * provider has its own thread it will be actually stopped in a unknown time.
+         * To fix that, the search engine marks itself for restarting if the client
+         * starts a new search and not all providers finished. Then it will emit
+         * its finished signal and restart all providers with the new search.
+         *
+         * That can cause that when the search engine emits its finished signal,
+         * it actually relates to old searchs that it stopped and not the one
+         * the client started lately.
+         * The client doesn't have a way to know wheter the finished signal
+         * relates to its current search or with an old search.
+         *
+         * To fix this situation, provide with the signal a status parameter, that
+         * provides a hint of how the search engine stopped or if it is going to realize
+         * some action afterwards, like restarting.
+         */
+        void (*finished) (NautilusSearchProvider       *provider,
+                          NautilusSearchProviderStatus  status);
         void (*error) (NautilusSearchProvider *provider, const char *error_message);
 };
 
@@ -56,7 +87,8 @@ void           nautilus_search_provider_stop            (NautilusSearchProvider
 
 void           nautilus_search_provider_hits_added      (NautilusSearchProvider *provider,
                                                          GList *hits);
-void           nautilus_search_provider_finished        (NautilusSearchProvider *provider);
+void           nautilus_search_provider_finished        (NautilusSearchProvider       *provider,
+                                                         NautilusSearchProviderStatus  status);
 void           nautilus_search_provider_error           (NautilusSearchProvider *provider,
                                                          const char *error_message);
 
diff --git a/test/test-nautilus-search-engine.c b/test/test-nautilus-search-engine.c
index d727035..5353a4b 100644
--- a/test/test-nautilus-search-engine.c
+++ b/test/test-nautilus-search-engine.c
@@ -13,7 +13,8 @@ hits_added_cb (NautilusSearchEngine *engine, GSList *hits)
 }
 
 static void
-finished_cb (NautilusSearchEngine *engine)
+finished_cb (NautilusSearchEngine         *engine,
+             NautilusSearchProviderStatus  status)
 {
        g_print ("finished!\n");
        gtk_main_quit ();


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]