Guarantee a thread safety when accessing source location info in pannerNode.

Source/modules/webaudio/PannerNode.cpp

Index: Source/modules/webaudio/PannerNode.cpp
diff --git a/Source/modules/webaudio/PannerNode.cpp b/Source/modules/webaudio/PannerNode.cpp
index 3f2dde89def0dc6e3f5920d71ebb7663e6bd3d12..3d3f0d1e6fe5c92f3959803b905659bcc07c84ac 100644
--- a/Source/modules/webaudio/PannerNode.cpp
+++ b/Source/modules/webaudio/PannerNode.cpp
@@ -58,7 +58,7 @@ PannerNode::PannerNode(AudioContext* context, float sampleRate)
     , m_lastGain(-1.0)
     , m_cachedAzimuth(0)
     , m_cachedElevation(0)
-    , m_cachedDistanceConeGain(0)
+    , m_cachedDistanceConeGain(1.0f)
     , m_cachedDopplerRate(1)
     , m_connectionCount(0)
 {
@@ -123,19 +123,19 @@ void PannerNode::process(size_t framesToProcess)
         return;
     }
 
-    // HRTFDatabase should be loaded before proceeding for offline audio context when panningModel() is "HRTF".
-    if (panningModel() == "HRTF" && !m_hrtfDatabaseLoader->isLoaded()) {
-        if (context()->isOfflineContext()) {
-            m_hrtfDatabaseLoader->waitForLoaderThreadCompletion();
-        } else {
-            destination->zero();
-            return;
-        }
-    }
-
     // The audio thread can't block on this lock, so we call tryLock() instead.
-    MutexTryLocker tryLocker(m_pannerLock);
+    MutexTryLocker tryLocker(m_processLock);
     if (tryLocker.locked()) {
+        // HRTFDatabase should be loaded before proceeding for offline audio context when panningModel() is "HRTF".
+        if (panningModel() == "HRTF" && !m_hrtfDatabaseLoader->isLoaded()) {
+            if (context()->isOfflineContext()) {
+                m_hrtfDatabaseLoader->waitForLoaderThreadCompletion();
+            } else {
+                destination->zero();
+                return;
+            }
+        }
+
         // Apply the panning effect.
         double azimuth;
         double elevation;
@@ -158,7 +158,8 @@ void PannerNode::process(size_t framesToProcess)
         // Now update the cached source location in case the source has changed.
         updateCachedSourceLocationInfo();
     } else {
-        // Too bad - The tryLock() failed. We must be in the middle of changing the panner.
+        // Too bad - The tryLock() failed.
+        // We must be in the middle of changing the panning model, the distance model, or the source's location information.
         destination->zero();
     }
 }
@@ -217,7 +218,7 @@ bool PannerNode::setPanningModel(unsigned model)
     case HRTF:
         if (!m_panner.get() || model != m_panningModel) {
             // This synchronizes with process().
-            MutexLocker processLocker(m_pannerLock);
+            MutexLocker processLocker(m_processLock);
 
             OwnPtr<Panner> newPanner = Panner::create(model, sampleRate(), m_hrtfDatabaseLoader.get());
             m_panner = newPanner.release();
@@ -233,37 +234,40 @@ bool PannerNode::setPanningModel(unsigned model)
 
 void PannerNode::setPosition(float x, float y, float z)
 {
-    // FIXME : consider thread safety about m_position in audio thread.
-    // See http://crbugs.com/350583.
     FloatPoint3D position = FloatPoint3D(x, y, z);
 
     if (m_position == position)
         return;
 
+    // This synchronizes with process().
+    MutexLocker processLocker(m_processLock);
+
     m_position = position;
 }
 
 void PannerNode::setOrientation(float x, float y, float z)
 {
-    // FIXME : consider thread safety about m_orientation in audio thread.
-    // See http://crbugs.com/350583.
     FloatPoint3D orientation = FloatPoint3D(x, y, z);
 
     if (m_orientation == orientation)
         return;
 
+    // This synchronizes with process().
+    MutexLocker processLocker(m_processLock);
+
     m_orientation = orientation;
 }
 
 void PannerNode::setVelocity(float x, float y, float z)
 {
-    // FIXME : consider thread safety about m_velocity in audio thread.
-    // See http://crbugs.com/350583.
     FloatPoint3D velocity = FloatPoint3D(x, y, z);
 
     if (m_velocity == velocity)
         return;
 
+    // This synchronizes with process().
+    MutexLocker processLocker(m_processLock);
+
     m_velocity = velocity;
 }
 
@@ -300,7 +304,11 @@ bool PannerNode::setDistanceModel(unsigned model)
     case DistanceEffect::ModelLinear:
     case DistanceEffect::ModelInverse:
     case DistanceEffect::ModelExponential:
-        m_distanceEffect.setModel(static_cast<DistanceEffect::ModelType>(model), true);
+        if (model != m_distanceEffect.model()) {
+            // This synchronizes with process().
+            MutexLocker processLocker(m_processLock);
+            m_distanceEffect.setModel(static_cast<DistanceEffect::ModelType>(model), true);
+        }
         break;
     default:
         return false;
@@ -371,7 +379,6 @@ void PannerNode::calculateAzimuthElevation(double* outAzimuth, double* outElevat
         *outElevation = elevation;
 }
 
-
 double PannerNode::calculateDopplerRate()
 {
     double dopplerShift = 1.0;
@@ -436,6 +443,8 @@ float PannerNode::calculateDistanceConeGain()
 
 void PannerNode::azimuthElevation(double* outAzimuth, double* outElevation)
 {
+    ASSERT(context()->isAudioThread());
+
     if (isAzimuthElevationDirty())
         calculateAzimuthElevation(&m_cachedAzimuth, &m_cachedElevation);
 
@@ -445,6 +454,8 @@ void PannerNode::azimuthElevation(double* outAzimuth, double* outElevation)
 
 double PannerNode::dopplerRate()
 {
+    ASSERT(context()->isAudioThread());

[Ken Russell (switch to Gerrit), 2014/03/19 00:37:41]

Since dopplerRate() is public in PannerNode.h please comment that it must be
called on the audio thread. It looks like it only has one caller.

[KhNo, 2014/03/19 01:46:42]

Thanks for sharing I missed. I have added. :)

+
     if (isDopplerRateDirty())
         m_cachedDopplerRate = calculateDopplerRate();
 
@@ -453,6 +464,8 @@ double PannerNode::dopplerRate()
 
 float PannerNode::distanceConeGain()
 {
+    ASSERT(context()->isAudioThread());
+
     if (isDistanceConeGainDirty())
         m_cachedDistanceConeGain = calculateDistanceConeGain();
 
@@ -534,6 +547,8 @@ void PannerNode::notifyAudioSourcesConnectedToNode(AudioNode* node, HashMap<Audi
 
 void PannerNode::updateCachedListener()
 {
+    ASSERT(context()->isAudioThread());
+
     m_cachedListener->setPosition(listener()->position());
     m_cachedListener->setOrientation(listener()->orientation());
     m_cachedListener->setUpVector(listener()->upVector());
@@ -544,6 +559,8 @@ void PannerNode::updateCachedListener()
 
 void PannerNode::updateCachedSourceLocationInfo()
 {
+    ASSERT(context()->isAudioThread());
+
     m_cachedPosition = m_position;
     m_cachedOrientation = m_orientation;
     m_cachedVelocity = m_velocity;