Process suspend() and resume() in call order

Source/modules/webaudio/AudioContext.cpp

Index: Source/modules/webaudio/AudioContext.cpp
diff --git a/Source/modules/webaudio/AudioContext.cpp b/Source/modules/webaudio/AudioContext.cpp
index 645145c54ff0c92282c932f40525833a6296e1d6..b971c15cc94227d60cf59f911907a488237fa3a5 100644
--- a/Source/modules/webaudio/AudioContext.cpp
+++ b/Source/modules/webaudio/AudioContext.cpp
@@ -78,6 +78,26 @@ const unsigned MaxHardwareContexts = 6;
 unsigned AudioContext::s_hardwareContextCount = 0;
 unsigned AudioContext::s_contextId = 0;
 
+AudioContext::SuspendResumeResolver::SuspendResumeResolver(
+    ResolverType resolverType,
+    PassRefPtrWillBeRawPtr<ScriptPromiseResolver> resolver)
+    : m_resolverType(resolverType)
+    , m_resolver(resolver)
+{
+}
+
+AudioContext::SuspendResumeResolver* AudioContext::SuspendResumeResolver::createSuspendResolver(
+    PassRefPtrWillBeRawPtr<ScriptPromiseResolver> resolver)
+{
+    return new SuspendResumeResolver(ResolverTypeSuspend, resolver);
+}
+
+AudioContext::SuspendResumeResolver* AudioContext::SuspendResumeResolver::createResumeResolver(
+    PassRefPtrWillBeRawPtr<ScriptPromiseResolver> resolver)
+{
+    return new SuspendResumeResolver(ResolverTypeResume, resolver);
+}
+
 AudioContext* AudioContext::create(Document& document, ExceptionState& exceptionState)
 {
     ASSERT(isMainThread());
@@ -103,7 +123,7 @@ AudioContext::AudioContext(Document* document)
     , m_isCleared(false)
     , m_isInitialized(false)
     , m_destinationNode(nullptr)
-    , m_isResolvingResumePromises(false)
+    , m_isResolvingSuspendResumePromises(false)
     , m_connectionCount(0)
     , m_didInitializeContextGraphMutex(false)
     , m_deferredTaskHandler(DeferredTaskHandler::create())
@@ -124,7 +144,7 @@ AudioContext::AudioContext(Document* document, unsigned numberOfChannels, size_t
     , m_isCleared(false)
     , m_isInitialized(false)
     , m_destinationNode(nullptr)
-    , m_isResolvingResumePromises(false)
+    , m_isResolvingSuspendResumePromises(false)
     , m_connectionCount(0)
     , m_didInitializeContextGraphMutex(false)
     , m_deferredTaskHandler(DeferredTaskHandler::create())
@@ -151,9 +171,8 @@ AudioContext::~AudioContext()
     ASSERT(!m_isInitialized);
     ASSERT(!m_activeSourceNodes.size());
     ASSERT(!m_finishedSourceHandlers.size());
-    ASSERT(!m_suspendResolvers.size());
-    ASSERT(!m_isResolvingResumePromises);
-    ASSERT(!m_resumeResolvers.size());
+    ASSERT(!m_suspendResumeResolvers.size());
+    ASSERT(!m_isResolvingSuspendResumePromises);
 }
 
 void AudioContext::initialize()
@@ -760,12 +779,12 @@ ScriptPromise AudioContext::suspendContext(ScriptState* scriptState)
 
     // Save the resolver.  If the context is running, it will get resolved at the end of a rendering
     // quantum.  Otherwise, resolve it now.
-    m_suspendResolvers.append(resolver);
+    m_suspendResumeResolvers.append(SuspendResumeResolver::createSuspendResolver(resolver));
 
     if (m_contextState != Running) {
         // Context is not running so we can't wait for a rendering quantum to resolve the
-        // promise. Just resolve it now (along with any other pending suspend promises).
-        resolvePromisesForSuspendOnMainThread();
+        // promise. Just resolve it now (along with any other pending promises).
+        resolvePromisesForSuspendResumeOnMainThread();
     }
 
     return promise;
@@ -801,7 +820,7 @@ ScriptPromise AudioContext::resumeContext(ScriptState* scriptState)
 
     // Save the resolver which will get resolved when the destination node starts pulling on the
     // graph again.
-    m_resumeResolvers.append(resolver);
+    m_suspendResumeResolvers.append(SuspendResumeResolver::createResumeResolver(resolver));
 
     return promise;
 }
@@ -869,7 +888,6 @@ void AudioContext::handlePreRenderTasks()
     if (tryLock()) {
         deferredTaskHandler().handleDeferredTasks();
 
-        resolvePromisesForResume();

[haraken, 2015/05/04 23:24:40]

Why don't we need to call resolvePromisesForSuspendResume() here?

 
         // Check to see if source nodes can be stopped because the end time has passed.
         handleStoppableSourceNodes();
@@ -898,75 +916,57 @@ void AudioContext::handlePostRenderTasks()
         deferredTaskHandler().handleDeferredTasks();
         deferredTaskHandler().requestToDeleteHandlersOnMainThread();
 
-        resolvePromisesForSuspend();
+        resolvePromisesForSuspendResume();
 
         unlock();
     }
 }
 
