Bug 144892

Summary: [Mac] Refine media playback target client configuration
Product: WebKit Reporter: Eric Carlson <eric.carlson>
Component: MediaAssignee: Eric Carlson <eric.carlson>
Status: RESOLVED FIXED    
Severity: Normal CC: webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Proposed patch.
none
Updated patch. none

Eric Carlson
Reported 2015-05-11 18:19:05 PDT
Refine media playback target client configuration
Attachments
Proposed patch. (22.13 KB, patch)
2015-05-11 18:52 PDT, Eric Carlson 		no flags
DetailsFormatted DiffDiff
Updated patch. (22.19 KB, patch)
2015-05-11 19:18 PDT, Eric Carlson 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1 2015-05-11 18:25:09 PDT
<rdar://problem/20909871>
Eric Carlson
Comment 2 2015-05-11 18:52:20 PDT
Created attachment 252921 [details] Proposed patch.
Eric Carlson
Comment 3 2015-05-11 19:18:32 PDT
Created attachment 252923 [details] Updated patch.
Brent Fulgham
Comment 4 2015-05-12 09:27:18 PDT
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.
Eric Carlson
Comment 5 2015-05-12 09:38:20 PDT
(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!
Eric Carlson
Comment 6 2015-05-12 09:43:54 PDT
Committed r184204: https://trac.webkit.org/r184204
Note You need to log in before you can comment on or make changes to this bug.