[Webkit-unassigned] [Bug 167214] Record whether a media element was prevented from playing without user interaction

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=167214

Jer Noble <jer.noble at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #299269|review?, commit-queue?      |review-
              Flags|                            |

--- Comment #3 from Jer Noble <jer.noble at apple.com> ---
Comment on attachment 299269
  --> https://bugs.webkit.org/attachment.cgi?id=299269
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?

> 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;
    }

-- 
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/aed2858b/attachment-0001.html>