[GStreamer] SleepDisabler not destroyed when video playback stops

• Component: WebKitGTK
• Status: RESOLVED FIXED

[Michael Catanzaro]

Bug #186971 fixed the media player such that it now (usually) creates a SleepDisabler when starting video playback. However, the SleepDisabler is not destroyed when the media is stopped. For example, say I stop a YouTube video and leave it open in a browser tab that I'm not currently viewing. The SleepDisabler will continue to inhibit sleep indefinitely (or would if SleepDisabler were working correctly; it is currently broken due to bug #219010.)

The SleepDisabler should be destroyed when video playback stops, then recreated when playback resumes.

[Philippe Normand]

Created attachment 432274 [details]
Patch

[Philippe Normand]

Eric, is this an issue on Apple ports too? The new test fails on the mac EWS.

[Eric Carlson]

(In reply to Philippe Normand from comment #2)
> Eric, is this an issue on Apple ports too? The new test fails on the mac EWS.

I haven't seen reports of it, so I don't believe it is a problem on the Apple ports. 

I'm not surprised the test fails as written though because we don't explicitly call `updateSleepDisabling` when playback ends.

I assume it is called as a side effect of something that happens later on Apple ports.  It would be better to make it explicit, something like this maybe:

diff --git a/Source/WebCore/html/HTMLMediaElement.cpp b/Source/WebCore/html/HTMLMediaElement.cpp
index 3f21dfe8ed17..69f4bb870622 100644
--- a/Source/WebCore/html/HTMLMediaElement.cpp
+++ b/Source/WebCore/html/HTMLMediaElement.cpp
@@ -5809,8 +5809,10 @@ void HTMLMediaElement::dispatchEvent(Event& event)
 {
     DEBUG_LOG(LOGIDENTIFIER, event.type());
 
-    if (m_removedBehaviorRestrictionsAfterFirstUserGesture && event.type() == eventNames().endedEvent)
+    if (m_removedBehaviorRestrictionsAfterFirstUserGesture && event.type() == eventNames().endedEvent) {
         document().userActivatedMediaFinishedPlaying();
+        updateSleepDisabling();
+    }
 
     HTMLElement::dispatchEvent(event);

[Philippe Normand]

Yes, indeed. Explicit is better!

[Philippe Normand]

Created attachment 432323 [details]
Patch

[Philippe Normand]

Created attachment 432326 [details]
Patch

[EWS]

Committed r279313 (239189@main): <https://commits.webkit.org/239189@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 432326 [details].