After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 761156 - [Wayland] Setting a popover 'relative_to' a widget on an offscreen window crashes gtk+
[Wayland] Setting a popover 'relative_to' a widget on an offscreen window cra...
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Backend: Wayland
3.19.x
Other Linux
: Normal normal
: ---
Assigned To: gtk-bugs
gtk-bugs
Depends on:
Blocks:
 
 
Reported: 2016-01-27 01:05 UTC by Andres Fernandez
Modified: 2016-03-02 03:20 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
wayland: Warn if trying to set transient to non-toplevel (1.12 KB, patch)
2016-02-27 16:00 UTC, Jonas Ådahl
accepted-commit_now Details | Review
GtkWindow: Don't realize popover if added to offscreen window (1.24 KB, patch)
2016-02-27 16:00 UTC, Jonas Ådahl
reviewed Details | Review
wayland: Check actual impl type in transient loop (1.75 KB, patch)
2016-02-29 09:35 UTC, Olivier Fourdan
committed Details | Review

Description Andres Fernandez 2016-01-27 01:05:33 UTC
We are mantaining an unofficial repo of Gnome Alphas for Arch Linux. We've built the whole Gnome 3.19.4 release.

When we try to launch Gnome Maps on wayland session it don't start.

Here you have gdb output:

http://pastebin.com/kzePYwef
Comment 1 Dominique Leuenberger 2016-02-26 21:40:24 UTC
Reproduced on openSUSE Tumbleweed - with GNOME Maps 3.19.90
Comment 2 Hashem Nasarat 2016-02-26 22:08:29 UTC
I noticed this too when I tried a few weeks ago in Debian Testing. Curiously, it seems to be a race condition because sometimes (5% of the time?) the app loads fine.
Comment 3 Jonas Ådahl 2016-02-27 05:54:29 UTC
The reason is something sets an window as transient for an offscreen window. This causes the transient-loop check introduced by bug 759299 to crash since it assumes the parent window is also a native window. What a transient-for-offcreen window should mean I'm not sure (whether it'll cause the toplevel window of the offscreen to be the transient parent or if it's just ignored).
Comment 4 Jonas Ådahl 2016-02-27 16:00:41 UTC
Created attachment 322535 [details] [review]
wayland: Warn if trying to set transient to non-toplevel

Setting a GdkWindow transient to a GdkWindow that is not a toplevel
(i.e. doesn't have a GdkWindowImplWayland) is invalid API usage, so
g_return_if_fail when the parent impl isn't a GdkWindowImplWayland.
Comment 5 Jonas Ådahl 2016-02-27 16:00:49 UTC
Created attachment 322536 [details] [review]
GtkWindow: Don't realize popover if added to offscreen window

Don't try to realize a popover widget if it was added to an offscreen
window. Doing so would cause the popover GdkWindow to be set transient
to the offscreen GdkWindow, when running the Wayland backend, which is
invalid. Instead wait until the popover is added to a onscreen window
before realizing it.
Comment 6 Matthias Clasen 2016-02-28 01:42:24 UTC
Review of attachment 322535 [details] [review]:

Looks right
Comment 7 Matthias Clasen 2016-02-28 02:02:25 UTC
Review of attachment 322536 [details] [review]:

This feels wrong to me to check for an offscreen window in this one place.
Can we not instead fix up the transient loop check to not croak ?
Comment 8 Olivier Fourdan 2016-02-29 09:35:47 UTC
Created attachment 322633 [details] [review]
wayland: Check actual impl type in transient loop

(In reply to Matthias Clasen from comment #7)
> This feels wrong to me to check for an offscreen window in this one place.
> Can we not instead fix up the transient loop check to not croak ?

Would this help ? (untested yet, still building gnome-maps here...)
Comment 9 Jonas Ådahl 2016-02-29 09:39:45 UTC
(In reply to Matthias Clasen from comment #7)
> Review of attachment 322536 [details] [review] [review]:
> 
> This feels wrong to me to check for an offscreen window in this one place.
> Can we not instead fix up the transient loop check to not croak ?

If we don't check it there, or skip setting the transiency in the realize function, we pass, according to the API, invalid input to gdk_window_set_transient_for(). gdk_window_set_transient_for() takes two toplevel GdkWindow's, so passing an offscreen GdkWindow as a parent is not valid.
Comment 10 Matthias Clasen 2016-02-29 18:28:26 UTC
Maybe we should do what the gdk_window_get_parent_docs say:

 * Note that you should use gdk_window_get_effective_parent() when
 * writing generic code that walks up a window hierarchy, because
 * gdk_window_get_parent() will most likely not do what you expect if
 * there are offscreen windows in the hierarchy.
Comment 11 Matthias Clasen 2016-03-02 03:20:51 UTC
The patch worked, so I'm going with it.

Attachment 322633 [details] pushed as de38380 - wayland: Check actual impl type in transient loop