Another chunk of review of kris-async-branch

[Federico Mena Quintero <federico ximian com>]

Hi, Kris,

Sorry that this is taking so long.  Here are more comments from
gtkfilechooserdialog and gtkfilechooserdefault:

- gtkpathbar.h

	Looks good.

- gtkpathbar.c

	Looks good.

- gtkfilechooserentry.h

	Looks good.

- gtkfilechooserentry.c

	Looks good.

- gtkfilechooserembed.h

	Looks good.

- gtkfilechooserembed.c

	Looks good.

- gtkfilechooserdialog.c

  + In response_cb():

	if (!priv->response_requested && !_gtk_file_chooser_embed_should_respond (GTK_FILE_CHOOSER_EMBED (priv->widget)))
	  g_signal_stop_emission_by_name (dialog, "response");

	You need to reset priv->response_requested to FALSE after
	this, for apps that re-use the same file chooser dialog.

	The variable *probably* needs to be reset in a connect_after
	handler.  I'm not sure what happens if you click on the "Open"
	button many times before the async layer wakes up, for example.

+ gtkfilechooserbutton.c

+ gtkfilechooserdefault.c

  + In shortcuts_reload_icons_get_info_cb():

	+  if (!g_slist_find (data->impl->reload_icon_handles, handle))
	+    goto out;

    You leak the handle.

  + In shortcuts_reload_icons():

	      info = g_new0 (struct ReloadIconsData, 1);
	      info->impl = g_object_ref (impl);
	      tree_path = gtk_tree_model_get_path (GTK_TREE_MODEL (impl->shortcuts_model), &iter);
	      info->row_ref = gtk_tree_row_reference_new (GTK_TREE_MODEL (impl->shortcuts_model), tree_path);
	      gtk_tree_path_free (tree_path);

    At the beginning of that function, you cancel all the pending
    handles.  But you never unref() the impl, not even in the
    callback.  Also, doesn't this in the callback

	  if (!g_slist_find (data->impl->reload_icon_handles, handle))
	    goto out;

    mean that you never really free the ReloadIconsData associated
    with handles?  The list no longer contains the old handles, or
    much worse, it has aliased pointers to new handles.

  + In shortcuts_remove_rows():

	+      shortcuts_free_row_data (impl, &iter);
	+      gtk_list_store_remove (impl->shortcuts_model, &iter);

    shortucts_free_row_data() will cancel the handle.  But what
    happens to the row_reference once the callback looks for it?  [Am I
    on crack?]

  + In get_file_info_finished():

	I'm thinking of not doing any validation at all for bookmarks,
	and just to let any valid path be put in there.  This will
	help with some things:

		- Bookmark files.  This will change shortly with
                  Emmanuele's RecentFilesAndBookmarks stuff, but
                  whatever.

		- Be friendlier to being disconnected from the
                  network.

		- Bookmarks to removable media?

	Other than that, looks good but it scares me that bookmarks
	got so complex :)

I hope to finish gtkfilechooserdefault today.  It's getting quite complex;
we'll have to refactor it after it gets merged.

This is *awesome* work.  Thanks again for tackling this big problem :)

  Federico