GNOME Bugzilla – Bug 668505
GtkWindow API can segfault with GtkOffscreenWindow
Last modified: 2018-04-15 00:35:45 UTC
It is documented that standard GtkWindow API shouldn't be used with GtkOffscreenWindow, but nevertheless, it shouldn't make apps crash. For instance, calling gtk_window_get_position() on a offscreen window can trigger a call to the GdkWindowImpl::get_frame_extents(), which for GdkOffscreenWindow is NULL. This crashes. I suspect there are other calls like this (after all, GdkOffscreenWindow does not implement every single method in GdkWindowImpl).
This is hit by ephy when using a GtkOffscreenWindow to render a WebKitWebview offscreen. WebKit needs to know about the window position and size and it asumes it can query this information from the toplevel ancestor of the webview. Perhaps GtkOffscreenWindow shouldn't be advertised as a toplevel?
Created attachment 205889 [details] minimal example
(In reply to comment #1) > This is hit by ephy when using a GtkOffscreenWindow to render a WebKitWebview > offscreen. WebKit needs to know about the window position and size and it > asumes it can query this information from the toplevel ancestor of the webview. > Perhaps GtkOffscreenWindow shouldn't be advertised as a toplevel? Well, its not a valid assumption to make. Tough for Webview...
Tough for GTK+ not to call its own NULL function pointers.. That's what's this bug is all about after all.
The reason we have GtkOffscreenWindow at all is that everything must be in some toplevel window, or all sort of things work, so not advertising it as a toplevel is not a good idea. We can make it not crash, but what kind of data would you expect to get back? There just is no "position" for an offscreen window, nor do most other non-supported things make sense.
Eh, "or all sort of things *don't* work"
Perhaps a critical warning and default values would do? We've fixed this in the webkitgtk side not to request for the data, but this could still hit some other libraries.
Isn't the whole point of of offscreen that you can stick to the same API and code paths and stop special-casing and hacking around? If you require widgets to be special-cased, it seems like half-way back to good old times.
For what I want Alexander's argument that position is undefined is perfectly reasonable. So maybe there should be a window_is_positionable() function. It could cover the offscreen use case. It might also cover backends or window managers which don't support window positions. It'd still require porting widgets but would be cleaner.
That sounds pretty reasonable. It would prevent us from fiddling with GTK_IS_WINDOW (w) && !GTK_IS_OFFSCREEN_WINDOW(w) and any other special-casing that might arise in the future.
Its not only about positioning. A lot of the stuff in GtkWindow makes no sense for offscreen windows. Fullscreen? Opacity? transient for? window type? skip taskbar? gravity? focus? is decorated? window icon? iconify? keep above? resize drag? Basically for technical reasons we need the topmost widget in any usable widget hierarchy to be a GtkWindow (which is why e.g. GtkPlug is also a window). To be able to sanely handle widgets rendering to offscreen pixmaps we added GtkOffscreenWindow to be this toplevel widget. However, this is far from an ideal fit, as GtkWindow contains a lot of crap that makes no sense for offscreen windows. The "proper" fix would be to introduce a GtkToplevel class that extracts the minimal stuff from GtkWindow and make it the parent class of GtkWindow and GtkOffscreenWindow, then make all existing Gtk+ code handle the toplevel not being a full GtkWindow. This is a large and incompatible undertaking though. Maybe something we can do for Gtk 4.0.
That said, it would perhaps be nice to replace the NULL vtable functions with some simple functions that warn and return some default values.
(In reply to comment #12) > That said, it would perhaps be nice to replace the NULL vtable functions with > some simple functions that warn and return some default values. Sounds like a reasonable idea.
*** Bug 678369 has been marked as a duplicate of this bug. ***
We're moving to gitlab! As part of this move, we are moving bugs to NEEDINFO if they haven't seen activity in more than a year. If this issue is still important to you and still relevant with GTK+ 3.22 or master, please reopen it and we will migrate it to gitlab.
As announced a while ago, we are migrating to gitlab, and bugs that haven't seen activity in the last year or so will be not be migrated, but closed out in bugzilla. If this bug is still relevant to you, you can open a new issue describing the symptoms and how to reproduce it with gtk 3.22.x or master in gitlab: https://gitlab.gnome.org/GNOME/gtk/issues/new