Diff
Modified: trunk/Source/WebCore/ChangeLog (268727 => 268728)
--- trunk/Source/WebCore/ChangeLog 2020-10-20 13:19:42 UTC (rev 268727)
+++ trunk/Source/WebCore/ChangeLog 2020-10-20 15:02:18 UTC (rev 268728)
@@ -1,3 +1,67 @@
+2020-10-20 Chris Dumez <cdu...@apple.com>
+
+ Drop explicit calls to BaseAudioContext::refNode()
+ https://bugs.webkit.org/show_bug.cgi?id=217926
+
+ Reviewed by Eric Carlson.
+
+ Drop explicit calls to BaseAudioContext::refNode(). Instead we now rely on the
+ sourceNodeWillBeginPlayback() signal to ref the source AudioNode. This simplifies
+ the code a bit. I also did some renaming for clarity, so be explicit about the
+ fact we are dealing with *source* nodes here.
+
+ No new tests, no Web-facing behavior change.
+
+ * Modules/webaudio/AudioBufferSourceNode.cpp:
+ (WebCore::AudioBufferSourceNode::create):
+ (WebCore::AudioBufferSourceNode::startPlaying):
+ * Modules/webaudio/AudioContext.cpp:
+ (WebCore::AudioContext::sourceNodeWillBeginPlayback):
+ * Modules/webaudio/AudioContext.h:
+ * Modules/webaudio/AudioScheduledSourceNode.cpp:
+ (WebCore::AudioScheduledSourceNode::startLater):
+ (WebCore::AudioScheduledSourceNode::finish):
+
+ * Modules/webaudio/AudioWorkletNode.cpp:
+ (WebCore::AudioWorkletNode::create):
+ (WebCore::AudioWorkletNode::didFinishProcessingOnRenderingThread):
+ Only call sourceNodeWillBeginPlayback() / sourceNodeDidFinishPlayback()
+ when the AudioWorklet is actually a *source* node, meaning that it has
+ outputs. This behavior is consistent with Blink.
+
+ * Modules/webaudio/BaseAudioContext.cpp:
+ (WebCore::BaseAudioContext::~BaseAudioContext):
+ (WebCore::BaseAudioContext::uninitialize):
+ (WebCore::BaseAudioContext::createScriptProcessor):
+ (WebCore::BaseAudioContext::derefFinishedSourceNodes):
+ (WebCore::BaseAudioContext::refSourceNode):
+ (WebCore::BaseAudioContext::derefSourceNode):
+ (WebCore::BaseAudioContext::derefUnfinishedSourceNodes):
+ (WebCore::BaseAudioContext::sourceNodeWillBeginPlayback):
+ (WebCore::BaseAudioContext::sourceNodeDidFinishPlayback):
+ * Modules/webaudio/BaseAudioContext.h:
+ * Modules/webaudio/ConstantSourceNode.cpp:
+ (WebCore::ConstantSourceNode::create):
+ * Modules/webaudio/MediaElementAudioSourceNode.cpp:
+ (WebCore::MediaElementAudioSourceNode::create):
+ * Modules/webaudio/MediaStreamAudioSourceNode.cpp:
+ (WebCore::MediaStreamAudioSourceNode::create):
+ * Modules/webaudio/OscillatorNode.cpp:
+ (WebCore::OscillatorNode::create):
+
+ * Modules/webaudio/ScriptProcessorNode.cpp:
+ (WebCore::ScriptProcessorNode::eventListenersDidChange):
+ (WebCore::ScriptProcessorNode::virtualHasPendingActivity const):
+ * Modules/webaudio/ScriptProcessorNode.h:
+ Use ActiveDOMObject logic to keep the script wrapper alive instead of relying on the
+ AudioContext to do so. We keep the wrapper alive as long as the context's state is
+ not closed and as long as there is an audioprocess event listener registered. This
+ behavior is consistent with what Blink does.
+
+ * Modules/webaudio/WebKitAudioContext.cpp:
+ (WebCore::WebKitAudioContext::createWebKitOscillator):
+ (WebCore::WebKitAudioContext::createWebKitBufferSource):
+
2020-10-20 Philippe Normand <pnorm...@igalia.com>
[GStreamer] REGRESSION(r266559): imported/w3c/web-platform-tests/webaudio/the-audio-api/the-destinationnode-interface/destination.html is failing
Modified: trunk/Source/WebCore/Modules/webaudio/AudioBufferSourceNode.cpp (268727 => 268728)
--- trunk/Source/WebCore/Modules/webaudio/AudioBufferSourceNode.cpp 2020-10-20 13:19:42 UTC (rev 268727)
+++ trunk/Source/WebCore/Modules/webaudio/AudioBufferSourceNode.cpp 2020-10-20 15:02:18 UTC (rev 268728)
@@ -76,10 +76,6 @@
node->setLoopStart(options.loopStart);
node->playbackRate().setValue(options.playbackRate);
- // 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(node);
-
return node;
}
@@ -473,8 +469,6 @@
ASSERT(isMainThread());
ALWAYS_LOG(LOGIDENTIFIER, "when = ", when, ", offset = ", grainOffset, ", duration = ", grainDuration.valueOr(0));
- context().nodeWillBeginPlayback();
-
if (m_playbackState != UNSCHEDULED_STATE)
return Exception { InvalidStateError, "Cannot call start more than once."_s };
@@ -487,6 +481,8 @@
if (grainDuration && (!std::isfinite(*grainDuration) || (*grainDuration < 0)))
return Exception { RangeError, "duration value should be positive"_s };
+ context().sourceNodeWillBeginPlayback(*this);
+
// This synchronizes with process().
auto locker = holdLock(m_processLock);
Modified: trunk/Source/WebCore/Modules/webaudio/AudioContext.cpp (268727 => 268728)
--- trunk/Source/WebCore/Modules/webaudio/AudioContext.cpp 2020-10-20 13:19:42 UTC (rev 268727)
+++ trunk/Source/WebCore/Modules/webaudio/AudioContext.cpp 2020-10-20 15:02:18 UTC (rev 268728)
@@ -252,8 +252,10 @@
});
}
-void AudioContext::nodeWillBeginPlayback()
+void AudioContext::sourceNodeWillBeginPlayback(AudioNode& audioNode)
{
+ BaseAudioContext::sourceNodeWillBeginPlayback(audioNode);
+
// Called by scheduled AudioNodes when clients schedule their start times.
// Prior to the introduction of suspend(), resume(), and stop(), starting
// a scheduled AudioNode would remove the user-gesture restriction, if present,
Modified: trunk/Source/WebCore/Modules/webaudio/AudioContext.h (268727 => 268728)
--- trunk/Source/WebCore/Modules/webaudio/AudioContext.h 2020-10-20 13:19:42 UTC (rev 268727)
+++ trunk/Source/WebCore/Modules/webaudio/AudioContext.h 2020-10-20 15:02:18 UTC (rev 268728)
@@ -71,7 +71,7 @@
void suspendRendering(DOMPromiseDeferred<void>&&);
void resumeRendering(DOMPromiseDeferred<void>&&);
- void nodeWillBeginPlayback() final;
+ void sourceNodeWillBeginPlayback(AudioNode&) final;
void lazyInitialize() final;
void startRendering();
Modified: trunk/Source/WebCore/Modules/webaudio/AudioScheduledSourceNode.cpp (268727 => 268728)
--- trunk/Source/WebCore/Modules/webaudio/AudioScheduledSourceNode.cpp 2020-10-20 13:19:42 UTC (rev 268727)
+++ trunk/Source/WebCore/Modules/webaudio/AudioScheduledSourceNode.cpp 2020-10-20 15:02:18 UTC (rev 268728)
@@ -152,8 +152,6 @@
ASSERT(isMainThread());
ALWAYS_LOG(LOGIDENTIFIER, when);
- context().nodeWillBeginPlayback();
-
if (m_playbackState != UNSCHEDULED_STATE)
return Exception { InvalidStateError };
@@ -160,6 +158,8 @@
if (!std::isfinite(when) || when < 0)
return Exception { RangeError, "when value should be positive"_s };
+ context().sourceNodeWillBeginPlayback(*this);
+
m_startTime = when;
m_playbackState = SCHEDULED_STATE;
@@ -193,7 +193,7 @@
{
ASSERT(!hasFinished());
// Let the context dereference this AudioNode.
- context().notifyNodeFinishedProcessing(this);
+ context().sourceNodeDidFinishPlayback(*this);
m_playbackState = FINISHED_STATE;
context().decrementActiveSourceCount();
Modified: trunk/Source/WebCore/Modules/webaudio/AudioWorkletNode.cpp (268727 => 268728)
--- trunk/Source/WebCore/Modules/webaudio/AudioWorkletNode.cpp 2020-10-20 13:19:42 UTC (rev 268727)
+++ trunk/Source/WebCore/Modules/webaudio/AudioWorkletNode.cpp 2020-10-20 15:02:18 UTC (rev 268728)
@@ -105,7 +105,9 @@
node->initializeAudioParameters(parameterDescriptors, parameterData);
// Will cause the context to ref the node until playback has finished.
- context.refNode(node);
+ // Note that a node with zero outputs cannot be a source node.
+ if (node->numberOfOutputs() > 0)
+ context.sourceNodeWillBeginPlayback(node);
context.audioWorklet().createProcessor(name, processorMessagePort->disentangle(), serializedOptions.releaseNonNull(), node);
@@ -228,7 +230,9 @@
m_processor = nullptr;
m_tailTime = 0;
- context().notifyNodeFinishedProcessing(this);
+
+ if (numberOfOutputs() > 0)
+ context().sourceNodeDidFinishPlayback(*this);
}
void AudioWorkletNode::updatePullStatus()
Modified: trunk/Source/WebCore/Modules/webaudio/BaseAudioContext.cpp (268727 => 268728)
--- trunk/Source/WebCore/Modules/webaudio/BaseAudioContext.cpp 2020-10-20 13:19:42 UTC (rev 268727)
+++ trunk/Source/WebCore/Modules/webaudio/BaseAudioContext.cpp 2020-10-20 15:02:18 UTC (rev 268728)
@@ -170,8 +170,8 @@
ASSERT(!m_isInitialized);
ASSERT(m_isStopScheduled);
ASSERT(m_nodesToDelete.isEmpty());
- ASSERT(m_referencedNodes.isEmpty());
- ASSERT(m_finishedNodes.isEmpty()); // FIXME (bug 105870): This assertion fails on tests sometimes.
+ ASSERT(m_referencedSourceNodes.isEmpty());
+ ASSERT(m_finishedSourceNodes.isEmpty());
ASSERT(m_automaticPullNodes.isEmpty());
if (m_automaticPullNodesNeedUpdating)
m_renderingAutomaticPullNodes.resize(m_automaticPullNodes.size());
@@ -244,7 +244,7 @@
AutoLocker locker(*this);
// This should have been called from handlePostRenderTasks() at the end of rendering.
// However, in case of lock contention, the tryLock() call could have failed in handlePostRenderTasks(),
- // leaving nodes in m_finishedNodes. Now that the audio thread is gone, make sure we deref those nodes
+ // leaving nodes in m_finishedSourceNodes. Now that the audio thread is gone, make sure we deref those nodes
// before the BaseAudioContext gets destroyed.
derefFinishedSourceNodes();
}
@@ -436,10 +436,7 @@
if (numberOfOutputChannels > maxNumberOfChannels())
return Exception { NotSupportedError };
- auto node = ScriptProcessorNode::create(*this, bufferSize, numberOfInputChannels, numberOfOutputChannels);
-
- refNode(node); // context keeps reference until we stop making _javascript_ rendering callbacks
- return node;
+ return ScriptProcessorNode::create(*this, bufferSize, numberOfInputChannels, numberOfOutputChannels);
}
ExceptionOr<Ref<BiquadFilterNode>> BaseAudioContext::createBiquadFilter()
@@ -578,42 +575,38 @@
return IIRFilterNode::create(scriptExecutionContext, *this, WTFMove(options));
}
-void BaseAudioContext::notifyNodeFinishedProcessing(AudioNode* node)
-{
- ASSERT(isAudioThread());
- m_finishedNodes.append(node);
-}
-
void BaseAudioContext::derefFinishedSourceNodes()
{
ASSERT(isGraphOwner());
ASSERT(isAudioThread() || isAudioThreadFinished());
- for (auto& node : m_finishedNodes)
- derefNode(*node);
+ for (auto& node : m_finishedSourceNodes)
+ derefSourceNode(*node);
- m_finishedNodes.clear();
+ m_finishedSourceNodes.clear();
}
-void BaseAudioContext::refNode(AudioNode& node)
+void BaseAudioContext::refSourceNode(AudioNode& node)
{
ASSERT(isMainThread());
AutoLocker locker(*this);
-
- m_referencedNodes.append(&node);
+
+ ASSERT(!m_referencedSourceNodes.contains(&node));
+ // Reference source node to keep it alive and playing even if its JS wrapper gets garbage collected.
+ m_referencedSourceNodes.append(&node);
}
-void BaseAudioContext::derefNode(AudioNode& node)
+void BaseAudioContext::derefSourceNode(AudioNode& node)
{
ASSERT(isGraphOwner());
- ASSERT(m_referencedNodes.contains(&node));
- m_referencedNodes.removeFirst(&node);
+ ASSERT(m_referencedSourceNodes.contains(&node));
+ m_referencedSourceNodes.removeFirst(&node);
}
void BaseAudioContext::derefUnfinishedSourceNodes()
{
ASSERT(isMainThread() && isAudioThreadFinished());
- m_referencedNodes.clear();
+ m_referencedSourceNodes.clear();
}
void BaseAudioContext::lock(bool& mustReleaseLock)
@@ -1050,6 +1043,18 @@
m_parameterDescriptorMap.add(processorName, WTFMove(descriptors));
}
+void BaseAudioContext::sourceNodeWillBeginPlayback(AudioNode& node)
+{
+ refSourceNode(node);
+}
+
+void BaseAudioContext::sourceNodeDidFinishPlayback(AudioNode& node)
+{
+ ASSERT(isAudioThread());
+
+ m_finishedSourceNodes.append(&node);
+}
+
#if !RELEASE_LOG_DISABLED
WTFLogChannel& BaseAudioContext::logChannel() const
{
Modified: trunk/Source/WebCore/Modules/webaudio/BaseAudioContext.h (268727 => 268728)
--- trunk/Source/WebCore/Modules/webaudio/BaseAudioContext.h 2020-10-20 13:19:42 UTC (rev 268727)
+++ trunk/Source/WebCore/Modules/webaudio/BaseAudioContext.h 2020-10-20 15:02:18 UTC (rev 268728)
@@ -163,9 +163,6 @@
ExceptionOr<Ref<StereoPannerNode>> createStereoPanner();
ExceptionOr<Ref<IIRFilterNode>> createIIRFilter(ScriptExecutionContext&, Vector<double>&& feedforward, Vector<double>&& feedback);
- // When a source node has no more processing to do (has finished playing), then it tells the context to dereference it.
- void notifyNodeFinishedProcessing(AudioNode*);
-
// Called at the start of each render quantum.
void handlePreRenderTasks(const AudioIOPosition& outputPosition);
@@ -249,7 +246,9 @@
void isPlayingAudioDidChange();
- virtual void nodeWillBeginPlayback() { }
+ virtual void sourceNodeWillBeginPlayback(AudioNode&);
+ // When a source node has no more processing to do (has finished playing), then it tells the context to dereference it.
+ void sourceNodeDidFinishPlayback(AudioNode&);
#if !RELEASE_LOG_DISABLED
const Logger& logger() const override { return m_logger.get(); }
@@ -283,13 +282,6 @@
bool m_mustReleaseLock;
};
- // 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&);
-
virtual void lazyInitialize();
static bool isSupportedSampleRate(float sampleRate);
@@ -326,6 +318,13 @@
void scheduleNodeDeletion();
+ // When source nodes begin playing, the BaseAudioContext keeps them alive inside m_referencedSourceNodes.
+ // When the nodes stop playing, they get added to m_finishedSourceNodes. After each rendering quantum,
+ // we call derefSourceNode() on every node in m_finishedSourceNodes since we no longer need to keep them
+ // alive.
+ void refSourceNode(AudioNode&);
+ void derefSourceNode(AudioNode&);
+
// EventTarget
void dispatchEvent(Event&) final;
@@ -352,10 +351,10 @@
Ref<AudioWorklet> m_worklet;
// Only accessed in the audio thread.
- Vector<AudioNode*> m_finishedNodes;
+ Vector<AudioNode*> m_finishedSourceNodes;
// Either accessed when the graph lock is held, or on the main thread when the audio thread has finished.
- Vector<AudioConnectionRefPtr<AudioNode>> m_referencedNodes;
+ Vector<AudioConnectionRefPtr<AudioNode>> m_referencedSourceNodes;
// Accumulate nodes which need to be deleted here.
// This is copied to m_nodesToDelete at the end of a render cycle in handlePostRenderTasks(), where we're assured of a stable graph
Modified: trunk/Source/WebCore/Modules/webaudio/ConstantSourceNode.cpp (268727 => 268728)
--- trunk/Source/WebCore/Modules/webaudio/ConstantSourceNode.cpp 2020-10-20 13:19:42 UTC (rev 268727)
+++ trunk/Source/WebCore/Modules/webaudio/ConstantSourceNode.cpp 2020-10-20 15:02:18 UTC (rev 268728)
@@ -41,11 +41,7 @@
ExceptionOr<Ref<ConstantSourceNode>> ConstantSourceNode::create(BaseAudioContext& context, const ConstantSourceOptions& options)
{
- auto node = adoptRef(*new ConstantSourceNode(context, options.offset));
-
- context.refNode(node);
-
- return node;
+ return adoptRef(*new ConstantSourceNode(context, options.offset));
}
ConstantSourceNode::ConstantSourceNode(BaseAudioContext& context, float offset)
Modified: trunk/Source/WebCore/Modules/webaudio/MediaElementAudioSourceNode.cpp (268727 => 268728)
--- trunk/Source/WebCore/Modules/webaudio/MediaElementAudioSourceNode.cpp 2020-10-20 13:19:42 UTC (rev 268727)
+++ trunk/Source/WebCore/Modules/webaudio/MediaElementAudioSourceNode.cpp 2020-10-20 15:02:18 UTC (rev 268728)
@@ -55,8 +55,10 @@
auto node = adoptRef(*new MediaElementAudioSourceNode(context, *options.mediaElement));
options.mediaElement->setAudioSourceNode(node.ptr());
- context.refNode(node.get()); // context keeps reference until node is disconnected
+ // context keeps reference until node is disconnected.
+ context.sourceNodeWillBeginPlayback(node);
+
return node;
}
Modified: trunk/Source/WebCore/Modules/webaudio/MediaStreamAudioSourceNode.cpp (268727 => 268728)
--- trunk/Source/WebCore/Modules/webaudio/MediaStreamAudioSourceNode.cpp 2020-10-20 13:19:42 UTC (rev 268727)
+++ trunk/Source/WebCore/Modules/webaudio/MediaStreamAudioSourceNode.cpp 2020-10-20 15:02:18 UTC (rev 268728)
@@ -60,7 +60,8 @@
auto node = adoptRef(*new MediaStreamAudioSourceNode(context, *options.mediaStream, *providerTrack));
node->setFormat(2, context.sampleRate());
- context.refNode(node); // context keeps reference until node is disconnected
+ // Context keeps reference until node is disconnected.
+ context.sourceNodeWillBeginPlayback(node);
return node;
}
Modified: trunk/Source/WebCore/Modules/webaudio/OscillatorNode.cpp (268727 => 268728)
--- trunk/Source/WebCore/Modules/webaudio/OscillatorNode.cpp 2020-10-20 13:19:42 UTC (rev 268727)
+++ trunk/Source/WebCore/Modules/webaudio/OscillatorNode.cpp 2020-10-20 15:02:18 UTC (rev 268728)
@@ -77,10 +77,6 @@
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;
}
Modified: trunk/Source/WebCore/Modules/webaudio/ScriptProcessorNode.cpp (268727 => 268728)
--- trunk/Source/WebCore/Modules/webaudio/ScriptProcessorNode.cpp 2020-10-20 13:19:42 UTC (rev 268727)
+++ trunk/Source/WebCore/Modules/webaudio/ScriptProcessorNode.cpp 2020-10-20 15:02:18 UTC (rev 268728)
@@ -270,6 +270,19 @@
return true;
}
+void ScriptProcessorNode::eventListenersDidChange()
+{
+ m_hasAudioProcessEventListener = hasEventListeners(eventNames().audioprocessEvent);
+}
+
+bool ScriptProcessorNode::virtualHasPendingActivity() const
+{
+ if (context().isClosed())
+ return false;
+
+ return m_hasAudioProcessEventListener;
+}
+
} // namespace WebCore
#endif // ENABLE(WEB_AUDIO)
Modified: trunk/Source/WebCore/Modules/webaudio/ScriptProcessorNode.h (268727 => 268728)
--- trunk/Source/WebCore/Modules/webaudio/ScriptProcessorNode.h 2020-10-20 13:19:42 UTC (rev 268727)
+++ trunk/Source/WebCore/Modules/webaudio/ScriptProcessorNode.h 2020-10-20 15:02:18 UTC (rev 268728)
@@ -76,6 +76,8 @@
ScriptProcessorNode(BaseAudioContext&, size_t bufferSize, unsigned numberOfInputChannels, unsigned numberOfOutputChannels);
+ bool virtualHasPendingActivity() const final;
+ void eventListenersDidChange() final;
void fireProcessEvent(unsigned doubleBufferIndex);
// Double buffering
@@ -94,6 +96,7 @@
RefPtr<AudioBus> m_internalInputBus;
RefPtr<PendingActivity<ScriptProcessorNode>> m_pendingActivity;
Lock m_processLock;
+ bool m_hasAudioProcessEventListener { false };
};
} // namespace WebCore
Modified: trunk/Source/WebCore/Modules/webaudio/WebKitAudioContext.cpp (268727 => 268728)
--- trunk/Source/WebCore/Modules/webaudio/WebKitAudioContext.cpp 2020-10-20 13:19:42 UTC (rev 268727)
+++ trunk/Source/WebCore/Modules/webaudio/WebKitAudioContext.cpp 2020-10-20 15:02:18 UTC (rev 268728)
@@ -145,15 +145,7 @@
lazyInitialize();
- auto node = WebKitOscillatorNode::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 WebKitOscillatorNode::create(*this);
}
ExceptionOr<Ref<PeriodicWave>> WebKitAudioContext::createPeriodicWave(Float32Array& real, Float32Array& imaginary)
@@ -180,13 +172,7 @@
lazyInitialize();
- auto node = WebKitAudioBufferSourceNode::create(*this);
-
- // Because this is an AudioScheduledSourceNode, the context keeps a reference until it has finished playing.
- // When this happens, AudioScheduledSourceNode::finish() calls BaseAudioContext::notifyNodeFinishedProcessing().
- refNode(node);
-
- return node;
+ return WebKitAudioBufferSourceNode::create(*this);
}
ExceptionOr<Ref<WebKitDynamicsCompressorNode>> WebKitAudioContext::createWebKitDynamicsCompressor()
