- Revision
- 278288
- Author
- cdu...@apple.com
- Date
- 2021-05-31 17:08:20 -0700 (Mon, 31 May 2021)
Log Message
Drop PendingActivity data member from BaseAudioContext
https://bugs.webkit.org/show_bug.cgi?id=226445
Reviewed by Darin Adler.
Drop PendingActivity data member from BaseAudioContext and instead have AudioContext / OfflineAudioContext
override virtualHasPendingActivity() to keep their JS wrapper alive. I find that PendingActivity data members
are too error prone and a frequent cause of leaks.
* Modules/webaudio/AudioContext.cpp:
(WebCore::AudioContext::AudioContext):
(WebCore::AudioContext::startRendering):
(WebCore::AudioContext::mayResumePlayback):
(WebCore::AudioContext::suspendPlayback):
(WebCore::AudioContext::virtualHasPendingActivity const):
* Modules/webaudio/AudioContext.h:
* Modules/webaudio/BaseAudioContext.cpp:
(WebCore::BaseAudioContext::clear):
(WebCore::BaseAudioContext::clearPendingActivity): Deleted.
(WebCore::BaseAudioContext::setPendingActivity): Deleted.
* Modules/webaudio/BaseAudioContext.h:
* Modules/webaudio/OfflineAudioContext.cpp:
(WebCore::OfflineAudioContext::startRendering):
(WebCore::OfflineAudioContext::resumeRendering):
(WebCore::OfflineAudioContext::didSuspendRendering):
(WebCore::OfflineAudioContext::finishedRendering):
(WebCore::OfflineAudioContext::virtualHasPendingActivity const):
(WebCore::OfflineAudioContext::dispatchEvent): Deleted.
* Modules/webaudio/OfflineAudioContext.h:
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (278287 => 278288)
--- trunk/Source/WebCore/ChangeLog 2021-06-01 00:07:14 UTC (rev 278287)
+++ trunk/Source/WebCore/ChangeLog 2021-06-01 00:08:20 UTC (rev 278288)
@@ -1,5 +1,37 @@
2021-05-31 Chris Dumez <cdu...@apple.com>
+ Drop PendingActivity data member from BaseAudioContext
+ https://bugs.webkit.org/show_bug.cgi?id=226445
+
+ Reviewed by Darin Adler.
+
+ Drop PendingActivity data member from BaseAudioContext and instead have AudioContext / OfflineAudioContext
+ override virtualHasPendingActivity() to keep their JS wrapper alive. I find that PendingActivity data members
+ are too error prone and a frequent cause of leaks.
+
+ * Modules/webaudio/AudioContext.cpp:
+ (WebCore::AudioContext::AudioContext):
+ (WebCore::AudioContext::startRendering):
+ (WebCore::AudioContext::mayResumePlayback):
+ (WebCore::AudioContext::suspendPlayback):
+ (WebCore::AudioContext::virtualHasPendingActivity const):
+ * Modules/webaudio/AudioContext.h:
+ * Modules/webaudio/BaseAudioContext.cpp:
+ (WebCore::BaseAudioContext::clear):
+ (WebCore::BaseAudioContext::clearPendingActivity): Deleted.
+ (WebCore::BaseAudioContext::setPendingActivity): Deleted.
+ * Modules/webaudio/BaseAudioContext.h:
+ * Modules/webaudio/OfflineAudioContext.cpp:
+ (WebCore::OfflineAudioContext::startRendering):
+ (WebCore::OfflineAudioContext::resumeRendering):
+ (WebCore::OfflineAudioContext::didSuspendRendering):
+ (WebCore::OfflineAudioContext::finishedRendering):
+ (WebCore::OfflineAudioContext::virtualHasPendingActivity const):
+ (WebCore::OfflineAudioContext::dispatchEvent): Deleted.
+ * Modules/webaudio/OfflineAudioContext.h:
+
+2021-05-31 Chris Dumez <cdu...@apple.com>
+
Fix thread safety issues in AudioBufferSourceNode
https://bugs.webkit.org/show_bug.cgi?id=226448
Modified: trunk/Source/WebCore/Modules/webaudio/AudioContext.cpp (278287 => 278288)
--- trunk/Source/WebCore/Modules/webaudio/AudioContext.cpp 2021-06-01 00:07:14 UTC (rev 278287)
+++ trunk/Source/WebCore/Modules/webaudio/AudioContext.cpp 2021-06-01 00:08:20 UTC (rev 278288)
@@ -115,10 +115,6 @@
, m_destinationNode(makeUniqueRef<DefaultAudioDestinationNode>(*this, contextOptions.sampleRate))
, m_mediaSession(PlatformMediaSession::create(PlatformMediaSessionManager::sharedManager(), *this))
{
- // According to spec AudioContext must die only after page navigate.
- // Lets mark it as ActiveDOMObject with pending activity and unmark it in clear method.
- setPendingActivity();
-
constructCommon();
// Initialize the destination node's muted state to match the page's current muted state.
@@ -130,7 +126,7 @@
// Unlike OfflineAudioContext, AudioContext does not require calling resume() to start rendering.
// Lazy initialization starts rendering so we schedule a task here to make sure lazy initialization
// ends up happening, even if no audio node gets constructed.
- postTask([this] {
+ postTask([this, pendingActivity = makePendingActivity(*this)] {
if (!isStopped())
lazyInitialize();
});
@@ -309,10 +305,8 @@
if (isStopped() || !willBeginPlayback() || m_wasSuspendedByScript)
return;
- setPendingActivity();
-
lazyInitialize();
- destination().startRendering([this, protectedThis = makeRef(*this)](std::optional<Exception>&& exception) {
+ destination().startRendering([this, protectedThis = makeRef(*this), pendingActivity = makePendingActivity(*this)](std::optional<Exception>&& exception) {
if (!exception)
setState(State::Running);
});
@@ -385,7 +379,7 @@
lazyInitialize();
- destination().resume([this, protectedThis = makeRef(*this)](std::optional<Exception>&& exception) {
+ destination().resume([this, protectedThis = makeRef(*this), pendingActivity = makePendingActivity(*this)](std::optional<Exception>&& exception) {
setState(exception ? State::Suspended : State::Running);
});
}
@@ -471,7 +465,7 @@
lazyInitialize();
- destination().suspend([this, protectedThis = makeRef(*this)](std::optional<Exception>&& exception) {
+ destination().suspend([this, protectedThis = makeRef(*this), pendingActivity = makePendingActivity(*this)](std::optional<Exception>&& exception) {
if (exception)
return;
@@ -555,6 +549,11 @@
#endif
+bool AudioContext::virtualHasPendingActivity() const
+{
+ return !isClosed();
+}
+
} // namespace WebCore
#endif // ENABLE(WEB_AUDIO)
Modified: trunk/Source/WebCore/Modules/webaudio/AudioContext.h (278287 => 278288)
--- trunk/Source/WebCore/Modules/webaudio/AudioContext.h 2021-06-01 00:07:14 UTC (rev 278287)
+++ trunk/Source/WebCore/Modules/webaudio/AudioContext.h 2021-06-01 00:08:20 UTC (rev 278288)
@@ -142,6 +142,7 @@
const char* activeDOMObjectName() const final;
void suspend(ReasonForSuspension) final;
void resume() final;
+ bool virtualHasPendingActivity() const final;
UniqueRef<DefaultAudioDestinationNode> m_destinationNode;
std::unique_ptr<PlatformMediaSession> m_mediaSession;
Modified: trunk/Source/WebCore/Modules/webaudio/BaseAudioContext.cpp (278287 => 278288)
--- trunk/Source/WebCore/Modules/webaudio/BaseAudioContext.cpp 2021-06-01 00:07:14 UTC (rev 278287)
+++ trunk/Source/WebCore/Modules/webaudio/BaseAudioContext.cpp 2021-06-01 00:08:20 UTC (rev 278288)
@@ -179,8 +179,6 @@
deleteMarkedNodes();
m_nodesToDelete = std::exchange(m_nodesMarkedForDeletion, { });
} while (!m_nodesToDelete.isEmpty());
-
- clearPendingActivity();
}
void BaseAudioContext::uninitialize()
@@ -877,17 +875,6 @@
m_scriptExecutionContext->addConsoleMessage(source, level, message);
}
-void BaseAudioContext::clearPendingActivity()
-{
- m_pendingActivity = nullptr;
-}
-
-void BaseAudioContext::setPendingActivity()
-{
- if (!m_pendingActivity)
- m_pendingActivity = makePendingActivity(*this);
-}
-
PeriodicWave& BaseAudioContext::periodicWave(OscillatorType type)
{
switch (type) {
Modified: trunk/Source/WebCore/Modules/webaudio/BaseAudioContext.h (278287 => 278288)
--- trunk/Source/WebCore/Modules/webaudio/BaseAudioContext.h 2021-06-01 00:07:14 UTC (rev 278287)
+++ trunk/Source/WebCore/Modules/webaudio/BaseAudioContext.h 2021-06-01 00:08:20 UTC (rev 278288)
@@ -227,9 +227,6 @@
protected:
explicit BaseAudioContext(Document&);
-
- void clearPendingActivity();
- void setPendingActivity();
virtual void uninitialize();
@@ -346,8 +343,6 @@
std::unique_ptr<AsyncAudioDecoder> m_audioDecoder;
- RefPtr<PendingActivity<BaseAudioContext>> m_pendingActivity;
-
AudioIOPosition m_outputPosition;
HashMap<String, Vector<AudioParamDescriptor>> m_parameterDescriptorMap;
Modified: trunk/Source/WebCore/Modules/webaudio/OfflineAudioContext.cpp (278287 => 278288)
--- trunk/Source/WebCore/Modules/webaudio/OfflineAudioContext.cpp 2021-06-01 00:07:14 UTC (rev 278287)
+++ trunk/Source/WebCore/Modules/webaudio/OfflineAudioContext.cpp 2021-06-01 00:08:20 UTC (rev 278288)
@@ -113,7 +113,6 @@
return;
}
- setPendingActivity();
m_pendingRenderingPromise = WTFMove(promise);
m_didStartRendering = true;
setState(State::Running);
@@ -175,7 +174,6 @@
return;
}
- setPendingActivity();
setState(State::Running);
promise->resolve();
});
@@ -195,8 +193,6 @@
{
setState(State::Suspended);
- clearPendingActivity();
-
RefPtr<DeferredPromise> promise;
{
Locker locker { graphLock() };
@@ -217,6 +213,9 @@
clear();
});
+ // Make sure our JSwrapper stays alive long enough to resolve the promise and queue the completion event.
+ // Otherwise, setting the state to Closed may cause our JS wrapper to get collected early.
+ auto protectedJSWrapper = makePendingActivity(*this);
setState(State::Closed);
// Avoid firing the event if the document has already gone away.
@@ -246,11 +245,9 @@
promise->resolve<IDLInterface<AudioBuffer>>(result.releaseReturnValue());
}
-void OfflineAudioContext::dispatchEvent(Event& event)
+bool OfflineAudioContext::virtualHasPendingActivity() const
{
- BaseAudioContext::dispatchEvent(event);
- if (event.eventInterface() == OfflineAudioCompletionEventInterfaceType)
- clearPendingActivity();
+ return state() == State::Running;
}
} // namespace WebCore
Modified: trunk/Source/WebCore/Modules/webaudio/OfflineAudioContext.h (278287 => 278288)
--- trunk/Source/WebCore/Modules/webaudio/OfflineAudioContext.h 2021-06-01 00:07:14 UTC (rev 278287)
+++ trunk/Source/WebCore/Modules/webaudio/OfflineAudioContext.h 2021-06-01 00:08:20 UTC (rev 278288)
@@ -60,10 +60,8 @@
// ActiveDOMObject
const char* activeDOMObjectName() const final;
+ bool virtualHasPendingActivity() const final;
- // EventTarget
- void dispatchEvent(Event&) final;
-
void settleRenderingPromise(ExceptionOr<Ref<AudioBuffer>>&&);
void uninitialize() final;
bool isOfflineContext() const final { return true; }
