Bug 157048

Summary: Update the compatibility version check for the ObjC API's InitConstructorSupport to use dyld_get_program_sdk_version().
Product: WebKit Reporter: Mark Lam <mark.lam>
Component: JavaScriptCoreAssignee: Mark Lam <mark.lam>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, ggaren, keith_miller, mitz, msaboff, saam, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Local Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
proposed patch. ggaren: review+

Description Mark Lam 2016-04-26 15:31:45 PDT
Patch coming.
Comment 1 Mark Lam 2016-04-26 15:35:55 PDT
<rdar://problem/25093101>
Comment 2 Mark Lam 2016-04-26 16:03:43 PDT
Created attachment 277419 [details]
proposed patch.
Comment 3 Geoffrey Garen 2016-04-26 17:05:15 PDT
Comment on attachment 277419 [details]
proposed patch.

r=me
Comment 4 Mark Lam 2016-04-26 17:30:30 PDT
Thanks for the review.  Landed in r200114: <http://trac.webkit.org/r200114>.
Comment 5 mitz 2016-04-26 19:36:40 PDT
Comment on attachment 277419 [details]
proposed patch.

View in context: https://bugs.webkit.org/attachment.cgi?id=277419&action=review

> Source/JavaScriptCore/API/JSWrapperMap.mm:50
> +#if __has_include(<mach-o/dyld_priv.h>)
> +#include <mach-o/dyld_priv.h>
> +#endif
> +extern "C" uint32_t dyld_get_program_sdk_version();

It’s incorrect to use __has_include to check for private headers. Instead, check for USE(APPLE_INTERNAL_SDK).

> Source/JavaScriptCore/API/JSWrapperMap.mm:55
> +static const uint32_t webkitFirstSDKVersionWithInitConstructorSupport = 0x80000; // iOS 8.0.0
> +#elif PLATFORM(MAC)
> +static const uint32_t webkitFirstSDKVersionWithInitConstructorSupport = 0xA0A00; // OSX 10.10.0

It doesn’t make sense to use “webkit” in these names, which aren’t WebKit versions.
Comment 6 Mark Lam 2016-04-27 11:49:19 PDT
(In reply to comment #5)
> It’s incorrect to use __has_include to check for private headers. Instead,
> check for USE(APPLE_INTERNAL_SDK).
> 
...
> It doesn’t make sense to use “webkit” in these names, which aren’t WebKit
> versions.

I'll address these in https://bugs.webkit.org/show_bug.cgi?id=157096.
Comment 7 Alexey Proskuryakov 2016-04-28 22:10:56 PDT
Comment on attachment 277419 [details]
proposed patch.

View in context: https://bugs.webkit.org/attachment.cgi?id=277419&action=review

> Source/JavaScriptCore/API/JSWrapperMap.mm:665
> +    static uint32_t programSDKVersion = 0;
> +    if (!programSDKVersion)
> +        programSDKVersion = dyld_get_program_sdk_version();

Do we need to keep trying if we get a zero value once? If not, this should be written as:

static uint32_t programSDKVersion = dyld_get_program_sdk_version();