Re: Hard code freeze break for Nautilus

[Cosimo Cecchi <cosimoc gnome org>]

On Sun, 2010-09-19 at 20:33 +0200, Vincent Untz wrote:
> Le dimanche 19 septembre 2010, à 20:04 +0200, Cosimo Cecchi a écrit :
> > Hi,
> > 
> > I've been fixing a few nasty crashers in Nautilus master this weekend,
> > and some of these apply to the gnome-2-32 branch too.
> > I created a gnome-2-32-fixes branch [1], which has four commits I'd like
> > to backport to 2.32.0; the most important commit are the top two, which
> > fix two frequent crashers, but I think it'd be nice to have the whole
> > branch merged to gnome-2-32.
> 
> Which ones are the "top two"? The two oldest? Or the two appearing at
> the top on git.g.o? :-)

The top two appearing on git.g.o or in `git log` :)

> Approval/questions for each patch:
> 
>  + "desktop-icon-view: initialize allocation before setting it"
>    I guess this is "just" to get something for width and height?
>    Approval 1 of 2, if yes.

It's a regression from GSEAL-ing. The allocation variable wasn't
initialized before setting it to the widget, and this caused stack
corruption.

>  + "Clear the list model in _finalize() instead of _dispose()"
>    I don't get this one. That's supposed to happen in dispose(), so I'm
>    not sure why moving things in finalize() would be better. Can you
>    elaborate?

Sure; FMListView is a FMDirectoryView. fm_directory_view_end_loading(),
and its subclass implementation fm_list_view_end_loading() might be
called both as a result of a successful directory change or when the
view is destroyed. In the latter case what happens is:

- dispose() is called on FMListView; the model is cleared
- destroy() is called on FMDirectoryView;
- fm_list_view_done_loading() is called, which then calls into the model
- crash

By moving the cleanup of the model in finalize() we ensure that the
model is always there when end_loading() is called.

>  + "pathbar: use another way to remove buttons (#627901)"
>    Was the issue that you were iterating over the list, while it also
>    got changed because of gtk_container_remove()?
>    Approval 1 of 2, if yes.

Yeah, exactly.

Thanks,

Cosimo