Bug 208568

Summary: Export NowPlaying commands to GPUProcess when media playing in GPUProcess is enabled
Product: WebKit Reporter: youenn fablet <youennf>
Component: MediaAssignee: youenn fablet <youennf>
Status: RESOLVED FIXED    
Severity: Normal CC: annulen, calvaris, cdumez, commit-queue, eric.carlson, esprehn+autocc, ews-watchlist, glenn, gyuyoung.kim, jer.noble, philipj, ryuan.choi, sergio, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch for landing none

Description youenn fablet 2020-03-04 01:51:19 PST
Export NowPlaying commands to GPUProcess when media playing inGPUProcess is enabled
Comment 1 youenn fablet 2020-03-04 02:13:07 PST
Created attachment 392395 [details]
Patch
Comment 2 youenn fablet 2020-03-04 02:16:13 PST
Created attachment 392396 [details]
Patch
Comment 3 youenn fablet 2020-03-04 05:41:24 PST
Created attachment 392402 [details]
Patch
Comment 4 youenn fablet 2020-03-04 06:16:09 PST
Created attachment 392404 [details]
Patch
Comment 5 youenn fablet 2020-03-04 06:27:06 PST
Created attachment 392407 [details]
Patch
Comment 6 youenn fablet 2020-03-04 06:44:18 PST
Created attachment 392409 [details]
Patch
Comment 7 youenn fablet 2020-03-04 11:07:36 PST
Crashes should go away with https://bugs.webkit.org/show_bug.cgi?id=208578
Comment 8 Eric Carlson 2020-03-04 12:08:09 PST
Comment on attachment 392409 [details]
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.

!!
Comment 9 youenn fablet 2020-03-05 02:24:13 PST
Created attachment 392551 [details]
Patch
Comment 10 youenn fablet 2020-03-05 03:08:19 PST
Created attachment 392554 [details]
Patch for landing
Comment 11 youenn fablet 2020-03-05 03:08:57 PST
Moved the bits from bug 208578 here instead.
Comment 12 WebKit Commit Bot 2020-03-05 03:53:42 PST
Comment on attachment 392554 [details]
Patch for landing

Clearing flags on attachment: 392554

Committed r257913: <https://trac.webkit.org/changeset/257913>
Comment 13 WebKit Commit Bot 2020-03-05 03:53:44 PST
All reviewed patches have been landed.  Closing bug.
Comment 14 Radar WebKit Bug Importer 2020-03-05 03:54:17 PST
<rdar://problem/60079072>