[Webkit-unassigned] [Bug 167214] Record whether a media element was prevented from playing without user interaction
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Fri Jan 20 13:59:13 PST 2017
https://bugs.webkit.org/show_bug.cgi?id=167214
--- Comment #4 from Matt Rajca <mrajca at apple.com> ---
(In reply to comment #3)
> Comment on attachment 299269 [details]
> Patch
>
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=299269&action=review
>
> > Source/WebCore/html/HTMLMediaElement.cpp:3106
> > + bool preventedPlaybackWithoutUserGesture = false;
> > + if (!m_mediaSession->playbackPermitted(*this, &preventedPlaybackWithoutUserGesture)) {
> > + if (preventedPlaybackWithoutUserGesture)
> > + m_preventedFromPlayingWithoutUserGesture = true;
> > return;
> > + }
>
> When is m_preventedFromPlayingWithoutUserGesture ever reset to false?
It will get reset when the user starts playing media again with a user gesture. There's currently a FIXME in that code path but I'll make it part of this patch.
>
> > Source/WebCore/html/HTMLMediaElement.h:719
> > - bool canTransitionFromAutoplayToPlay() const;
> > + bool canTransitionFromAutoplayToPlay(bool* preventedPlaybackWithoutUserGesture) const;
>
> This and ...
>
> > Source/WebCore/html/MediaElementSession.h:55
> > - bool playbackPermitted(const HTMLMediaElement&) const;
> > + bool playbackPermitted(const HTMLMediaElement&, bool* preventedPlaybackWithoutUserGesture) const;
>
> This.
>
> I don't like this "out boolean pointer parameter" change. We should some
> other way of representing failure, on the order of "ExceptionOr<>". For
> example, something like:
>
> enum class PlaybackDenialReason {
> UserGestureRequired,
> PageConsentRequired,
> FullscreenRequired,
> };
> template <typename T>
> class SuccessOr : public optional<T> {
> public:
> explicit constexpr operator bool() const { return !optional::operator
> bool(); }
> };
>
> And have these methods return SuccessOr<PlaybackDenialReason>. I.e.:
>
> SuccessOr<PlaybackDenialReason> playbackPermitted(const
> HTMLMediaElement&) const;
>
> So the call sites can remain exactly the same:
>
> if (!playbackPermitted(m_element)) { ... }
>
> And in the places we care about why playback was denied, we can look at the
> return value:
>
> auto success =
> canTransitionFromAutoplayToPlay(&preventedPlaybackWithoutUserGesture));
> if (success) { ... } else {
> m_preventedFromPlayingWithoutUserGesture = success.value() ==
> PlaybackDenialReason:: UserGestureRequired;
> }
Thanks for the suggestion - that would make the call sites cleaner. Working on a v2 of the patch now.
--
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20170120/ea631512/attachment.html>
More information about the webkit-unassigned
mailing list