RESOLVED FIXED 147189
Fix fullscreen and PiP video animation and sizing regressions.
https://bugs.webkit.org/show_bug.cgi?id=147189
Summary Fix fullscreen and PiP video animation and sizing regressions.
Jeremy Jones
Reported 2015-07-22 05:49:58 PDT
Fix fullscreen and PiP video animation and sizing regressions.
Attachments
Patch (6.32 KB, patch)
2015-07-22 05:59 PDT, Jeremy Jones
no flags
Patch (7.69 KB, patch)
2015-07-22 13:36 PDT, Jeremy Jones
jeremyj-wk: commit-queue+
Patch for landing. (7.66 KB, patch)
2015-07-22 13:39 PDT, Jeremy Jones
no flags
Jeremy Jones
Comment 1 2015-07-22 05:52:54 PDT
Jeremy Jones
Comment 2 2015-07-22 05:59:41 PDT
Jer Noble
Comment 3 2015-07-22 10:05:13 PDT
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).
Jeremy Jones
Comment 4 2015-07-22 12:03:50 PDT
(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.
Jeremy Jones
Comment 5 2015-07-22 13:36:25 PDT
Jeremy Jones
Comment 6 2015-07-22 13:39:16 PDT
Created attachment 257293 [details] Patch for landing.
WebKit Commit Bot
Comment 7 2015-07-22 14:32:59 PDT
Comment on attachment 257293 [details] Patch for landing. Clearing flags on attachment: 257293 Committed r187183: <http://trac.webkit.org/changeset/187183>
WebKit Commit Bot
Comment 8 2015-07-22 14:33:02 PDT
All reviewed patches have been landed. Closing bug.
Tim Horton
Comment 9 2015-07-23 03:13:08 PDT
Note You need to log in before you can comment on or make changes to this bug.