Do not exit fullscreen when starting optimized fullscreen since this is done automatically.
Created attachment 252860 [details] Patch
rdar://problem/20719473
Created attachment 255173 [details] Patch
Comment on attachment 255173 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=255173&action=review Is there a way to make a regression test for this? > Source/WebCore/platform/ios/WebVideoFullscreenInterfaceAVKit.h:108 > + typedef enum { > + ExitFullScreenReasonDoneButtonTapped, > + ExitFullScreenReasonFullScreenButtonTapped, > + ExitFullScreenReasonPinchGestureHandled, > + ExitFullScreenReasonRemoteControlStopEventReceived, > + ExitFullScreenReasonPictureInPictureStarted > + } ExitFullscreenReason; That’s C style. C++ style would be: enum ExitFullScreenReason { ExitFullScreenReasonDoneButtonTapped ... }; And modern C++ style would be: enum class ExitFullScreenReason { DoneButtonTapped ... }; Please use one of those styles. > Source/WebCore/platform/ios/WebVideoFullscreenInterfaceAVKit.mm:245 > + WebVideoFullscreenInterfaceAVKit::ExitFullscreenReason exitReason; > + switch (reason) { > + case AVPlayerViewControllerExitFullScreenReasonDoneButtonTapped: > + exitReason = WebVideoFullscreenInterfaceAVKit::ExitFullScreenReasonDoneButtonTapped; > + break; > + case AVPlayerViewControllerExitFullScreenReasonFullScreenButtonTapped: > + exitReason = WebVideoFullscreenInterfaceAVKit::ExitFullScreenReasonFullScreenButtonTapped; > + break; > + case AVPlayerViewControllerExitFullScreenReasonPictureInPictureStarted: > + exitReason = WebVideoFullscreenInterfaceAVKit::ExitFullScreenReasonPictureInPictureStarted; > + break; > + case AVPlayerViewControllerExitFullScreenReasonPinchGestureHandled: > + exitReason = WebVideoFullscreenInterfaceAVKit::ExitFullScreenReasonPinchGestureHandled; > + break; > + case AVPlayerViewControllerExitFullScreenReasonRemoteControlStopEventReceived: > + exitReason = WebVideoFullscreenInterfaceAVKit::ExitFullScreenReasonRemoteControlStopEventReceived; > + break; > + } Seems like we should factor this out into a helper function that makes the AVPlayerViewController codes to the WebVideoFullscreenInterfaceAVKit ones and does nothing else. Build failed on iOS; I think the codes might be different iOS vs. Mac or iOS 8 vs. iOS 9.
Created attachment 255227 [details] Patch
(In reply to comment #5) > Comment on attachment 255173 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=255173&action=review > > Is there a way to make a regression test for this? > > > Source/WebCore/platform/ios/WebVideoFullscreenInterfaceAVKit.h:108 > > + typedef enum { > > + ExitFullScreenReasonDoneButtonTapped, > > + ExitFullScreenReasonFullScreenButtonTapped, > > + ExitFullScreenReasonPinchGestureHandled, > > + ExitFullScreenReasonRemoteControlStopEventReceived, > > + ExitFullScreenReasonPictureInPictureStarted > > + } ExitFullscreenReason; > > That’s C style. C++ style would be: > > enum ExitFullScreenReason { > ExitFullScreenReasonDoneButtonTapped > ... > }; > > And modern C++ style would be: > > enum class ExitFullScreenReason { > DoneButtonTapped > ... > }; > > Please use one of those styles. Updated to Modern C++ style. > > > Source/WebCore/platform/ios/WebVideoFullscreenInterfaceAVKit.mm:245 > > + WebVideoFullscreenInterfaceAVKit::ExitFullscreenReason exitReason; > > + switch (reason) { > > + case AVPlayerViewControllerExitFullScreenReasonDoneButtonTapped: > > + exitReason = WebVideoFullscreenInterfaceAVKit::ExitFullScreenReasonDoneButtonTapped; > > + break; > > + case AVPlayerViewControllerExitFullScreenReasonFullScreenButtonTapped: > > + exitReason = WebVideoFullscreenInterfaceAVKit::ExitFullScreenReasonFullScreenButtonTapped; > > + break; > > + case AVPlayerViewControllerExitFullScreenReasonPictureInPictureStarted: > > + exitReason = WebVideoFullscreenInterfaceAVKit::ExitFullScreenReasonPictureInPictureStarted; > > + break; > > + case AVPlayerViewControllerExitFullScreenReasonPinchGestureHandled: > > + exitReason = WebVideoFullscreenInterfaceAVKit::ExitFullScreenReasonPinchGestureHandled; > > + break; > > + case AVPlayerViewControllerExitFullScreenReasonRemoteControlStopEventReceived: > > + exitReason = WebVideoFullscreenInterfaceAVKit::ExitFullScreenReasonRemoteControlStopEventReceived; > > + break; > > + } > > Seems like we should factor this out into a helper function that makes the > AVPlayerViewController codes to the WebVideoFullscreenInterfaceAVKit ones > and does nothing else. Done. > > Build failed on iOS; I think the codes might be different iOS vs. Mac or iOS > 8 vs. iOS 9. Added Missing enums in AVKitSPI.h.
Radar updated to: rdar://problem/21467999
Comment on attachment 255227 [details] Patch While I am not an expert on this code, it looks good. r=me
Comment on attachment 255227 [details] Patch Clearing flags on attachment: 255227 Committed r185834: <http://trac.webkit.org/changeset/185834>
All reviewed patches have been landed. Closing bug.