From d811b0f62baaaa5c1b5fca71b03a177d5eb13ee7 Mon Sep 17 00:00:00 2001 From: Xavier Claessens Date: Thu, 17 Sep 2015 15:54:43 -0400 Subject: filesystem: avoid duplicate change notifications - Make sure to have only one GFileMonitor per directory - When deleting a directory avoid having both monitors (deleted dir's and its parent's) notifying it. - Fix typo causing notification when a monitor emits G_FILE_MONITOR_EVENT_CHANGES_DONE_HINT which happens after each set of changes. https://bugzilla.gnome.org/show_bug.cgi?id=755181 --- src/filesystem/grl-filesystem.c | 138 +++++++++++++++++++++++++--------------- 1 file changed, 88 insertions(+), 50 deletions(-) diff --git a/src/filesystem/grl-filesystem.c b/src/filesystem/grl-filesystem.c index 7e40fc2..7d5f85c 100644 --- a/src/filesystem/grl-filesystem.c +++ b/src/filesystem/grl-filesystem.c @@ -73,8 +73,8 @@ struct _GrlFilesystemSourcePrivate { gboolean handle_pls; /* a mapping operation_id -> GCancellable to cancel this operation */ GHashTable *cancellables; - /* Monitors for changes in directories */ - GList *monitors; + /* URI -> GFileMonitor */ + GHashTable *monitors; GCancellable *cancellable_monitors; }; @@ -247,6 +247,8 @@ grl_filesystem_source_init (GrlFilesystemSource *source) { source->priv = GRL_FILESYSTEM_SOURCE_GET_PRIVATE (source); source->priv->cancellables = g_hash_table_new (NULL, NULL); + source->priv->monitors = g_hash_table_new_full (g_str_hash, g_str_equal, + g_free, g_object_unref); } static void @@ -255,6 +257,7 @@ grl_filesystem_source_finalize (GObject *object) GrlFilesystemSource *filesystem_source = GRL_FILESYSTEM_SOURCE (object); g_list_free_full (filesystem_source->priv->chosen_uris, g_free); g_hash_table_unref (filesystem_source->priv->cancellables); + g_hash_table_unref (filesystem_source->priv->monitors); G_OBJECT_CLASS (grl_filesystem_source_parent_class)->finalize (object); } @@ -928,73 +931,108 @@ directory_changed (GFileMonitor *monitor, gpointer data) { GrlSource *source = GRL_SOURCE (data); - GFile *file_parent, *other_file_parent; + GrlFilesystemSource *fs_source = GRL_FILESYSTEM_SOURCE (data); + GFileInfo *info = NULL; + + /* Keep only signals we are interested in */ + if (event != G_FILE_MONITOR_EVENT_CREATED && + event != G_FILE_MONITOR_EVENT_CHANGED && + event != G_FILE_MONITOR_EVENT_MOVED && + event != G_FILE_MONITOR_EVENT_DELETED) + return; - if (event == G_FILE_MONITOR_EVENT_CHANGES_DONE_HINT || - event == G_FILE_MONITOR_EVENT_CREATED) { - GFileInfo *info; - info = g_file_query_info (file, grl_pls_get_file_attributes (), G_FILE_QUERY_INFO_NONE, NULL, NULL); - if (info && file_is_valid_content (info, TRUE, NULL)) { - notify_parent_change (source, - file, - (event == G_FILE_MONITOR_EVENT_CREATED)? GRL_CONTENT_ADDED: GRL_CONTENT_CHANGED); - if (event == G_FILE_MONITOR_EVENT_CREATED) { - if (g_file_info_get_file_type (info) == G_FILE_TYPE_DIRECTORY) { - add_monitor (GRL_FILESYSTEM_SOURCE (source), file); - } - } - } - g_object_unref (info); - } else if (event == G_FILE_MONITOR_EVENT_DELETED) { - notify_parent_change (source, file, GRL_CONTENT_REMOVED); - } else if (event == G_FILE_MONITOR_EVENT_MOVED) { - GFileInfo *info; - info = g_file_query_info (file, grl_pls_get_file_attributes (), G_FILE_QUERY_INFO_NONE, NULL, NULL); - if (file_is_valid_content (info, TRUE, NULL)) { - file_parent = g_file_get_parent (file); - other_file_parent = g_file_get_parent (other_file); + /* File DELETED */ + if (event == G_FILE_MONITOR_EVENT_DELETED) { + gchar *uri; - if (g_file_equal (file_parent, other_file_parent) == 0) { - notify_parent_change (source, file, GRL_CONTENT_CHANGED); - } else { - notify_parent_change (source, file, GRL_CONTENT_REMOVED); - notify_parent_change (source, other_file, GRL_CONTENT_ADDED); - } - g_object_unref (file_parent); - g_object_unref (other_file_parent); + /* Avoid duplicated notification when a directory being monitored is + * deleted. The signal will be emitted by the monitor tracking its parent. + */ + uri = g_file_get_uri (file); + if (g_hash_table_lookup (fs_source->priv->monitors, uri) != monitor) + notify_parent_change (source, file, GRL_CONTENT_REMOVED); + g_free (uri); + + goto out; + } + + /* Query the file and leave if we are not interested in it */ + info = g_file_query_info (file, + grl_pls_get_file_attributes (), + G_FILE_QUERY_INFO_NONE, + NULL, NULL); + if (!info || !file_is_valid_content (info, TRUE, NULL)) + goto out; + + /* File CHANGED */ + if (event == G_FILE_MONITOR_EVENT_CHANGED) { + notify_parent_change (source, file, GRL_CONTENT_CHANGED); + goto out; + } + + /* File CREATED */ + if (event == G_FILE_MONITOR_EVENT_CREATED) { + notify_parent_change (source, file, GRL_CONTENT_ADDED); + if (g_file_info_get_file_type (info) == G_FILE_TYPE_DIRECTORY) + add_monitor (GRL_FILESYSTEM_SOURCE (source), file); + goto out; + } + + /* File MOVED */ + if (event == G_FILE_MONITOR_EVENT_MOVED) { + GFile *file_parent = g_file_get_parent (file); + GFile *other_file_parent = g_file_get_parent (other_file); + + if (g_file_equal (file_parent, other_file_parent)) { + notify_parent_change (source, file, GRL_CONTENT_CHANGED); + } else { + notify_parent_change (source, file, GRL_CONTENT_REMOVED); + notify_parent_change (source, other_file, GRL_CONTENT_ADDED); } + + g_object_unref (file_parent); + g_object_unref (other_file_parent); + goto out; } + +out: + g_clear_object (&info); } static void cancel_monitors (GrlFilesystemSource *fs_source) { - g_list_foreach (fs_source->priv->monitors, - (GFunc) g_file_monitor_cancel, - NULL); - g_list_free_full (fs_source->priv->monitors, g_object_unref); - fs_source->priv->monitors = NULL; + /* That table holds the only ref to our GFileMonitor, and dispose will + * cancel them. */ + g_hash_table_remove_all (fs_source->priv->monitors); } static void add_monitor (GrlFilesystemSource *fs_source, GFile *dir) { GFileMonitor *monitor; + gchar *uri; + + uri = g_file_get_uri (dir); + if (g_hash_table_contains (fs_source->priv->monitors, uri)) + goto out; monitor = g_file_monitor_directory (dir, G_FILE_MONITOR_SEND_MOVED, NULL, NULL); - if (monitor) { - fs_source->priv->monitors = g_list_prepend (fs_source->priv->monitors, - monitor); - g_signal_connect (monitor, - "changed", - G_CALLBACK (directory_changed), - fs_source); - } else { - char *uri; - uri = g_file_get_uri (dir); + if (!monitor) { GRL_DEBUG ("Unable to set up monitor in %s\n", uri); - g_free (uri); + goto out; } + + /* transfer ownership of uri and monitor */ + g_hash_table_insert (fs_source->priv->monitors, uri, monitor); + g_signal_connect (monitor, + "changed", + G_CALLBACK (directory_changed), + fs_source); + uri = NULL; + +out: + g_free (uri); } static gboolean -- cgit v1.2.1