[nautilus] batch-rename-dialog: Simplify name conflicts check

[António Fernandes <antoniof src gnome org>]

commit 72ebd7af89b532d58843361711be24516e85340e
Author: Sachin Daluja <30343-sachindaluja users noreply gitlab gnome org>
Date:   Thu Jan 14 08:54:01 2021 -0500

    batch-rename-dialog: Simplify name conflicts check
    
    We were spawning a background thread for performing the name conflict
    check. Since the logic and data used for checking name conflicts
    is not thread-safe we were executing the actual conflict check function
    in the main thread (via g_main_context_invoke()). That routine in turn
    ended up scheduling the actual work to execute in the callback
    on_directory_conflicts_ready() per distinct parent directory. Thus
    almost all the computationally intensive work was done via multiple
    layers of indirection in the main thread.
    
    Modify file_names_list_has_duplicates_async() to simply schedule the
    actual name check function to be called per distict parent directory.
    As before, most of the computationally intensive work is still done in
    the callback registered via nautilus_directory_call_when_ready().
    This greatly simplifies the design of the name conflict check by
    eliminating the need for thread-safe synchronization of access and
    modification of data that is used in the check.

 src/nautilus-batch-rename-dialog.c | 205 ++++++++-----------------------------
 1 file changed, 41 insertions(+), 164 deletions(-)
---
diff --git a/src/nautilus-batch-rename-dialog.c b/src/nautilus-batch-rename-dialog.c
index dde67158e..3464045c3 100644
--- a/src/nautilus-batch-rename-dialog.c
+++ b/src/nautilus-batch-rename-dialog.c
@@ -90,8 +90,7 @@ struct _NautilusBatchRenameDialog
 
     GList *duplicates;
     GList *distinct_parent_directories;
-    GCancellable *conflict_cancellable;
-    gboolean checking_conflicts;
+    GList *directories_pending_conflict_check;
 
     /* this hash table has information about the status
      * of all tags: availability, if it's currently used
@@ -119,6 +118,7 @@ typedef struct
 
 
 static void     update_display_text (NautilusBatchRenameDialog *dialog);
+static void     cancel_conflict_check (NautilusBatchRenameDialog *self);
 
 G_DEFINE_TYPE (NautilusBatchRenameDialog, nautilus_batch_rename_dialog, GTK_TYPE_DIALOG);
 
@@ -667,7 +667,7 @@ prepare_batch_rename (NautilusBatchRenameDialog *dialog)
 
     /* wait for checking conflicts to finish, to be sure that
      * the rename can actually take place */
-    if (dialog->checking_conflicts)
+    if (dialog->directories_pending_conflict_check != NULL)
     {
         dialog->rename_clicked = TRUE;
         return;
@@ -693,12 +693,6 @@ prepare_batch_rename (NautilusBatchRenameDialog *dialog)
     gtk_widget_hide (GTK_WIDGET (dialog));
     begin_batch_rename (dialog, dialog->new_names);
 
-    if (dialog->conflict_cancellable)
-    {
-        g_cancellable_cancel (dialog->conflict_cancellable);
-        g_clear_object (&dialog->conflict_cancellable);
-    }
-
     gtk_widget_destroy (GTK_WIDGET (dialog));
 }
 
