<html>
<head>
<base href="https://bugs.webkit.org/" />
</head>
<body>
<p>
<div>
<b><a class="bz_bug_link
bz_status_NEW "
title="NEW - Record whether a media element was prevented from playing without user interaction"
href="https://bugs.webkit.org/show_bug.cgi?id=167214#c4">Comment # 4</a>
on <a class="bz_bug_link
bz_status_NEW "
title="NEW - Record whether a media element was prevented from playing without user interaction"
href="https://bugs.webkit.org/show_bug.cgi?id=167214">bug 167214</a>
from <span class="vcard"><a class="email" href="mailto:mrajca@apple.com" title="Matt Rajca <mrajca@apple.com>"> <span class="fn">Matt Rajca</span></a>
</span></b>
<pre>(In reply to <a href="show_bug.cgi?id=167214#c3">comment #3</a>)
<span class="quote">> Comment on <span class=""><a href="attachment.cgi?id=299269&action=diff" name="attach_299269" title="Patch">attachment 299269</a> <a href="attachment.cgi?id=299269&action=edit" title="Patch">[details]</a></span>
> Patch
>
> View in context:
> <a href="https://bugs.webkit.org/attachment.cgi?id=299269&action=review">https://bugs.webkit.org/attachment.cgi?id=299269&action=review</a>
>
> > 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?</span >
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.
<span class="quote">>
> > 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;
> }</span >
Thanks for the suggestion - that would make the call sites cleaner. Working on a v2 of the patch now.</pre>
</div>
</p>
<hr>
<span>You are receiving this mail because:</span>
<ul>
<li>You are the assignee for the bug.</li>
</ul>
</body>
</html>