Improve enter and exit fullscreen transition by using two step transition.
radar://16878025
Created attachment 232830 [details] Patch
Comment on attachment 232830 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=232830&action=review > Source/WebCore/platform/ios/WebVideoFullscreenInterfaceAVKit.mm:742 > + __block RefPtr<WebVideoFullscreenInterfaceAVKit> protect2(this); Nit: You don't need the "2" on the variable name now that isn't in setExternalPlayback. > Source/WebKit2/WebProcess/ios/WebVideoFullscreenManager.mm:195 > + > + dispatch_async(dispatch_get_main_queue(), ^{ No need to protect "this"?
(In reply to comment #3) > (From update of attachment 232830 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=232830&action=review > > > Source/WebCore/platform/ios/WebVideoFullscreenInterfaceAVKit.mm:742 > > + __block RefPtr<WebVideoFullscreenInterfaceAVKit> protect2(this); > > Nit: You don't need the "2" on the variable name now that isn't in setExternalPlayback. Removed the 2. > > > Source/WebKit2/WebProcess/ios/WebVideoFullscreenManager.mm:195 > > + > > + dispatch_async(dispatch_get_main_queue(), ^{ > > No need to protect "this"? Added a protect.
Created attachment 232971 [details] Patch
Comment on attachment 232971 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=232971&action=review > Source/WebCore/platform/ios/WebVideoFullscreenControllerAVKit.mm:156 > + _model->setVideoFullscreenLayer(nullptr); nil if that's a CALayer* > Source/WebCore/platform/ios/WebVideoFullscreenInterfaceAVKit.mm:792 > + [m_window setHidden:YES]; > + [m_window setRootViewController:nil]; > + [m_playerViewController setDelegate:nil]; > + [m_playerViewController setPlayerController:nil]; > + m_playerViewController = nil; > + m_viewController = nil; > + m_window = nil; > + [m_videoLayer removeFromSuperlayer]; > + m_videoLayer = nil; > + [m_videoLayerContainer removeFromSuperlayer]; > + [m_videoLayerContainer setPlayerViewController:nil]; > + m_videoLayerContainer = nil; So much stuff. > Source/WebKit2/UIProcess/ios/WebVideoFullscreenManagerProxy.messages.in:33 > + SetupFullscreenWithID(uint32_t videoLayerID, WebCore::IntRect initialRect) Is this a GraphicsLayer::PlatformLayerID? If so, it needs to be 64-bit. > Source/WebKit2/UIProcess/ios/WebVideoFullscreenManagerProxy.mm:121 > + Blank line.
(In reply to comment #6) > (From update of attachment 232971 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=232971&action=review > > > Source/WebCore/platform/ios/WebVideoFullscreenControllerAVKit.mm:156 > > + _model->setVideoFullscreenLayer(nullptr); > > nil if that's a CALayer* It is a PlatformLayer, which I guess is close enough in an Obj-c file. Change. > > > Source/WebCore/platform/ios/WebVideoFullscreenInterfaceAVKit.mm:792 > > + [m_window setHidden:YES]; > > + [m_window setRootViewController:nil]; > > + [m_playerViewController setDelegate:nil]; > > + [m_playerViewController setPlayerController:nil]; > > + m_playerViewController = nil; > > + m_viewController = nil; > > + m_window = nil; > > + [m_videoLayer removeFromSuperlayer]; > > + m_videoLayer = nil; > > + [m_videoLayerContainer removeFromSuperlayer]; > > + [m_videoLayerContainer setPlayerViewController:nil]; > > + m_videoLayerContainer = nil; > > So much stuff. Sigh. Maybe a session object that cleans up all this implicitly in its destructor. https://bugs.webkit.org/show_bug.cgi?id=133960 > > > Source/WebKit2/UIProcess/ios/WebVideoFullscreenManagerProxy.messages.in:33 > > + SetupFullscreenWithID(uint32_t videoLayerID, WebCore::IntRect initialRect) > > Is this a GraphicsLayer::PlatformLayerID? If so, it needs to be 64-bit. It is one of these: uint32_t LayerHostingContext::contextID() so it stays 32-bit. > > > Source/WebKit2/UIProcess/ios/WebVideoFullscreenManagerProxy.mm:121 > > + > > Blank line. Deleted.
Created attachment 233203 [details] Patch for landing
Comment on attachment 233203 [details] Patch for landing Rejecting attachment 233203 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.appspot.com', '--bot-id=webkit-cq-02', 'validate-changelog', '--check-oops', '--non-interactive', 233203, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit ChangeLog entry in Source/WebCore/ChangeLog contains OOPS!. Full output: http://webkit-queues.appspot.com/results/5873666126512128
Created attachment 233263 [details] Patch for landing.
Comment on attachment 233263 [details] Patch for landing. Clearing flags on attachment: 233263 Committed r170083: <http://trac.webkit.org/changeset/170083>