Changeset 265052 in webkit

Timestamp:

Jul 29, 2020 1:24:46 PM (4 years ago)

Author:

Chris Dumez

Message:

Calling AudioNode constructors should have identical behavior to using create*() function on BaseAudioContext
​https://bugs.webkit.org/show_bug.cgi?id=214931

Reviewed by Eric Carlson.

Calling AudioNode constructors should have identical behavior to using create*() function on BaseAudioContext.
Otherwise, this leads to assertion hits on the bots like we saw for Bug 214851.

We need to make sure that the AudioContext gets initialized when an AudioNode constructor gets called. We also
need to make sure refNode() gets called if necessary.

• Modules/webaudio/BaseAudioContext.cpp:

(WebCore::BaseAudioContext::createPanner):
(WebCore::BaseAudioContext::createOscillator):
(WebCore::BaseAudioContext::createPeriodicWave):

• Modules/webaudio/BaseAudioContext.h:
• Modules/webaudio/OscillatorNode.cpp:

(WebCore::OscillatorNode::create):

• Modules/webaudio/PannerNode.cpp:

(WebCore::PannerNode::create):

• Modules/webaudio/PeriodicWave.cpp:

(WebCore::PeriodicWave::create):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• Modules/webaudio/BaseAudioContext.cpp (modified) (4 diffs)
• Modules/webaudio/BaseAudioContext.h (modified) (3 diffs)
• Modules/webaudio/OscillatorNode.cpp (modified) (2 diffs)
• Modules/webaudio/PannerNode.cpp (modified) (1 diff)
• Modules/webaudio/PeriodicWave.cpp (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2020-07-29  Chris Dumez  <cdumez@apple.com>
+
+        Calling AudioNode constructors should have identical behavior to using create*() function on BaseAudioContext
+        https://bugs.webkit.org/show_bug.cgi?id=214931
+
+        Reviewed by Eric Carlson.
+
+        Calling AudioNode constructors should have identical behavior to using create*() function on BaseAudioContext.
+        Otherwise, this leads to assertion hits on the bots like we saw for Bug 214851.
+
+        We need to make sure that the AudioContext gets initialized when an AudioNode constructor gets called. We also
+        need to make sure refNode() gets called if necessary.
+
+        * Modules/webaudio/BaseAudioContext.cpp:
+        (WebCore::BaseAudioContext::createPanner):
+        (WebCore::BaseAudioContext::createOscillator):
+        (WebCore::BaseAudioContext::createPeriodicWave):
+        * Modules/webaudio/BaseAudioContext.h:
+        * Modules/webaudio/OscillatorNode.cpp:
+        (WebCore::OscillatorNode::create):
+        * Modules/webaudio/PannerNode.cpp:
+        (WebCore::PannerNode::create):
+        * Modules/webaudio/PeriodicWave.cpp:
+        (WebCore::PeriodicWave::create):
+
 2020-07-29  Simon Fraser  <simon.fraser@apple.com>
 

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

@@ -519 +519 @@
     
     ASSERT(isMainThread());
-    if (m_isStopScheduled)
-        return Exception { InvalidStateError };
-
-    lazyInitialize();
     return PannerNode::create(*this);
 }
@@ -621 +617 @@
     
     ASSERT(isMainThread());
-    if (m_isStopScheduled)
-        return Exception { InvalidStateError };
-
-    lazyInitialize();
-
-    auto node = OscillatorNode::create(*this);
-    if (node.hasException())
-        return node.releaseException();
-
-    // Because this is an AudioScheduledSourceNode, the context keeps a reference until it has finished playing.
-    // When this happens, AudioScheduledSourceNode::finish() calls BaseAudioContext::notifyNodeFinishedProcessing().
-    auto nodeValue = node.releaseReturnValue();
-    refNode(nodeValue);
-    return nodeValue;
+    return OscillatorNode::create(*this);
 }
 
@@ -642 +625 @@
     
     ASSERT(isMainThread());
-    if (m_isStopScheduled)
-        return Exception { InvalidStateError };
-    
-    if (real.size() != imaginary.size())
-        return Exception { IndexSizeError, "real and imaginary must have the same length"_s };
     
     PeriodicWaveOptions options;
@@ -652 +630 @@
     options.imag = WTFMove(imaginary);
     options.disableNormalization = constraints.disableNormalization;
-    lazyInitialize();
     return PeriodicWave::create(*this, WTFMove(options));
 }

trunk/Source/WebCore/Modules/webaudio/BaseAudioContext.h

@@ -286 +286 @@
     };
 
+    // The context itself keeps a reference to all source nodes. The source nodes, then reference all nodes they're connected to.
+    // In turn, these nodes reference all nodes they're connected to. All nodes are ultimately connected to the AudioDestinationNode.
+    // When the context dereferences a source node, it will be deactivated from the rendering graph along with all other nodes it is
+    // uniquely connected to. See the AudioNode::ref() and AudioNode::deref() methods for more details.
+    void refNode(AudioNode&);
+    void derefNode(AudioNode&);
+
+    void lazyInitialize();
+
 protected:
     explicit BaseAudioContext(Document&, const AudioContextOptions& = { });
@@ -296 +305 @@
     AudioDestinationNode* destinationNode() const { return m_destinationNode.get(); }
 
-    void lazyInitialize();
     void uninitialize();
 
@@ -302 +310 @@
     const char* logClassName() const final { return "BaseAudioContext"; }
 #endif
-
-    // The context itself keeps a reference to all source nodes.  The source nodes, then reference all nodes they're connected to.
-    // In turn, these nodes reference all nodes they're connected to.  All nodes are ultimately connected to the AudioDestinationNode.
-    // When the context dereferences a source node, it will be deactivated from the rendering graph along with all other nodes it is
-    // uniquely connected to.  See the AudioNode::ref() and AudioNode::deref() methods for more details.
-    void refNode(AudioNode&);
-    void derefNode(AudioNode&);
 
     void addReaction(State, DOMPromiseDeferred<void>&&);

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

@@ -48 +48 @@
 ExceptionOr<Ref<OscillatorNode>> OscillatorNode::create(BaseAudioContext& context, const OscillatorOptions& options)
 {
+    if (context.isStopped())
+        return Exception { InvalidStateError };
+
+    context.lazyInitialize();
+
     if (options.type == OscillatorType::Custom && !options.periodicWave)
         return Exception { InvalidStateError, "Must provide periodicWave when using custom type."_s };
@@ -72 +77 @@
             return result.releaseException();
     }
-    
+
+    // Because this is an AudioScheduledSourceNode, the context keeps a reference until it has finished playing.
+    // When this happens, AudioScheduledSourceNode::finish() calls BaseAudioContext::notifyNodeFinishedProcessing().
+    context.refNode(oscillator);
+
     return oscillator;
 }

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

@@ -63 +63 @@
 ExceptionOr<Ref<PannerNode>> PannerNode::create(BaseAudioContext& context, const PannerOptions& options)
 {
+    if (context.isStopped())
+        return Exception { InvalidStateError };
+
+    context.lazyInitialize();
+
     auto panner = adoptRef(*new PannerNode(context, options));
 

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

@@ -57 +57 @@
 ExceptionOr<Ref<PeriodicWave>> PeriodicWave::create(BaseAudioContext& context, PeriodicWaveOptions&& options)
 {
+    if (context.isStopped())
+        return Exception { InvalidStateError };
+
+    context.lazyInitialize();
+
     Vector<float> real;
     Vector<float> imag;