191378 2018-11-07 09:47:23 -0800 Simplify macros in platform 2018-11-07 14:13:20 -0800 1 1 1 Unclassified WebKit Platform WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 don.olmstead don.olmstead achristensen annulen commit-queue fujii mcatanzaro webkit-bug-importer oldest_to_newest 1476024 0 don.olmstead 2018-11-07 09:47:23 -0800 There are a few cases where the #if PLATFORM macros could be simplified withing WebCore/platform 1476025 1 354101 don.olmstead 2018-11-07 09:49:14 -0800 Created attachment 354101 Patch 1476030 2 354101 annulen 2018-11-07 09:59:21 -0800 Comment on attachment 354101 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=354101&action=review > Source/WebCore/page/EventHandler.cpp:2643 > +#if !USE(GLIB) This code has nothing to do with GLib > Source/WebCore/platform/network/NetworkStateNotifier.h:70 > +#if USE(GLIB) Same here 1476034 3 354101 don.olmstead 2018-11-07 10:04:21 -0800 Comment on attachment 354101 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=354101&action=review With the GLIB stuff I found implementations in corresponding Glib platform files. Both WPE and GTK rely on Glib. >> Source/WebCore/page/EventHandler.cpp:2643 >> +#if !USE(GLIB) > > This code has nothing to do with GLib There is an implementation at Source/WebCore/platform/glib/EventHandlerGlib.cpp so !USE(GLIB) is applicable here. >> Source/WebCore/platform/network/NetworkStateNotifier.h:70 >> +#if USE(GLIB) > > Same here There is an implementation at Source/WebCore/platform/network/glib/NetworkStateNotifierGlib.cpp so USE(GLIB) is applicable here. 1476175 4 354101 mcatanzaro 2018-11-07 13:39:55 -0800 Comment on attachment 354101 Patch This is iffy, but Don's changes seem reasonable. The problem is we do have a lot of GTK/WPE-specific code used by both ports in files with the GLib (or older Glib) suffix. So these macros *are* guarding WebKit's GLib implementations of these functions... even if they have nothing to do with the library GLib itself. 1476183 5 don.olmstead 2018-11-07 13:47:26 -0800 (In reply to Michael Catanzaro from comment #4) > Comment on attachment 354101 [details] > Patch > > This is iffy, but Don's changes seem reasonable. The problem is we do have a > lot of GTK/WPE-specific code used by both ports in files with the GLib (or > older Glib) suffix. So these macros *are* guarding WebKit's GLib > implementations of these functions... even if they have nothing to do with > the library GLib itself. I currently have platform/glib/EventHandlerGLib.cpp in my CMake for PlayStation so its on my list to turn into a generic implementation. So this should be temporary 1476210 6 354101 commit-queue 2018-11-07 14:12:51 -0800 Comment on attachment 354101 Patch Clearing flags on attachment: 354101 Committed r237944: <https://trac.webkit.org/changeset/237944> 1476211 7 commit-queue 2018-11-07 14:12:53 -0800 All reviewed patches have been landed. Closing bug. 1476212 8 webkit-bug-importer 2018-11-07 14:13:20 -0800 <rdar://problem/45888762> 354101 2018-11-07 09:49:14 -0800 2018-11-07 14:12:51 -0800 Patch bug-191378.diff text/plain 2211 don.olmstead ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJDb3JlL0No YW5nZUxvZwppbmRleCA2Njc4YzllY2Q0Yi4uNjA1MzNhZjdmNGIgMTAwNjQ0Ci0tLSBhL1NvdXJj ZS9XZWJDb3JlL0NoYW5nZUxvZworKysgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKQEAgLTEs MyArMSwxOCBAQAorMjAxOC0xMS0wNyAgRG9uIE9sbXN0ZWFkICA8ZG9uLm9sbXN0ZWFkQHNvbnku Y29tPgorCisgICAgICAgIFNpbXBsaWZ5IG1hY3JvcyBpbiBwbGF0Zm9ybQorICAgICAgICBodHRw czovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTkxMzc4CisKKyAgICAgICAgUmV2 aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgTm8gbmV3IHRlc3RzLiBObyBjaGFu Z2UgaW4gYmVoYXZpb3IuCisKKyAgICAgICAgTW9kaWZpZXMgYSBmZXcgY2FzZXMgd2hlcmUgYSBw bGF0Zm9ybSBtYWNybyBjb3VsZCBiZSBzaW1wbGlmaWVkLgorCisgICAgICAgICogcGFnZS9FdmVu dEhhbmRsZXIuY3BwOgorICAgICAgICAqIHBsYXRmb3JtL25ldHdvcmsvRE5TLmg6CisgICAgICAg ICogcGxhdGZvcm0vbmV0d29yay9OZXR3b3JrU3RhdGVOb3RpZmllci5oOgorCiAyMDE4LTExLTA3 ICBTaW1vbiBGcmFzZXIgIDxzaW1vbi5mcmFzZXJAYXBwbGUuY29tPgogCiAgICAgICAgIFRpbGVD b250cm9sbGVyOjp0aWxlU2l6ZSgpIHNob3VsZCBub3QgaGF2ZSBzaWRlIGVmZmVjdHMKZGlmZiAt LWdpdCBhL1NvdXJjZS9XZWJDb3JlL3BhZ2UvRXZlbnRIYW5kbGVyLmNwcCBiL1NvdXJjZS9XZWJD b3JlL3BhZ2UvRXZlbnRIYW5kbGVyLmNwcAppbmRleCA5OWI3NGJmYjY2My4uYjgwYjc2ODFjMGEg MTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL3BhZ2UvRXZlbnRIYW5kbGVyLmNwcAorKysgYi9T b3VyY2UvV2ViQ29yZS9wYWdlL0V2ZW50SGFuZGxlci5jcHAKQEAgLTI2NDAsNyArMjY0MCw3IEBA IGJvb2wgRXZlbnRIYW5kbGVyOjppc0luc2lkZVNjcm9sbGJhcihjb25zdCBJbnRQb2ludCYgd2lu ZG93UG9pbnQpIGNvbnN0CiAgICAgcmV0dXJuIGZhbHNlOwogfQogCi0jaWYgIVBMQVRGT1JNKEdU SykgJiYgIVBMQVRGT1JNKFdQRSkKKyNpZiAhVVNFKEdMSUIpCiAKIGJvb2wgRXZlbnRIYW5kbGVy OjpzaG91bGRTd2FwU2Nyb2xsRGlyZWN0aW9uKGNvbnN0IEhpdFRlc3RSZXN1bHQmLCBjb25zdCBQ bGF0Zm9ybVdoZWVsRXZlbnQmKSBjb25zdAogewpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUv cGxhdGZvcm0vbmV0d29yay9ETlMuaCBiL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL25ldHdvcmsv RE5TLmgKaW5kZXggYjZiNTE4OTNkMTYuLmJhZTRhMmQwZWZhIDEwMDY0NAotLS0gYS9Tb3VyY2Uv V2ViQ29yZS9wbGF0Zm9ybS9uZXR3b3JrL0ROUy5oCisrKyBiL1NvdXJjZS9XZWJDb3JlL3BsYXRm b3JtL25ldHdvcmsvRE5TLmgKQEAgLTI1LDEwICsyNSwxMCBAQAogCiAjcHJhZ21hIG9uY2UKIAot I2lmIFBMQVRGT1JNKENPQ09BKSB8fCBQTEFURk9STShHVEspIHx8IFBMQVRGT1JNKFdQRSkKLSNp bmNsdWRlIDxuZXRpbmV0L2luLmg+Ci0jZWxpZiBQTEFURk9STShXSU4pCisjaWYgT1MoV0lORE9X UykKICNpbmNsdWRlIDx3aW5zb2NrMi5oPgorI2Vsc2UKKyNpbmNsdWRlIDxuZXRpbmV0L2luLmg+ CiAjZW5kaWYKIAogI2luY2x1ZGUgPHd0Zi9Gb3J3YXJkLmg+CmRpZmYgLS1naXQgYS9Tb3VyY2Uv V2ViQ29yZS9wbGF0Zm9ybS9uZXR3b3JrL05ldHdvcmtTdGF0ZU5vdGlmaWVyLmggYi9Tb3VyY2Uv V2ViQ29yZS9wbGF0Zm9ybS9uZXR3b3JrL05ldHdvcmtTdGF0ZU5vdGlmaWVyLmgKaW5kZXggMzE3 MmZlZWI0YjIuLjNmYzI1OTg2OGI2IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9y bS9uZXR3b3JrL05ldHdvcmtTdGF0ZU5vdGlmaWVyLmgKKysrIGIvU291cmNlL1dlYkNvcmUvcGxh dGZvcm0vbmV0d29yay9OZXR3b3JrU3RhdGVOb3RpZmllci5oCkBAIC02Nyw3ICs2Nyw3IEBAIHBy aXZhdGU6CiAgICAgc3RhdGljIHZvaWQgQ0FMTEJBQ0sgYWRkcmVzc0NoYW5nZUNhbGxiYWNrKHZv aWQqLCBCT09MRUFOIHRpbWVkT3V0KTsKICNlbmRpZgogCi0jaWYgUExBVEZPUk0oR1RLKSB8fCBQ TEFURk9STShXUEUpCisjaWYgVVNFKEdMSUIpCiAgICAgc3RhdGljIHZvaWQgbmV0d29ya0NoYW5n ZWRDYWxsYmFjayhOZXR3b3JrU3RhdGVOb3RpZmllciopOwogI2VuZGlmCiAK