-void AudioContext::resolvePromisesForResumeOnMainThread()
+void AudioContext::resolvePromisesForSuspendResumeOnMainThread()
 {
     ASSERT(isMainThread());
     AutoLocker locker(this);
 
-    for (auto& resolver : m_resumeResolvers) {
-        if (m_contextState == Closed) {
-            resolver->reject(
-                DOMException::create(InvalidStateError, "Cannot resume a context that has been closed"));
-        } else {
-            resolver->resolve();
+    for (auto& handler : m_suspendResumeResolvers) {
+        bool isSuspend = handler->handlerType() == SuspendResumeResolver::ResolverTypeSuspend;
+
+        // Stop or start rendering depending on the type of handler.
+        if (m_destinationNode) {
+            if (isSuspend) {
+                stopRendering();
+            } else {
+                // Normally we don't need this, but if suspend() and resume() calls have piled up,
+                // the context may have been suspended again, and we want to start rendering for
+                // sure.  See crbug.com/483269.
+                startRendering();
+            }
         }
-    }
 
-    m_resumeResolvers.clear();
-    m_isResolvingResumePromises = false;
-}
-
-void AudioContext::resolvePromisesForResume()
-{
-    // This runs inside the AudioContext's lock when handling pre-render tasks.
-    ASSERT(isAudioThread());
-    ASSERT(isGraphOwner());
-
-    // Resolve any pending promises created by resume(). Only do this if we haven't already started
-    // resolving these promises. This gets called very often and it takes some time to resolve the
-    // promises in the main thread.

[haraken, 2015/05/04 23:24:40]

Let's keep this comment.

-    if (!m_isResolvingResumePromises && m_resumeResolvers.size() > 0) {
-        m_isResolvingResumePromises = true;
-        Platform::current()->mainThread()->postTask(FROM_HERE, threadSafeBind(&AudioContext::resolvePromisesForResumeOnMainThread, this));
-    }
-}
-
-void AudioContext::resolvePromisesForSuspendOnMainThread()
-{
-    ASSERT(isMainThread());
-    AutoLocker locker(this);
-
-    // We can stop rendering now.
-    if (m_destinationNode)
-        stopRendering();
-
-    for (auto& resolver : m_suspendResolvers) {
         if (m_contextState == Closed) {
-            resolver->reject(
-                DOMException::create(InvalidStateError, "Cannot suspend a context that has been closed"));
+            String message = isSuspend ? "suspend" : "resume";
+            handler->resolver()->reject(DOMException::create(InvalidStateError,
+                "cannot " + message + " a context that has been closed"));
         } else {
-            resolver->resolve();
+            handler->resolver()->resolve();
         }
     }
 
-    m_suspendResolvers.clear();
+    m_suspendResumeResolvers.clear();
+    m_isResolvingSuspendResumePromises = false;
 }
 
-void AudioContext::resolvePromisesForSuspend()
+void AudioContext::resolvePromisesForSuspendResume()
 {
     // This runs inside the AudioContext's lock when handling pre-render tasks.
     ASSERT(isAudioThread());
     ASSERT(isGraphOwner());
 
     // Resolve any pending promises created by suspend()

[haraken, 2015/05/04 23:24:40]

suspend() or resume().

-    if (m_suspendResolvers.size() > 0)
-        Platform::current()->mainThread()->postTask(FROM_HERE, threadSafeBind(&AudioContext::resolvePromisesForSuspendOnMainThread, this));
+    if (!m_isResolvingSuspendResumePromises && m_suspendResumeResolvers.size() > 0) {
+        m_isResolvingSuspendResumePromises = true;
+        Platform::current()->mainThread()->postTask(FROM_HERE,
+            threadSafeBind(&AudioContext::resolvePromisesForSuspendResumeOnMainThread, this));
+    }
 }
 
 void AudioContext::rejectPendingResolvers()
@@ -976,16 +976,9 @@ void AudioContext::rejectPendingResolvers()
     // Audio context is closing down so reject any suspend or resume promises that are still
     // pending.
 
-    for (auto& resolver : m_suspendResolvers) {
-        resolver->reject(DOMException::create(InvalidStateError, "Audio context is going away"));
-    }
-    m_suspendResolvers.clear();
-
-    for (auto& resolver : m_resumeResolvers) {
-        resolver->reject(DOMException::create(InvalidStateError, "Audio context is going away"));
-    }
-    m_resumeResolvers.clear();
-    m_isResolvingResumePromises = false;
+    for (auto& handler : m_suspendResumeResolvers)
+        handler->resolver()->reject(DOMException::create(InvalidStateError, "Audio context is going away"));
+    m_suspendResumeResolvers.clear();

[haraken, 2015/05/04 23:24:40]

Don't we need to set m_isResolvingSuspendResumePromise to false here?

 }
 
 const AtomicString& AudioContext::interfaceName() const
@@ -1062,8 +1055,6 @@ DEFINE_TRACE(AudioContext)
     } else {
         visitor->trace(m_activeSourceNodes);
     }
-    visitor->trace(m_resumeResolvers);
-    visitor->trace(m_suspendResolvers);
     RefCountedGarbageCollectedEventTargetWithInlineData<AudioContext>::trace(visitor);
     ActiveDOMObject::trace(visitor);
 }