Bug 157048 - Update the compatibility version check for the ObjC API's InitConstructorSupport to use dyld_get_program_sdk_version().
Summary: Update the compatibility version check for the ObjC API's InitConstructorSupp...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Local Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mark Lam
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2016-04-26 15:31 PDT by Mark Lam
Modified: 2016-04-28 22:10 PDT (History)
7 users (show)

See Also:


Attachments
proposed patch. (2.19 KB, patch)
2016-04-26 16:03 PDT, Mark Lam
ggaren: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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();