Bug 197171

Summary: Create AVFoundationSoftLink.{h,mm} to reduce duplicate code
Product: WebKit Reporter: Eric Carlson <eric.carlson>
Component: MediaAssignee: Eric Carlson <eric.carlson>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, darin, ddkilzer, ews-watchlist, rniwa, webkit-bug-importer, youennf
Priority: P2 Keywords: InRadar
Version: Other   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 197212, 197282    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Archive of layout-test-results from ews102 for mac-highsierra
none
Archive of layout-test-results from ews115 for mac-highsierra
none
Archive of layout-test-results from ews107 for mac-highsierra-wk2
none
Archive of layout-test-results from ews123 for ios-simulator-wk2
none
Patch for landing
none
Updated patch for landing
none
Updated patch for landing
none
Archive of layout-test-results from ews124 for ios-simulator-wk2
none
Updated patch for landing none

Description Eric Carlson 2019-04-22 11:21:50 PDT
Create AVFoundationSoftLink.{h,mm} to reduce dirty memory pages and duplicate code.
Comment 1 Eric Carlson 2019-04-22 11:22:04 PDT
<rdar://problem/47454979>
Comment 2 Eric Carlson 2019-04-22 12:53:29 PDT
Created attachment 367961 [details]
Patch
Comment 3 EWS Watchlist 2019-04-22 12:56:25 PDT
Attachment 367961 [details] did not pass style-queue:


ERROR: Tools/TestWebKitAPI/Tests/WebCore/cocoa/AVFoundationSoftLinkTest.mm:139:  Misplaced OS version check. Please use a named macro in wtf/Platform.h, wtf/FeatureDefines.h, or an appropriate internal file.  [build/version_check] [5]
ERROR: Tools/TestWebKitAPI/Tests/WebCore/cocoa/AVFoundationSoftLinkTest.mm:144:  Misplaced OS version check. Please use a named macro in wtf/Platform.h, wtf/FeatureDefines.h, or an appropriate internal file.  [build/version_check] [5]
ERROR: Tools/TestWebKitAPI/Tests/WebCore/cocoa/AVFoundationSoftLinkTest.mm:159:  Misplaced OS version check. Please use a named macro in wtf/Platform.h, wtf/FeatureDefines.h, or an appropriate internal file.  [build/version_check] [5]
ERROR: Source/WebCore/platform/graphics/avfoundation/objc/MediaPlaybackTargetPickerMac.mm:40:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/objc/CDMInstanceFairPlayStreamingAVFObjC.mm:45:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/objc/CDMInstanceFairPlayStreamingAVFObjC.mm:418:  Missing space around : in range-based for statement  [whitespace/colon] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/objc/CDMInstanceFairPlayStreamingAVFObjC.mm:477:  Missing space around : in range-based for statement  [whitespace/colon] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.mm:224:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebCore/platform/audio/ios/MediaSessionManagerIOS.mm:48:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/mediarecorder/cocoa/MediaRecorderPrivateWriterCocoa.mm:40:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/MediaPlaybackTargetMac.mm:35:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebKit/WebProcess/WebPage/RemoteLayerTree/PlatformCALayerRemoteCustom.mm:39:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebKit/Shared/mac/WebCoreArgumentCodersMac.mm:49:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/mediastream/mac/AVVideoCaptureSource.mm:49:  *SoftLink.h header should be included after all other headers.  [build/include_order] [4]
ERROR: Source/WebCore/platform/mediastream/mac/AVVideoCaptureSource.mm:50:  *SoftLink.h header should be included after all other headers.  [build/include_order] [4]
ERROR: Source/WebCore/platform/audio/ios/AudioSessionIOS.mm:37:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/mediastream/ios/CoreAudioCaptureSourceIOS.mm:36:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebKitLegacy/mac/WebView/WebVideoFullscreenController.mm:40:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/mediastream/mac/AVCaptureDeviceManager.mm:46:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/mac/SerializedPlatformRepresentationMac.mm:44:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/MediaSelectionGroupAVFObjC.mm:38:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/objc/CDMSessionAVContentKeySession.mm:45:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/objc/CDMSessionAVStreamSession.mm:46:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/ios/PlatformSpeechSynthesizerIOS.mm:36:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebKit/Shared/ios/WebIconUtilities.mm:41:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/objc/InbandTextTrackPrivateLegacyAVFObjC.mm:38:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:143:  *SoftLink.h header should be included after all other headers.  [build/include_order] [4]
Total errors found: 27 in 45 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 4 EWS Watchlist 2019-04-22 14:30:23 PDT
Comment on attachment 367961 [details]
Patch

