Andrew Comminos [:acomminos] Assignee
	Description • 9 years ago

The mochitest browser/base/content/test/general/browser_minimize.js hangs on GTK3 builds when waiting for the window to be restored.

	Andrew Comminos [:acomminos] Assignee
	Comment 1 • 9 years ago

Using present instead of deiconify might be a potential solution; deiconification isn't guaranteed by the WM.

Thoughts, Karl?

	Andrew Comminos [:acomminos] Assignee
	Comment 2 • 9 years ago

This doesn't appear to be an issue with the behaviour of deiconify on GTK3 after a window-state-event notifies us that the window is iconified (at least on Ubuntu 12.04 with Unity 2D, the try server WM). This short program (correctly) endlessly iconifies and deiconifies on test machines.

> #include <gtk/gtk.h>
> #include <gdk/gdk.h>
> gboolean window_state_cb(GtkWidget* widget, GdkEvent* event, gpointer data) {
>   if (!(event->window_state.changed_mask & GDK_WINDOW_STATE_ICONIFIED))
>     return FALSE;
>   if (event->window_state.new_window_state & GDK_WINDOW_STATE_ICONIFIED) {
>     printf("iconified, now deiconifying...\n");
>     gtk_window_deiconify(GTK_WINDOW(widget));
>   } else {
>     printf("deiconified, now iconifying...\n");
>     gtk_window_iconify(GTK_WINDOW(widget));
>   }
>   return FALSE;
> }
> int main(int argc, char* argv[]) {
>   gtk_init(&argc, &argv);
>   GtkWidget* window = gtk_window_new(GTK_WINDOW_TOPLEVEL);
>   gtk_window_present(GTK_WINDOW(window));
>   g_signal_connect(G_OBJECT(window), "window-state-event", G_CALLBACK(window_state_cb), NULL);
>   gtk_window_iconify(GTK_WINDOW(window));
>   gtk_main();
>   return 0;
> }

	Karl Tomlinson (:karlt)
	Comment 3 • 9 years ago

Comment on attachment 8614345 [details] [diff] [review]
Present instead of deiconify on GTK.

I'm not keen on using present to de-iconify because present will request focus
and that is not necessarily the intention of SetSizeMode().

The test actually seems confused about this because it seems to assume that
the window will receive focus after window.restore().  I don't know that
restore() requests focus.

Do you know whether the window is still not restored, or just not active?

Does your test program fail to restore, if modified to not wait for
window-state-event?

The reason the test waits for focus is because it can't wait for the correct
windowState because the windowState is not giving the true reflection of the
current state.  It would be nice to fix that.  That might be not updating
mSizeState in SetSizeMode() but only in OnWindowStateEvent(), and override
SizeMode() to return mSizeState.  I don't know what might be depending on the
current behavior though.

If this is not causing a problem in real usage, then I'd disable the test or
part of the test for now and leave the bug open.  If the tests can be
annotated with their current state, or disabled where that is not possible,
then we're more likely to notice regressions in tests that are currently
passing.

	Andrew Comminos [:acomminos] Assignee
	Comment 4 • 9 years ago

You're right; this is a focus issue upon debugging it. In which case, it's likely related to bug 1170783, and the fact that using present worked around it is probably a fluke.

	Andrew Comminos [:acomminos] Assignee
	Comment 5 • 9 years ago

Marking as dupe of 1170342, focus seems to be the issue here.