Summary: | Add UIClient callback for when picture-in-picture is activated. | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Jeremy Jones <jeremyj-wk> | ||||||||||
Component: | WebKit2 | Assignee: | Jeremy Jones <jeremyj-wk> | ||||||||||
Status: | REOPENED --- | ||||||||||||
Severity: | Normal | CC: | buildbot, commit-queue, jlewis3, ryanhaddad, simon.fraser, thorton, webkit-bug-importer | ||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||
Version: | WebKit Nightly Build | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Bug Depends on: | 172121 | ||||||||||||
Bug Blocks: | 172048 | ||||||||||||
Attachments: |
|
Description
Jeremy Jones
2017-05-12 01:17:15 PDT
Created attachment 309882 [details]
Patch
Comment on attachment 309882 [details] Patch Attachment 309882 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/3724924 New failing tests: fast/dom/Window/querySelectorAll-with-pseudo-elements.html Created attachment 309886 [details]
Archive of layout-test-results from ews115 for mac-elcapitan
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews115 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Created attachment 309913 [details]
Patch
Comment on attachment 309913 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=309913&action=review > Source/WebKit2/UIProcess/API/APIUIClient.h:82 > + virtual void setPictureInPictureActive(WebKit::WebPageProxy*, bool) { } I think "active" is too vague here. It could be taken to mean that the PIP window was brought to the front. Maybe setIsShowingPictureInPicture or something? Created attachment 309920 [details]
Patch
(In reply to Simon Fraser (smfr) from comment #6) > Comment on attachment 309913 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=309913&action=review > > > Source/WebKit2/UIProcess/API/APIUIClient.h:82 > > + virtual void setPictureInPictureActive(WebKit::WebPageProxy*, bool) { } > > I think "active" is too vague here. It could be taken to mean that the PIP > window was brought to the front. Maybe setIsShowingPictureInPicture or > something? OK. setHasVideoInPictureInPicture Comment on attachment 309920 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=309920&action=review > Source/WebKit2/UIProcess/Cocoa/WebVideoFullscreenManagerProxy.mm:373 > + m_page->uiClient().setHasVideoInPictureInPicture(m_page, videoFullscreenMode & MediaPlayerEnums::VideoFullscreenModePictureInPicture); Do we always want to call the UI client, even when the PiP mode doesn't change? > Source/WebKit2/UIProcess/Cocoa/WebVideoFullscreenManagerProxy.mm:502 > + m_page->uiClient().setHasVideoInPictureInPicture(m_page, mode & MediaPlayerEnums::VideoFullscreenModePictureInPicture); Ditto. > Tools/TestWebKitAPI/Tests/WebKit2Cocoa/PictureInPictureDelegate.mm:131 > + sleep(1); // Wait for PIPAgent to launch, or it won't call -pipDidClose: callback. Is 1 second always enough, even on loaded bots? Comment on attachment 309920 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=309920&action=review >> Source/WebKit2/UIProcess/Cocoa/WebVideoFullscreenManagerProxy.mm:373 >> + m_page->uiClient().setHasVideoInPictureInPicture(m_page, videoFullscreenMode & MediaPlayerEnums::VideoFullscreenModePictureInPicture); > > Do we always want to call the UI client, even when the PiP mode doesn't change? With semantics of a setter, it s fine. And that prevents us from having to cache the value here. >> Tools/TestWebKitAPI/Tests/WebKit2Cocoa/PictureInPictureDelegate.mm:131 >> + sleep(1); // Wait for PIPAgent to launch, or it won't call -pipDidClose: callback. > > Is 1 second always enough, even on loaded bots? Unknown. It has worked consistently in my testing. Comment on attachment 309920 [details] Patch Clearing flags on attachment: 309920 Committed r216791: <http://trac.webkit.org/changeset/216791> All reviewed patches have been landed. Closing bug. (In reply to WebKit Commit Bot from comment #11) > Comment on attachment 309920 [details] > Patch > > Clearing flags on attachment: 309920 > > Committed r216791: <http://trac.webkit.org/changeset/216791> The API test added with this change has been timing out on El Capitan since it was added: PictureInPicture.WKPageUIClient https://build.webkit.org/builders/Apple%20El%20Capitan%20Release%20WK1%20%28Tests%29/builds/1610 Re-opened since this is blocked by bug 172121 |