Attachment 367961 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/11964524

New failing tests:
http/wpt/mediarecorder/MediaRecorder-AV-audio-video-dataavailable.html
Comment 5 EWS Watchlist 2019-04-22 14:30:25 PDT
Created attachment 367975 [details]
Archive of layout-test-results from ews102 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-highsierra  Platform: Mac OS X 10.13.6
Comment 6 youenn fablet 2019-04-22 14:36:22 PDT
Comment on attachment 367961 [details]
Patch

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

> Source/WebCore/PAL/pal/cocoa/AVFoundationSoftLink.h:67
> +SOFT_LINK_CLASS_FOR_HEADER(PAL, AVMutableAudioMixInputParameters)

Should we sort them lexicographically?

> Source/WebCore/WebCore.xcodeproj/project.pbxproj:32568
> +				070F2DDB226DF54200D7BBA3 /* AudioSessionIOS.mm in Sources */,

Changes not needed.

> Source/WebCore/platform/audio/ios/MediaSessionManagerIOS.mm:291
> +            [[NSNotificationCenter defaultCenter] addObserver:protectedSelf selector:@selector(wirelessRoutesAvailableDidChange:) name:AVRouteDetectorMultipleRoutesDetectedDidChangeNotification object:protectedSelf->_routeDetector.get()];

I would mention somewhere why we have AVRouteDetectorMultipleRoutesDetectedDidChangeNotification here but we have PAL::getAVAudioSessionClass() now

> Source/WebCore/platform/graphics/avfoundation/AVTrackPrivateAVFObjCImpl.mm:38
> +#import <pal/cocoa/AVFoundationSoftLink.h>

Add a space so that it is kept after all includes/imports?

> Source/WebCore/platform/graphics/avfoundation/MediaPlaybackTargetMac.mm:33
>  #import <wtf/SoftLinking.h>

To remove.

> Source/WebCore/platform/graphics/avfoundation/objc/CDMInstanceFairPlayStreamingAVFObjC.mm:39
> +#import <pal/cocoa/AVFoundationSoftLink.h>

Should we move this import one below with pal/spi?
Or keep pal/spi/Mac/AVFoundationSPI.h and add below <pal/cocoa/AVFoundationSoftLink.h>

> Source/WebCore/platform/graphics/avfoundation/objc/CDMSessionAVContentKeySession.mm:43
>  #import <wtf/SoftLinking.h>

To remove

> Source/WebCore/platform/graphics/avfoundation/objc/CDMSessionAVFoundationObjC.mm:41
>  #import <wtf/SoftLinking.h>

Needed?

> Source/WebCore/platform/graphics/avfoundation/objc/CDMSessionAVStreamSession.mm:43
>  #import <wtf/SoftLinking.h>

Needed?

> Source/WebCore/platform/graphics/avfoundation/objc/InbandTextTrackPrivateLegacyAVFObjC.mm:36
>  #import <wtf/SoftLinking.h>

Could be removed.

> Source/WebCore/platform/graphics/avfoundation/objc/MediaPlaybackTargetPickerMac.mm:40
> +#import <wtf/SoftLinking.h>

Not needed

> Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:139
>  #pragma mark - Soft Linking

Do we need this pragma?

> Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:-2072
> -#if HAVE(AVFOUNDATION_MEDIA_SELECTION_GROUP)

If we remove all cases of this one, maybe we should update

> Source/WebCore/platform/mediastream/ios/AVAudioSessionCaptureDeviceManager.mm:34
> +#import <pal/cocoa/AVFoundationSoftLink.h>

To put separately?

> Source/WebCore/platform/mediastream/ios/CoreAudioCaptureSourceIOS.mm:34
>  #import <wtf/SoftLinking.h>

Not needed.

> Source/WebCore/platform/mediastream/mac/AVVideoCaptureSource.mm:587
> +        for (AVFrameRateRange *range in [format videoSupportedFrameRateRanges])

AVFrameRateRange* ?
Or on the contrary AVCaptureDeviceFormat *format above?
Comment 7 EWS Watchlist 2019-04-22 16:13:37 PDT
Comment on attachment 367961 [details]
Patch

