Bug 128098

Summary: Stop using PLATFORM(MAC) in JavaScriptCore except where it means “OS X but not iOS”
Product: WebKit Reporter: mitz
Component: JavaScriptCoreAssignee: mitz
Status: RESOLVED FIXED    
Severity: Normal CC: andersca, darin
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Replace PLATFORM(MAC) with alternatives appropriate for the context darin: review+

Description mitz 2014-02-02 23:40:02 PST
So that we can make PLATFORM(MAC) false when building for iOS, we need to replace all uses of it that don’t take iOS into account. Most such uses are about targeting Darwin.
Comment 1 mitz 2014-02-02 23:47:47 PST
Created attachment 222964 [details]
Replace PLATFORM(MAC) with alternatives appropriate for the context
Comment 2 Darin Adler 2014-02-03 00:06:27 PST
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.
Comment 3 mitz 2014-02-03 00:07:58 PST
(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.
Comment 4 mitz 2014-02-03 00:12:55 PST
Committed <http://trac.webkit.org/r163291>.
Comment 5 Darin Adler 2014-02-03 00:15:25 PST
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.
Comment 6 mitz 2014-02-03 00:20:28 PST
(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 7 Darin Adler 2014-02-03 00:21:16 PST
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.