System sleeps while playing to wireless target, ending stream.

[Jer Noble]

System sleeps while playing to wireless target, ending stream.

[Jer Noble]

Created attachment 311121 [details]
Patch

[Jer Noble]

Created attachment 311123 [details]
Patch

[Jer Noble]

Created attachment 311126 [details]
Patch

[Jer Noble]

Created attachment 311134 [details]
Patch

[Jer Noble]

Created attachment 311136 [details]
Patch

[Eric Carlson]

Comment on attachment 311136 [details]
Patch

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

> Source/WebCore/ChangeLog:14
> +        one that is playing but not visible, and one that is playing remotely.

Nit: "one that is" => "and one that is"

> Source/WebCore/html/HTMLMediaElement.cpp:6356
> +            m_sleepDisabler = SleepDisabler::create("com.apple.WebCore: HTMLMediaElement playback", type);

Nit: it may help debugging later if the type is in the name string.

> Source/WebCore/html/HTMLMediaElement.cpp:6360
> +        m_player->setShouldDisableSleep(shouldDisableSleep == SleepType::Display);

Nit: setShouldDisableSleep should probably be renamed to something like setShouldDisableDisplaySleep

[Jer Noble]

(In reply to Eric Carlson from comment #6)
> Comment on attachment 311136 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=311136&action=review
> 
> > Source/WebCore/ChangeLog:14
> > +        one that is playing but not visible, and one that is playing remotely.
> 
> Nit: "one that is" => "and one that is"

?

> > Source/WebCore/html/HTMLMediaElement.cpp:6356
> > +            m_sleepDisabler = SleepDisabler::create("com.apple.WebCore: HTMLMediaElement playback", type);
> 
> Nit: it may help debugging later if the type is in the name string.

In `pmset -g assertions` it displays the type immediately before the string passed in here, which is why I didn't add it to the string itself. If we add logging for sleep assertions, we can just add the type to the log string itself.

> > Source/WebCore/html/HTMLMediaElement.cpp:6360
> > +        m_player->setShouldDisableSleep(shouldDisableSleep == SleepType::Display);
> 
> Nit: setShouldDisableSleep should probably be renamed to something like
> setShouldDisableDisplaySleep

Ok, but I'd prefer to do that in a future patch (it'll touch a lot of files).

[WebKit Commit Bot]

Comment on attachment 311136 [details]
Patch

Clearing flags on attachment: 311136

Committed r217447: <http://trac.webkit.org/changeset/217447>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/32479770>