GNOME Bugzilla – Bug 757623
Qt5 apps cause mutter to crash
Last modified: 2016-05-03 02:20:05 UTC
Created attachment 314870 [details] [review] Patch If you run any Qt5 app (e.g. "textedit" from Qt5 examples), it will cause the GNOME shell to crash. Clients are allowed to commit a NULL buffer to a wl_shell; that causes the window to get unmapped. After that, any attempt to modify the window will cause a crash. (This is arguably broken behavior on the client's part. However, treating such requests as errors would break Qt5 apps. Instead, we can just ignore them.)
(In reply to Dima Ryazanov from comment #0) > Created attachment 314870 [details] [review] [review] > Patch > > If you run any Qt5 app (e.g. "textedit" from Qt5 examples), it will cause > the GNOME shell to crash. > > Clients are allowed to commit a NULL buffer to a wl_shell; that causes the > window to get unmapped. After that, any attempt to modify the window will > cause a crash. > > (This is arguably broken behavior on the client's part. However, treating > such requests as errors would break Qt5 apps. Instead, we can just ignore > them.) I wouldn't say its broken. wl_shell doesn't define the behaviour when a null-buffer is attached and committed and it has typically been implemented as something similar to hiding. In mutter, attaching+committing a NULL simply means destroy the window, and attaching+committing a non-NULL buffer after that showing it again, though with most of the previous state (such as title, wm class etc) lost. So, I'm not sure whether we should just NULL check and drop any state flat on the floor, or create a new MetaWindow on demand, assuming the client intended to actually attach a valid buffer to it. I guess simple NULL check is in line with the broken wl_shell support we have in mutter (since we already loose most state on NULL buffer attach), but IMHO, we should at least try a bit harder to play along with original wl_shell practices.
A simple NULL check seems to be enough for Qt5 apps to work. Also, wl_shell is supposed to eventually get replaced by xdg_shell, right? So maybe fixing it is not worth the effort.
Looks like this bug has already been reported earlier. *** This bug has been marked as a duplicate of bug 754501 ***
Created attachment 317593 [details] [review] wayland: Split out shell surface code from meta-wayland-surface.c Move xdg_shell related functionality to a new meta-wayland-xdg-shell.c and wl_shell related functionality to a new meta-wayland-wl-shell.c, and adapt role object tree. Common functionality related to the surface being drawn as a MetaSurfaceActor was moved to a MetaWaylandSurfaceRoleActorSurface role. The subsurface role GObject is made to inherit the actor surface GObject. Shell surface hooks (configure, ping, close, popup done) were added to a MetaWaylandSurfaceRoleShellSurface GObject which inherits the surface actor role GObject. The shell surface roles (xdg_surface, xdg_popup, wl_shell_surface) are made to inherit the shell surface GObject and implement the relevant API.
Created attachment 317594 [details] [review] wayland: Keep wl_shell_surface state during loss of window It has been common practice (in QT5 for example) to set wl_shell_surface state at situations where mutter will have destroyed the MetaWindow. This commit keeps track of the relevant state separately from MetaWindow, and synchronizes when needed.
Created attachment 317595 [details] [review] wayland: Let the roles handle their windows being managed Move xdg_shell specific code from generic Wayland code into the xdg shell code unit by letting the roles handle the corresponding MetaWindow being managed.
Created attachment 317596 [details] [review] wayland: Set window type of wl_shell_surface popups to 'dropdown menu' The wl_shell_surface popups are mostly used in the same way as xdg_popup, so set the same window type.
Created attachment 317597 [details] [review] wayland: Remove wl_shell_surface parent destroy listener in destructor If the surface is destroyed before the popup is dismissed, the destroy listener needs to be removed from the parent destroy signal.
*** Bug 754501 has been marked as a duplicate of this bug. ***
*** Bug 760165 has been marked as a duplicate of this bug. ***
The patches for making non-trivial wl_shell clients (i.e. QT5) work at least somewhat well are now part of bug 763431. Please consider the patches for that attached to this bug obsolete.
would be great to get these reviewed and landed
I thought Jonas wanted to go with the patch set in bug 763431 instead? Should we close this bug ?
(In reply to Rui Matos from comment #13) > I thought Jonas wanted to go with the patch set in bug 763431 instead? > Should we close this bug ? Yep, lets close this one. *** This bug has been marked as a duplicate of bug 763431 ***