Bug 174430 - Add iOS 11 SPI
Summary: Add iOS 11 SPI
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: Other
Hardware: Unspecified iOS 11
: P2 Normal
Assignee: Jonathan Bedard
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-07-12 11:21 PDT by Jonathan Bedard
Modified: 2017-07-15 14:48 PDT (History)
9 users (show)

See Also:


Attachments
Patch (25.16 KB, patch)
2017-07-12 11:59 PDT, Jonathan Bedard
no flags Details | Formatted Diff | Diff
Patch (25.32 KB, patch)
2017-07-14 11:21 PDT, Jonathan Bedard
no flags Details | Formatted Diff | Diff
Patch (25.11 KB, patch)
2017-07-14 13:55 PDT, Jonathan Bedard
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews101 for mac-elcapitan (1.19 MB, application/zip)
2017-07-14 14:58 PDT, Build Bot
no flags Details
Patch (22.67 KB, patch)
2017-07-14 15:25 PDT, Jonathan Bedard
no flags Details | Formatted Diff | Diff
Patch (25.92 KB, patch)
2017-07-14 16:56 PDT, Jonathan Bedard
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews116 for mac-elcapitan (1.86 MB, application/zip)
2017-07-14 18:46 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews124 for ios-simulator-wk2 (1.08 MB, application/zip)
2017-07-14 19:51 PDT, Build Bot
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Bedard 2017-07-12 11:21:37 PDT
Add iOS 11 SPI required to build WebKit with the iOS 11 SDK.
Comment 1 Radar WebKit Bug Importer 2017-07-12 11:22:03 PDT
<rdar://problem/33269288>
Comment 2 Jonathan Bedard 2017-07-12 11:59:11 PDT
Created attachment 315266 [details]
Patch
Comment 3 Build Bot 2017-07-12 12:01:57 PDT
Attachment 315266 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/spi/ios/UIKitSPI.h:110:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/platform/spi/ios/UIKitSPI.h:111:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/platform/spi/ios/UIKitSPI.h:118:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Tools/TestWebKitAPI/ios/DataInteractionSimulator.h:53:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Tools/TestWebKitAPI/ios/DataInteractionSimulator.h:54:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Tools/TestWebKitAPI/ios/DataInteractionSimulator.h:60:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Tools/TestWebKitAPI/Tests/ios/DataInteractionTests.mm:51:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Tools/TestWebKitAPI/Tests/ios/DataInteractionTests.mm:52:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 8 in 21 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 4 Jonathan Bedard 2017-07-14 11:21:23 PDT
Created attachment 315462 [details]
Patch
Comment 5 Jonathan Bedard 2017-07-14 11:21:57 PDT
(In reply to Jonathan Bedard from comment #4)
> Created attachment 315462 [details]
> Patch

Needed to rebase the patch due to the WebKit2->WebKit, WebKit->WebKitLegacy move.
Comment 6 Build Bot 2017-07-14 11:24:08 PDT
Attachment 315462 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/spi/ios/UIKitSPI.h:110:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/platform/spi/ios/UIKitSPI.h:111:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/platform/spi/ios/UIKitSPI.h:118:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Tools/TestWebKitAPI/ios/DataInteractionSimulator.h:53:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Tools/TestWebKitAPI/ios/DataInteractionSimulator.h:54:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Tools/TestWebKitAPI/ios/DataInteractionSimulator.h:60:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Tools/TestWebKitAPI/Tests/ios/DataInteractionTests.mm:51:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Tools/TestWebKitAPI/Tests/ios/DataInteractionTests.mm:52:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 8 in 21 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 7 Tim Horton 2017-07-14 11:32:41 PDT
Comment on attachment 315462 [details]
Patch

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

> Source/WebCore/platform/network/cf/FormDataStreamCFNet.cpp:46
> -#include <MacErrors.h>
> +static const int fnfErr = -43;

I still see this header in my SDK, but w/e.

> Source/WebCore/platform/spi/cocoa/IOSurfaceSPI.h:41
>  

Shouldn't we just make it (PLATFORM(MAC) || (PLATFORM(IOS) && __IPHONE_OS_VERSION_MIN_REQUIRED < 110000)) above instead? Not sure.

> Source/WebCore/platform/spi/ios/UIKitSPI.h:98
> +#ifndef _WEBKIT_UIKITSPI_UIITEMPROVIDER

What

> Source/WebKit/Platform/spi/ios/UIKitSPI.h:971
> -void UIImageDataWriteToSavedPhotosAlbum(NSData *imageData, id completionTarget, SEL completionSelector, void *contextInfo);
> +void UIImageDataWriteToSavedPhotosAlbum(NSData *imageData, id completionTarget, SEL completionSelector, void * contextInfo);

Star's still on the wrong side.

> Source/WebKit/Platform/spi/ios/UIKitSPI.h:992
> +@interface UITapGestureRecognizer ()
> +- (void)setDelegate:(id)delegate;
> +@end

This seems wrong; setDelegate is API on UIGestureRecognizer (which UITapGestureRecognizer inherits from)

> Tools/TestWebKitAPI/Tests/ios/DataInteractionTests.mm:49
> ++ (UIItemProvider*) itemProviderWithURL:(NSURL *)url title:(NSString *)title;

space before star, no space after )

