Bug 157401 - Add dyldSPI.h for linked on or after checks, and add one for link preview
Summary: Add dyldSPI.h for linked on or after checks, and add one for link preview
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-05-05 16:57 PDT by Beth Dakin
Modified: 2016-05-13 11:47 PDT (History)
17 users (show)

See Also:

Attachments
Patch (10.02 KB, patch)
2016-05-05 17:00 PDT, Beth Dakin 		mitz: review-
Details | Formatted Diff | Diff
Patch (9.47 KB, patch)
2016-05-12 14:05 PDT, Beth Dakin 		no flags Details | Formatted Diff | Diff
Patch (20.65 KB, patch)
2016-05-12 15:06 PDT, Beth Dakin 		no flags Details | Formatted Diff | Diff
Patch (20.61 KB, patch)
2016-05-12 16:37 PDT, Beth Dakin 		darin: review+
Details | Formatted Diff | Diff
Show Obsolete (3) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Beth Dakin 2016-05-05 16:57:06 PDT 
Add dyldSPI.h for linked on or after checks, and add one for link preview
Comment 1 Beth Dakin 2016-05-05 17:00:48 PDT 
Created attachment 278206 [details]
Patch
Comment 2 mitz 2016-05-05 17:38:16 PDT 
Comment on attachment 278206 [details]
Patch

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

> Source/WTF/wtf/spi/darwin/dyldSPI.h:2
> + * Copyright (C) 2015 Apple Inc. All rights reserved.

Pretty sure this is all new 2016 stuff.

> Source/WTF/wtf/spi/darwin/dyldSPI.h:27
> +#ifndef WTF_dyldSPI_h
> +#define WTF_dyldSPI_h

We’ve switched to using #pragma once in new files

> Source/WTF/wtf/spi/darwin/dyldSPI.h:40
> +#if PLATFORM(APPLETV)
> +#elif PLATFORM(IOS)
> +static const uint32_t firstSDKVersionWithInitConstructorSupport = 0x80000; // iOS 8.0.0
> +static const uint32_t firstSDKVersionWithLinkPreviewEnabledByDefault = 0xA0000;
> +#elif PLATFORM(MAC)
> +static const uint32_t firstSDKVersionWithInitConstructorSupport = 0xA0A00; // OSX 10.10.0
> +#endif

This things are neither dyld SPI nor WTF-related. They should stay in their respective projects.

> Source/WebKit2/UIProcess/API/Cocoa/WKWebView.h:235
> + @discussion The default value is NO on iOS 9, and YES on iOS linked on or after WK_IOS_TBS.

TBS? The script that post-processes API headers is not going to do anything pretty with this WK_IOS_TBS (nor with WK_IOS_TBA). More importantly, anyone seeing this comment will be looking at the iOS TBA SDK, and therefore linking against that SDK. There’s no need to confuse them with telling them about behavior that won’t apply to them.

> Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:493
> +    static uint32_t programSDKVersion = 0;
> +    if (!programSDKVersion)
> +        programSDKVersion = dyld_get_program_sdk_version();

Since this is C++, you just say
static uint32_t programSDKVersion = dyld_get_program_sdk_version();
and it will do one-time initialization.
Comment 3 Beth Dakin 2016-05-12 14:05:28 PDT 
Created attachment 278757 [details]
Patch
Comment 4 Beth Dakin 2016-05-12 15:06:08 PDT 
Created attachment 278767 [details]
Patch
Comment 5 Beth Dakin 2016-05-12 16:37:20 PDT 
Created attachment 278778 [details]
Patch
Comment 6 Darin Adler 2016-05-13 00:13:24 PDT 
Comment on attachment 278778 [details]
Patch

I kind of want all these quirk policies gathered in one place in the future. The actual quirks have to be spread around, but I wish all the functions that define the various minimum versions for the expiration various old behaviors were grouped together.
Comment 7 Beth Dakin 2016-05-13 11:34:23 PDT 
http://trac.webkit.org/changeset/200866 Thanks Darin! I agree, that would be great. I was not aware that this was the "best" way to write a quirk until I started working on this and talking to Dan. There are still lot of (recent!) examples in the code that that take a similar-yet-different approach.
Comment 8 Beth Dakin 2016-05-13 11:47:19 PDT 
Build fix: http://trac.webkit.org/changeset/200867