[webkit-reviews] review granted: [Bug 209610] Swipe down gestures cause the video layer to stick for a moment before bouncing back into place : [Attachment 394629] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Peng Liu <peng.liu6 at apple.com>'s
request for review:
Bug 209610: Swipe down gestures cause the video layer to stick for a moment
before bouncing back into place
https://bugs.webkit.org/show_bug.cgi?id=209610

Attachment 394629: Patch

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




--- Comment #3 from Darin Adler <darin at apple.com> ---
Comment on attachment 394629
  --> https://bugs.webkit.org/attachment.cgi?id=394629
Patch

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

We *must* find a way to test these animations rather than just making code
changes and manually trying it out each time. This has turned out to be a
difficult area to code properly and seems likely we will continue to break it
over and over again in the future. It’s worth lots of extra effort to find a
way to test this. Even if we can’t do it right at this moment and need to land
this fix first.

> Source/WebCore/html/HTMLMediaElement.cpp:6132
> +	   scheduleEvent(eventNames().webkitendfullscreenEvent);

Is this correct in the case where we are about to call
*enter*VideoFullscreenForVideoElement because m_videoFullscreenStandby is true?

> Source/WebCore/html/HTMLMediaElement.cpp:6138
>	   else
> -	      
document().page()->chrome().client().exitVideoFullscreenForVideoElement(downcas
t<HTMLVideoElement>(*this));
> -	   scheduleEvent(eventNames().webkitendfullscreenEvent);
> +	       m_fullscreenTaskQueue.enqueueTask([this] {
> +		  
document().page()->chrome().client().exitVideoFullscreenForVideoElement(downcas
t<HTMLVideoElement>(*this));
> +	       });

Change log does not make it clear why enqueing this on the task queue will
properly sequence with the scheduled event. Is there something that makes sure
that the events and the full screen task queue work in sequence?

Multiple-line else body gets braces in WebKit coding style.