Bug 191378 - Simplify macros in platform
Summary: Simplify macros in platform
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Don Olmstead
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-11-07 09:47 PST by Don Olmstead
Modified: 2018-11-07 14:13 PST (History)
6 users (show)

See Also:


Attachments
Patch (2.16 KB, patch)
2018-11-07 09:49 PST, Don Olmstead
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Don Olmstead 2018-11-07 09:47:23 PST
There are a few cases where the #if PLATFORM macros could be simplified withing WebCore/platform
Comment 1 Don Olmstead 2018-11-07 09:49:14 PST
Created attachment 354101 [details]
Patch
Comment 2 Konstantin Tokarev 2018-11-07 09:59:21 PST
Comment on attachment 354101 [details]
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
Comment 3 Don Olmstead 2018-11-07 10:04:21 PST
Comment on attachment 354101 [details]
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.
Comment 4 Michael Catanzaro 2018-11-07 13:39:55 PST
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.
Comment 5 Don Olmstead 2018-11-07 13:47:26 PST
(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
Comment 6 WebKit Commit Bot 2018-11-07 14:12:51 PST
Comment on attachment 354101 [details]
Patch

Clearing flags on attachment: 354101

Committed r237944: <https://trac.webkit.org/changeset/237944>
Comment 7 WebKit Commit Bot 2018-11-07 14:12:53 PST
All reviewed patches have been landed.  Closing bug.
Comment 8 Radar WebKit Bug Importer 2018-11-07 14:13:20 PST
<rdar://problem/45888762>