Implement suspend() and resume() for OfflineAudioContext

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 a6e0aebbeeab2c4dc1278d17ec43f392fd25f5df..9fab5d87215882999675c42b88f1275091834081 100644
--- a/Source/modules/webaudio/AudioContext.cpp
+++ b/Source/modules/webaudio/AudioContext.cpp
@@ -99,15 +99,15 @@ AudioContext* AudioContext::create(Document& document, ExceptionState& exception
 // Constructor for rendering to the audio hardware.
 AudioContext::AudioContext(Document* document)
     : ActiveDOMObject(document)
+    , m_didInitializeContextGraphMutex(false)
+    , m_isOfflineContext(false)
+    , m_destinationNode(nullptr)
     , m_isStopScheduled(false)
     , m_isCleared(false)
     , m_isInitialized(false)
-    , m_destinationNode(nullptr)
     , m_isResolvingResumePromises(false)
     , m_connectionCount(0)
-    , m_didInitializeContextGraphMutex(false)
     , m_deferredTaskHandler(DeferredTaskHandler::create())
-    , m_isOfflineContext(false)
     , m_contextState(Suspended)
 {
     m_didInitializeContextGraphMutex = true;
@@ -119,18 +119,19 @@ AudioContext::AudioContext(Document* document)
 // Constructor for offline (non-realtime) rendering.
 AudioContext::AudioContext(Document* document, unsigned numberOfChannels, size_t numberOfFrames, float sampleRate)
     : ActiveDOMObject(document)
+    , m_didInitializeContextGraphMutex(false)
+    , m_isOfflineContext(true)
+    , m_destinationNode(nullptr)
     , m_isStopScheduled(false)
     , m_isCleared(false)
     , m_isInitialized(false)
-    , m_destinationNode(nullptr)
     , m_isResolvingResumePromises(false)
     , m_connectionCount(0)
-    , m_didInitializeContextGraphMutex(false)
     , m_deferredTaskHandler(DeferredTaskHandler::create())
-    , m_isOfflineContext(true)
     , m_contextState(Suspended)
 {
     m_didInitializeContextGraphMutex = true;
+
     // Create a new destination for offline rendering.
     m_renderTarget = AudioBuffer::create(numberOfChannels, numberOfFrames, sampleRate);
     if (m_renderTarget.get())
@@ -718,8 +719,10 @@ void AudioContext::setContextState(AudioContextState newState)
     m_contextState = newState;
 
     // Notify context that state changed
-    if (executionContext())
-        executionContext()->postTask(FROM_HERE, createSameThreadTask(&AudioContext::notifyStateChange, this));
+    if (executionContext()) {
+        executionContext()->postTask(FROM_HERE,
+            createSameThreadTask(&AudioContext::notifyStateChange, this));
+    }

[Raymond Toy, 2015/05/28 16:37:35]

This change isn't really necessary, is it?

[hongchan, 2015/06/09 20:49:59]

No, I am reverting this in PS6.

 }
 
 void AudioContext::notifyStateChange()
@@ -730,15 +733,9 @@ void AudioContext::notifyStateChange()
 ScriptPromise AudioContext::suspendContext(ScriptState* scriptState)
 {
     ASSERT(isMainThread());
-    AutoLocker locker(this);
+    ASSERT(!isOfflineContext());
 
-    if (isOfflineContext()) {
-        return ScriptPromise::rejectWithDOMException(
-            scriptState,
-            DOMException::create(
-                InvalidAccessError,
-                "cannot suspend an OfflineAudioContext"));
-    }
+    AutoLocker locker(this);
 
     RefPtrWillBeRawPtr<ScriptPromiseResolver> resolver = ScriptPromiseResolver::create(scriptState);
     ScriptPromise promise = resolver->promise();
@@ -945,7 +942,7 @@ ExecutionContext* AudioContext::executionContext() const
 
 void AudioContext::startRendering()
 {
-    // This is called for both online and offline contexts.
+    // This is only for the real-time context.

[Raymond Toy, 2015/05/28 16:37:35]

Add assert for !isOfflineContext()?

[hongchan, 2015/06/09 20:49:59]

Done.

     ASSERT(isMainThread());
     ASSERT(m_destinationNode);
 
@@ -968,34 +965,20 @@ void AudioContext::stopRendering()
     }
 }
 
-void AudioContext::fireCompletionEvent()
+bool AudioContext::shouldSuspendNow()

[Raymond Toy, 2015/05/28 16:37:35]

Can't this be a method just on an OfflineAudioContext?

[hongchan, 2015/06/09 20:49:58]

The problem is OAC is a very thin wrapper for AC. So calling context() inside of
OfflineAudioDestintion always goes to AudioContext first. That's why I made this
as public/virtual so I can override in OfflineAudioContext. Does it make sense?

[Raymond Toy, 2015/06/09 22:34:59]

Why not make context() be virtual? I don't know which would be better.

 {
-    ASSERT(isMainThread());
-    if (!isMainThread())
-        return;
-
-    AudioBuffer* renderedBuffer = m_renderTarget.get();
-
-    // For an offline context, we set the state to closed here so that the oncomplete handler sees
-    // that the context has been closed.
-    setContextState(Closed);
-
-    ASSERT(renderedBuffer);
-    if (!renderedBuffer)
-        return;
+    ASSERT_WITH_MESSAGE(1, "shouldSuspendNow() only valid for offline audio context");
+    return false;
+}
 
-    // Avoid firing the event if the document has already gone away.
-    if (executionContext()) {
-        // Call the offline rendering completion event listener and resolve the promise too.
-        dispatchEvent(OfflineAudioCompletionEvent::create(renderedBuffer));
-        m_offlineResolver->resolve(renderedBuffer);
-    }
+void AudioContext::fireCompletionEvent()

[Raymond Toy, 2015/05/28 16:37:35]

Could this be moved to the OfflineAudioContext class?

[hongchan, 2015/06/09 20:49:59]

The same reason above.

+{
+    ASSERT_WITH_MESSAGE(1, "fireCompletionEvent() only valid for offline audio context");
 }
 
 DEFINE_TRACE(AudioContext)
 {
     visitor->trace(m_closeResolver);
-    visitor->trace(m_offlineResolver);
     visitor->trace(m_renderTarget);
     visitor->trace(m_destinationNode);
     visitor->trace(m_listener);