Changeset 268893 in webkit

Timestamp:

Oct 22, 2020 2:56:24 PM (21 months ago)

Author:

Chris Dumez

Message:

Web Audio continues to play when navigating off the web page via an iframe
​https://bugs.webkit.org/show_bug.cgi?id=218078

Reviewed by Eric Carlson.

The page was suspending playback when clicking the link and then resuming playback in the pagehide
event handler. The issue is that the AudioContext's state gets updated asynchronously when the
page suspends or resume rendering, as per specification. When entering the back/forward cache,
AudioContext::suspend() would be called but would do nothing because the state was "suspended",
despite the script having just called "resume()". To address the issue, AudioContext::suspend()
now early returns only if the context is closed or based on the m_wasSuspendedByScript flag
when gets updated synchronously when the script calls suspend() / resume(). Similarly,
AudioContext::resume() checks those same flags now.

No new tests, this impacts audio rendering on speakers but is not web-observable.

• Modules/webaudio/AudioContext.cpp:

(WebCore::AudioContext::resumeRendering):
Check if the MediaSession is interrupted before asynchronously setting the AudioContext's state
to "running". This is important because the state gets updated asynchronously but the MediaSession
can be interrupted synchronously when AudioContext::suspend() gets called, which would have set
the AudioContext's state to "interrupted". We don't want to incorrectly change the state from
"interrupted" to "running" in this case. Note that AudioContext::suspendRendering() already does
the same thing.

(WebCore::AudioContext::suspend):
(WebCore::AudioContext::resume):
Only early return if the AudioContext is closed or if the m_wasSuspendedByScript flag is set. The
m_wasSuspendedByScript gets updated synchronously when suspendRendering() / resumeRendering() get
called, unlike the AudioContext's state which gets updated asynchronously.

(WebCore::AudioContext::suspendPlayback):
Stop early returning if the state is "suspended". This was wrong since the state gets updated
asynchronously.

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• Modules/webaudio/AudioContext.cpp (modified) (3 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2020-10-22  Chris Dumez  <cdumez@apple.com>
+
+        Web Audio continues to play when navigating off the web page via an iframe
+        https://bugs.webkit.org/show_bug.cgi?id=218078
+
+        Reviewed by Eric Carlson.
+
+        The page was suspending playback when clicking the link and then resuming playback in the pagehide
+        event handler. The issue is that the AudioContext's state gets updated asynchronously when the
+        page suspends or resume rendering, as per specification. When entering the back/forward cache,
+        AudioContext::suspend() would be called but would do nothing because the state was "suspended",
+        despite the script having just called "resume()". To address the issue, AudioContext::suspend()
+        now early returns only if the context is closed or based on the m_wasSuspendedByScript flag
+        when gets updated synchronously when the script calls suspend() / resume(). Similarly,
+        AudioContext::resume() checks those same flags now.
+
+        No new tests, this impacts audio rendering on speakers but is not web-observable.
+
+        * Modules/webaudio/AudioContext.cpp:
+        (WebCore::AudioContext::resumeRendering):
+        Check if the MediaSession is interrupted before asynchronously setting the AudioContext's state
+        to "running". This is important because the state gets updated asynchronously but the MediaSession
+        can be interrupted synchronously when AudioContext::suspend() gets called, which would have set
+        the AudioContext's state to "interrupted". We don't want to incorrectly change the state from
+        "interrupted" to "running" in this case. Note that AudioContext::suspendRendering() already does
+        the same thing.
+
+        (WebCore::AudioContext::suspend):
+        (WebCore::AudioContext::resume):
+        Only early return if the AudioContext is closed or if the m_wasSuspendedByScript flag is set. The
+        m_wasSuspendedByScript gets updated synchronously when suspendRendering() / resumeRendering() get
+        called, unlike the AudioContext's state which gets updated asynchronously.
+
+        (WebCore::AudioContext::suspendPlayback):
+        Stop early returning if the state is "suspended". This was wrong since the state gets updated
+        asynchronously.
+
 2020-10-22  Martin Robinson  <mrobinson@igalia.com>
 

trunk/Source/WebCore/Modules/webaudio/AudioContext.cpp

@@ -249 +249 @@
 
     destinationNode()->resume([this, protectedThis = makeRef(*this)] {
-        setState(State::Running);
+        // Since we update the state asynchronously, we may have been interrupted after the
+        // call to resume() and before this lambda runs. In this case, we don't want to
+        // reset the state to running.
+        bool interrupted = m_mediaSession->state() == PlatformMediaSession::Interrupted;
+        setState(interrupted ? State::Interrupted : State::Running);
     });
 }
@@ -403 +407 @@
 void AudioContext::suspend(ReasonForSuspension)
 {
-    if (state() == State::Running) {
-        m_mediaSession->beginInterruption(PlatformMediaSession::PlaybackSuspended);
-        document()->updateIsPlayingMedia();
-    }
+    if (isClosed() || m_wasSuspendedByScript)
+        return;
+
+    m_mediaSession->beginInterruption(PlatformMediaSession::PlaybackSuspended);
+    document()->updateIsPlayingMedia();
 }
 
 void AudioContext::resume()
 {
-    if (state() == State::Interrupted) {
-        m_mediaSession->endInterruption(PlatformMediaSession::MayResumePlaying);
-        document()->updateIsPlayingMedia();
-    }
+    if (isClosed() || m_wasSuspendedByScript)
+        return;
+
+    m_mediaSession->endInterruption(PlatformMediaSession::MayResumePlaying);
+    document()->updateIsPlayingMedia();
 }
 
@@ -421 +427 @@
     if (!destinationNode() || state() == State::Closed)
         return;
-
-    if (state() == State::Suspended) {
-        if (m_mediaSession->state() == PlatformMediaSession::Interrupted)
-            setState(State::Interrupted);
-        return;
-    }
 
     lazyInitialize();