Improve and simplify AudioOutputDispatcher.

media/audio/audio_output_dispatcher_impl.h

Index: media/audio/audio_output_dispatcher_impl.h
diff --git a/media/audio/audio_output_dispatcher_impl.h b/media/audio/audio_output_dispatcher_impl.h
index b59f835f9b0a2e85720b10964167eccba7c5f048..6bf7eb1b479964ab40ac3b1abc4799a9a96259c2 100644
--- a/media/audio/audio_output_dispatcher_impl.h
+++ b/media/audio/audio_output_dispatcher_impl.h
@@ -13,12 +13,11 @@
 #ifndef MEDIA_AUDIO_AUDIO_OUTPUT_DISPATCHER_IMPL_H_
 #define MEDIA_AUDIO_AUDIO_OUTPUT_DISPATCHER_IMPL_H_
 
-#include <list>
+#include <deque>
 #include <map>
 
 #include "base/basictypes.h"
 #include "base/memory/ref_counted.h"
-#include "base/memory/weak_ptr.h"
 #include "base/timer/timer.h"
 #include "media/audio/audio_io.h"
 #include "media/audio/audio_manager.h"
@@ -31,8 +30,8 @@ class AudioOutputProxy;
 
 class MEDIA_EXPORT AudioOutputDispatcherImpl : public AudioOutputDispatcher {
  public:
-  // |close_delay_ms| specifies delay after the stream is paused until
-  // the audio device is closed.
+  // |close_delay| specifies delay after the stream is idle until the audio
+  // device is closed.
   AudioOutputDispatcherImpl(AudioManager* audio_manager,
                             const AudioParameters& params,
                             const std::string& output_device_id,
@@ -60,38 +59,25 @@ class MEDIA_EXPORT AudioOutputDispatcherImpl : public AudioOutputDispatcher {
   virtual void Shutdown() OVERRIDE;
 
  private:
-  typedef std::map<AudioOutputProxy*, AudioOutputStream*> AudioStreamMap;
   friend class base::RefCountedThreadSafe<AudioOutputDispatcherImpl>;
   virtual ~AudioOutputDispatcherImpl();
 
-  friend class AudioOutputProxyTest;
-
   // Creates a new physical output stream, opens it and pushes to
   // |idle_streams_|.  Returns false if the stream couldn't be created or
   // opened.
   bool CreateAndOpenStream();
 
-  // A task scheduled by StartStream(). Opens a new stream and puts
-  // it in |idle_streams_|.
-  void OpenTask();
-
-  // Before a stream is reused, it should sit idle for a bit.  This task is
-  // called once that time has elapsed.
-  void StopStreamTask();
+  // Closes all |idle_streams_|.
+  void CloseIdleStreams();
 
-  // Called by |close_timer_|. Closes all pending streams.
-  void ClosePendingStreams();
-
-  base::TimeDelta pause_delay_;
-  size_t paused_proxies_;
-  typedef std::list<AudioOutputStream*> AudioOutputStreamList;
+  size_t idle_proxies_;
+  typedef std::deque<AudioOutputStream*> AudioOutputStreamList;

[scherkus (not reviewing), 2013/10/17 19:19:53]

OOC why the change from std::list to std::deque? I didn't see it mentioned in
the CL description.

[DaleCurtis, 2013/10/17 21:11:55]

We're always popping elements off the back or erasing a couple of elements from
the front (but could also from the back).  At most there will be 50 elements
(due to stream cap), on average probably 1 to 5.  We don't need any of the
iterator safety...

Hmmm, as I wrote this I realized we can just use vector.  We don't even need any
of the fancy deque abilities either.

[scherkus (not reviewing), 2013/10/17 23:01:47]

deque will still have an advantage for exactly the reasons you describe
(appending to end, removing N-1 elements from the front in CloseStream()). it'll
make smaller allocations when push_back()'ing as well as make less moves when
erase()'ing

[DaleCurtis, 2013/10/17 23:09:10]

I changed the CloseStream() call to remove the last N-1 elements, so we are now
only accessing the back of the structure, which obviates any of the deque
advantages.

Otherwise, vector and deque both have amortized constant time push_back() AFAIK,
are you saying this is not the case?

I was also under the impression vector and deque had similar allocation
strategies.  Do you have a source for "it'll
make smaller allocations when push_back()'ing"

[scherkus (not reviewing), 2013/10/17 23:25:05]

the biggest difference between vectors and deques is that unlike vector, deque
doesn't guarantee the underlying memory for the structure is contiguous

deques are typically implemented as a list of vectors. if you push_back() and
capacity needs to be increased, a small vector is allocated and linked to the
rest of the vectors (compared to vector which would double in size and copy all
elements)

same benefit happens when removing from the front: only the first vector in the
list of vectors is modified

http://www.cplusplus.com/reference/deque/deque/ and
http://stackoverflow.com/questions/6292332/what-really-is-a-deque-in-stl are
have good summaries

[DaleCurtis, 2013/10/17 23:33:55]

Thanks for the links.  vector will be fine for <50 elements.

   AudioOutputStreamList idle_streams_;
-  AudioOutputStreamList pausing_streams_;
 
   // Used to post delayed tasks to ourselves that we cancel inside Shutdown().
-  base::WeakPtrFactory<AudioOutputDispatcherImpl> weak_this_;
   base::DelayTimer<AudioOutputDispatcherImpl> close_timer_;
 
+  typedef std::map<AudioOutputProxy*, AudioOutputStream*> AudioStreamMap;
   AudioStreamMap proxy_to_physical_map_;
 
   DISALLOW_COPY_AND_ASSIGN(AudioOutputDispatcherImpl);