Re: Reverting to non-async file chooser

[Federico Mena Quintero <federico ximian com>]

On Thu, 2006-08-17 at 20:03 -0400, Matthias Clasen wrote:

> I hate to say it, but the blame for that goes largely to the person
> doing the merge.

So yes, it is in large part my fault :(

Right after Kris merged his async branch into HEAD, I did run
autotestfilechooser, but erroneously assumed that failures were "just
bugs" that would be fixed in time.  That did not happen.

Then, I merged my location-entry branch and again some tests didn't
pass, and I didn't fix them right away.

[In my feeble defense, I did ask Kris to write a test suite for the
async calls right when he was starting to do the async work; that didn't
happen either.  I should have been more anal^H^H^H^Hresponsible and not
taken the async patches until that got done.]

> > gtkfilesystemgnomevfs.c:979: warning: format '%s' expects type 'char *',
> > but argument 5 has type 'struct GnomeVFSURI *'
> 
> Ugly, but fixable and hardly a reason to throw the child out with the
> bath water.

Fixed.  Was a thinko with a similarly-named struct field.

I admit to getting a twitchy trigger finger when things get this broken.

> I don't think changing the filechooser backend ABI like this in the middle of
> a stable series is compatible with the stability guarantees we associate with
> stable GTK+ releases.

OK, so here's the plan of action.

Semantics of callbacks and cancellation
=======================================

- You need to hit the idle loop for your callback to be called, just
  like in GnomeVFS.

- You do not need to ever ref/unref async handles; they are opaque
  cookies.

- Once you call gtk_file_system_cancel_operation(), you can assume
  that the callback corresponding to the handle will NOT be called.
  Therefore, the caller must free the stuff in the callback's closure
  as appropriate.

  Rationale:  the idle loop may be far away in time.  By that time, it
  is very likely that the closure of the callback is no longer valid.
  So, we want to get rid of it as soon as possible.

  Caveat:  gnome-vfs does not let you cancel the operation of mounting
  a drive or volume.  So if you cancel the handle from
  gtk_file_system_volume_mount(), your callback will not be called,
  but the volume *will* be mounted (unless an error occurs during
  mounting, of course).

  Notes:  The current scheme (callbacks get called even when
  cancelled; caller must unref the handle in the callback) doesn't
  make sense.  Callbacks test if the handle being passed is the same
  that got remembered; this causes leaks if an old handle had been
  canceled but the idle loop did not run yet.


Plan of action
==============

1. Add assertions in GtkFileSystemGnomeVFS so that no async handles
   are outstanding when the file_system's finalizer gets run.  DONE.

2. Add assertions to GtkFileSystemUnix in a similar fashion.

3. Write a test suite for the async operations of GtkFileSystem,
   similar in spirit to gnome-vfs/test/test-async-cancel.c.  IN PROGRESS.

   [I think op_data is being leaked quite often in
   gtkfilesystemgnomevfs.c, but haven't proved it yet.]

4. Run that test suite against GtkFileSystemGnomeVFS.

5. Run that test suite against GtkFileSystemUnix.

6. Fix gtkfilechooser*.c to handle the new semantics for async
   handles.  See the patches in these bugs:

      http://bugzilla.gnome.org/show_bug.cgi?id=349998
      http://bugzilla.gnome.org/show_bug.cgi?id=348113

7. Ensure that autotestfilechooser passes all tests.  This will take
   care of the big overwrite-confirmation bug:

      http://bugzilla.gnome.org/show_bug.cgi?id=347883

8. ???

9. Profit!!!

I'm getting kind of nervous that RC1 for GNOME 2.16 is a few days away.
Help is much appreciated to finish the tasks above that are not done.

  Federico