Bug 184188 – [Extra zoom mode] Replace video with a placeholder image during fullscreen transition

• Status: RESOLVED
• Resolution: FIXED
• Component: Media

[Eric Carlson]

Replace video with a placeholder image during fullscreen transition

[Eric Carlson]

<rdar://problem/38940307>

[Eric Carlson]

Created attachment 336876 [details]
Patch

[EWS Watchlist]

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.

[Eric Carlson]

Created attachment 336882 [details]
Patch

[EWS Watchlist]

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.

[Eric Carlson]

Created attachment 336885 [details]
Patch

[EWS Watchlist]

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.

[youenn fablet]

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.

[Eric Carlson]

Created attachment 336971 [details]
Patch

[EWS Watchlist]

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.

[WebKit Commit Bot]

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

[Eric Carlson]

Created attachment 336978 [details]
Patch

[EWS Watchlist]

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.

[WebKit Commit Bot]

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

[Eric Carlson]

Created attachment 336980 [details]
Patch

[EWS Watchlist]

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.

[WebKit Commit Bot]

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

[Eric Carlson]

Created attachment 337021 [details]
Patch

[EWS Watchlist]

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.

[WebKit Commit Bot]

Comment on attachment 337021 [details]
Patch

Clearing flags on attachment: 337021

Committed r230194: <https://trac.webkit.org/changeset/230194>

[WebKit Commit Bot]

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.