[webkit-reviews] review granted: [Bug 193128] Make DidPlayMediaPreventedFromPlaying autoplay event more generic : [Attachment 358288] Patch

[bugzilla-daemon at webkit.org]

Jer Noble <jer.noble at apple.com> has granted Matt Rajca <mrajca at apple.com>'s
request for review:
Bug 193128: Make DidPlayMediaPreventedFromPlaying autoplay event more generic
https://bugs.webkit.org/show_bug.cgi?id=193128

Attachment 358288: Patch

https://bugs.webkit.org/attachment.cgi?id=358288&action=review




--- Comment #4 from Jer Noble <jer.noble at apple.com> ---
Comment on attachment 358288
  --> https://bugs.webkit.org/attachment.cgi?id=358288
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=358288&action=review

r=me, with nits.

> Source/WebCore/html/HTMLMediaElement.cpp:-3576
> -	      
handleAutoplayEvent(AutoplayEvent::DidPlayMediaPreventedFromPlaying);

This line...

> Source/WebCore/html/HTMLMediaElement.cpp:3577
> +	       handleAutoplayEvent(AutoplayEvent::DidPlayMediaWithUserGesture);

And this line seem to not be parallel changes. Is this new enum correct?

> Source/WebCore/html/HTMLMediaElement.cpp:3938
> +	   if (m_autoplayEventPlaybackState ==
AutoplayEventPlaybackState::StartedWithoutUserGesture)
> +	      
handleAutoplayEvent(AutoplayEvent::DidAutoplayMediaPastThresholdWithoutUserInte
rference);
> +	   else {
> +	       ASSERT(m_autoplayEventPlaybackState ==
AutoplayEventPlaybackState::StartedWithUserGesture);
> +	       handleAutoplayEvent(AutoplayEvent::DidPlayMediaWithUserGesture);
> +	   }

I think this could be dramatically simplified to:

handleAutoplayEvent(m_autoplayEventPlaybackState ==
AutoplayEventPlaybackState::StartedWithoutUserGesture ?
AutoplayEvent::DidAutoplayMediaPastThresholdWithoutUserInterference :
AutoplayEvent::DidPlayMediaWithUserGesture);