NautilusPropertyDialog: Fix problems with fast group/owner changes

If the owner or group of a file was changed twice very quicky then
the second change would cancel the first. However, the code assumed
that after the nautilus_file_cancel() call the result of the first
change would not be called. That is not true anymore, and when
it did get called it tried to stop the timed wait, which gave a warning,
and in general touched stuff that the new operation should only touch.

We fix this by making A group/owner change its own struct which we
delay freeing until the operation has actually finished, although we
track if it has been cancelled so we can avoid multiple calls to
eel_timed_wait_stop().

1 files changed, 145 insertions, 167 deletions

diff --git a/src/nautilus-properties-window.c b/src/nautilus-properties-window.c
index 3b15dd997..c57b4a544 100644
--- a/src/nautilus-properties-window.c
+++ b/src/nautilus-properties-window.c
@@ -88,6 +88,22 @@
 static GHashTable *windows;
 static GHashTable *pending_lists;
 
+typedef struct {
+	NautilusFile *file;
+	char         *owner;
+	GtkWindow    *window;
+	unsigned int  timeout;
+	gboolean      cancelled;
+} OwnerChange;
+
+typedef struct {
+	NautilusFile *file;
+	char         *group;
+	GtkWindow    *window;
+	unsigned int  timeout;
+	gboolean      cancelled;
+} GroupChange;
+
 struct NautilusPropertiesWindowDetails {	
 	GList *original_files;
 	GList *target_files;
@@ -111,12 +127,8 @@ struct NautilusPropertiesWindowDetails {
 	guint update_directory_contents_timeout_id;
 	guint update_files_timeout_id;
 
-	NautilusFile *group_change_file;
-	char         *group_change_group;
-	unsigned int  group_change_timeout;
-	NautilusFile *owner_change_file;
-	char         *owner_change_owner;
-	unsigned int  owner_change_timeout;
+	GroupChange  *group_change;
+	OwnerChange  *owner_change;
 
 	GList *permission_buttons;
 	GList *permission_combos;
@@ -207,8 +219,8 @@ static void properties_window_update              (NautilusPropertiesWindow *win
 						   GList              *files);
 static void is_directory_ready_callback           (NautilusFile       *file,
 						   gpointer            data);
-static void cancel_group_change_callback          (NautilusPropertiesWindow *window);
-static void cancel_owner_change_callback          (NautilusPropertiesWindow *window);
+static void cancel_group_change_callback          (GroupChange        *change);
+static void cancel_owner_change_callback          (OwnerChange        *change);
 static void parent_widget_destroyed_callback      (GtkWidget          *widget,
 						   gpointer            callback_data);
 static void select_image_button_callback          (GtkWidget          *widget,
@@ -1368,78 +1380,70 @@ attach_ellipsizing_value_field (NautilusPropertiesWindow *window,
 }
 
 static void
+group_change_free (GroupChange *change)
+{
+	nautilus_file_unref (change->file);
+	g_free (change->group);
+	g_object_unref (change->window);
+
+	g_free (change);
+}
+
+static void
 group_change_callback (NautilusFile *file,
 		       GFile *res_loc,
 		       GError *error,
-		       NautilusPropertiesWindow *window)
+		       GroupChange *change)
 {
-	char *group;
-
-	g_assert (NAUTILUS_IS_PROPERTIES_WINDOW (window));
-	g_assert (window->details->group_change_file == file);
+	NautilusPropertiesWindow *window;
 
-	group = window->details->group_change_group;
-	g_assert (group != NULL);
+	g_assert (NAUTILUS_IS_PROPERTIES_WINDOW (change->window));
+	g_assert (NAUTILUS_IS_FILE (change->file));
+	g_assert (change->group != NULL);
 
-	/* Report the error if it's an error. */
-	eel_timed_wait_stop ((EelCancelCallback) cancel_group_change_callback, window);
-	nautilus_report_error_setting_group (file, error, GTK_WINDOW (window));
+	if (!change->cancelled) {
+		/* Report the error if it's an error. */
+		eel_timed_wait_stop ((EelCancelCallback) cancel_group_change_callback, change);
+		nautilus_report_error_setting_group (change->file, error, change->window);
+	}
 
-	nautilus_file_unref (file);
-	g_free (group);
+	window = NAUTILUS_PROPERTIES_WINDOW(change->window);
+	if (window->details->group_change == change) {
+		window->details->group_change = NULL;
+	}
 
-	window->details->group_change_file = NULL;
-	window->details->group_change_group = NULL;
-	g_object_unref (G_OBJECT (window));
+	group_change_free (change);
 }
 
 static void
-cancel_group_change_callback (NautilusPropertiesWindow *window)
+cancel_group_change_callback (GroupChange *change)
 {
-	NautilusFile *file;
-	char *group;
-
-	file = window->details->group_change_file;
-	g_assert (NAUTILUS_IS_FILE (file));
-
-	group = window->details->group_change_group;
-	g_assert (group != NULL);
-
-	nautilus_file_cancel (file, (NautilusFileOperationCallback) group_change_callback, window);
+	g_assert (NAUTILUS_IS_FILE (change->file));
+	g_assert (change->group != NULL);
 
-	g_free (group);
-	nautilus_file_unref (file);
-
-	window->details->group_change_file = NULL;
-	window->details->group_change_group = NULL;
-	g_object_unref (window);
+	change->cancelled = TRUE;
+	nautilus_file_cancel (change->file, (NautilusFileOperationCallback) group_change_callback, change);
 }
 
 static gboolean
-schedule_group_change_timeout (NautilusPropertiesWindow *window)
+schedule_group_change_timeout (GroupChange *change)
 {
-	NautilusFile *file;
-	char *group;
+	g_assert (NAUTILUS_IS_PROPERTIES_WINDOW (change->window));
+	g_assert (NAUTILUS_IS_FILE (change->file));
+	g_assert (change->group != NULL);
 
-	g_assert (NAUTILUS_IS_PROPERTIES_WINDOW (window));
-
-	file = window->details->group_change_file;
-	g_assert (NAUTILUS_IS_FILE (file));
-
-	group = window->details->group_change_group;
-	g_assert (group != NULL);
+	change->timeout = 0;
 
 	eel_timed_wait_start
 		((EelCancelCallback) cancel_group_change_callback,
-		 window,
+		 change,
 		 _("Cancel Group Change?"),
-		 GTK_WINDOW (window));
+		 change->window);
 
 	nautilus_file_set_group
-		(file,  group,
-		 (NautilusFileOperationCallback) group_change_callback, window);
+		(change->file, change->group,
+		 (NautilusFileOperationCallback) group_change_callback, change);
 
-	window->details->group_change_timeout = 0;
 	return FALSE;
 }
 
@@ -1448,55 +1452,45 @@ schedule_group_change (NautilusPropertiesWindow *window,
 		       NautilusFile       *file,
 		       const char         *group)
 {
+	GroupChange *change;
+
 	g_assert (NAUTILUS_IS_PROPERTIES_WINDOW (window));
-	g_assert (window->details->group_change_group == NULL);
-	g_assert (window->details->group_change_file == NULL);
+	g_assert (window->details->group_change == NULL);
 	g_assert (NAUTILUS_IS_FILE (file));
 
-	window->details->group_change_file = nautilus_file_ref (file);
-	window->details->group_change_group = g_strdup (group);
-	g_object_ref (G_OBJECT (window));
-	window->details->group_change_timeout =
+	change = g_new0 (GroupChange, 1);
+
+	change->file = nautilus_file_ref (file);
+	change->group = g_strdup (group);
+	change->window = g_object_ref (G_OBJECT (window));
+	change->timeout =
 		g_timeout_add (CHOWN_CHGRP_TIMEOUT,
 			       (GSourceFunc) schedule_group_change_timeout,
-			       window);
+			       change);
+
+	window->details->group_change = change;
 }
 
 static void
 unschedule_or_cancel_group_change (NautilusPropertiesWindow *window)
 {
-	NautilusFile *file;
-	char *group;
+	GroupChange *change;
 
 	g_assert (NAUTILUS_IS_PROPERTIES_WINDOW (window));
 
-	file = window->details->group_change_file;
-	group = window->details->group_change_group;
-
-	g_assert ((file == NULL && group == NULL) ||
-		  (file != NULL && group != NULL));
+	change = window->details->group_change;
 
-	if (file != NULL) {
-		g_assert (NAUTILUS_IS_FILE (file));
-
-		if (window->details->group_change_timeout == 0) {
-			nautilus_file_cancel (file,
-					      (NautilusFileOperationCallback) group_change_callback, window);
-			eel_timed_wait_stop ((EelCancelCallback) cancel_group_change_callback, window);
+	if (change != NULL) {
+		if (change->timeout == 0) {
+			/* The operation was started, cancel it and let the operation callback free the change */
+			cancel_group_change_callback (change);
+			eel_timed_wait_stop ((EelCancelCallback) cancel_group_change_callback, change);
+		} else {
+			g_source_remove (change->timeout);
+			group_change_free (change);
 		}
 
-		nautilus_file_unref (file);
-		g_free (group);
-
-		window->details->group_change_file = NULL;
-		window->details->group_change_group = NULL;
-		g_object_unref (G_OBJECT (window));
-	}
-
-	if (window->details->group_change_timeout > 0) {
-		g_assert (file != NULL);
-		g_source_remove (window->details->group_change_timeout);
-		window->details->group_change_timeout = 0;
+		window->details->group_change = NULL;
 	}
 }
 
@@ -1783,78 +1777,70 @@ attach_group_combo_box (GtkGrid *grid,
 }	
 
 static void
+owner_change_free (OwnerChange *change)
+{
+	nautilus_file_unref (change->file);
+	g_free (change->owner);
+	g_object_unref (change->window);
+
+	g_free (change);
+}
+
+static void
 owner_change_callback (NautilusFile *file,
                        GFile 	    *result_location,
 		       GError        *error,
-		       NautilusPropertiesWindow *window)
+		       OwnerChange *change)
 {
-	char *owner;
-
-	g_assert (NAUTILUS_IS_PROPERTIES_WINDOW (window));
-	g_assert (window->details->owner_change_file == file);
+	NautilusPropertiesWindow *window;
 
-	owner = window->details->owner_change_owner;
-	g_assert (owner != NULL);
+	g_assert (NAUTILUS_IS_PROPERTIES_WINDOW (change->window));
+	g_assert (NAUTILUS_IS_FILE (change->file));
+	g_assert (change->owner != NULL);
 
-	/* Report the error if it's an error. */
-	eel_timed_wait_stop ((EelCancelCallback) cancel_owner_change_callback, window);
-	nautilus_report_error_setting_owner (file, error, GTK_WINDOW (window));
+	if (!change->cancelled) {
+		/* Report the error if it's an error. */
+		eel_timed_wait_stop ((EelCancelCallback) cancel_owner_change_callback, change);
+		nautilus_report_error_setting_owner (file, error, change->window);
+	}
 
-	nautilus_file_unref (file);
-	g_free (owner);
+	window = NAUTILUS_PROPERTIES_WINDOW(change->window);
+	if (window->details->owner_change == change) {
+		window->details->owner_change = NULL;
+	}
 
-	window->details->owner_change_file = NULL;
-	window->details->owner_change_owner = NULL;
-	g_object_unref (G_OBJECT (window));
+	owner_change_free (change);
 }
 
 static void
-cancel_owner_change_callback (NautilusPropertiesWindow *window)
+cancel_owner_change_callback (OwnerChange *change)
 {
-	NautilusFile *file;
-	char *owner;
-
-	file = window->details->owner_change_file;
-	g_assert (NAUTILUS_IS_FILE (file));
+	g_assert (NAUTILUS_IS_FILE (change->file));
+	g_assert (change->owner != NULL);
 
-	owner = window->details->owner_change_owner;
-	g_assert (owner != NULL);
-
-	nautilus_file_cancel (file, (NautilusFileOperationCallback) owner_change_callback, window);
-
-	nautilus_file_unref (file);
-	g_free (owner);
-
-	window->details->owner_change_file = NULL;
-	window->details->owner_change_owner = NULL;
-	g_object_unref (window);
+	change->cancelled = TRUE;
+	nautilus_file_cancel (change->file, (NautilusFileOperationCallback) owner_change_callback, change);
 }
 
 static gboolean
-schedule_owner_change_timeout (NautilusPropertiesWindow *window)
+schedule_owner_change_timeout (OwnerChange *change)
 {
-	NautilusFile *file;
-	char *owner;
+	g_assert (NAUTILUS_IS_PROPERTIES_WINDOW (change->window));
+	g_assert (NAUTILUS_IS_FILE (change->file));
+	g_assert (change->owner != NULL);
 
-	g_assert (NAUTILUS_IS_PROPERTIES_WINDOW (window));
-
-	file = window->details->owner_change_file;
-	g_assert (NAUTILUS_IS_FILE (file));
-
-	owner = window->details->owner_change_owner;
-	g_assert (owner != NULL);
+	change->timeout = 0;
 
 	eel_timed_wait_start
 		((EelCancelCallback) cancel_owner_change_callback,
-		 window,
+		 change,
 		 _("Cancel Owner Change?"),
-		 GTK_WINDOW (window));
+		 change->window);
 
 	nautilus_file_set_owner
-		(file,  owner,
-		 (NautilusFileOperationCallback) owner_change_callback, window);
+		(change->file, change->owner,
+		 (NautilusFileOperationCallback) owner_change_callback, change);
 
-	window->details->owner_change_timeout = 0;
 	return FALSE;
 }
 