> Tools/TestWebKitAPI/ios/DataInteractionSimulator.h:63
> +@interface NSAttributedString (UIItemProvederReadingWriting) <UIItemProviderReading, UIItemProviderWriting>

Proveder!
Comment 8 Jonathan Bedard 2017-07-14 12:47:00 PDT
Comment on attachment 315462 [details]
Patch

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

>> Source/WebCore/platform/spi/cocoa/IOSurfaceSPI.h:41
>>  
> 
> Shouldn't we just make it (PLATFORM(MAC) || (PLATFORM(IOS) && __IPHONE_OS_VERSION_MIN_REQUIRED < 110000)) above instead? Not sure.

Good point, need to test this

>> Source/WebCore/platform/spi/ios/UIKitSPI.h:98
>> +#ifndef _WEBKIT_UIKITSPI_UIITEMPROVIDER
> 
> What

DumpRenderTree can include WebCore headers but WebKitTestRunner can't.  We do the same thing with _WEBKIT_UIKITSPI_UIKEYBOARD.  Its unfortunate.  That's why there's a bug.
Comment 9 Jonathan Bedard 2017-07-14 13:55:20 PDT
Created attachment 315485 [details]
Patch
Comment 10 Build Bot 2017-07-14 13:56:29 PDT
Attachment 315485 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/spi/ios/UIKitSPI.h:110:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/platform/spi/ios/UIKitSPI.h:111:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/platform/spi/ios/UIKitSPI.h:118:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Tools/TestWebKitAPI/ios/DataInteractionSimulator.h:53:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Tools/TestWebKitAPI/ios/DataInteractionSimulator.h:54:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Tools/TestWebKitAPI/ios/DataInteractionSimulator.h:60:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Tools/TestWebKitAPI/Tests/ios/DataInteractionTests.mm:51:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Tools/TestWebKitAPI/Tests/ios/DataInteractionTests.mm:52:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 8 in 21 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 11 Wenson Hsieh 2017-07-14 14:36:53 PDT
Comment on attachment 315485 [details]
Patch

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

> Source/WebCore/platform/sql/SQLiteFileSystem.cpp:40
> +#include "sqlite3SPI.h"

"SQLiteSPI.h" would follow our WebCore header conventions better. I assume you're using sqliteSPI.h here to match the style of the platform header, but there are other places (e.g. XPCSPI.h) where even though the original platform header is lowercase (xpc.h), we still uppercase it for our private header.

> Source/WebKit/Platform/spi/ios/UIKitSPI.h:878
> +#ifndef _WEBKIT_UIKITSPI_UIITEMPROVIDER

As we discussed in person, let's see if we can avoid having to add this :P

> Source/WebKit/Platform/spi/ios/UIKitSPI.h:885
> +#endif

We can rename the ENABLE(DATA_INTERACTION) above to ENABLE(DRAG_SUPPRT) and remove the #endif here and the #if ENABLE_DRAG_SUPPORT) right below.
Comment 12 Build Bot 2017-07-14 14:58:19 PDT
Comment on attachment 315485 [details]
Patch

Attachment 315485 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/4121517

New failing tests:
security/contentSecurityPolicy/video-with-data-url-allowed-by-media-src-star.html
Comment 13 Build Bot 2017-07-14 14:58:20 PDT
Created attachment 315493 [details]
Archive of layout-test-results from ews101 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-elcapitan  Platform: Mac OS X 10.11.6
Comment 14 Jonathan Bedard 2017-07-14 15:25:05 PDT
Created attachment 315497 [details]
Patch
Comment 15 Tim Horton 2017-07-14 15:55:48 PDT
Comment on attachment 315497 [details]
Patch

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

