Add WebThread in AudioDestination to support AudioWorkletThread

third_party/WebKit/Source/platform/audio/PushPullFIFO.h

Index: third_party/WebKit/Source/platform/audio/PushPullFIFO.h
diff --git a/third_party/WebKit/Source/platform/audio/PushPullFIFO.h b/third_party/WebKit/Source/platform/audio/PushPullFIFO.h
index 447ee221e9f764bcf27b1e72a6ccbc934bd167b9..d3bd47c879d281a60f7975e84cd8e9202a0354d7 100644
--- a/third_party/WebKit/Source/platform/audio/PushPullFIFO.h
+++ b/third_party/WebKit/Source/platform/audio/PushPullFIFO.h
@@ -7,6 +7,9 @@
 
 #include "platform/audio/AudioBus.h"
 #include "platform/wtf/Allocator.h"
+#include "platform/wtf/Functional.h"
+#include "platform/wtf/Threading.h"
+#include "platform/wtf/ThreadingPrimitives.h"
 #include "public/platform/WebCommon.h"
 
 namespace blink {
@@ -26,6 +29,14 @@ struct PushPullFIFOStateForTest {
 // Blink-WebAudio and the renderer. The renderer's hardware callback buffer size
 // varies on the platform, but the WebAudio always renders 128 frames (render
 // quantum, RQ) thus FIFO is needed to handle the general case.
+//
+// Note that this object is concurrently accessed by two threads; WebAudio
+// rendering thread (WebThread) in Blink and the audio device thread
+// (AudioDeviceThread) from the media renderer. The push/pull operations touch
+// most of variables in the class (index_write_, index_read_, frames_available_,
+// and fifo_Bus_) so the thread safety must be handled with care.
+//
+// TODO(hongchan): add a unit test for multi-thread access.
 class BLINK_PLATFORM_EXPORT PushPullFIFO {
   USING_FAST_MALLOC(PushPullFIFO);
   WTF_MAKE_NONCOPYABLE(PushPullFIFO);
@@ -53,12 +64,13 @@ class BLINK_PLATFORM_EXPORT PushPullFIFO {
   //  - In case of underflow (FIFO empty while pull), the remaining space in the
   //    requested output bus will be filled with silence. Thus it will fulfill
   //    the request from the consumer without causing error, but with a glitch.
-  void Pull(AudioBus* output_bus, size_t frames_requested);
+  size_t Pull(AudioBus* output_bus, size_t frames_requested);

[o1ka, 2017/04/18 14:14:57]

Could you add a description of the return value?

[hongchan, 2017/04/18 18:16:09]

Done.

 
-  size_t FramesAvailable() const { return frames_available_; }
   size_t length() const { return fifo_length_; }
   unsigned NumberOfChannels() const { return fifo_bus_->NumberOfChannels(); }
-  AudioBus* Bus() const { return fifo_bus_.Get(); }
+
+  // TODO(hongchan): This is unsafe even for testing. Consider refactoring.
+  AudioBus* GetFIFOBusForTest() const { return fifo_bus_.Get(); }
 
   // For unit test. Get the current configuration that consists of FIFO length,
   // number of channels, read/write index position and under/overflow count.
@@ -78,6 +90,10 @@ class BLINK_PLATFORM_EXPORT PushPullFIFO {
 
   unsigned overflow_count_;
   unsigned underflow_count_;
+
+  // This lock protects: |fifo_bus_|, |frames_available_|, |index_read_| and
+  // |index_write_|.

[o1ka, 2017/04/18 14:14:57]

I've seen it done this way: a mutex member is placed above the variables it
protects, and the comment says "protects the variables below". Might be
convenient here.

[hongchan, 2017/04/18 18:16:09]

Done.

+  mutable Mutex lock_;

[o1ka, 2017/04/18 14:14:57]

No need for mutable any more?

[hongchan, 2017/04/18 18:16:09]

Done.

 };
 
 }  // namespace blink