@@ -713,10 +707,9 @@ batch_rename_dialog_on_response (NautilusBatchRenameDialog *dialog,
     }
     else
     {
-        if (dialog->conflict_cancellable)
+        if (dialog->directories_pending_conflict_check != NULL)
         {
-            g_cancellable_cancel (dialog->conflict_cancellable);
-            g_clear_object (&dialog->conflict_cancellable);
+            cancel_conflict_check (dialog);
         }
 
         gtk_widget_destroy (GTK_WIDGET (dialog));
@@ -1177,174 +1170,70 @@ check_conflict_for_files (NautilusBatchRenameDialog *dialog,
     g_hash_table_destroy (names_conflicts_table);
 }
 
-static gboolean
-file_names_list_has_duplicates_finish (NautilusBatchRenameDialog  *self,
-                                       GAsyncResult               *res,
-                                       GError                    **error)
-{
-    g_return_val_if_fail (g_task_is_valid (res, self), FALSE);
-
-    return g_task_propagate_boolean (G_TASK (res), error);
-}
-
-static void
-on_file_names_list_has_duplicates (GObject      *object,
-                                   GAsyncResult *res,
-                                   gpointer      user_data)
-{
-    NautilusBatchRenameDialog *self;
-    GError *error = NULL;
-    gboolean success;
-
-    self = NAUTILUS_BATCH_RENAME_DIALOG (object);
-    success = file_names_list_has_duplicates_finish (self, res, &error);
-
-    if (!success)
-    {
-        g_clear_error (&error);
-        return;
-    }
-
-    self->duplicates = g_list_reverse (self->duplicates);
-    self->checking_conflicts = FALSE;
-    update_listbox (self);
-}
-
-typedef struct
-{
-    GList *directories;
-    NautilusDirectory *current_directory;
-    GMutex wait_ready_mutex;
-    GCond wait_ready_condition;
-    gboolean directory_conflicts_ready;
-} CheckConflictsData;
-
 static void
 on_directory_conflicts_ready (NautilusDirectory *conflict_directory,
                               GList             *files,
                               gpointer           callback_data)
 {
     NautilusBatchRenameDialog *self;
-    GTask *task;
-    CheckConflictsData *task_data;
-    GCancellable *cancellable;
 
-    task = G_TASK (callback_data);
-    task_data = g_task_get_task_data (task);
-    cancellable = g_task_get_cancellable (task);
-    self = NAUTILUS_BATCH_RENAME_DIALOG (g_task_get_source_object (task));
-    if (!g_cancellable_is_cancelled (cancellable))
-    {
-        check_conflict_for_files (self, conflict_directory, files);
-    }
+    self = NAUTILUS_BATCH_RENAME_DIALOG (callback_data);
 
-    g_mutex_lock (&task_data->wait_ready_mutex);
-    task_data->directory_conflicts_ready = TRUE;
-    g_cond_signal (&task_data->wait_ready_condition);
-    g_mutex_unlock (&task_data->wait_ready_mutex);
-}
+    check_conflict_for_files (self, conflict_directory, files);
 
-static gboolean
-check_conflicts_on_main_thread (gpointer user_data)
-{
-    GTask *task = (GTask *) user_data;
-    CheckConflictsData *task_data = g_task_get_task_data (task);
-
-    nautilus_directory_call_when_ready (task_data->current_directory,
-                                        NAUTILUS_FILE_ATTRIBUTE_INFO,
-                                        TRUE,
-                                        on_directory_conflicts_ready,
-                                        task);
-    return FALSE;
-}
+    g_assert (g_list_find (self->directories_pending_conflict_check, conflict_directory) != NULL);
 
-static void
-file_names_list_has_duplicates_async_thread (GTask        *task,
-                                             gpointer      object,
-                                             gpointer      data,
-                                             GCancellable *cancellable)
-{
-    NautilusBatchRenameDialog *self;
-    CheckConflictsData *task_data;
-    GList *l;
-
-    self = g_task_get_source_object (task);
-    task_data = g_task_get_task_data (task);
-    self->duplicates = NULL;
+    self->directories_pending_conflict_check = g_list_remove (self->directories_pending_conflict_check, 
conflict_directory);
 
-    g_mutex_init (&task_data->wait_ready_mutex);
-    g_cond_init (&task_data->wait_ready_condition);
+    nautilus_directory_unref (conflict_directory);
 
-    /* check if this is the last call of the callback */
-    for (l = self->distinct_parent_directories; l != NULL; l = l->next)
+    if (self->directories_pending_conflict_check == NULL)
     {
-        if (g_task_return_error_if_cancelled (task))
-        {
-            return;
-        }
-
-        g_mutex_lock (&task_data->wait_ready_mutex);
-        task_data->directory_conflicts_ready = FALSE;
+        self->duplicates = g_list_reverse (self->duplicates);
 
-
-        task_data->current_directory = l->data;
-        /* NautilusDirectory and NautilusFile are not thread safe, we need to call
-         * them on the main thread.
-         */
-        g_main_context_invoke (NULL, check_conflicts_on_main_thread, task);
-
-        /* We need to block this thread until the call_when_ready call is done,
-         * if not the GTask would finalize. */
-        while (!task_data->directory_conflicts_ready)
-        {
-            g_cond_wait (&task_data->wait_ready_condition,
-                         &task_data->wait_ready_mutex);
-        }
-
-        g_mutex_unlock (&task_data->wait_ready_mutex);
+        update_listbox (self);
     }
-
-    g_task_return_boolean (task, TRUE);
 }
 
 static void
-destroy_conflicts_task_data (gpointer data)
+cancel_conflict_check (NautilusBatchRenameDialog *self)
 {
-    CheckConflictsData *task_data = data;
+    GList *l;
+    NautilusDirectory *directory;
 
-    if (task_data->directories)
+    for (l = self->directories_pending_conflict_check; l != NULL; l = l->next)
     {
-        g_list_free (task_data->directories);
-    }
+        directory = l->data;
 
-    g_mutex_clear (&task_data->wait_ready_mutex);
-    g_cond_clear (&task_data->wait_ready_condition);
+        nautilus_directory_cancel_callback (directory,
+                                            on_directory_conflicts_ready,
+                                            self);
+    }
 
-    g_free (task_data);
+    g_clear_list (&self->directories_pending_conflict_check, g_object_unref);
 }
 
 static void
