Clean-up and bug fixes in AudioOutputController:

media/audio/audio_output_controller.h

Index: media/audio/audio_output_controller.h
diff --git a/media/audio/audio_output_controller.h b/media/audio/audio_output_controller.h
index 21f8efe7fe038db1c7228aa1be362d040cd68473..c4dfe1c32993a32858871be8f377f04cd56d8eff 100644
--- a/media/audio/audio_output_controller.h
+++ b/media/audio/audio_output_controller.h
@@ -5,10 +5,10 @@
 #ifndef MEDIA_AUDIO_AUDIO_OUTPUT_CONTROLLER_H_
 #define MEDIA_AUDIO_AUDIO_OUTPUT_CONTROLLER_H_
 
+#include "base/atomic_ref_count.h"
 #include "base/callback.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/weak_ptr.h"
-#include "base/synchronization/lock.h"
 #include "media/audio/audio_buffers_state.h"
 #include "media/audio/audio_io.h"
 #include "media/audio/audio_manager.h"
@@ -16,10 +16,6 @@
 #include "media/audio/simple_sources.h"
 #include "media/base/media_export.h"
 
-namespace base {
-class WaitableEvent;
-}  // namespace base
-
 class MessageLoop;
 
 // An AudioOutputController controls an AudioOutputStream and provides data
@@ -30,29 +26,26 @@ class MessageLoop;
 // All the public methods of AudioOutputController are non-blocking.
 // The actual operations are performed on the audio manager thread.
 //
-// Here is a state diagram for the AudioOutputController:
+// Here is a state transition diagram for the AudioOutputController:
 //
-//             .----------------------->  [ Closed / Error ]  <------.
-//             |                                   ^                 |
-//             |                                   |                 |
-//        [ Created ]  -->  [ Starting ]  -->  [ Playing ]  -->  [ Paused ]
-//             ^                 |                 ^                |  ^
-//             |                 |                 |                |  |
-//             |                 |                 `----------------'  |
-//             |                 V                                     |
-//             |        [ PausedWhenStarting ] ------------------------'
-//             |
-//       *[  Empty  ]
+//   *[ Empty ]  -->  [ Created ]  -->  [ Starting ]  -->  [ Playing ]  --.
+//        |                |               |    ^               |         |
+//        |                |               |    |               |         |
+//        |                |               |    |               v         |
+//        |                |               |    `---------  [ Paused ]    |
+//        |                |               |                    |         |
+//        |                v               v                    |         |
+//        `----------->  [      Closed       ]  <-------------------------'
 //
 // * Initial state
 //
-// At any time after reaching the Created state but before Closed / Error, the
-// AudioOutputController may be notified of a device change via OnDeviceChange()
-// and transition to the Recreating state.  If OnDeviceChange() completes
-// successfully the state will transition back to an equivalent pre-call state.
-// E.g., if the state was Paused or PausedWhenStarting, the new state will be
-// Created, since these states are all functionally equivalent and require a
-// Play() call to continue to the next state.
+// At any time after reaching the Created state but before Closed, the
+// AudioOutputController may be notified of a device change via
+// OnDeviceChange().  As the OnDeviceChange() is processed, state transitions
+// will occur, ultimately ending up in an equivalent pre-call state.  E.g., if
+// the state was Paused, the new state will be Created, since these states are
+// all functionally equivalent and require a Play() call to continue to the next
+// state.
 //
 // The AudioOutputStream can request data from the AudioOutputController via the
 // AudioSourceCallback interface. AudioOutputController uses the SyncReader
@@ -166,13 +159,11 @@ class MEDIA_EXPORT AudioOutputController
   enum State {
     kEmpty,
     kCreated,
-    kPlaying,
     kStarting,
-    kPausedWhenStarting,
+    kPlaying,
     kPaused,
     kClosed,
     kError,
-    kRecreating,
   };
 
   friend class base::RefCountedThreadSafe<AudioOutputController>;
@@ -187,7 +178,7 @@ class MEDIA_EXPORT AudioOutputController
                         const AudioParameters& params, SyncReader* sync_reader);
 
   // The following methods are executed on the audio manager thread.
-  void DoCreate();
+  void DoCreate(bool is_for_device_change);
   void DoPlay();
   void PollAndStartIfDataReady();
   void DoPause();
@@ -198,18 +189,21 @@ class MEDIA_EXPORT AudioOutputController
   void DoStartDiverting(AudioOutputStream* to_stream);
   void DoStopDiverting();
 
-  // Helper method that starts physical stream.
+  // Helper methods that start/stop physical stream.
   void StartStream();
+  void StopStream();
 
   // Helper method that stops, closes, and NULLs |*stream_|.
-  // Signals event when done if it is not NULL.
-  void DoStopCloseAndClearStream(base::WaitableEvent *done);
+  void DoStopCloseAndClearStream();
+
+  // Sanity-check that entry/exit to OnMoreIOData() by the hardware audio thread
+  // happens only between AudioOutputStream::Start() and Stop().
+  void AllowEntryToOnMoreIOData();
+  void DisallowEntryToOnMoreIOData();
 
   AudioManager* const audio_manager_;
   const AudioParameters params_;
-
-  // |handler_| may be called only if |state_| is not kClosed.
-  EventHandler* handler_;
+  EventHandler* const handler_;
 
   // Note: It's important to invalidate the weak pointers whenever stream_ is
   // changed.  See comment for weak_this_.
@@ -226,15 +220,18 @@ class MEDIA_EXPORT AudioOutputController
   // is not required for reading on the audio manager thread.
   State state_;
 
-  // The |lock_| must be acquired whenever we access |state_| from a thread
-  // other than the audio manager thread.
-  base::Lock lock_;
+  // Binary semaphore, used to ensure that only one thread enters the
+  // OnMoreIOData() method, and only when it is valid to do so.  This is for
+  // sanity-checking the behavior of platform implementations of
+  // AudioOutputStream.  In other words, multiple contention is not expected,
+  // nor in the design here.
+  base::AtomicRefCount num_allowed_io_;
 
   // SyncReader is used only in low latency mode for synchronous reading.
-  SyncReader* sync_reader_;
+  SyncReader* const sync_reader_;
 
   // The message loop of audio manager thread that this object runs on.
-  scoped_refptr<base::MessageLoopProxy> message_loop_;
+  const scoped_refptr<base::MessageLoopProxy> message_loop_;
 
   // When starting stream we wait for data to become available.
   // Number of times left.