Bug 128098 – Stop using PLATFORM(MAC) in JavaScriptCore except where it means “OS X but not iOS”

• Component: JavaScriptCore
• Status: RESOLVED FIXED

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.

Created attachment 222964 [details]
Replace PLATFORM(MAC) with alternatives appropriate for the context

[Darin Adler]

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>.

[Darin Adler]

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.

[Darin Adler]

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.