-file_names_list_has_duplicates_async (NautilusBatchRenameDialog *dialog,
-                                      GAsyncReadyCallback        callback,
-                                      gpointer                   user_data)
+file_names_list_has_duplicates_async (NautilusBatchRenameDialog *self)
 {
-    g_autoptr (GTask) task = NULL;
-    CheckConflictsData *task_data;
+    GList *l;
 
-    if (dialog->checking_conflicts == TRUE)
+    if (self->directories_pending_conflict_check != NULL)
     {
-        g_cancellable_cancel (dialog->conflict_cancellable);
-        g_clear_object (&dialog->conflict_cancellable);
+        cancel_conflict_check (self);
     }
 
-    dialog->conflict_cancellable = g_cancellable_new ();
-
-    dialog->checking_conflicts = TRUE;
+    self->directories_pending_conflict_check = nautilus_directory_list_copy 
(self->distinct_parent_directories);
+    self->duplicates = NULL;
 
-    task = g_task_new (dialog, dialog->conflict_cancellable, callback, user_data);
-    task_data = g_new0 (CheckConflictsData, 1);
-    g_task_set_task_data (task, task_data, destroy_conflicts_task_data);
-    g_task_run_in_thread (task, file_names_list_has_duplicates_async_thread);
+    for (l = self->distinct_parent_directories; l != NULL; l = l->next)
+    {
+        nautilus_directory_call_when_ready (l->data,
+                                            NAUTILUS_FILE_ATTRIBUTE_INFO,
+                                            TRUE,
+                                            on_directory_conflicts_ready,
+                                            self);
+    }
 }
 
 static gboolean
@@ -1486,12 +1375,6 @@ numbering_tag_is_some_added (NautilusBatchRenameDialog *self)
 static void
 update_display_text (NautilusBatchRenameDialog *dialog)
 {
-    if (dialog->conflict_cancellable != NULL)
-    {
-        g_cancellable_cancel (dialog->conflict_cancellable);
-        g_clear_object (&dialog->conflict_cancellable);
-    }
-
     if(dialog->selection == NULL)
     {
         return;
@@ -1524,9 +1407,7 @@ update_display_text (NautilusBatchRenameDialog *dialog)
         return;
     }
 
-    file_names_list_has_duplicates_async (dialog,
-                                          on_file_names_list_has_duplicates,
-                                          NULL);
+    file_names_list_has_duplicates_async (dialog);
 }
 
 static void
@@ -2034,10 +1915,9 @@ nautilus_batch_rename_dialog_finalize (GObject *object)
 
     dialog = NAUTILUS_BATCH_RENAME_DIALOG (object);
 
-    if (dialog->checking_conflicts)
+    if (dialog->directories_pending_conflict_check != NULL)
     {
-        g_cancellable_cancel (dialog->conflict_cancellable);
-        g_clear_object (&dialog->conflict_cancellable);
+        cancel_conflict_check (dialog);
     }
 
     g_clear_object (&dialog->numbering_order_menu);
@@ -2268,13 +2148,10 @@ nautilus_batch_rename_dialog_init (NautilusBatchRenameDialog *self)
 
     self->duplicates = NULL;
     self->distinct_parent_directories = NULL;
+    self->directories_pending_conflict_check = NULL;
     self->new_names = NULL;
-
-    self->checking_conflicts = FALSE;
-
     self->rename_clicked = FALSE;
 
-
     self->tag_info_table = g_hash_table_new_full (g_str_hash,
                                                   g_str_equal,
                                                   (GDestroyNotify) g_free,