WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
197171
Create AVFoundationSoftLink.{h,mm} to reduce duplicate code
https://bugs.webkit.org/show_bug.cgi?id=197171
Summary
Create AVFoundationSoftLink.{h,mm} to reduce duplicate code
Eric Carlson
Reported
2019-04-22 11:21:50 PDT
Create AVFoundationSoftLink.{h,mm} to reduce dirty memory pages and duplicate code.
Attachments
Patch
(254.44 KB, patch)
2019-04-22 12:53 PDT
,
Eric Carlson
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from ews102 for mac-highsierra
(3.46 MB, application/zip)
2019-04-22 14:30 PDT
,
EWS Watchlist
no flags
Details
Archive of layout-test-results from ews115 for mac-highsierra
(3.52 MB, application/zip)
2019-04-22 16:13 PDT
,
EWS Watchlist
no flags
Details
Archive of layout-test-results from ews107 for mac-highsierra-wk2
(3.64 MB, application/zip)
2019-04-22 17:18 PDT
,
EWS Watchlist
no flags
Details
Archive of layout-test-results from ews123 for ios-simulator-wk2
(24.68 MB, application/zip)
2019-04-22 19:04 PDT
,
EWS Watchlist
no flags
Details
Patch for landing
(256.58 KB, patch)
2019-04-23 10:46 PDT
,
Eric Carlson
no flags
Details
Formatted Diff
Diff
Updated patch for landing
(258.28 KB, patch)
2019-04-24 12:52 PDT
,
Eric Carlson
no flags
Details
Formatted Diff
Diff
Updated patch for landing
(256.72 KB, patch)
2019-04-24 12:57 PDT
,
Eric Carlson
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from ews124 for ios-simulator-wk2
(2.87 MB, application/zip)
2019-04-24 14:53 PDT
,
EWS Watchlist
no flags
Details
Updated patch for landing
(257.00 KB, patch)
2019-04-25 11:37 PDT
,
Eric Carlson
no flags
Details
Formatted Diff
Diff
Show Obsolete
(8)
View All
Add attachment
proposed patch, testcase, etc.
Eric Carlson
Comment 1
2019-04-22 11:22:04 PDT
<
rdar://problem/47454979
>
Eric Carlson
Comment 2
2019-04-22 12:53:29 PDT
Created
attachment 367961
[details]
Patch
EWS Watchlist
Comment 3
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.
EWS Watchlist
Comment 4
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
EWS Watchlist
Comment 5
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
youenn fablet
Comment 6
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?
EWS Watchlist
Comment 7
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
EWS Watchlist
Comment 8
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
EWS Watchlist
Comment 9
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
EWS Watchlist
Comment 10
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
EWS Watchlist
Comment 11
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
EWS Watchlist
Comment 12
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
Eric Carlson
Comment 13
2019-04-23 10:46:57 PDT
Created
attachment 368041
[details]
Patch for landing
EWS Watchlist
Comment 14
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.
Eric Carlson
Comment 15
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.
WebKit Commit Bot
Comment 16
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
>
WebKit Commit Bot
Comment 17
2019-04-23 13:13:49 PDT
All reviewed patches have been landed. Closing bug.
WebKit Commit Bot
Comment 18
2019-04-23 14:35:22 PDT
Re-opened since this is blocked by
bug 197212
Eric Carlson
Comment 19
2019-04-24 12:52:17 PDT
Created
attachment 368162
[details]
Updated patch for landing
EWS Watchlist
Comment 20
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.
Eric Carlson
Comment 21
2019-04-24 12:57:03 PDT
Created
attachment 368164
[details]
Updated patch for landing
EWS Watchlist
Comment 22
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.
EWS Watchlist
Comment 23
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
EWS Watchlist
Comment 24
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
Eric Carlson
Comment 25
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 ]
WebKit Commit Bot
Comment 26
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
>
WebKit Commit Bot
Comment 27
2019-04-24 16:56:16 PDT
All reviewed patches have been landed. Closing bug.
WebKit Commit Bot
Comment 28
2019-04-25 08:40:04 PDT
Re-opened since this is blocked by
bug 197282
David Kilzer (:ddkilzer)
Comment 29
2019-04-25 11:19:34 PDT
***
Bug 193678
has been marked as a duplicate of this bug. ***
David Kilzer (:ddkilzer)
Comment 30
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))
Eric Carlson
Comment 31
2019-04-25 11:37:03 PDT
Created
attachment 368254
[details]
Updated patch for landing
EWS Watchlist
Comment 32
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.
WebKit Commit Bot
Comment 33
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
>
WebKit Commit Bot
Comment 34
2019-04-26 12:58:49 PDT
All reviewed patches have been landed. Closing bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug