Bug 221886 – Cleanup exit/enterFullScreenHandler and how the global states are set.

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

[Jean-Yves Avenard [:jya]]

This is a follow-up to bug 221863 that saw crashes due to an infinite recursion happening when entering/exiting full screen.

The crash occurs when VideoFullscreenInterfaceAVKit::doEnterFullscreen is called, attempts to enter full screen by calling [AVPlayerViewController _transitionFromFullScreenAnimated] which errors causing doEnterFullScreen to be called again.


```
Thread 0 name:  Dispatch queue: com.apple.main-thread
Thread 0 Crashed ↩:
0   libsystem_trace.dylib         	0x00000001a0fc92fc _os_log_fmt_flatten_to_blob + 68 (format.c:739)
1   libsystem_trace.dylib         	0x00000001a0fce608 _os_log_impl_flatten_and_send + 2188 (format.c:839)
2   libsystem_trace.dylib         	0x00000001a0fce608 _os_log_impl_flatten_and_send + 2188 (format.c:839)
3   libsystem_trace.dylib         	0x00000001a0fd098c _os_log_impl_dynamic + 316 (log.c:2346)
4   libsystem_trace.dylib         	0x00000001a0fd0db4 _os_log_with_args_impl + 160 (log.c:2611)
5   JavaScriptCore                	0x0000000195090d9c vprintf_stderr_common(char const*, char*) + 296 (Assertions.cpp:152)
6   JavaScriptCore                	0x000000019508f6e0 vprintf_stderr_with_trailing_newline(char const*, char*) + 284 (Assertions.cpp:204)
7   JavaScriptCore                	0x000000019508f794 WTFLogAlways + 28 (Assertions.cpp:478)
8   WebCore                       	0x0000000199749a80 WebCore::VideoFullscreenInterfaceAVKit::enterFullscreenHandler(bool, NSError*) + 68 (VideoFullscreenInterfaceAVKit.mm:1573)
9   AVKit                         	0x00000001a0696ff8 -[AVPlayerViewController _transitionToFullScreenAnimated:interactive:completionHandler:] + 388 (AVPlayerViewController_Mobile.m:2066)
10  AVKit                         	0x00000001a069d418 -[AVPlayerViewController(AVPlayerViewController_WebKitOnly) enterFullScreenAnimated:completionHandler:] + 884 (AVPlayerViewController_Mobile.m:2848)
11  WebCore                       	0x00000001997488d8 WebCore::VideoFullscreenInterfaceAVKit::doEnterFullscreen() + 212 (VideoFullscreenInterfaceAVKit.mm:1445)
12  AVKit                         	0x00000001a0696ff8 -[AVPlayerViewController _transitionToFullScreenAnimated:interactive:completionHandler:] + 388 (AVPlayerViewController_Mobile.m:2066)
13  AVKit                         	0x00000001a069d418 -[AVPlayerViewController(AVPlayerViewController_WebKitOnly) enterFullScreenAnimated:completionHandler:] + 884 (AVPlayerViewController_Mobile.m:2848)
14  WebCore                       	0x00000001997488d8 WebCore::VideoFullscreenInterfaceAVKit::doEnterFullscreen() + 212 (VideoFullscreenInterfaceAVKit.mm:1445)
15  AVKit                         	0x00000001a0696ff8 -[AVPlayerViewController _transitionToFullScreenAnimated:interactive:completionHandler:] + 388 (AVPlayerViewController_Mobile.m:2066)
16  AVKit                         	0x00000001a069d418 -[AVPlayerViewController(AVPlayerViewController_WebKitOnly) enterFullScreenAnimated:completionHandler:] + 884 (AVPlayerViewController_Mobile.m:2848)
17  WebCore                       	0x00000001997488d8 WebCore::VideoFullscreenInterfaceAVKit::doEnterFullscreen() + 212 (VideoFullscreenInterfaceAVKit.mm:1445)
18  AVKit                         	0x00000001a0696ff8 -[AVPlayerViewController _transitionToFullScreenAnimated:interactive:completionHandler:] + 388 (AVPlayerViewController_Mobile.m:2066)
19  AVKit                         	0x00000001a069d418 -[AVPlayerViewController(AVPlayerViewController_WebKitOnly) enterFullScreenAnimated:completionHandler:] + 884 (AVPlayerViewController_Mobile.m:2848)
20  WebCore                       	0x00000001997488d8 WebCore::VideoFullscreenInterfaceAVKit::doEnterFullscreen() + 212 (VideoFullscreenInterfaceAVKit.mm:1445)
21  AVKit                         	0x00000001a0696ff8 -[AVPlayerViewController _transitionToFullScreenAnimated:interactive:completionHandler:] + 388 (AVPlayerViewController_Mobile.m:2066)
22  AVKit                         	0x00000001a069d418 -[AVPlayerViewController(AVPlayerViewController_WebKitOnly) enterFullScreenAnimated:completionHandler:] + 884 (AVPlayerViewController_Mobile.m:2848)
23  WebCore                       	0x00000001997488d8 WebCore::VideoFullscreenInterfaceAVKit::doEnterFullscreen() + 212 (VideoFullscreenInterfaceAVKit.mm:1445)
24  AVKit                         	0x00000001a0696ff8 -[AVPlayerViewController _transitionToFullScreenAnimated:interactive:completionHandler:] + 388 (AVPlayerViewController_Mobile.m:2066)
25  AVKit                         	0x00000001a069d418 -[AVPlayerViewController(AVPlayerViewController_WebKitOnly) enterFullScreenAnimated:completionHandler:] + 884 (AVPlayerViewController_Mobile.m:2848)
26  WebCore                       	0x00000001997488d8 WebCore::VideoFullscreenInterfaceAVKit::doEnterFullscreen() + 212 (VideoFullscreenInterfaceAVKit.mm:1445)
27  AVKit                         	0x00000001a0696ff8 -[AVPlayerViewController _transitionToFullScreenAnimated:interactive:completionHandler:] + 388 (AVPlayerViewController_Mobile.m:2066)
28  AVKit                         	0x00000001a069d418 -[AVPlayerViewController(AVPlayerViewController_WebKitOnly) enterFullScreenAnimated:completionHandler:] + 884 (AVPlayerViewController_Mobile.m:2848)
29  WebCore                       	0x00000001997488d8 WebCore::VideoFullscreenInterfaceAVKit::doEnterFullscreen() + 212 (VideoFullscreenInterfaceAVKit.mm:1445)
30  AVKit                         	0x00000001a0696ff8 -[AVPlayerViewController _transitionToFullScreenAnimated:interactive:completionHandler:] + 388 (AVPlayerViewController_Mobile.m:2066)
31  AVKit                         	0x00000001a069d418 -[AVPlayerViewController(AVPlayerViewController_WebKitOnly) enterFullScreenAnimated:completionHandler:] + 884 (AVPlayerViewController_Mobile.m:2848)
32  WebCore                       	0x00000001997488d8 WebCore::VideoFullscreenInterfaceAVKit::doEnterFullscreen() + 212 (VideoFullscreenInterfaceAVKit.mm:1445)
33  AVKit                         	0x00000001a0696ff8 -[AVPlayerViewController _transitionToFullScreenAnimated:interactive:completionHandler:] + 388 (AVPlayerViewController_Mobile.m:2066)
34  AVKit                         	0x00000001a069d418 -[AVPlayerViewController(AVPlayerViewController_WebKitOnly) enterFullScreenAnimated:completionHandler:] + 884 (AVPlayerViewController_Mobile.m:2848)
35  WebCore                       	0x00000001997488d8 WebCore::VideoFullscreenInterfaceAVKit::doEnterFullscreen() + 212 (VideoFullscreenInterfaceAVKit.mm:1445)
...
```

As described in bug 221863 comment 3, when switching from fullscreen to PiP; I see the following stack trace occurring:
```
    frame #0: 0x0000000109c4c734 WebCore`WebCore::VideoFullscreenInterfaceAVKit::exitFullscreenHandler(this=0x0000000112e85c18, success=NO, error=domain: "AVKitErrorDomain" - code: 0) at VideoFullscreenInterfaceAVKit.mm:1553:115 [opt]
    frame #1: 0x00000001b9300074 AVKit`-[AVPlayerViewController _transitionFromFullScreenAnimated:interactive:completionHandler:] + 484
    frame #2: 0x00000001b930bfa8 AVKit`-[AVPlayerViewController(AVPlayerViewController_WebKitOnly) exitFullScreenAnimated:completionHandler:] + 1196
    frame #3: 0x0000000109c4b7c0 WebCore`WebCore::VideoFullscreenInterfaceAVKit::doEnterFullscreen(this=0x0000000112e85c18) at VideoFullscreenInterfaceAVKit.mm:1474:9 [opt]
    frame #4: 0x0000000105cafe10 WebKit`WebKit::VideoFullscreenManagerProxy::enterFullscreen(this=0x0000000112eb9448, contextId=(m_identifier = 353)) at VideoFullscreenManagerProxy.mm:603:15 [opt]
    frame #5: 0x0000000103ea3f70 JavaScriptCore`WTF::RunLoop::performWork() [inlined] WTF::Function<void ()>::operator(this=<unavailable>)() const at Function.h:83:35 [opt]
    frame #6: 0x0000000103ea3f5c JavaScriptCore`WTF::RunLoop::performWork(this=0x0000000112edc000) at RunLoop.cpp:128 [opt]
  * frame #7: 0x0000000103ea49f4 JavaScriptCore`WTF::RunLoop::performWork(context=<unavailable>) at RunLoopCF.cpp:46:37 [opt]
    frame #8: 0x00000001a2ddc858 CoreFoundation`__CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ + 28
    frame #9: 0x00000001a2ddc754 CoreFoundation`__CFRunLoopDoSource0 + 208
    frame #10: 0x00000001a2ddba58 CoreFoundation`__CFRunLoopDoSources0 + 268
    frame #11: 0x00000001a2dd5a38 CoreFoundation`__CFRunLoopRun + 820
    frame #12: 0x00000001a2dd51d0 CoreFoundation`CFRunLoopRunSpecific + 600
    frame #13: 0x00000001baa4a734 GraphicsServices`GSEventRunModal + 164
    frame #14: 0x00000001a583bfb8 UIKitCore`-[UIApplication _run] + 1072
    frame #15: 0x00000001a5841828 UIKitCore`UIApplicationMain + 168
    frame #16: 0x0000000102ca535c MobileSafari`main(argc=<unavailable>, argv=<unavailable>) at main.m:123:5 [opt]
    frame #17: 0x00000001a2a91d54 libdyld.dylib`start + 4
```

For some reasons when going into PiP mode from fullscreen, we get a call to -[AVPlayerViewController _transitionFromFullScreenAnimated:interactive:completionHandler:] which will error.
I have a working theory on how bug 221863 could occur: if that method got to get called while we were still in the middle of entering full screen, we would have VideoFullscreenInterfaceAVKit::m_enterFullscreenNeedsEnterFullscreen set to true as the handler is called asynchronously.

The aim of that boolean is to have enterFullScreenHandler set to call again doEnterFullScreen; however it is possible that it is set when the new call to WebKit::VideoFullscreenManagerProxy::enterFullscreen occurs; this would erroneously cause the recursion.

Using member variables to control how a future method will behave is dangerous in asynchronous mode. By limiting the scope of that state to only that specific callback instance we will prevent the above case to occur.

[Jean-Yves Avenard [:jya]]

Forgot to add in the description above (and I unfortunately don't have edit rights): I have the strong suspicion than with the crash report attached to bug 221863, those stack traces are truncated. And that the original called was WebKit::VideoFullscreenManagerProxy::enterFullscreen
following use of PiP

[Jean-Yves Avenard [:jya]]

Created attachment 420270 [details]
Patch

[Jean-Yves Avenard [:jya]]

Comment on attachment 420270 [details]
Patch

This is ios code only

[Darin Adler]

Comment on attachment 420270 [details]
Patch

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

> Source/WebCore/platform/ios/VideoFullscreenInterfaceAVKit.h:236
> +    void exitFullscreenHandler(BOOL success, NSError *, NextActions = NextActions());
> +    void enterFullscreenHandler(BOOL success, NSError *, NextActions = NextActions());

Could consider { } instead of NextActions()

[EWS]

Committed r272853: <https://commits.webkit.org/r272853>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 420270 [details].

[Radar WebKit Bug Importer]

<rdar://problem/74351997>