[webkit-reviews] review granted: [Bug 201024] [Picture-in-Picture Web API] Implement HTMLVideoElement.requestPictureInPicture() / Document.exitPictureInPicture() : [Attachment 380584] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Fri Oct 11 08:17:22 PDT 2019
Eric Carlson <eric.carlson at apple.com> has granted Peng Liu
<peng.liu6 at apple.com>'s request for review:
Bug 201024: [Picture-in-Picture Web API] Implement
HTMLVideoElement.requestPictureInPicture() / Document.exitPictureInPicture()
https://bugs.webkit.org/show_bug.cgi?id=201024
Attachment 380584: Patch
https://bugs.webkit.org/attachment.cgi?id=380584&action=review
--- Comment #22 from Eric Carlson <eric.carlson at apple.com> ---
Comment on attachment 380584
--> https://bugs.webkit.org/attachment.cgi?id=380584
Patch
View in context: https://bugs.webkit.org/attachment.cgi?id=380584&action=review
> Source/WebCore/Modules/pictureinpicture/DocumentPictureInPicture.cpp:50
> + auto element = document.pictureInPictureElement();
> + if (!element || !is<HTMLVideoElement>(*element)) {
> + promise->reject(InvalidStateError);
> + return;
> + }
The PiP element should *always* be an HTMLVideoElement, so this ASSERT so we
can catch it in debug builds.
> Source/WebCore/Modules/pictureinpicture/DocumentPictureInPicture.cpp:53
> + HTMLVideoElement& videoElement = downcast<HTMLVideoElement>(*element);
> +
HTMLVideoElementPictureInPicture::from(videoElement)->exitPictureInPicture(WTFM
ove(promise));
Nit: this can be done in one line.
>
Source/WebCore/Modules/pictureinpicture/HTMLVideoElementPictureInPicture.cpp:10
2
> + videoElementPictureInPicture->m_enteringPictureInPicture = true;
> + videoElementPictureInPicture->m_enterPictureInPicturePromise =
WTFMove(promise);
Do you need a both a bool and a Promise? Couldn't you use just the promises to
signal that enter or exit is in progress?
> Source/WebKit/UIProcess/API/C/WKPreferencesRef.h:191
> +// Defaults to false
> +WK_EXPORT void WKPreferencesSetPictureInPictureAPIEnabled(WKPreferencesRef
preferencesRef, bool enabled);
> +WK_EXPORT bool WKPreferencesGetPictureInPictureAPIEnabled(WKPreferencesRef
preferencesRef);
> +
Do we need to add new C API?
More information about the webkit-reviews
mailing list