Bug 220562

The bug that triggered this change was a case of a library ignoring the return
value of gdk_pixbuf_loader_write and feeding more garbage into the loader after it 
already returned FALSE. This unfortunately triggered a crash inside one of the
laoders. 

So I thought it would be better to make write() behave as documented and close
the loader when it returns FALSE. This also shows that you are not correct, it
is in fact possible for a user of this api to know if the loader is closed -
simply pay attention to the return value of write(). Anyway, a further change
that I have already made in upstream cvs is to in fact make close() silently
return if the loader is already closed. I'll push that out as an FC6 update
tonight.

I wasn't sure if it is a good idea to emit the closed signal when the loader
is closed due to error, but I can do that too.

(In reply to comment #1)
> This also shows that you are not correct, it
> is in fact possible for a user of this api to know if the loader is closed -
> simply pay attention to the return value of write().

No, I don't know because I cannot tell whether I use a version of Gtk+ that
actually closes it, or version that does not close it.

> Anyway, a further change
> that I have already made in upstream cvs is to in fact make close() silently
> return if the loader is already closed.

Thanks.

> I wasn't sure if it is a good idea to emit the closed signal when the loader
> is closed due to error, but I can do that too.

I cannot tell how other people write their programs, but if they use "closed", I
suppose they bound some clean-up action to it.  For years, they have been
calling gdk_pixbuf_loader_close() themselves after failure -- because not doing
so  used to lead to

GdkPixbuf-WARNING **: GdkPixbufLoader finalized without calling
gdk_pixbuf_loader_close() - this is not allowed. You must explicitly end the
data stream to the loader before dropping the last reference

-- and their clean up actions were always run because "closed" was always
emitted before finalization of the loader.  Now the pixbuf loader closes itself
on failure, further gdk_pixbuf_loader_close() just returns (once it's fixed),
"closed" is not emitted and the clean-up action is not run.

On second thought, emitting "closed" in failure-incuded close makes it happen in
different time than it used to, so some assertions in its handler can fail.  To
make it really backward compatible, one would have to add a flag to priv whether
the loader was closed *explicitly* by calling gdk_pixbuf_loader_close().  It
could then:

1. If it was not closed at all, behave normally and set closed and the new flag.

2. If it was closed implicitly, do nothing except emitting "closed" and setting
the flag.

3. If it was closed explicitly, fail noisily.

So this also allows preservation of an analogue to

g_return_val_if_fail (priv->closed == FALSE, TRUE);

Of course, it is a matter of how you value backward compatibility...

Please try version thats in updates-testing now, and tell me if you see any
further problems with it. I'll leave the signal emission where it is now unless
it causes problems.

I can see only 2.10.4-9 in testing updates with an unrelated changelog

* Thu Dec 21 2006 Matthias Clasen <mclasen> - 2.10.4-9
- Make update scripts handle slight variations in $host

and it behaves the same.  I suppose I should wait for another testing update
(2.10.4-10?) to get to the mirrors.

OK, with gtk2-2.10.4-10.fc6 programs expecting both the old behaviour and the
behaviour specified in the documentation seem to work and print no warnings.