WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
REOPENED
172023
Add UIClient callback for when picture-in-picture is activated.
https://bugs.webkit.org/show_bug.cgi?id=172023
Summary
Add UIClient callback for when picture-in-picture is activated.
Jeremy Jones
Reported
2017-05-12 01:17:15 PDT
Add UIClient callback for when picture-in-picture is activated.
Attachments
Patch
(29.20 KB, patch)
2017-05-12 01:28 PDT
,
Jeremy Jones
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from ews115 for mac-elcapitan
(1.76 MB, application/zip)
2017-05-12 02:50 PDT
,
Build Bot
no flags
Details
Patch
(27.22 KB, patch)
2017-05-12 10:37 PDT
,
Jeremy Jones
no flags
Details
Formatted Diff
Diff
Patch
(27.33 KB, patch)
2017-05-12 11:29 PDT
,
Jeremy Jones
no flags
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Jeremy Jones
Comment 1
2017-05-12 01:28:00 PDT
Created
attachment 309882
[details]
Patch
Jeremy Jones
Comment 2
2017-05-12 01:28:34 PDT
rdar://problem/30882102
Build Bot
Comment 3
2017-05-12 02:50:49 PDT
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
Build Bot
Comment 4
2017-05-12 02:50:51 PDT
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
Jeremy Jones
Comment 5
2017-05-12 10:37:38 PDT
Created
attachment 309913
[details]
Patch
Simon Fraser (smfr)
Comment 6
2017-05-12 10:47:13 PDT
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?
Jeremy Jones
Comment 7
2017-05-12 11:29:25 PDT
Created
attachment 309920
[details]
Patch
Jeremy Jones
Comment 8
2017-05-12 11:35:27 PDT
(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
Simon Fraser (smfr)
Comment 9
2017-05-12 11:44:08 PDT
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?
Jeremy Jones
Comment 10
2017-05-12 13:20:58 PDT
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.
WebKit Commit Bot
Comment 11
2017-05-12 13:49:30 PDT
Comment on
attachment 309920
[details]
Patch Clearing flags on attachment: 309920 Committed
r216791
: <
http://trac.webkit.org/changeset/216791
>
WebKit Commit Bot
Comment 12
2017-05-12 13:49:31 PDT
All reviewed patches have been landed. Closing bug.
Ryan Haddad
Comment 13
2017-05-13 17:23:53 PDT
(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
WebKit Commit Bot
Comment 14
2017-05-15 09:29:04 PDT
Re-opened since this is blocked by
bug 172121
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug