Fix fullscreen and PiP video animation and sizing regressions.
rdar://problem/21930899
Created attachment 257261 [details] Patch
Comment on attachment 257261 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=257261&action=review > Source/WebCore/platform/ios/WebVideoFullscreenControllerAVKit.mm:467 > - RefPtr<WebVideoFullscreenControllerContext> strongThis(this); > - RetainPtr<CALayer> videoFullscreenLayer = [m_videoFullscreenView layer]; > - > - mach_port_name_t fencePort = [[videoFullscreenLayer context] createFencePort]; > - > - WebThreadRun([strongThis, this, frame, fencePort, videoFullscreenLayer] { > - [CATransaction begin]; > - [CATransaction setAnimationDuration:0]; > - if (m_model) > - m_model->setVideoLayerFrame(frame); > - [[videoFullscreenLayer context] setFencePort:fencePort]; > - mach_port_deallocate(mach_task_self(), fencePort); > - [CATransaction commit]; > - }); > + if (m_model) > + m_model->setVideoLayerFrame(frame); Is there anything we do inside setVideoLayerFrame() which assumes it's being run on the web thread? > Source/WebCore/platform/ios/WebVideoFullscreenControllerAVKit.mm:532 > + FloatRect videoLayerFrame = FloatRect(0, 0, videoElementClientRect.width(), videoElementClientRect.height()); You can just do: FloatRect videoLayerFrame = FloatRect(FloatPoint(), videoElementClientRect.size()); Or use constructor syntax rather than assignment. FloatSize is constructible from an IntSize (but not vice-versa).
(In reply to comment #3) > Comment on attachment 257261 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=257261&action=review > > > Source/WebCore/platform/ios/WebVideoFullscreenControllerAVKit.mm:467 > > - RefPtr<WebVideoFullscreenControllerContext> strongThis(this); > > - RetainPtr<CALayer> videoFullscreenLayer = [m_videoFullscreenView layer]; > > - > > - mach_port_name_t fencePort = [[videoFullscreenLayer context] createFencePort]; > > - > > - WebThreadRun([strongThis, this, frame, fencePort, videoFullscreenLayer] { > > - [CATransaction begin]; > > - [CATransaction setAnimationDuration:0]; > > - if (m_model) > > - m_model->setVideoLayerFrame(frame); > > - [[videoFullscreenLayer context] setFencePort:fencePort]; > > - mach_port_deallocate(mach_task_self(), fencePort); > > - [CATransaction commit]; > > - }); > > + if (m_model) > > + m_model->setVideoLayerFrame(frame); > > Is there anything we do inside setVideoLayerFrame() which assumes it's being > run on the web thread? Maybe. Just to be base I'll move the operations that need to be sync'd into the web thread instead of into the UIThread. > > > Source/WebCore/platform/ios/WebVideoFullscreenControllerAVKit.mm:532 > > + FloatRect videoLayerFrame = FloatRect(0, 0, videoElementClientRect.width(), videoElementClientRect.height()); > > You can just do: > > FloatRect videoLayerFrame = FloatRect(FloatPoint(), > videoElementClientRect.size()); > > Or use constructor syntax rather than assignment. FloatSize is constructible > from an IntSize (but not vice-versa). I thought there had to be something more succinct.
Created attachment 257290 [details] Patch
Created attachment 257293 [details] Patch for landing.
Comment on attachment 257293 [details] Patch for landing. Clearing flags on attachment: 257293 Committed r187183: <http://trac.webkit.org/changeset/187183>
All reviewed patches have been landed. Closing bug.
Build fix: http://trac.webkit.org/changeset/187225