Attachment 367961 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/11965826

New failing tests:
imported/w3c/web-platform-tests/mediacapture-record/MediaRecorder-constructor.html
http/wpt/mediarecorder/MediaRecorder-AV-audio-video-dataavailable.html
Comment 8 EWS Watchlist 2019-04-22 16:13:39 PDT
Created attachment 367990 [details]
Archive of layout-test-results from ews115 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews115  Port: mac-highsierra  Platform: Mac OS X 10.13.6
Comment 9 EWS Watchlist 2019-04-22 17:18:55 PDT
Comment on attachment 367961 [details]
Patch

Attachment 367961 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/11966848

New failing tests:
imported/w3c/web-platform-tests/mediacapture-record/MediaRecorder-constructor.html
http/wpt/mediarecorder/MediaRecorder-AV-audio-video-dataavailable.html
Comment 10 EWS Watchlist 2019-04-22 17:18:57 PDT
Created attachment 367999 [details]
Archive of layout-test-results from ews107 for mac-highsierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-highsierra-wk2  Platform: Mac OS X 10.13.6
Comment 11 EWS Watchlist 2019-04-22 19:04:27 PDT
Comment on attachment 367961 [details]
Patch

Attachment 367961 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/11967564

New failing tests:
imported/w3c/web-platform-tests/service-workers/cache-storage/serviceworker/cache-abort.https.html
imported/w3c/web-platform-tests/mediacapture-record/MediaRecorder-constructor.html
http/wpt/mediarecorder/MediaRecorder-AV-audio-video-dataavailable.html
Comment 12 EWS Watchlist 2019-04-22 19:04:29 PDT
Created attachment 368004 [details]
Archive of layout-test-results from ews123 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews123  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.6
Comment 13 Eric Carlson 2019-04-23 10:46:57 PDT
Created attachment 368041 [details]
Patch for landing
Comment 14 EWS Watchlist 2019-04-23 10:50:33 PDT
Attachment 368041 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/mediastream/mac/MockRealtimeVideoSourceMac.mm:50:  *SoftLink.h header should be included after all other headers.  [build/include_order] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/objc/ImageDecoderAVFObjC.mm:59:  *SoftLink.h header should be included after all other headers.  [build/include_order] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/objc/ImageDecoderAVFObjC.mm:60:  *SoftLink.h header should be included after all other headers.  [build/include_order] [4]
ERROR: Source/WebCore/platform/graphics/cocoa/HEVCUtilitiesCocoa.mm:36:  *SoftLink.h header should be included after all other headers.  [build/include_order] [4]
ERROR: Tools/TestWebKitAPI/Tests/WebCore/cocoa/AVFoundationSoftLinkTest.mm:139:  Misplaced OS version check. Please use a named macro in wtf/Platform.h, wtf/FeatureDefines.h, or an appropriate internal file.  [build/version_check] [5]
ERROR: Tools/TestWebKitAPI/Tests/WebCore/cocoa/AVFoundationSoftLinkTest.mm:144:  Misplaced OS version check. Please use a named macro in wtf/Platform.h, wtf/FeatureDefines.h, or an appropriate internal file.  [build/version_check] [5]
ERROR: Tools/TestWebKitAPI/Tests/WebCore/cocoa/AVFoundationSoftLinkTest.mm:159:  Misplaced OS version check. Please use a named macro in wtf/Platform.h, wtf/FeatureDefines.h, or an appropriate internal file.  [build/version_check] [5]
ERROR: Source/WebCore/platform/graphics/avfoundation/objc/MediaPlaybackTargetPickerMac.mm:40:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/objc/CDMInstanceFairPlayStreamingAVFObjC.mm:44:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/objc/CDMInstanceFairPlayStreamingAVFObjC.mm:417:  Missing space around : in range-based for statement  [whitespace/colon] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/objc/CDMInstanceFairPlayStreamingAVFObjC.mm:476:  Missing space around : in range-based for statement  [whitespace/colon] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.mm:50:  *SoftLink.h header should be included after all other headers.  [build/include_order] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.mm:51:  *SoftLink.h header should be included after all other headers.  [build/include_order] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.mm:56:  *SoftLink.h header should be included after all other headers.  [build/include_order] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.mm:57:  *SoftLink.h header should be included after all other headers.  [build/include_order] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.mm:222:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebCore/platform/audio/ios/MediaSessionManagerIOS.mm:48:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/mediarecorder/cocoa/MediaRecorderPrivateWriterCocoa.mm:40:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/MediaPlaybackTargetMac.mm:34:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebKit/WebProcess/WebPage/RemoteLayerTree/PlatformCALayerRemoteCustom.mm:39:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/mediastream/ios/AVAudioSessionCaptureDeviceManager.mm:36:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/mediastream/mac/AVVideoCaptureSource.mm:49:  *SoftLink.h header should be included after all other headers.  [build/include_order] [4]
ERROR: Source/WebCore/platform/mediastream/mac/AVVideoCaptureSource.mm:50:  *SoftLink.h header should be included after all other headers.  [build/include_order] [4]
ERROR: Source/WebCore/platform/audio/ios/AudioSessionIOS.mm:37:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/mediastream/ios/CoreAudioCaptureSourceIOS.mm:35:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebKitLegacy/mac/WebView/WebVideoFullscreenController.mm:40:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/mediastream/mac/AVCaptureDeviceManager.mm:45:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/mac/SerializedPlatformRepresentationMac.mm:43:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/MediaSelectionGroupAVFObjC.mm:43:  *SoftLink.h header should be included after all other headers.  [build/include_order] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/objc/CDMSessionAVContentKeySession.mm:44:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/objc/MediaSampleAVFObjC.mm:36:  *SoftLink.h header should be included after all other headers.  [build/include_order] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/objc/CDMSessionAVStreamSession.mm:45:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/ios/PlatformSpeechSynthesizerIOS.mm:36:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebKit/Shared/ios/WebIconUtilities.mm:40:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:140:  *SoftLink.h header should be included after all other headers.  [build/include_order] [4]
Total errors found: 35 in 47 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 15 Eric Carlson 2019-04-23 11:05:24 PDT
Comment on attachment 367961 [details]
Patch

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

