[webkit-reviews] review granted: [Bug 212729] A YouTube video gets stuck after rapidly tapping on touchbar’s PIP button : [Attachment 400996] 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 212729: A YouTube video gets stuck after rapidly tapping on touchbar’s PIP
button
https://bugs.webkit.org/show_bug.cgi?id=212729

Attachment 400996: Patch

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




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

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

> Source/WebCore/platform/cocoa/PlaybackSessionModelMediaElement.mm:37
>  #import "HTMLElement.h"
>  #import "HTMLMediaElement.h"
> +#import "HTMLVideoElement.h"

Don’t need HTMLElement.h or HTMLMediaElement.h if we are including
HTMLVideoElement.h.

> Source/WebCore/platform/cocoa/PlaybackSessionModelMediaElement.mm:319
> +    ASSERT(is<HTMLVideoElement>(*m_mediaElement));
> +    if (is<HTMLVideoElement>(*m_mediaElement)) {

I’d do early return rather than nesting everything in an if statement.

> Source/WebCore/platform/cocoa/PlaybackSessionModelMediaElement.mm:320
> +	   HTMLVideoElement& asVideo =
downcast<HTMLVideoElement>(*m_mediaElement);

I’d write this:

    auto& element = downcast<HTMLVideoElement>(*m_mediaElement);

And then I would use element exclusively in the rest of the function, and not
use m_mediaElement at all.