@@ -1863,55 +1849,47 @@ schedule_owner_change (NautilusPropertiesWindow *window,
 		       NautilusFile       *file,
 		       const char         *owner)
 {
+	OwnerChange *change;
+
 	g_assert (NAUTILUS_IS_PROPERTIES_WINDOW (window));
-	g_assert (window->details->owner_change_owner == NULL);
-	g_assert (window->details->owner_change_file == NULL);
+	g_assert (window->details->owner_change == NULL);
 	g_assert (NAUTILUS_IS_FILE (file));
 
-	window->details->owner_change_file = nautilus_file_ref (file);
-	window->details->owner_change_owner = g_strdup (owner);
-	g_object_ref (G_OBJECT (window));
-	window->details->owner_change_timeout =
+	change = g_new0 (OwnerChange, 1);
+
+	change->file = nautilus_file_ref (file);
+	change->owner = g_strdup (owner);
+	change->window = g_object_ref (G_OBJECT (window));
+	change->timeout =
 		g_timeout_add (CHOWN_CHGRP_TIMEOUT,
 			       (GSourceFunc) schedule_owner_change_timeout,
-			       window);
+			       change);
+
+	window->details->owner_change = change;
 }
 
 static void
 unschedule_or_cancel_owner_change (NautilusPropertiesWindow *window)
 {
-	NautilusFile *file;
-	char *owner;
+	OwnerChange *change;
 
 	g_assert (NAUTILUS_IS_PROPERTIES_WINDOW (window));
 
-	file = window->details->owner_change_file;
-	owner = window->details->owner_change_owner;
+	change = window->details->owner_change;
 
-	g_assert ((file == NULL && owner == NULL) ||
-		  (file != NULL && owner != NULL));
+	if (change != NULL) {
+		g_assert (NAUTILUS_IS_FILE (change->file));
 
-	if (file != NULL) {
-		g_assert (NAUTILUS_IS_FILE (file));
-
-		if (window->details->owner_change_timeout == 0) {
-			nautilus_file_cancel (file,
-					      (NautilusFileOperationCallback) owner_change_callback, window);
-			eel_timed_wait_stop ((EelCancelCallback) cancel_owner_change_callback, window);
+		if (change->timeout == 0) {
+			/* The operation was started, cancel it and let the operation callback free the change */
+			cancel_owner_change_callback (change);
+			eel_timed_wait_stop ((EelCancelCallback) cancel_owner_change_callback, change);
+		} else {
+			g_source_remove (change->timeout);
+			owner_change_free (change);
 		}
 
-		nautilus_file_unref (file);
-		g_free (owner);
-
-		window->details->owner_change_file = NULL;
-		window->details->owner_change_owner = NULL;
-		g_object_unref (G_OBJECT (window));
-	}
-
-	if (window->details->owner_change_timeout > 0) {
-		g_assert (file != NULL);
-		g_source_remove (window->details->owner_change_timeout);
-		window->details->owner_change_timeout = 0;
+		window->details->owner_change = NULL;
 	}
 }