146627 2015-07-06 01:34:29 -0700 [GTK] Guard X11-specific code in webkitWebViewBaseDidRelaunchWebProcess() 2015-07-08 07:49:16 -0700 1 1 1 Unclassified WebKit WebKitGTK 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 zan zan berto cgarcia commit-queue gustavo mcatanzaro mrobinson oldest_to_newest 1107047 0 zan 2015-07-06 01:34:29 -0700 Should be guarded with PLATFORM(X11). 1107050 1 256201 zan 2015-07-06 01:38:23 -0700 Created attachment 256201 Patch 1107052 2 commit-queue 2015-07-06 01:39:40 -0700 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 1107053 3 256201 cgarcia 2015-07-06 01:40:50 -0700 Comment on attachment 256201 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. 1107065 4 zan 2015-07-06 02:49:40 -0700 Committed r186333: <http://trac.webkit.org/changeset/186333> 1107077 5 mcatanzaro 2015-07-06 05:49:21 -0700 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)? 1107079 6 cgarcia 2015-07-06 05:55:03 -0700 (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)? 1107678 7 zan 2015-07-07 22:45:34 -0700 (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. 256201 2015-07-06 01:38:23 -0700 2015-07-06 01:40:50 -0700 Patch pppatch.patch text/plain 1776 zan ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQyL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQyL0No YW5nZUxvZwppbmRleCA1NDYyMmE0Li4wMThjMzUzIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViS2l0 Mi9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYktpdDIvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTUg QEAKKzIwMTUtMDctMDYgIFphbiBEb2JlcnNlayAgPHpkb2JlcnNla0BpZ2FsaWEuY29tPgorCisg ICAgICAgIFtHVEtdIEd1YXJkIFgxMS1zcGVjaWZpYyBjb2RlIGluIHdlYmtpdFdlYlZpZXdCYXNl RGlkUmVsYXVuY2hXZWJQcm9jZXNzKCkKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcv c2hvd19idWcuY2dpP2lkPTE0NjYyNworCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09Q UyEpLgorCisgICAgICAgICogVUlQcm9jZXNzL0FQSS9ndGsvV2ViS2l0V2ViVmlld0Jhc2UuY3Bw OgorICAgICAgICAod2Via2l0V2ViVmlld0Jhc2VEaWRSZWxhdW5jaFdlYlByb2Nlc3MpOgorICAg ICAgICBHdWFyZCBjYWxscyB0byBEcmF3aW5nQXJlYUltcGw6OnNldE5hdGl2ZVN1cmZhY2VIYW5k bGVGb3JDb21wb3NpdGluZygpCisgICAgICAgIHRoYXQgcGFzcyBpbiBhbiBYMTEgV2luZG93IElE IHdpdGggdGhlIFBMQVRGT1JNKFgxMSkgYnVpbGQgZ3VhcmRzLgorCiAyMDE1LTA3LTA1ICBDaHJp cyBEdW1leiAgPGNkdW1lekBhcHBsZS5jb20+CiAKICAgICAgICAgW1dLMl0gQ3VycmVudCBwYWdl J3Mgc2Nyb2xsIHBvc2l0aW9uIGlzIGxvc3Qgd2hlbiB0cmFuc2ZlcnJpbmcgc2Vzc2lvblN0YXRl IGZyb20gb25lIHZpZXcgdG8gYW5vdGhlcgpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYktpdDIvVUlQ cm9jZXNzL0FQSS9ndGsvV2ViS2l0V2ViVmlld0Jhc2UuY3BwIGIvU291cmNlL1dlYktpdDIvVUlQ cm9jZXNzL0FQSS9ndGsvV2ViS2l0V2ViVmlld0Jhc2UuY3BwCmluZGV4IDAxYzVlNTguLjlkNjZm NjEgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQyL1VJUHJvY2Vzcy9BUEkvZ3RrL1dlYktpdFdl YlZpZXdCYXNlLmNwcAorKysgYi9Tb3VyY2UvV2ViS2l0Mi9VSVByb2Nlc3MvQVBJL2d0ay9XZWJL aXRXZWJWaWV3QmFzZS5jcHAKQEAgLTEzOTMsNiArMTM5Myw3IEBAIHZvaWQgd2Via2l0V2ViVmll d0Jhc2VEaWRSZWxhdW5jaFdlYlByb2Nlc3MoV2ViS2l0V2ViVmlld0Jhc2UqIHdlYmtpdFdlYlZp ZXdCYXNlCiAgICAgV2ViS2l0V2ViVmlld0Jhc2VQcml2YXRlKiBwcml2ID0gd2Via2l0V2ViVmll d0Jhc2UtPnByaXY7CiAgICAgRHJhd2luZ0FyZWFQcm94eUltcGwqIGRyYXdpbmdBcmVhID0gc3Rh dGljX2Nhc3Q8RHJhd2luZ0FyZWFQcm94eUltcGwqPihwcml2LT5wYWdlUHJveHktPmRyYXdpbmdB cmVhKCkpOwogICAgIEFTU0VSVChkcmF3aW5nQXJlYSk7CisjaWYgUExBVEZPUk0oWDExKQogI2lm IFVTRShSRURJUkVDVEVEX1hDT01QT1NJVEVfV0lORE9XKQogICAgIGlmICghcHJpdi0+cmVkaXJl Y3RlZFdpbmRvdykKICAgICAgICAgcmV0dXJuOwpAQCAtMTQwMiw0ICsxNDAzLDUgQEAgdm9pZCB3 ZWJraXRXZWJWaWV3QmFzZURpZFJlbGF1bmNoV2ViUHJvY2VzcyhXZWJLaXRXZWJWaWV3QmFzZSog d2Via2l0V2ViVmlld0Jhc2UKICAgICAgICAgcmV0dXJuOwogICAgIGRyYXdpbmdBcmVhLT5zZXRO YXRpdmVTdXJmYWNlSGFuZGxlRm9yQ29tcG9zaXRpbmcoR0RLX1dJTkRPV19YSUQoZ3RrX3dpZGdl dF9nZXRfd2luZG93KEdUS19XSURHRVQod2Via2l0V2ViVmlld0Jhc2UpKSkpOwogI2VuZGlmCisj ZW5kaWYKIH0K