Summary: | Stop using PLATFORM(MAC) in JavaScriptCore except where it means “OS X but not iOS” | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | mitz | ||||
Component: | JavaScriptCore | Assignee: | mitz | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | andersca, darin | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
mitz
2014-02-02 23:40:02 PST
Created attachment 222964 [details]
Replace PLATFORM(MAC) with alternatives appropriate for the context
Comment on attachment 222964 [details] Replace PLATFORM(MAC) with alternatives appropriate for the context View in context: https://bugs.webkit.org/attachment.cgi?id=222964&action=review > Source/JavaScriptCore/runtime/Watchdog.h:29 > +#if OS(DARWIN) && !PLATFORM(EFL) && !PLATFORM(GTK) I probably would have guarded this just by OS(DARWIN) even though the actual code is turned off for EFL and GTK. (In reply to comment #2) > (From update of attachment 222964 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=222964&action=review > > > Source/JavaScriptCore/runtime/Watchdog.h:29 > > +#if OS(DARWIN) && !PLATFORM(EFL) && !PLATFORM(GTK) > > I probably would have guarded this just by OS(DARWIN) even though the actual code is turned off for EFL and GTK. Makes sense. I’ll change that to OS(DARWIN) alone. Committed <http://trac.webkit.org/r163291>. Comment on attachment 222964 [details] Replace PLATFORM(MAC) with alternatives appropriate for the context View in context: https://bugs.webkit.org/attachment.cgi?id=222964&action=review >>> Source/JavaScriptCore/runtime/Watchdog.h:29 >>> +#if OS(DARWIN) && !PLATFORM(EFL) && !PLATFORM(GTK) >> >> I probably would have guarded this just by OS(DARWIN) even though the actual code is turned off for EFL and GTK. > > Makes sense. I’ll change that to OS(DARWIN) alone. Glad you took my suggestion for the include here, but ... > Source/JavaScriptCore/runtime/Watchdog.h:96 > +#if OS(DARWIN) && !PLATFORM(EFL) && !PLATFORM(GTK) Looks like you also made this check just OS(DARWIN), and that was not what I was suggesting. (In reply to comment #5) > (From update of attachment 222964 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=222964&action=review > > >>> Source/JavaScriptCore/runtime/Watchdog.h:29 > >>> +#if OS(DARWIN) && !PLATFORM(EFL) && !PLATFORM(GTK) > >> > >> I probably would have guarded this just by OS(DARWIN) even though the actual code is turned off for EFL and GTK. > > > > Makes sense. I’ll change that to OS(DARWIN) alone. > > Glad you took my suggestion for the include here, but ... > > > Source/JavaScriptCore/runtime/Watchdog.h:96 > > +#if OS(DARWIN) && !PLATFORM(EFL) && !PLATFORM(GTK) > > Looks like you also made this check just OS(DARWIN), and that was not what I was suggesting. I misunderstood you. I thought you meant that it would be OK to give EFL and GTK on Darwin those extra member variables in their Watchdog instances, even though they don’t use the WatchDogMac (though they could!). But I’ll undo that bit. Comment on attachment 222964 [details] Replace PLATFORM(MAC) with alternatives appropriate for the context View in context: https://bugs.webkit.org/attachment.cgi?id=222964&action=review >>>>> Source/JavaScriptCore/runtime/Watchdog.h:29 >>>>> +#if OS(DARWIN) && !PLATFORM(EFL) && !PLATFORM(GTK) >>>> >>>> I probably would have guarded this just by OS(DARWIN) even though the actual code is turned off for EFL and GTK. >>> >>> Makes sense. I’ll change that to OS(DARWIN) alone. >> >> Glad you took my suggestion for the include here, but ... > > I misunderstood you. I thought you meant that it would be OK to give EFL and GTK on Darwin those extra member variables in their Watchdog instances, even though they don’t use the WatchDogMac (though they could!). But I’ll undo that bit. Who knows, that might be a good idea. It just wasn’t my idea. |