refactor AudioInputDevice to remove race condition.

content/renderer/media/audio_input_device.h

Index: content/renderer/media/audio_input_device.h
===================================================================
--- content/renderer/media/audio_input_device.h	(revision 94305)
+++ content/renderer/media/audio_input_device.h	(working copy)
@@ -53,10 +53,9 @@
 #include "base/memory/scoped_ptr.h"
 #include "base/shared_memory.h"
 #include "base/threading/simple_thread.h"
+#include "base/threading/thread.h"
 #include "content/renderer/media/audio_input_message_filter.h"
 
-struct AudioParameters;
-
 // TODO(henrika): This class is based on the AudioDevice class and it has
 // many components in common. Investigate potential for re-factoring.
 class AudioInputDevice
@@ -105,14 +104,15 @@
   virtual void OnVolume(double volume);
 
  private:
-  // Methods called on IO thread ----------------------------------------------
-  // The following methods are tasks posted on the IO thread that needs to
-  // be executed on that thread. They interact with AudioInputMessageFilter and
-  // sends IPC messages on that thread.
-  void InitializeOnIOThread(const AudioParameters& params);
-  void StartOnIOThread();
-  void ShutDownOnIOThread();
-  void SetVolumeOnIOThread(double volume);
+  // Methods called on capture thread-------------------------------------------
+  // The following methods are tasks posted on the capture thread that needs to
+  // be executed on that thread.

[henrika_dont_use, 2011/07/30 15:50:59]

Perhaps mention that this thread is the main working thread.

[wjia(left Chromium), 2011/08/01 22:40:13]

Done.

+  void StartOnCaptureThread();
+  void StartRecordingOnCaptureThread(base::SharedMemoryHandle handle,
+                                     base::SyncSocket::Handle socket_handle,
+                                     uint32 length);
+  void ShutDownOnCaptureThread();
+  void SetVolumeOnCaptureThread(double volume);
 
   void Send(IPC::Message* message);
 
@@ -123,6 +123,10 @@
   // DelegateSimpleThread::Delegate implementation.
   virtual void Run();
 
+  // Add/Remove delegate on IO thread.

[henrika_dont_use, 2011/07/30 15:50:59]

Use same comment style as above, i.e., --------------------?

[wjia(left Chromium), 2011/08/01 22:40:13]

Done.

+  void AddDelegateOnIOThread();
+  void RemoveDelegateOnIOThread(int32 stream_id);
+
   // Format
   size_t buffer_size_;  // in sample-frames
   int channels_;
@@ -142,6 +146,22 @@
   // The current volume scaling [0.0, 1.0] of the audio stream.
   double volume_;
 
+  // The working thread for AudioInputDevice.
+  base::Thread capture_thread_;
+  scoped_refptr<base::MessageLoopProxy> message_loop_proxy_;

[henrika_dont_use, 2011/07/30 15:50:59]

Sorry, but I actually don't understand why a proxy is needed. What is the
advantage? Why not access the thread message loop directly?

[wjia(left Chromium), 2011/08/01 22:40:13]

MessageLoop and MessageLoopProxy are very similar, except that MessageLoopProxy
can outlive the target thread and is a safe way to hold on to message loop
pointers and leads to far less brittle code.

+
+  // These 2 WaitableEvents are added to translate async operation (between

[henrika_dont_use, 2011/07/30 15:50:59]

two

[wjia(left Chromium), 2011/08/01 22:40:13]

Done.

+  // AudioInputDevice and AudioInputMessageFilter) to sync operation (between
+  // AudioInputDevice and ADM). The main reason is ADM can delete

[henrika_dont_use, 2011/07/30 15:50:59]

that the

[xians, 2011/08/01 11:41:23]

AudioInputDevice and client). The main reason is client can ..

[wjia(left Chromium), 2011/08/01 22:40:13]

Done.

+  // AudioInputDevice anytime after Stop() is called. It's highly recommended

[henrika_dont_use, 2011/07/30 15:50:59]

Hmm, we state that events are added to emulate sync operation and then we say
that async is highly recommended. Can this comment/section be more clear? Also,
today Start is async and Stop is sync. Does this section deal with Stop
only?Feels like the complete comment section can be even more clear.

[wjia(left Chromium), 2011/08/01 22:40:13]

The recommendation is to get rid of emulating sync operation. Actually, Start
always returns "true" now, which is not quite right. This is just an
intermediate stage before changing API between AudioInputDevice and ADM.
Eventually, ADM will be an EventHanlder of AudioInputDevice which receives
status update events from AudioInputDevice.

+  // to use async API between ADM and AudioInputDevice.

[xians, 2011/08/01 11:41:23]

client?

[wjia(left Chromium), 2011/08/01 22:40:13]

Done.

+  // WaitableEvent used to synchronize delegate operation since delegate
+  // needs to be added/removed on IO thread.
+  base::WaitableEvent delegate_event_;
+  // WaitableEvent used to synchronize Stop() function call to ensure it's safe
+  // to destruct AudioInputDevice.
+  base::WaitableEvent stop_event_;
+
   // Callbacks for capturing audio occur on this thread.
   scoped_ptr<base::DelegateSimpleThread> audio_thread_;