bgo#687196 - Fix model corruption during file removal

The main problem is that we were emitting the row-deleted signal for the model in the middle
of the process that actually deletes the row from the model (remove the row from the array,
update the model->file_lookup hash table, etc.).  In the model's caller, one of the row-deleted
callbacks was requesting an iter, which caused the model to revalidate itself - but it did
this while it was in an inconsistent state.  This led to an assertion failure later when the
model resorted itself.

The fix in remove_file() is like this:

* The filteredness/visibility of the deleted node is not updated.  The
  node will simply be gone; we don't need to update those values at
  all.

* We invalidate just the node that is being deleted.

* The model->file_lookup hash table is not completely nuked; instead,
  we carefully adjust its indices.

* The row-deleted signal is only emitted at the very end, when
  deletion is complete and the model is consistent.

Many thanks to William Hua for doing the detective work on this bug!

Signed-off-by: Federico Mena Quintero <federico@gnome.org>
This commit is contained in:
Federico Mena Quintero 2012-11-01 17:48:23 -06:00
parent 94e3d7faf1
commit ea3a750f13

View File

@ -1807,6 +1807,33 @@ _gtk_file_system_model_get_iter_for_file (GtkFileSystemModel *model,
return TRUE; return TRUE;
} }
/* When an element is added or removed to the model->files array, we need to
* update the model->file_lookup mappings of (node, index), as the indexes
* change. This function adds the specified increment to the index in that pair
* if the index is equal or after the specified id. We use this to slide the
* mappings up or down when a node is added or removed, respectively.
*/
static void
adjust_file_lookup (GtkFileSystemModel *model, guint id, int increment)
{
GHashTableIter iter;
gpointer key;
gpointer value;
g_hash_table_iter_init (&iter, model->file_lookup);
while (g_hash_table_iter_next (&iter, &key, &value))
{
guint index = GPOINTER_TO_UINT (value);
if (index >= id)
{
index += increment;
g_hash_table_iter_replace (&iter, GUINT_TO_POINTER (index));
}
}
}
/** /**
* add_file: * add_file:
* @model: the model * @model: the model
@ -1857,6 +1884,7 @@ remove_file (GtkFileSystemModel *model,
{ {
FileModelNode *node; FileModelNode *node;
guint id; guint id;
guint row;
g_return_if_fail (GTK_IS_FILE_SYSTEM_MODEL (model)); g_return_if_fail (GTK_IS_FILE_SYSTEM_MODEL (model));
g_return_if_fail (G_IS_FILE (file)); g_return_if_fail (G_IS_FILE (file));
@ -1866,17 +1894,22 @@ remove_file (GtkFileSystemModel *model,
return; return;
node = get_node (model, id); node = get_node (model, id);
node_set_visible_and_filtered_out (model, id, FALSE, FALSE); row = node_get_tree_row (model, id);
node_invalidate_index (model, id);
g_hash_table_remove (model->file_lookup, file); g_hash_table_remove (model->file_lookup, file);
g_object_unref (node->file); g_object_unref (node->file);
adjust_file_lookup (model, id, -1);
if (node->info) if (node->info)
g_object_unref (node->info); g_object_unref (node->info);
g_array_remove_index (model->files, id); g_array_remove_index (model->files, id);
g_hash_table_remove_all (model->file_lookup);
/* We don't need to resort, as removing a row doesn't change the sorting order */ /* We don't need to resort, as removing a row doesn't change the sorting order of the other rows */
emit_row_deleted_for_row (model, row);
} }
/** /**