> Source/WTF/Configurations/WTF.xcconfig:33
> +EXCLUDED_SOURCE_FILE_NAMES[sdk=iphonesimulator11*] = $(EXCLUDED_SOURCE_FILE_NAMES_$(USE_INTERNAL_SDK))

This should be inverted so that it remains true in future.

> Source/WebCore/platform/spi/cocoa/IOSurfaceSPI.h:44
> +// iOS 11 has this value defined.

I don't think we need the comment.

> Source/WebCore/platform/spi/cocoa/QuartzCoreSPI.h:135
> ++ (CATransactionPhase) currentPhase;

No space after the ). Also this is available on Mac too!

> Source/WebCore/platform/spi/ios/UIKitSPI.h:88
> +@interface NSURL (UIKit_Private)

Leave all the category names blank (make them class extensions or whatever)

> Source/WebKit/Platform/spi/ios/UIKitSPI.h:310
> +#if __IPHONE_OS_VERSION_MAX_ALLOWED < 90100

I can't imagine we still need this

> Source/WebKit/Platform/spi/ios/UIKitSPI.h:332
> +@property (nonatomic, setter=_setContentInsetAdjustmentBehavior:, getter=_contentInsetAdjustmentBehavior) UIScrollViewContentInsetAdjustmentBehavior _contentInsetAdjustmentBehavior;

Shoudn't need this. If anybody's calling it, they should be using the one without the underscore.

> Source/WebKit/Platform/spi/ios/UIKitSPI.h:341
> +@interface UITapGestureRecognizer (UITapGestureRecognizer)

Blank

> Source/WebKit/Platform/spi/ios/UIKitSPI.h:915
> ++ (UITextEffectsWindow *) sharedTextEffectsWindow;

No space after )

> Source/WebKitLegacy/Storage/StorageTracker.cpp:44
> -#include <sqlite3_private.h>
> +#define SQLITE_TRUNCATE_DATABASE 101

You should be able to use your nice SPI header if you just make it private (like some of the other WebCore ones are!).

> Tools/TestWebKitAPI/Tests/ios/DataInteractionTests.mm:49
> ++ (UIItemProvider *) itemProviderWithURL:(NSURL *)url title:(NSString *)title;

Spaces after ) again

> Tools/TestWebKitAPI/ios/DataInteractionSimulator.h:40
> +@interface NSURL (UIKit_Private)

Drop all the category names.
Comment 16 Jonathan Bedard 2017-07-14 16:56:04 PDT
Created attachment 315506 [details]
Patch
Comment 17 Build Bot 2017-07-14 18:46:28 PDT
Comment on attachment 315506 [details]
Patch

Attachment 315506 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/4122680

New failing tests:
security/contentSecurityPolicy/video-with-data-url-allowed-by-media-src-star.html
Comment 18 Build Bot 2017-07-14 18:46:29 PDT
Created attachment 315520 [details]
Archive of layout-test-results from ews116 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews116  Port: mac-elcapitan  Platform: Mac OS X 10.11.6
Comment 19 Build Bot 2017-07-14 19:51:36 PDT
Comment on attachment 315506 [details]
Patch

Attachment 315506 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/4122913

New failing tests:
imported/w3c/IndexedDB-private-browsing/idbfactory_open12.html
Comment 20 Build Bot 2017-07-14 19:51:38 PDT
Created attachment 315525 [details]
Archive of layout-test-results from ews124 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews124  Port: ios-simulator-wk2  Platform: Mac OS X 10.12.5
Comment 21 Jonathan Bedard 2017-07-14 22:13:09 PDT
Comment on attachment 315506 [details]
Patch

These test failures are unrelated to this patch.
Comment 22 WebKit Commit Bot 2017-07-14 22:41:58 PDT
Comment on attachment 315506 [details]
Patch

Clearing flags on attachment: 315506

Committed r219537: <http://trac.webkit.org/changeset/219537>
Comment 23 WebKit Commit Bot 2017-07-14 22:42:00 PDT
All reviewed patches have been landed.  Closing bug.
Comment 24 Jonathan Bedard 2017-07-15 14:48:14 PDT
Build fix committed in <http://trac.webkit.org/changeset/219538>.