Summary: | [GTK] Guard X11-specific code in webkitWebViewBaseDidRelaunchWebProcess() | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Zan Dobersek <zan> | ||||
Component: | WebKitGTK | Assignee: | Zan Dobersek <zan> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | berto, cgarcia, commit-queue, gustavo, mcatanzaro, mrobinson | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Zan Dobersek
2015-07-06 01:34:29 PDT
Created attachment 256201 [details]
Patch
Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API Comment on attachment 256201 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=256201&action=review > Source/WebKit2/UIProcess/API/gtk/WebKitWebViewBase.cpp:1396 > +#if PLATFORM(X11) I think you can move this to the beginning of the function, the drawing area pointer is useless otherwise. Committed r186333: <http://trac.webkit.org/changeset/186333> Now that since we now support compiling with ENABLE_X11_TARGET and ENABLE_WAYLAND_TARGET at the same time, checks like these need to be done at both compile time *and* runtime. (So this patch is not wrong, but incomplete.) One way to do that would be: bool runningInX11 = GDK_IS_X11_DISPLAY(gdk_window_get_display(gtk_widget_get_window(GTK_WIDGET(webkitWebViewBase)))); Maybe we have a better way? Do we need to audit our code to check for places this was missed in the past? What about around uses of USE(REDIRECTED_XCOMPOSITE_WINDOW)? (In reply to comment #5) > Now that since we now support compiling with ENABLE_X11_TARGET and > ENABLE_WAYLAND_TARGET at the same time, checks like these need to be done at > both compile time *and* runtime. (So this patch is not wrong, but > incomplete.) > > One way to do that would be: > > bool runningInX11 = > GDK_IS_X11_DISPLAY(gdk_window_get_display(gtk_widget_get_window(GTK_WIDGET(we > bkitWebViewBase)))); > > Maybe we have a better way? Yes, PlatformDisplay::sharedDisplay().type() == PlatformDisplay::X11 > Do we need to audit our code to check for places this was missed in the > past? What about around uses of USE(REDIRECTED_XCOMPOSITE_WINDOW)? (In reply to comment #6) > (In reply to comment #5) > > Now that since we now support compiling with ENABLE_X11_TARGET and > > ENABLE_WAYLAND_TARGET at the same time, checks like these need to be done at > > both compile time *and* runtime. (So this patch is not wrong, but > > incomplete.) > > > > One way to do that would be: > > > > bool runningInX11 = > > GDK_IS_X11_DISPLAY(gdk_window_get_display(gtk_widget_get_window(GTK_WIDGET(we > > bkitWebViewBase)))); > > > > Maybe we have a better way? > > Yes, PlatformDisplay::sharedDisplay().type() == PlatformDisplay::X11 > Bug #146711. |