RESOLVED FIXED 189478
Expose a few WebPlaybackControlsManager-driven PIP APIs to clients
https://bugs.webkit.org/show_bug.cgi?id=189478
Summary Expose a few WebPlaybackControlsManager-driven PIP APIs to clients
Matt Rajca
Reported 2018-09-10 12:22:55 PDT
Expose a few WebPlaybackControlsManager-driven PIP APIs to clients.
Attachments
Patch (12.22 KB, patch)
2018-09-10 16:08 PDT, Matt Rajca
no flags
Patch (12.41 KB, patch)
2018-09-10 17:29 PDT, Matt Rajca
no flags
Patch (13.86 KB, patch)
2018-09-11 09:50 PDT, Matt Rajca
eric.carlson: review+
Matt Rajca
Comment 1 2018-09-10 12:23:11 PDT
Matt Rajca
Comment 2 2018-09-10 16:08:08 PDT
Matt Rajca
Comment 3 2018-09-10 17:29:05 PDT
Matt Rajca
Comment 4 2018-09-11 09:50:47 PDT
Eric Carlson
Comment 5 2018-09-11 15:32:04 PDT
Comment on attachment 349402 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=349402&action=review > Source/WebKit/UIProcess/Cocoa/WebViewImpl.mm:1226 > +bool WebViewImpl::isPictureInPictureActive() > +{ > + if (!m_playbackControlsManager) > + return false; > + > + return [m_playbackControlsManager isPictureInPictureActive]; > +} > + > +void WebViewImpl::togglePictureInPicture() > +{ > + [m_playbackControlsManager togglePictureInPicture]; > +} Nit: do you need to NULL-test m_playbackControlsManager in one method but not the other?
Matt Rajca
Comment 6 2018-09-12 12:01:26 PDT
(In reply to Eric Carlson from comment #5) > Comment on attachment 349402 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=349402&action=review > > > Source/WebKit/UIProcess/Cocoa/WebViewImpl.mm:1226 > > +bool WebViewImpl::isPictureInPictureActive() > > +{ > > + if (!m_playbackControlsManager) > > + return false; > > + > > + return [m_playbackControlsManager isPictureInPictureActive]; > > +} > > + > > +void WebViewImpl::togglePictureInPicture() > > +{ > > + [m_playbackControlsManager togglePictureInPicture]; > > +} > > Nit: do you need to NULL-test m_playbackControlsManager in one method but > not the other? Nope, simplified ::isPictureInPictureActive to avoid the nil check since messaging a nil Obj-C object will result in `NO`.
Matt Rajca
Comment 7 2018-09-12 12:10:11 PDT
Note You need to log in before you can comment on or make changes to this bug.