Replace video with a placeholder image during fullscreen transition
<rdar://problem/38940307>
Created attachment 336876 [details] Patch
Attachment 336876 [details] did not pass style-queue: ERROR: Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.h:266: The parameter name "type" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 1 in 26 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 336882 [details] Patch
Attachment 336882 [details] did not pass style-queue: ERROR: Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.h:266: The parameter name "type" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 1 in 27 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 336885 [details] Patch
Attachment 336885 [details] did not pass style-queue: ERROR: Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.h:266: The parameter name "type" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 1 in 27 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 336885 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=336885&action=review > Source/WebCore/html/HTMLMediaElement.cpp:6028 > + if (m_player) Is there a case where m_player is nullified before exiting full screen? > Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.h:265 > + enum UpdateType { UpdateSynchronously, UpdateAsynchronously }; Can it be an enum class? > Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:1175 > void MediaPlayerPrivateAVFoundationObjC::setVideoFullscreenLayer(PlatformLayer* videoFullscreenLayer, WTF::Function<void()>&& completionHandler) WTF:: is no longer needed. > Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.mm:1069 > void MediaPlayerPrivateMediaSourceAVFObjC::setVideoFullscreenLayer(PlatformLayer *videoFullscreenLayer, WTF::Function<void()>&& completionHandler) Ditto here. > Source/WebCore/platform/graphics/avfoundation/objc/VideoFullscreenLayerManagerObjC.mm:77 > + [m_videoInlineLayer setContentsGravity:kCAGravityResizeAspect]; Not sure why this is needed now, maybe a comment in changelog? > Source/WebCore/platform/graphics/avfoundation/objc/VideoFullscreenLayerManagerObjC.mm:91 > + [m_videoInlineLayer setContents:(id)image.get()]; This is ObjC so if m_videoInlinLayer is null, would this line be a no op, in which case we can remove if(m_videoInlineLayer) ? > Source/WebCore/platform/ios/VideoFullscreenInterfaceAVKit.mm:1112 > + if (m_waitingForPreparedToExit) { early return? > Source/WebCore/platform/ios/WebVideoFullscreenControllerAVKit.mm:287 > + WebThreadRun([protectedThis = makeRefPtr(this), this] () mutable { makeRef(*this) is usually what we use. Is it supposed to work in WK1? > Source/WebKit/WebProcess/cocoa/VideoFullscreenManager.mm:443 > + dispatch_async(dispatch_get_main_queue(), [protectedThis = makeRefPtr(this), videoElement, contextId] { makeRef(*this) maybe and WTFMove(videoElement) to reduce count churn.
Created attachment 336971 [details] Patch
Attachment 336971 [details] did not pass style-queue: ERROR: Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.h:266: The parameter name "type" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 1 in 27 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 336971 [details] Patch Rejecting attachment 336971 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-03', 'build', '--no-clean', '--no-update', '--build-style=release', '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit Last 500 characters of output: Kit/Source/WebKit/UIProcess/WebContextMenuListenerProxy.cpp -o /Volumes/Data/EWS/WebKit/WebKitBuild/WebKit.build/Release/WebKit.build/Objects-normal/x86_64/WebContextMenuListenerProxy.o ** BUILD FAILED ** The following build commands failed: CompileC /Volumes/Data/EWS/WebKit/WebKitBuild/WebKit.build/Release/WebKit.build/Objects-normal/x86_64/VideoFullscreenManager.o WebProcess/cocoa/VideoFullscreenManager.mm normal x86_64 objective-c++ com.apple.compilers.llvm.clang.1_0.compiler (1 failure) Full output: http://webkit-queues.webkit.org/results/7172584
Created attachment 336978 [details] Patch
Attachment 336978 [details] did not pass style-queue: ERROR: Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.h:266: The parameter name "type" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 1 in 27 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 336978 [details] Patch Rejecting attachment 336978 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-02', 'build', '--no-clean', '--no-update', '--build-style=release', '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit Last 500 characters of output: /Data/EWS/WebKit/Source/WebKit/Shared/WebContextMenuItemData.cpp -o /Volumes/Data/EWS/WebKit/WebKitBuild/WebKit.build/Release/WebKit.build/Objects-normal/x86_64/WebContextMenuItemData.o ** BUILD FAILED ** The following build commands failed: CompileC /Volumes/Data/EWS/WebKit/WebKitBuild/WebKit.build/Release/WebKit.build/Objects-normal/x86_64/VideoFullscreenManager.o WebProcess/cocoa/VideoFullscreenManager.mm normal x86_64 objective-c++ com.apple.compilers.llvm.clang.1_0.compiler (1 failure) Full output: http://webkit-queues.webkit.org/results/7174886
Created attachment 336980 [details] Patch
Attachment 336980 [details] did not pass style-queue: ERROR: Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.h:266: The parameter name "type" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 1 in 27 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 336980 [details] Patch Rejecting attachment 336980 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-01', 'build', '--no-clean', '--no-update', '--build-style=release', '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit Last 500 characters of output: /Data/EWS/WebKit/Source/WebKit/Shared/WebContextMenuItemData.cpp -o /Volumes/Data/EWS/WebKit/WebKitBuild/WebKit.build/Release/WebKit.build/Objects-normal/x86_64/WebContextMenuItemData.o ** BUILD FAILED ** The following build commands failed: CompileC /Volumes/Data/EWS/WebKit/WebKitBuild/WebKit.build/Release/WebKit.build/Objects-normal/x86_64/VideoFullscreenManager.o WebProcess/cocoa/VideoFullscreenManager.mm normal x86_64 objective-c++ com.apple.compilers.llvm.clang.1_0.compiler (1 failure) Full output: http://webkit-queues.webkit.org/results/7175981
Created attachment 337021 [details] Patch
Attachment 337021 [details] did not pass style-queue: ERROR: Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.h:266: The parameter name "type" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 1 in 27 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 337021 [details] Patch Clearing flags on attachment: 337021 Committed r230194: <https://trac.webkit.org/changeset/230194>
All reviewed patches have been landed. Closing bug.
This seems to have broken the !HAVE(AVFOUNDATION_VIDEO_OUTPUT) build.
(In reply to mitz from comment #22) > This seems to have broken the !HAVE(AVFOUNDATION_VIDEO_OUTPUT) build. Bug 184309, already fixed.