Web Audio: Oilpan: Correctly trace m_referencedNodes elements within a lock.

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 fde9cf2c51d36e4aea01626108c36d93ef42d011..8ee36aea0f2b4ee95cf59c7ff26a91e62f4e2385 100644
--- a/Source/modules/webaudio/AudioContext.cpp
+++ b/Source/modules/webaudio/AudioContext.cpp
@@ -106,11 +106,12 @@ AudioContext::AudioContext(Document* document)
     , m_isResolvingResumePromises(false)
     , m_automaticPullNodesNeedUpdating(false)
     , m_connectionCount(0)
+    , m_didInitializeContextGraphMutex(false)
     , m_audioThread(0)
     , m_isOfflineContext(false)
     , m_contextState(Suspended)
 {
-    m_referencedNodes = new HeapVector<Member<AudioNode>>();
+    m_didInitializeContextGraphMutex = true;
     m_destinationNode = DefaultAudioDestinationNode::create(this);
 
     initialize();
@@ -129,11 +130,12 @@ AudioContext::AudioContext(Document* document, unsigned numberOfChannels, size_t
     , m_isResolvingResumePromises(false)
     , m_automaticPullNodesNeedUpdating(false)
     , m_connectionCount(0)
+    , m_didInitializeContextGraphMutex(false)
     , m_audioThread(0)
     , m_isOfflineContext(true)
     , m_contextState(Suspended)
 {
-    m_referencedNodes = new HeapVector<Member<AudioNode>>();
+    m_didInitializeContextGraphMutex = true;
     // Create a new destination for offline rendering.
     m_renderTarget = AudioBuffer::create(numberOfChannels, numberOfFrames, sampleRate);
     if (m_renderTarget.get())
@@ -149,6 +151,7 @@ AudioContext::~AudioContext()
 #endif
     // AudioNodes keep a reference to their context, so there should be no way to be in the destructor if there are still AudioNodes around.
     ASSERT(!m_isInitialized);
+    ASSERT(!m_referencedNodes.size());
     ASSERT(!m_finishedNodes.size());
     ASSERT(!m_automaticPullNodes.size());
     if (m_automaticPullNodesNeedUpdating)
@@ -678,7 +681,7 @@ void AudioContext::refNode(AudioNode* node)
     ASSERT(isMainThread());
     AutoLocker locker(this);
 
-    m_referencedNodes->append(node);
+    m_referencedNodes.append(node);
     node->makeConnection();
 }
 
@@ -686,10 +689,10 @@ void AudioContext::derefNode(AudioNode* node)
 {
     ASSERT(isGraphOwner());
 
-    for (unsigned i = 0; i < m_referencedNodes->size(); ++i) {
-        if (node == m_referencedNodes->at(i).get()) {
+    for (unsigned i = 0; i < m_referencedNodes.size(); ++i) {
+        if (node == m_referencedNodes.at(i).get()) {
             node->breakConnection();
-            m_referencedNodes->remove(i);
+            m_referencedNodes.remove(i);
             break;
         }
     }
@@ -698,10 +701,10 @@ void AudioContext::derefNode(AudioNode* node)
 void AudioContext::derefUnfinishedSourceNodes()
 {
     ASSERT(isMainThread());
-    for (unsigned i = 0; i < m_referencedNodes->size(); ++i)
-        m_referencedNodes->at(i)->breakConnection();
+    for (unsigned i = 0; i < m_referencedNodes.size(); ++i)
+        m_referencedNodes.at(i)->breakConnection();
 
-    m_referencedNodes->clear();
+    m_referencedNodes.clear();
 }
 
 void AudioContext::lock()
@@ -1078,10 +1081,8 @@ void AudioContext::trace(Visitor* visitor)
     visitor->trace(m_destinationNode);
     visitor->trace(m_listener);
     // trace() can be called in AudioContext constructor, and
-    // m_contextGraphMutex might be unavailable.  We can use m_contextGraphMutex
-    // if m_referencedNodes is not null because m_referencedNodes is initialized
-    // after m_contextGraphMutex.
-    if (m_referencedNodes) {
+    // m_contextGraphMutex might be unavailable.
+    if (m_didInitializeContextGraphMutex) {
         AutoLocker lock(this);
         visitor->trace(m_referencedNodes);
     }

[haraken, 2014/12/04 05:43:35]

Shouldn't this be:

if (m_didInitializeContextGraphMutex) {
  AutoLocker lock(this);
  visitor->trace(m_referencedNodes);
} else {
  visitor->trace(m_referencedNodes);
}

?

[tkent, 2014/12/04 05:58:24]

m_referencedNode is always empty if m_didInitializeContextGraphMutex is false.
But tracing it is safer.  Will do.