>> Source/WebCore/PAL/pal/cocoa/AVFoundationSoftLink.h:67
>> +SOFT_LINK_CLASS_FOR_HEADER(PAL, AVMutableAudioMixInputParameters)
> 
> Should we sort them lexicographically?

Fixed.

>> Source/WebCore/WebCore.xcodeproj/project.pbxproj:32568
>> +				070F2DDB226DF54200D7BBA3 /* AudioSessionIOS.mm in Sources */,
> 
> Changes not needed.

Fixed.

>> Source/WebCore/platform/audio/ios/MediaSessionManagerIOS.mm:291
>> +            [[NSNotificationCenter defaultCenter] addObserver:protectedSelf selector:@selector(wirelessRoutesAvailableDidChange:) name:AVRouteDetectorMultipleRoutesDetectedDidChangeNotification object:protectedSelf->_routeDetector.get()];
> 
> I would mention somewhere why we have AVRouteDetectorMultipleRoutesDetectedDidChangeNotification here but we have PAL::getAVAudioSessionClass() now

OK.

>> Source/WebCore/platform/graphics/avfoundation/AVTrackPrivateAVFObjCImpl.mm:38
>> +#import <pal/cocoa/AVFoundationSoftLink.h>
> 
> Add a space so that it is kept after all includes/imports?

Fixed this and the others

>> Source/WebCore/platform/mediastream/mac/AVVideoCaptureSource.mm:587
>> +        for (AVFrameRateRange *range in [format videoSupportedFrameRateRanges])
> 
> AVFrameRateRange* ?
> Or on the contrary AVCaptureDeviceFormat *format above?

Fixed.
Comment 16 WebKit Commit Bot 2019-04-23 13:13:47 PDT
Comment on attachment 368041 [details]
Patch for landing

Clearing flags on attachment: 368041

