AudioContext.decodeAudioData returns a Promise

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 ebab3b8851a59fb698968cac7c46e373c5966ed7..1f44deec437dcaae2bccd8abe43658aec57bcac9 100644
--- a/Source/modules/webaudio/AudioContext.cpp
+++ b/Source/modules/webaudio/AudioContext.cpp
@@ -112,6 +112,7 @@ AudioContext::AudioContext(Document* document)
     , m_isOfflineContext(false)
     , m_contextState(Suspended)
     , m_cachedSampleFrame(0)
+    , m_closedContextSampleRate(-1)

[tkent, 2015/04/07 23:24:47]

This member isn't initialized in another constructor.

 {
     m_didInitializeContextGraphMutex = true;
     m_destinationNode = DefaultAudioDestinationNode::create(this);
@@ -150,6 +151,14 @@ AudioContext::~AudioContext()
     fprintf(stderr, "%p: AudioContext::~AudioContext(): %u\n", this, m_contextId);
 #endif
     // AudioNodes keep a reference to their context, so there should be no way to be in the destructor if there are still AudioNodes around.
+
+    // Context is being destroyed.  Reject any decodeAudioData promises that haven't been fulfilled
+    // yet.
+    for (auto& resolver : m_audioDecoderResolvers) {

[haraken, 2015/04/07 23:39:14]

This is unsafe. You're not allowed to touch other on-heap objects in a
destructor. Can we move this reject logic to AudioContext::uninitialize or
something like that?

[yhirano, 2015/04/08 03:19:27]

Rejection is needed when deleting a resolver if 
 - the resolver is not resolved nor rejected, and
 - the execution context is not stopped.

In other words, even when m_audioDecoderResolvers is not empty, you don't have
to reject them if the execution context is already stopped (You need a comment
and / or assertion though).

I'm not sure if the above assumption holds: Just FYI

[Raymond Toy, 2015/04/08 16:47:11]

uninitialize should work.

[Raymond Toy, 2015/04/08 16:47:11]

If this is moved to uninitialize(), then the execution context should still be
around because hasPendingActivity() is true until uninitialize is finished, at
which point hasPendingActivity() returns false.

+        resolver->reject(DOMException::create(InvalidStateError, "Audio context is going away"));
+    }
+    m_audioDecoderResolvers.clear();
+
     ASSERT(!m_isInitialized);
     ASSERT(!m_referencedNodes.size());
     ASSERT(!m_finishedNodes.size());
@@ -160,6 +169,7 @@ AudioContext::~AudioContext()
     ASSERT(!m_suspendResolvers.size());
     ASSERT(!m_isResolvingResumePromises);
     ASSERT(!m_resumeResolvers.size());
+    ASSERT(!m_audioDecoderResolvers.size());
 }
 
 void AudioContext::initialize()
@@ -272,20 +282,30 @@ AudioBuffer* AudioContext::createBuffer(unsigned numberOfChannels, size_t number
     return AudioBuffer::create(numberOfChannels, numberOfFrames, sampleRate, exceptionState);
 }
 
-void AudioContext::decodeAudioData(DOMArrayBuffer* audioData, AudioBufferCallback* successCallback, AudioBufferCallback* errorCallback, ExceptionState& exceptionState)
+ScriptPromise AudioContext::decodeAudioData(ScriptState* scriptState, DOMArrayBuffer* audioData, AudioBufferCallback* successCallback, AudioBufferCallback* errorCallback, ExceptionState& exceptionState)
 {
-    if (isContextClosed()) {
-        throwExceptionForClosedState(exceptionState);
-        return;
-    }
-
     if (!audioData) {
-        exceptionState.throwDOMException(
-            SyntaxError,
+        RefPtr<DOMException> error = DOMException::create(
+            NotSupportedError,
             "invalid ArrayBuffer for audioData.");
-        return;
+        if (errorCallback) {
+            errorCallback->handleEvent(error.get());
+        }
+        return ScriptPromise::rejectWithDOMException(scriptState, error);
     }
-    m_audioDecoder.decodeAsync(audioData, sampleRate(), successCallback, errorCallback);
+
+    RefPtrWillBeRawPtr<ScriptPromiseResolver> resolver = ScriptPromiseResolver::create(scriptState);
+    ScriptPromise promise = resolver->promise();
+
+    m_audioDecoderResolvers.append(resolver);
+
+    float rate = isContextClosed() ? m_closedContextSampleRate : sampleRate();
+
+    ASSERT(rate > 0);
+
+    m_audioDecoder.decodeAsync(audioData, rate, successCallback, errorCallback, resolver.get(), this);

[yhirano, 2015/04/08 03:19:27]

Just to confirm: Is this |resolver.get()| safe in non-oilpan environment? Does
AudioContext live long enough?

[Raymond Toy, 2015/04/08 16:47:11]

I believe so.  As long as uninitialize() has not been called, the AudioContext
should still be alive.

+
+    return promise;
 }
 
 AudioBufferSourceNode* AudioContext::createBufferSource(ExceptionState& exceptionState)
@@ -948,6 +968,19 @@ void AudioContext::handleStoppableSourceNodes()
         }
     }
 }
+
+void AudioContext::removeAudioDecoderResolver(ScriptPromiseResolver* resolver)
+{
+    ASSERT(isMainThread());
+
+    for (size_t k = 0; k < m_audioDecoderResolvers.size(); ++k) {
+        if (resolver == m_audioDecoderResolvers.at(k)) {
+            m_audioDecoderResolvers.remove(k);
+            break;
+        }
+    }
+}
+
 void AudioContext::handlePreRenderTasks()
 {
     ASSERT(isAudioThread());
@@ -1198,8 +1231,7 @@ void AudioContext::rejectPendingResolvers()
 {
     ASSERT(isMainThread());
 
-    // Audio context is closing down so reject any suspend or resume promises that are still
-    // pending.
+    // Audio context is closing down so reject any promises that are still pending.
 
     for (auto& resolver : m_suspendResolvers) {
         resolver->reject(DOMException::create(InvalidStateError, "Audio context is going away"));
@@ -1273,6 +1305,7 @@ void AudioContext::fireCompletionEvent()
 
 DEFINE_TRACE(AudioContext)
 {
+    visitor->trace(m_audioDecoderResolvers);
     visitor->trace(m_closeResolver);
     visitor->trace(m_offlineResolver);
     visitor->trace(m_renderTarget);
@@ -1342,6 +1375,9 @@ ScriptPromise AudioContext::closeContext(ScriptState* scriptState)
                 "Cannot close a context that is being closed or has already been closed."));
     }
 
+    // Save the current sample rate for any subsequent decodeAudioData calls.
+    m_closedContextSampleRate = sampleRate();
+
     m_closeResolver = ScriptPromiseResolver::create(scriptState);
     ScriptPromise promise = m_closeResolver->promise();