Reviewers: Raymond Toy
CL:
https://codereview.chromium.org/2283053002/Message:
PTAL.
Description:
Fix BaseAudioContext::hasPendingActivity() to make it GCed correctly
The realtime AudioContext is not GCed properly even after the tear-down.
It is because the context mark itself as 'has pending activity' no
matter what the actual status is.
This CL changes heuristic of hasPendingActivity method to signal Oilpan
that this context is ready for GC; rtoy@ and I discussed that the best
logic is when the destination handler does not have incoming connection
any more.
BUG=503845
TEST=LayoutTests/webaudio/audiocontext-leak.html
Affected files (+79, -2 lines):
A third_party/WebKit/LayoutTests/webaudio/audiocontext-leak.html
M third_party/WebKit/LayoutTests/webaudio/resources/audio-testing.js
M third_party/WebKit/Source/modules/webaudio/AudioDestinationNode.h
M third_party/WebKit/Source/modules/webaudio/AudioDestinationNode.cpp
M third_party/WebKit/Source/modules/webaudio/BaseAudioContext.cpp
Index: third_party/WebKit/LayoutTests/webaudio/audiocontext-leak.html
diff --git a/third_party/WebKit/LayoutTests/webaudio/audiocontext-leak.html b/third_party/WebKit/LayoutTests/webaudio/audiocontext-leak.html
new file mode 100644
index 0000000000000000000000000000000000000000..4d76874a75b240ff97b0bb1281e5f9fafb2c650c
--- /dev/null
+++ b/third_party/WebKit/LayoutTests/webaudio/audiocontext-leak.html
@@ -0,0 +1,37 @@
+<!DOCTYPE html>
+<html>
+<head>
+ <title></title>
+ <script src="../resources/testharness.js"></script>
+ <script src="../resources/testharnessreport.js"></script>
+ <script src="resources/audio-testing.js"></script>
+</head>
+<body>
+ <script>
+ var audioContextObservation;
+
+ var audit = Audit.createTaskRunner();
+
+
+ // Create an instance of AudioContext without JS reference, and observe
+ // if it goes away after the forcible GC.
+ audit.defineTask('observe-leak', function (taskDone) {
+
+ // This layout test requires internal helpers from run-webkit-tests.
+ Should('window.internals', window.internals).exist();
+ Should('window.GCController', window.GCController).exist();
+
+ audioContextObservation = internals.observeGC(new AudioContext());
+ GCController.collectAll();
+
+ Should('audioContextObservation.wasCollected',
+ audioContextObservation.wasCollected).beEqualTo(true);
+
+ taskDone();
+ });
+
+
+ audit.runTasks();
+ </script>
+</body>
+</html>
Index: third_party/WebKit/LayoutTests/webaudio/resources/audio-testing.js
diff --git a/third_party/WebKit/LayoutTests/webaudio/resources/audio-testing.js b/third_party/WebKit/LayoutTests/webaudio/resources/audio-testing.js
index 5cbe92c150b07aa9938853951645f4b81ed64729..12479de0a57b597b77159a3e9a64dda3a09b0611 100644
--- a/third_party/WebKit/LayoutTests/webaudio/resources/audio-testing.js
+++ b/third_party/WebKit/LayoutTests/webaudio/resources/audio-testing.js
@@ -597,6 +597,20 @@ var Should = (function () {
throw failureMessage;
};
+ // Check if |target| does exist. (not undefined nor null)
+ //
+ // Example:
+ // Should('null', null).exist();
+ // Result:
+ // "FAIL null does not exist."
+ ShouldModel.prototype.exist = function () {
+ if (this.target !== null && this.target !== undefined) {
+ this._testPassed('exist');
+ } else {
+ this._testFailed('does not exist');
+ }
+ };
+
// Check if |target| is equal to |value|.
//
// Example:
Index: third_party/WebKit/Source/modules/webaudio/AudioDestinationNode.cpp
diff --git a/third_party/WebKit/Source/modules/webaudio/AudioDestinationNode.cpp b/third_party/WebKit/Source/modules/webaudio/AudioDestinationNode.cpp
index f4a86ca65181ee9dc233c803168a815022438191..a02406807862e9e17b8bd26b83f85bb59a341156 100644
--- a/third_party/WebKit/Source/modules/webaudio/AudioDestinationNode.cpp
+++ b/third_party/WebKit/Source/modules/webaudio/AudioDestinationNode.cpp
@@ -35,6 +35,7 @@ namespace blink {
AudioDestinationHandler::AudioDestinationHandler(AudioNode& node, float sampleRate)
: AudioHandler(NodeTypeDestination, node, sampleRate)
, m_currentSampleFrame(0)
+ , m_numberOfConnections(0)
{
addInput();
}
@@ -103,6 +104,14 @@ void AudioDestinationHandler::render(AudioBus* sourceBus, AudioBus* destinationB
// Advance current sample-frame.
size_t newSampleFrame = m_currentSampleFrame + numberOfFrames;
releaseStore(&m_currentSampleFrame, newSampleFrame);
+
+ // Update connection status.
+ releaseStore(&m_numberOfConnections, input(0).numberOfRenderingConnections());
+}
+
+unsigned AudioDestinationHandler::numberOfConnections() const
+{
+ return acquireLoad(&m_numberOfConnections);
}
// ----------------------------------------------------------------
@@ -122,5 +131,10 @@ unsigned long AudioDestinationNode::maxChannelCount() const
return audioDestinationHandler().maxChannelCount();
}
+bool AudioDestinationNode::hasConnection() const
+{
+ return audioDestinationHandler().numberOfConnections() > 0;
+}
+
} // namespace blink
Index: third_party/WebKit/Source/modules/webaudio/AudioDestinationNode.h
diff --git a/third_party/WebKit/Source/modules/webaudio/AudioDestinationNode.h b/third_party/WebKit/Source/modules/webaudio/AudioDestinationNode.h
index a2c419a469b5e4eb25eeb45044ca653c52585bbc..40758cb907402dce56116c4d1411719d36089846 100644
--- a/third_party/WebKit/Source/modules/webaudio/AudioDestinationNode.h
+++ b/third_party/WebKit/Source/modules/webaudio/AudioDestinationNode.h
@@ -56,6 +56,8 @@ public:
virtual void startRendering() = 0;
virtual void stopRendering() = 0;
+ unsigned numberOfConnections() const;
+
protected:
// LocalAudioInputProvider allows us to expose an AudioSourceProvider for local/live audio input.
// If there is local/live audio input, we call set() with the audio input data every render quantum.
@@ -88,6 +90,12 @@ protected:
// Counts the number of sample-frames processed by the destination.
size_t m_currentSampleFrame;
+ // Represents the number of incoming connections toward this destination
+ // node. It is updated every render quantum atomically, and it proxies
+ // the connection count for the rendering operation used in
+ // AudioSummingJuntion.
+ unsigned m_numberOfConnections;
+
LocalAudioInputProvider m_localAudioInputProvider;
};
@@ -98,6 +106,8 @@ public:
unsigned long maxChannelCount() const;
+ bool hasConnection() const;
+
protected:
AudioDestinationNode(BaseAudioContext&);
};
Index: third_party/WebKit/Source/modules/webaudio/BaseAudioContext.cpp
diff --git a/third_party/WebKit/Source/modules/webaudio/BaseAudioContext.cpp b/third_party/WebKit/Source/modules/webaudio/BaseAudioContext.cpp
index 2e72ec0778d2f4d10bedb4b47c8f0f8f00d29de2..0914dfafd913d50de7e1151223037dc794d762e9 100644
--- a/third_party/WebKit/Source/modules/webaudio/BaseAudioContext.cpp
+++ b/third_party/WebKit/Source/modules/webaudio/BaseAudioContext.cpp
@@ -213,8 +213,10 @@ void BaseAudioContext::stop()
bool BaseAudioContext::hasPendingActivity() const
{
- // There's no pending activity if the audio context has been cleared.
- return !m_isCleared;
+ // If the destination node (handler) has any active connection and the audio
+ // graph needs to be rendered, we mark the context as 'hasPendingActivity'.
+ // This keeps this node from getting GCed.
+ return destination()->hasConnection();
}
AudioDestinationNode* BaseAudioContext::destination() const