Summary: | Fix fullscreen and PiP video animation and sizing regressions. | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Jeremy Jones <jeremyj-wk> | ||||||||
Component: | Media | Assignee: | Jeremy Jones <jeremyj-wk> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | commit-queue, jer.noble, jonlee, thorton, webkit-bug-importer | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | iPhone / iPad | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
Jeremy Jones
2015-07-22 05:49:58 PDT
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 |