sub-task of bug 223298 The NowPlayingManager already does almost the same work as done by MediaSessionManagerCocoa: 1- Setup and control RemoteCommandListener 2- Allow to set NowPlayingInfo to RemoteMedia instance. It does 2) via calling back the MediaSessionManagerCocoa::setNowPlayingInfo/clearPlayingInfo static method. NowPlayingManager is currently only used in the GPU process and acts as a proxy to MediaSessionManagerCocoa in the WebContent process. The flow currently being: with GPU process: MediaSessionManagerCocoa -> WebPlatformStrategies -> XPC to GPU -> GPUConnectionToWebProcess -> NowPlayingInfoManager -> MediaSessionManagerCocoa(static) -> MediaRemote without GPU process: MediaSessionManagerCocoa -> WebPlatformStrategies -> MediaSessionManagerCocoa(static) -> MediaRemote in bug 223298 we want to add special handling so that the MediaSessionManagerCocoa can set the NowPlayingInfo's artwork without having to always send the image. By doing: MediaSessionManagerCocoa -> NowPlayingInfoManager -> WebPlatformStrategies [-> XPC to GPU -> GPUConnectionToWebProcess] -> NowPlayingInfoManager -> MediaRemote We can simplify and centralise the code so that NowPlayingInfoManager will be responsible in calling the web strategies and the caching at both end of the XPC channel.
<rdar://problem/75567198>
Created attachment 423809 [details] Patch
Created attachment 423814 [details] Patch
Comment on attachment 423814 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=423814&action=review > Source/WebCore/platform/NowPlayingManager.cpp:65 > + m_setAsNowPlayingApplication = false; Do we need both `m_nowPlayingInfo` and `m_setAsNowPlayingApplication` now - couldn't we clear `m_nowPlayingInfo` here and check `!m_nowPlayingInfo` where we currently check `m_setAsNowPlayingApplication`? > Source/WebKit/GPUProcess/GPUConnectionToWebProcess.cpp:390 > + // FIXME: Eric, why is this call needed? This will reset the seek information from the one set in setNowPlayingInfo > updateSupportedRemoteCommands(); It is needed if we are just switched to a different element, which may have a different set of supported commands.
Comment on attachment 423814 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=423814&action=review >> Source/WebCore/platform/NowPlayingManager.cpp:65 >> + m_setAsNowPlayingApplication = false; > > Do we need both `m_nowPlayingInfo` and `m_setAsNowPlayingApplication` now - couldn't we clear `m_nowPlayingInfo` here and check `!m_nowPlayingInfo` where we currently check `m_setAsNowPlayingApplication`? Not with the logic as-is m_setAsNowPlayingApplication is only set after the first call to setNowPlayingInfoPrivate, if we used to just check if m_nowPlayingInfo is set, then we would always pass true to MediaSessionManagerCocoa::setNowPlayingInfo And we need to set m_nowPlayingInfo before calling it as in bug 223298 as the parameter is const. So while we could probably combined the two members, I do feel like it would make the code harder to read. As such, I prefer to leave it as-is.
Created attachment 423960 [details] Patch remove FIXME question:
Committed r274829: <https://commits.webkit.org/r274829> All reviewed patches have been landed. Closing bug and clearing flags on attachment 423960 [details].