Bug 146733 - [Mac] Two clicks required to enable AirPlay at youtube.com
Summary: [Mac] Two clicks required to enable AirPlay at youtube.com
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Eric Carlson
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2015-07-08 11:13 PDT by Eric Carlson
Modified: 2015-07-08 11:58 PDT (History)
1 user (show)

See Also:


Attachments
Proposed patch. (7.07 KB, patch)
2015-07-08 11:33 PDT, Eric Carlson
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Carlson 2015-07-08 11:13:28 PDT
Some videos at youtube.com don't switch to AirPlay the first time a route is chosen.
Comment 1 Eric Carlson 2015-07-08 11:13:58 PDT
<rdar://problem/21556356>
Comment 2 Eric Carlson 2015-07-08 11:33:46 PDT
Created attachment 256390 [details]
Proposed patch.
Comment 3 Brent Fulgham 2015-07-08 11:41:37 PDT
Comment on attachment 256390 [details]
Proposed patch.

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

r=me. I had a couple of questions about the m_targetChanged flag.

> Source/WebCore/Modules/mediasession/WebMediaSessionManager.cpp:203
>      // Do not take the target if another client has it and the client reporting a state change is not playing.

You should revise this comment to match the code below (or get rid of it?)

> Source/WebCore/Modules/mediasession/WebMediaSessionManager.cpp:283
> +            state->requestedPicker = false;

Do we ever need to reset the 'm_targetChanged' state? WebMediaSessionManager exists for the life of the process, right? So does that mean future changes to target won't work right?
Comment 4 Eric Carlson 2015-07-08 11:56:01 PDT
(In reply to comment #3)
> Comment on attachment 256390 [details]
> Proposed patch.
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=256390&action=review
> 
> r=me. I had a couple of questions about the m_targetChanged flag.
> 
> > Source/WebCore/Modules/mediasession/WebMediaSessionManager.cpp:203
> >      // Do not take the target if another client has it and the client reporting a state change is not playing.
> 
> You should revise this comment to match the code below (or get rid of it?)
> 
Removed.

> > Source/WebCore/Modules/mediasession/WebMediaSessionManager.cpp:283
> > +            state->requestedPicker = false;
> 
> Do we ever need to reset the 'm_targetChanged' state? WebMediaSessionManager
> exists for the life of the process, right? So does that mean future changes
> to target won't work right?
>
Good point, fixed.

Thanks!
Comment 5 Eric Carlson 2015-07-08 11:56:22 PDT
Committed r186519: https://trac.webkit.org/r186519