Bug 199471 – HTMLMediaElement can hold onto display sleep assertion while process is suspended.

• Status: RESOLVED
• Resolution: FIXED
• Component: Media

[Jer Noble]

HTMLMediaElement can hold onto display sleep assertion while process is suspended.

[Jer Noble]

<rdar://problem/52124320>

[Jer Noble]

Created attachment 373424 [details]
Patch

[Darin Adler]

Comment on attachment 373424 [details]
Patch

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

> Source/WebCore/html/HTMLMediaElement.cpp:6927
> +    if (PlatformMediaSessionManager::sharedManager().processIsSuspended())
> +        return SleepType::None;

Is this correct even for audio? You can’t hear audio when the process is suspended?

[Jer Noble]

(In reply to Darin Adler from comment #3)
> Comment on attachment 373424 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=373424&action=review
> 
> > Source/WebCore/html/HTMLMediaElement.cpp:6927
> > +    if (PlatformMediaSessionManager::sharedManager().processIsSuspended())
> > +        return SleepType::None;
> 
> Is this correct even for audio? You can’t hear audio when the process is
> suspended?

The danger is that, if the process is suspended, it will never get a chance to release it's process assertion, and so even if audio playback ends in mediaserverd (like when it reaches the end of the file), we'll keep the system from sleeping.

[Darin Adler]

(In reply to Jer Noble from comment #4)
> The danger is that, if the process is suspended, it will never get a chance
> to release it's process assertion, and so even if audio playback ends in
> mediaserverd (like when it reaches the end of the file), we'll keep the
> system from sleeping.

OK, so we’re losing the feature where you can listen to a whole podcast without the system sleeping? And need to find some other way to implement that?

[Jer Noble]

No. (In reply to Darin Adler from comment #5)
> (In reply to Jer Noble from comment #4)
> > The danger is that, if the process is suspended, it will never get a chance
> > to release it's process assertion, and so even if audio playback ends in
> > mediaserverd (like when it reaches the end of the file), we'll keep the
> > system from sleeping.
> 
> OK, so we’re losing the feature where you can listen to a whole podcast
> without the system sleeping? And need to find some other way to implement
> that?

No, for two reasons: 1) on iOS, mediaserverd will keep the system from sleeping while audio is being generated and 2) WebKit in the UIProcess will take out a process assertion on the WebContent when it is generating audio. So 1) makes having our own system sleep assertion a “belt and suspenders” thing, and 2) should ensure the WebCintent process is never suspended in the first place during the “podcast playback” scenario. 

But because the WebContent process could get suspended the next turn of the run loop after the processWillSuspend() call turns, we may not ever receive the mediaPlayerRateDidChange() callback before suspending. This patch just protects against the situation where a suspend breaks our ability to release our system sleep assertion.

[WebKit Commit Bot]

Comment on attachment 373424 [details]
Patch

Clearing flags on attachment: 373424

Committed r247118: <https://trac.webkit.org/changeset/247118>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.