Add process lock when setting looping, loop start, and loop end

The audio thread reads the looping parameters (looping, start, and
end) possibly many times while rendering the data.  The main thread
can modify any of these at any time.  This is a race.

To eliminate the race, modify the setters for looping, loop start, and
loop end to add a mutex for the process lock so that the main thread
can't modify these until audio thread is finished processing.

Bug: 1107500
Change-Id: Ied5cfb35cfb91e7982dc68e694dead09b1c74aa2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2306555
Commit-Queue: Raymond Toy <rtoy@chromium.org>
Reviewed-by: Hongchan Choi <hongchan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#791825}

third_party/blink/renderer/modules/webaudio/audio_buffer_source_node.cc

@@ -566,6 +566,34 @@ void AudioBufferSourceHandler::StartSource(double when,
   SetPlaybackState(SCHEDULED_STATE);
 }
 
+void AudioBufferSourceHandler::SetLoop(bool looping) {
+  DCHECK(IsMainThread());
+
+  // This synchronizes with |Process()|.
+  MutexLocker process_locker(process_lock_);
+
+  is_looping_ = looping;
+  SetDidSetLooping(looping);
+}
+
+void AudioBufferSourceHandler::SetLoopStart(double loop_start) {
+  DCHECK(IsMainThread());
+
+  // This synchronizes with |Process()|.
+  MutexLocker process_locker(process_lock_);
+
+  loop_start_ = loop_start;
+}
+
+void AudioBufferSourceHandler::SetLoopEnd(double loop_end) {
+  DCHECK(IsMainThread());
+
+  // This synchronizes with |Process()|.
+  MutexLocker process_locker(process_lock_);
+
+  loop_end_ = loop_end;
+}
+
 double AudioBufferSourceHandler::ComputePlaybackRate() {
   // Incorporate buffer's sample-rate versus BaseAudioContext's sample-rate.
   // Normally it's not an issue because buffers are loaded at the

third_party/blink/renderer/modules/webaudio/audio_buffer_source_node.h

@@ -81,16 +81,13 @@ class AudioBufferSourceHandler final : public AudioScheduledSourceHandler {
   // specification, the proper attribute name is |.loop|. The old attribute is
   // kept for backwards compatibility.
   bool Loop() const { return is_looping_; }
-  void SetLoop(bool looping) {
-    is_looping_ = looping;
-    SetDidSetLooping(looping);
-  }
+  void SetLoop(bool looping);
 
   // Loop times in seconds.
   double LoopStart() const { return loop_start_; }
   double LoopEnd() const { return loop_end_; }
-  void SetLoopStart(double loop_start) { loop_start_ = loop_start; }
-  void SetLoopEnd(double loop_end) { loop_end_ = loop_end; }
+  void SetLoopStart(double loop_start);
+  void SetLoopEnd(double loop_end);
 
   // If we are no longer playing, propogate silence ahead to downstream nodes.
   bool PropagatesSilence() const override;
@@ -147,23 +144,26 @@ class AudioBufferSourceHandler final : public AudioScheduledSourceHandler {
   scoped_refptr<AudioParamHandler> playback_rate_;
   scoped_refptr<AudioParamHandler> detune_;
 
-  bool DidSetLooping() const {
-    return did_set_looping_.load(std::memory_order_acquire);
-  }
+  bool DidSetLooping() const { return did_set_looping_; }
   void SetDidSetLooping(bool loop) {
     if (loop)
-      did_set_looping_.store(true, std::memory_order_release);
+      did_set_looping_ = true;
   }
 
   // If m_isLooping is false, then this node will be done playing and become
   // inactive after it reaches the end of the sample data in the buffer.  If
   // true, it will wrap around to the start of the buffer each time it reaches
   // the end.
+  //
+  // A process lock must be used to protect access.
   bool is_looping_;
 
   // True if the source .loop attribute was ever set.
-  std::atomic_bool did_set_looping_;
+  // A process lock must be used to protect access.
+  bool did_set_looping_;
 
+  // A process lock must be used to protect access to both |loop_start_| and
+  // |loop_end_|.
   double loop_start_;
   double loop_end_;