Committed r244556: <https://trac.webkit.org/changeset/244556>
Comment 17 WebKit Commit Bot 2019-04-23 13:13:49 PDT
All reviewed patches have been landed.  Closing bug.
Comment 18 WebKit Commit Bot 2019-04-23 14:35:22 PDT
Re-opened since this is blocked by bug 197212
Comment 19 Eric Carlson 2019-04-24 12:52:17 PDT
Created attachment 368162 [details]
Updated patch for landing
Comment 20 EWS Watchlist 2019-04-24 12:55:06 PDT
Attachment 368162 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/mediastream/mac/MockRealtimeVideoSourceMac.mm:50:  *SoftLink.h header should be included after all other headers.  [build/include_order] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/objc/ImageDecoderAVFObjC.mm:59:  *SoftLink.h header should be included after all other headers.  [build/include_order] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/objc/ImageDecoderAVFObjC.mm:60:  *SoftLink.h header should be included after all other headers.  [build/include_order] [4]
ERROR: Source/WebCore/platform/graphics/cocoa/HEVCUtilitiesCocoa.mm:36:  *SoftLink.h header should be included after all other headers.  [build/include_order] [4]
ERROR: Tools/TestWebKitAPI/Tests/WebCore/cocoa/AVFoundationSoftLinkTest.mm:139:  Misplaced OS version check. Please use a named macro in wtf/Platform.h, wtf/FeatureDefines.h, or an appropriate internal file.  [build/version_check] [5]
ERROR: Tools/TestWebKitAPI/Tests/WebCore/cocoa/AVFoundationSoftLinkTest.mm:144:  Misplaced OS version check. Please use a named macro in wtf/Platform.h, wtf/FeatureDefines.h, or an appropriate internal file.  [build/version_check] [5]
ERROR: Tools/TestWebKitAPI/Tests/WebCore/cocoa/AVFoundationSoftLinkTest.mm:159:  Misplaced OS version check. Please use a named macro in wtf/Platform.h, wtf/FeatureDefines.h, or an appropriate internal file.  [build/version_check] [5]
ERROR: Source/WebCore/platform/graphics/avfoundation/objc/MediaPlaybackTargetPickerMac.mm:40:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/objc/CDMInstanceFairPlayStreamingAVFObjC.mm:44:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/objc/CDMInstanceFairPlayStreamingAVFObjC.mm:417:  Missing space around : in range-based for statement  [whitespace/colon] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/objc/CDMInstanceFairPlayStreamingAVFObjC.mm:476:  Missing space around : in range-based for statement  [whitespace/colon] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.mm:50:  *SoftLink.h header should be included after all other headers.  [build/include_order] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.mm:51:  *SoftLink.h header should be included after all other headers.  [build/include_order] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.mm:56:  *SoftLink.h header should be included after all other headers.  [build/include_order] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.mm:57:  *SoftLink.h header should be included after all other headers.  [build/include_order] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.mm:222:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebCore/platform/audio/ios/MediaSessionManagerIOS.mm:48:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/mediarecorder/cocoa/MediaRecorderPrivateWriterCocoa.mm:40:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/MediaPlaybackTargetMac.mm:34:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebKit/WebProcess/WebPage/RemoteLayerTree/PlatformCALayerRemoteCustom.mm:39:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/mediastream/ios/AVAudioSessionCaptureDeviceManager.mm:36:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/mediastream/mac/AVVideoCaptureSource.mm:49:  *SoftLink.h header should be included after all other headers.  [build/include_order] [4]
ERROR: Source/WebCore/platform/mediastream/mac/AVVideoCaptureSource.mm:50:  *SoftLink.h header should be included after all other headers.  [build/include_order] [4]
ERROR: Source/WebCore/platform/audio/ios/AudioSessionIOS.mm:37:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/mediastream/ios/CoreAudioCaptureSourceIOS.mm:35:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebKitLegacy/mac/WebView/WebVideoFullscreenController.mm:40:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/mediastream/mac/AVCaptureDeviceManager.mm:45:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/mac/SerializedPlatformRepresentationMac.mm:43:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/MediaSelectionGroupAVFObjC.mm:43:  *SoftLink.h header should be included after all other headers.  [build/include_order] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/objc/CDMSessionAVContentKeySession.mm:44:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/objc/MediaSampleAVFObjC.mm:36:  *SoftLink.h header should be included after all other headers.  [build/include_order] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/objc/CDMSessionAVStreamSession.mm:45:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/ios/PlatformSpeechSynthesizerIOS.mm:36:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebKit/Shared/ios/WebIconUtilities.mm:40:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:140:  *SoftLink.h header should be included after all other headers.  [build/include_order] [4]
Total errors found: 35 in 48 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 21 Eric Carlson 2019-04-24 12:57:03 PDT
Created attachment 368164 [details]
Updated patch for landing
Comment 22 EWS Watchlist 2019-04-24 12:59:43 PDT
Attachment 368164 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/mediastream/mac/MockRealtimeVideoSourceMac.mm:50:  *SoftLink.h header should be included after all other headers.  [build/include_order] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/objc/ImageDecoderAVFObjC.mm:59:  *SoftLink.h header should be included after all other headers.  [build/include_order] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/objc/ImageDecoderAVFObjC.mm:60:  *SoftLink.h header should be included after all other headers.  [build/include_order] [4]
ERROR: Source/WebCore/platform/graphics/cocoa/HEVCUtilitiesCocoa.mm:36:  *SoftLink.h header should be included after all other headers.  [build/include_order] [4]
ERROR: Tools/TestWebKitAPI/Tests/WebCore/cocoa/AVFoundationSoftLinkTest.mm:139:  Misplaced OS version check. Please use a named macro in wtf/Platform.h, wtf/FeatureDefines.h, or an appropriate internal file.  [build/version_check] [5]
ERROR: Tools/TestWebKitAPI/Tests/WebCore/cocoa/AVFoundationSoftLinkTest.mm:144:  Misplaced OS version check. Please use a named macro in wtf/Platform.h, wtf/FeatureDefines.h, or an appropriate internal file.  [build/version_check] [5]
ERROR: Tools/TestWebKitAPI/Tests/WebCore/cocoa/AVFoundationSoftLinkTest.mm:159:  Misplaced OS version check. Please use a named macro in wtf/Platform.h, wtf/FeatureDefines.h, or an appropriate internal file.  [build/version_check] [5]
ERROR: Source/WebCore/platform/graphics/avfoundation/objc/MediaPlaybackTargetPickerMac.mm:40:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/objc/CDMInstanceFairPlayStreamingAVFObjC.mm:44:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/objc/CDMInstanceFairPlayStreamingAVFObjC.mm:417:  Missing space around : in range-based for statement  [whitespace/colon] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/objc/CDMInstanceFairPlayStreamingAVFObjC.mm:476:  Missing space around : in range-based for statement  [whitespace/colon] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.mm:50:  *SoftLink.h header should be included after all other headers.  [build/include_order] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.mm:51:  *SoftLink.h header should be included after all other headers.  [build/include_order] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.mm:56:  *SoftLink.h header should be included after all other headers.  [build/include_order] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.mm:57:  *SoftLink.h header should be included after all other headers.  [build/include_order] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.mm:222:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebCore/platform/audio/ios/MediaSessionManagerIOS.mm:48:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/mediarecorder/cocoa/MediaRecorderPrivateWriterCocoa.mm:40:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/MediaPlaybackTargetMac.mm:34:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebKit/WebProcess/WebPage/RemoteLayerTree/PlatformCALayerRemoteCustom.mm:39:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/mediastream/ios/AVAudioSessionCaptureDeviceManager.mm:36:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/mediastream/mac/AVVideoCaptureSource.mm:49:  *SoftLink.h header should be included after all other headers.  [build/include_order] [4]
ERROR: Source/WebCore/platform/mediastream/mac/AVVideoCaptureSource.mm:50:  *SoftLink.h header should be included after all other headers.  [build/include_order] [4]
ERROR: Source/WebCore/platform/audio/ios/AudioSessionIOS.mm:37:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/mediastream/ios/CoreAudioCaptureSourceIOS.mm:35:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebKitLegacy/mac/WebView/WebVideoFullscreenController.mm:40:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/mediastream/mac/AVCaptureDeviceManager.mm:45:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/mac/SerializedPlatformRepresentationMac.mm:43:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/MediaSelectionGroupAVFObjC.mm:43:  *SoftLink.h header should be included after all other headers.  [build/include_order] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/objc/CDMSessionAVContentKeySession.mm:44:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/objc/MediaSampleAVFObjC.mm:36:  *SoftLink.h header should be included after all other headers.  [build/include_order] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/objc/CDMSessionAVStreamSession.mm:45:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/ios/PlatformSpeechSynthesizerIOS.mm:36:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebKit/Shared/ios/WebIconUtilities.mm:40:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:140:  *SoftLink.h header should be included after all other headers.  [build/include_order] [4]
Total errors found: 35 in 47 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 23 EWS Watchlist 2019-04-24 14:53:53 PDT
Comment on attachment 368164 [details]
Updated patch for landing

