Refine media playback target client configuration
<rdar://problem/20909871>
Created attachment 252921 [details] Proposed patch.
Created attachment 252923 [details] Updated patch.
Comment on attachment 252923 [details] Updated patch. View in context: https://bugs.webkit.org/attachment.cgi?id=252923&action=review r=me. I had a couple of minor questions, but nothing that needs to be addressed before landing. > Source/WebCore/Modules/mediasession/WebMediaSessionManager.cpp:37 > +static double taskDelayInterval = 1. / 10.; Do we use this style of constant elsewhere? I thought we always said "1.0 / 10.0" (for example). > Source/WebCore/html/HTMLMediaSession.cpp:389 > + client().setShouldPlayToPlaybackTarget(true); I'm just curious here -- did you encounter raciness with the m_shouldPlayToPlaybackTarget state? Because it seems like the original version should have worked.
(In reply to comment #4) > Comment on attachment 252923 [details] > Updated patch. > > View in context: > https://bugs.webkit.org/attachment.cgi?id=252923&action=review > > r=me. I had a couple of minor questions, but nothing that needs to be > addressed before landing. > > > Source/WebCore/Modules/mediasession/WebMediaSessionManager.cpp:37 > > +static double taskDelayInterval = 1. / 10.; > > Do we use this style of constant elsewhere? I thought we always said "1.0 / > 10.0" (for example). > Hmm, not sure what I was thinking. > > Source/WebCore/html/HTMLMediaSession.cpp:389 > > + client().setShouldPlayToPlaybackTarget(true); > > I'm just curious here -- did you encounter raciness with the > m_shouldPlayToPlaybackTarget state? Because it seems like the original > version should have worked. It worked, but the "if (m_shouldPlayToPlaybackTarget)" above means we actually always call "client().setShouldPlayToPlaybackTarget(true)" so I just made it explicit. Thanks!
Committed r184204: https://trac.webkit.org/r184204