Bug 146627 – [GTK] Guard X11-specific code in webkitWebViewBaseDidRelaunchWebProcess()

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Zan Dobersek]

Should be guarded with PLATFORM(X11).

[Zan Dobersek]

Created attachment 256201 [details]
Patch

[WebKit Commit Bot]

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

[Carlos Garcia Campos]

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.

[Zan Dobersek]

Committed r186333: <http://trac.webkit.org/changeset/186333>

[Michael Catanzaro]

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)?

[Carlos Garcia Campos]

(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)?

[Zan Dobersek]

(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.