Attachment 368164 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/11987074

New failing tests:
imported/w3c/web-platform-tests/service-workers/cache-storage/window/cache-abort.https.html
Comment 24 EWS Watchlist 2019-04-24 14:53:55 PDT
Created attachment 368180 [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.13.6
Comment 25 Eric Carlson 2019-04-24 16:28:42 PDT
(In reply to Build Bot from comment #24)
> Created attachment 368180 [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.13.6

None of the tests that fail use AVFoundation so this seem unrelated:

Unexpected flakiness: text-only failures (5)
  editing/pasteboard/5761530-1.html [ Failure Pass ]
  editing/pasteboard/ios/dom-paste-confirmation.html [ Failure Pass ]
  editing/pasteboard/ios/dom-paste-consecutive-confirmations.html [ Failure Pass ]
  editing/pasteboard/ios/dom-paste-requires-user-gesture.html [ Failure Pass ]
  imported/w3c/web-platform-tests/service-workers/service-worker/update-on-navigation.https.html [ Failure Pass ]

Unexpected flakiness: image-only failures (1)
  editing/pasteboard/copy-element-with-conflicting-background-color-from-rule.html [ ImageOnlyFailure Pass ]


Regressions: Unexpected text-only failures (1)
  imported/w3c/web-platform-tests/service-workers/cache-storage/window/cache-abort.https.html [ Failure ]

Regressions: Unexpected crashes (1)
  webgl/2.0.0/conformance/glsl/constructors/glsl-construct-vec3.html [ Crash ]
Comment 26 WebKit Commit Bot 2019-04-24 16:56:14 PDT
Comment on attachment 368164 [details]
Updated patch for landing

Clearing flags on attachment: 368164

Committed r244627: <https://trac.webkit.org/changeset/244627>
Comment 27 WebKit Commit Bot 2019-04-24 16:56:16 PDT
All reviewed patches have been landed.  Closing bug.
Comment 28 WebKit Commit Bot 2019-04-25 08:40:04 PDT
Re-opened since this is blocked by bug 197282
Comment 29 David Kilzer (:ddkilzer) 2019-04-25 11:19:34 PDT
*** Bug 193678 has been marked as a duplicate of this bug. ***
Comment 30 David Kilzer (:ddkilzer) 2019-04-25 11:21:23 PDT
Not to kick you after this was rolled out, but it would be great if we could make this change at the same time so others who write new soft-linking code to AVFoundation.framework get a warning from the style checker:

diff --git a/Tools/Scripts/webkitpy/style/checkers/cpp.py b/Tools/Scripts/webkitpy/style/checkers/cpp.py
index a1972205b4d..f909b60cf6b 100644
--- a/Tools/Scripts/webkitpy/style/checkers/cpp.py
+++ b/Tools/Scripts/webkitpy/style/checkers/cpp.py
@@ -3356,7 +3356,7 @@ def check_language(filename, clean_lines, line_number, file_extension, include_s
             error(line_number, 'softlink/header', 5,
                   'Never soft-link frameworks in headers. Put the soft-link macros in a source file, or create {framework}SoftLink.{{cpp,mm}} instead.'.format(framework=framework_name))
 
-        frameworks_with_soft_links = ['CoreMedia', 'CoreVideo', 'DataDetectorsCore', 'LocalAuthentication', 'MediaAccessibility', 'MediaRemote', 'PassKit', 'QuickLook', 'UIKit', 'VideoToolbox']
+        frameworks_with_soft_links = ['AVFoundation', 'CoreMedia', 'CoreVideo', 'DataDetectorsCore', 'LocalAuthentication', 'MediaAccessibility', 'MediaRemote', 'PassKit', 'QuickLook', 'UIKit', 'VideoToolbox']
         if framework_name in frameworks_with_soft_links and not re.compile('^\s*SOFT_LINK_(PRIVATE_)?FRAMEWORK_FOR_(HEADER|SOURCE)(_WITH_EXPORT)?\({}\)'.format(framework_name)).search(line):
             error(line_number, 'softlink/framework', 5,
                   'Use {framework}SoftLink.{{cpp,h,mm}} to soft-link to {framework}.framework.'.format(framework=framework_name))
Comment 31 Eric Carlson 2019-04-25 11:37:03 PDT
Created attachment 368254 [details]
Updated patch for landing
Comment 32 EWS Watchlist 2019-04-25 11:39:09 PDT
Attachment 368254 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/mediastream/mac/MockRealtimeVideoSourceMac.mm:50:  *SoftLink.h header should be included after all other headers.  [build/include_order] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/objc/ImageDecoderAVFObjC.mm:59:  *SoftLink.h header should be included after all other headers.  [build/include_order] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/objc/ImageDecoderAVFObjC.mm:60:  *SoftLink.h header should be included after all other headers.  [build/include_order] [4]
ERROR: Source/WebCore/platform/graphics/cocoa/HEVCUtilitiesCocoa.mm:36:  *SoftLink.h header should be included after all other headers.  [build/include_order] [4]
ERROR: Tools/TestWebKitAPI/Tests/WebCore/cocoa/AVFoundationSoftLinkTest.mm:139:  Misplaced OS version check. Please use a named macro in wtf/Platform.h, wtf/FeatureDefines.h, or an appropriate internal file.  [build/version_check] [5]
ERROR: Tools/TestWebKitAPI/Tests/WebCore/cocoa/AVFoundationSoftLinkTest.mm:144:  Misplaced OS version check. Please use a named macro in wtf/Platform.h, wtf/FeatureDefines.h, or an appropriate internal file.  [build/version_check] [5]
ERROR: Tools/TestWebKitAPI/Tests/WebCore/cocoa/AVFoundationSoftLinkTest.mm:159:  Misplaced OS version check. Please use a named macro in wtf/Platform.h, wtf/FeatureDefines.h, or an appropriate internal file.  [build/version_check] [5]
ERROR: Source/WebCore/platform/graphics/avfoundation/objc/MediaPlaybackTargetPickerMac.mm:40:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/objc/CDMInstanceFairPlayStreamingAVFObjC.mm:44:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/objc/CDMInstanceFairPlayStreamingAVFObjC.mm:417:  Missing space around : in range-based for statement  [whitespace/colon] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/objc/CDMInstanceFairPlayStreamingAVFObjC.mm:476:  Missing space around : in range-based for statement  [whitespace/colon] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.mm:50:  *SoftLink.h header should be included after all other headers.  [build/include_order] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.mm:51:  *SoftLink.h header should be included after all other headers.  [build/include_order] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.mm:56:  *SoftLink.h header should be included after all other headers.  [build/include_order] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.mm:57:  *SoftLink.h header should be included after all other headers.  [build/include_order] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.mm:222:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebCore/platform/audio/ios/MediaSessionManagerIOS.mm:48:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/mediarecorder/cocoa/MediaRecorderPrivateWriterCocoa.mm:40:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/MediaPlaybackTargetMac.mm:34:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebKit/WebProcess/WebPage/RemoteLayerTree/PlatformCALayerRemoteCustom.mm:39:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/mediastream/ios/AVAudioSessionCaptureDeviceManager.mm:36:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/mediastream/mac/AVVideoCaptureSource.mm:49:  *SoftLink.h header should be included after all other headers.  [build/include_order] [4]
ERROR: Source/WebCore/platform/mediastream/mac/AVVideoCaptureSource.mm:50:  *SoftLink.h header should be included after all other headers.  [build/include_order] [4]
ERROR: Source/WebCore/platform/audio/ios/AudioSessionIOS.mm:37:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/mediastream/ios/CoreAudioCaptureSourceIOS.mm:35:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebKitLegacy/mac/WebView/WebVideoFullscreenController.mm:40:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/mediastream/mac/AVCaptureDeviceManager.mm:45:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/mac/SerializedPlatformRepresentationMac.mm:43:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/MediaSelectionGroupAVFObjC.mm:43:  *SoftLink.h header should be included after all other headers.  [build/include_order] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/objc/CDMSessionAVContentKeySession.mm:44:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/objc/MediaSampleAVFObjC.mm:36:  *SoftLink.h header should be included after all other headers.  [build/include_order] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/objc/CDMSessionAVStreamSession.mm:45:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/ios/PlatformSpeechSynthesizerIOS.mm:36:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebKit/Shared/ios/WebIconUtilities.mm:40:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:140:  *SoftLink.h header should be included after all other headers.  [build/include_order] [4]
Total errors found: 35 in 47 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 33 WebKit Commit Bot 2019-04-26 12:58:47 PDT
Comment on attachment 368254 [details]
Updated patch for landing

Clearing flags on attachment: 368254

Committed r244704: <https://trac.webkit.org/changeset/244704>
Comment 34 WebKit Commit Bot 2019-04-26 12:58:49 PDT
All reviewed patches have been landed.  Closing bug.