[webkit-reviews] review granted: [Bug 208568] Export NowPlaying commands to GPUProcess when media playing in GPUProcess is enabled : [Attachment 392409] Patch

[bugzilla-daemon at webkit.org]

Eric Carlson <eric.carlson at apple.com> has granted  review:
Bug 208568: Export NowPlaying commands to GPUProcess when media playing in
GPUProcess is enabled
https://bugs.webkit.org/show_bug.cgi?id=208568

Attachment 392409: Patch

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




--- Comment #8 from Eric Carlson <eric.carlson at apple.com> ---
Comment on attachment 392409
  --> https://bugs.webkit.org/attachment.cgi?id=392409
Patch

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

r=me once the bots are happy

> Source/WebCore/html/MediaElementSession.cpp:993
> +    double currentTime = std::isfinite(m_element.currentTime()) &&
m_element.supportsSeeking() ? m_element.currentTime() :
MediaPlayer::invalidTime();

Nit: m_element.currentTime() can be fairly expensive, so it would be good to
cache it in a local instead of calling twice.

> Source/WebCore/html/MediaElementSession.cpp:995
> +    return NowPlayingInfo { m_element.mediaSessionTitle(),
m_element.sourceApplicationIdentifier(), duration, currentTime,
m_element.supportsSeeking(), m_element.mediaSessionUniqueIdentifier(),
isPlaying, allowsNowPlayingControlsVisibility };

May as well also have a local for m_element.supportsSeeking() since it is used
three times.

> Source/WebCore/platform/audio/cocoa/MediaSessionManagerCocoa.mm:242
> +    double rate =  nowPlayingInfo.isPlaying ? 1 : 0;

Nit: two spaces after '='

> Source/WebCore/platform/audio/cocoa/MediaSessionManagerCocoa.mm:276
> +    // FIXME: Fix this layering violation.

!!