GNOME Bugzilla – Bug 749012
GtkStack: Don't emit notify::visible-child during destruction
Last modified: 2016-12-19 23:36:27 UTC
GtkStack emits notify::visible-child during destruction - after dispose is run and before finalize. This can be a bit surprising for applications [1]. So either it shouldn't do that, or it should be clearly documented. If possible, I would prefer the former. [1] https://git.gnome.org/browse/gnome-photos/tree/src/photos-embed.c#n576
Created attachment 302978 [details] [review] GtkStack: Don't emit notify::visible-child during destruction
Created attachment 302979 [details] Sample program
Review of attachment 302978 [details] [review]: ::: gtk/gtkstack.c @@ +214,3 @@ + GtkStackPrivate *priv = gtk_stack_get_instance_private (stack); + + priv->destroyed = TRUE; Instead of doing this dance, why not unsetting the visible child during dispose()?
Created attachment 302981 [details] [review] GtkStack: Don't emit notify::visible-child during destruction By the way, given that this changes the run-time behaviour, is it suitable for gtk-3-16?
Review of attachment 302981 [details] [review]: Looks good to me. Wrt to 3.16, I think this is in 'try it and see if anything breaks' area.
Comment on attachment 302981 [details] [review] GtkStack: Don't emit notify::visible-child during destruction Pushed to both master and gtk-3-16. I am not picky about having it in gtk-3-16, because I already have the workaround in place.
Turns out that I stepped on Paolo's toes here. (In reply to Emmanuele Bassi (:ebassi) from comment #3) > Review of attachment 302978 [details] [review] [review]: > > ::: gtk/gtkstack.c > @@ +214,3 @@ > + GtkStackPrivate *priv = gtk_stack_get_instance_private (stack); > + > + priv->destroyed = TRUE; > > Instead of doing this dance, why not unsetting the visible child during > dispose()? He submitted the same patch (bug 724506) but it got committed after mine (as commit 4e155d784dda384cd4bba6653134b1a0542ee569), which was later fixed up by Matthias (as commit 7ce96cb6ac28eeb62e003dcb9e0a8ce7d48e09e0). Hence, I don't think my patch is needed anymore.
Created attachment 341845 [details] [review] GtkStack: Remove redundant code
Review of attachment 341845 [details] [review]: ok
Comment on attachment 341845 [details] [review] GtkStack: Remove redundant code Pushed only to master.