Associate audio streams with their source/destination RenderView.

media/audio/audio_output_device_unittest.cc

Index: media/audio/audio_output_device_unittest.cc
diff --git a/media/audio/audio_output_device_unittest.cc b/media/audio/audio_output_device_unittest.cc
index 3952cf38d128f304beba5d0bfc4590f2f0719aa0..06aa95e374d9e1770f5f5d5b416aab1f2882d2bd 100644
--- a/media/audio/audio_output_device_unittest.cc
+++ b/media/audio/audio_output_device_unittest.cc
@@ -100,14 +100,23 @@ class AudioOutputDeviceTest
       : default_audio_parameters_(AudioParameters::AUDIO_PCM_LINEAR,
                                   CHANNEL_LAYOUT_STEREO,
                                   48000, 16, 1024),
-        audio_device_(new AudioOutputDevice(
-            &audio_output_ipc_, io_loop_.message_loop_proxy())),
         synchronized_io_(GetParam()),
         input_channels_(synchronized_io_ ? 2 : 0) {
   }
 
   ~AudioOutputDeviceTest() {}
 
+  virtual void SetUp() OVERRIDE {

[scherkus (not reviewing), 2012/11/28 21:55:01]

this is a pet peeve of mine... if your test fixture class is already doing work
in the ctor/dtor, there's no reason to use SetUp/TearDown()

https://code.google.com/p/googletest/wiki/FAQ#Should_I_use_the_constructor/de...

[miu, 2012/11/28 22:20:54]

History: Each TEST_F() used to call Construct(), Initialize() and Destruct(). 
Dale suggested I remove the "copy and paste" and put these calls in
SetUp()/TearDown().

Now I'm thinking I should just put things back.  The main reason is that, if the
argument has shifted into "should this be in the ctor or a SetUp() method?" I
think we've gone entirely in the wrong direction.  Keep in mind that the
Construct(), etc. methods are actually *testing* something (i.e., using
EXPECT_CALL() on a mock object and so forth).

So, what do you think?  Put things back?  There may have been some "copy and
paste," but I'd argue things were still clean and simple.

[DaleCurtis, 2012/11/28 22:34:39]

I think you should just get rid of construct/destruct and roll them into the
constructor and destructor as andrew suggests. There's nothing wrong with doing
test work in those places.

[scherkus (not reviewing), 2012/11/28 22:37:32]

We're splitting hairs here, but I'm OK having test code in ctor/dtor + inlining
Construct+Initialize into the ctor and Destruct into the dtor.

It's one less thing to think about when reading existing tests / writing new
tests.

+    Construct();
+    Initialize();
+  }
+
+  virtual void TearDown() OVERRIDE {
+    Destruct();
+  }
+
+  void Construct();
+  void Destruct();
   void Initialize();
   void StartAudioDevice();
   void CreateStream();
@@ -157,6 +166,20 @@ int AudioOutputDeviceTest::CalculateMemorySize() {
   return TotalSharedMemorySizeInBytes(io_buffer_size);
 }
 
+void AudioOutputDeviceTest::Construct() {
+  EXPECT_CALL(audio_output_ipc_, AddDelegate(_))
+      .WillOnce(Return(kStreamId));
+
+  audio_device_ = new AudioOutputDevice(
+      &audio_output_ipc_, io_loop_.message_loop_proxy());
+}
+
+void AudioOutputDeviceTest::Destruct() {
+  EXPECT_CALL(audio_output_ipc_, RemoveDelegate(kStreamId));
+
+  audio_device_ = NULL;
+}
+
 void AudioOutputDeviceTest::Initialize() {
   if (synchronized_io_) {
     audio_device_->InitializeIO(default_audio_parameters_,
@@ -172,8 +195,6 @@ void AudioOutputDeviceTest::Initialize() {
 void AudioOutputDeviceTest::StartAudioDevice() {
   audio_device_->Start();
 
-  EXPECT_CALL(audio_output_ipc_, AddDelegate(audio_device_.get()))
-      .WillOnce(Return(kStreamId));
   EXPECT_CALL(audio_output_ipc_, CreateStream(kStreamId, _, _));
 
   io_loop_.RunUntilIdle();
@@ -245,26 +266,24 @@ void AudioOutputDeviceTest::StopAudioDevice() {
   audio_device_->Stop();
 
   EXPECT_CALL(audio_output_ipc_, CloseStream(kStreamId));
-  EXPECT_CALL(audio_output_ipc_, RemoveDelegate(kStreamId));
 
   io_loop_.RunUntilIdle();
 }
 
 TEST_P(AudioOutputDeviceTest, Initialize) {
-  Initialize();
+  // Tests that the object can be constructed, initialized and destructed
+  // without having ever been started/stopped.
 }
 
 // Calls Start() followed by an immediate Stop() and check for the basic message
 // filter messages being sent in that case.
 TEST_P(AudioOutputDeviceTest, StartStop) {
-  Initialize();
   StartAudioDevice();
   StopAudioDevice();
 }
 
 // AudioOutputDevice supports multiple start/stop sequences.
 TEST_P(AudioOutputDeviceTest, StartStopStartStop) {
-  Initialize();
   StartAudioDevice();
   StopAudioDevice();
   StartAudioDevice();
@@ -274,7 +293,6 @@ TEST_P(AudioOutputDeviceTest, StartStopStartStop) {
 // Simulate receiving OnStreamCreated() prior to processing ShutDownOnIOThread()
 // on the IO loop.
 TEST_P(AudioOutputDeviceTest, StopBeforeRender) {
-  Initialize();
   StartAudioDevice();
 
   // Call Stop() but don't run the IO loop yet.
@@ -283,13 +301,11 @@ TEST_P(AudioOutputDeviceTest, StopBeforeRender) {
   // Expect us to shutdown IPC but not to render anything despite the stream
   // getting created.
   EXPECT_CALL(audio_output_ipc_, CloseStream(kStreamId));
-  EXPECT_CALL(audio_output_ipc_, RemoveDelegate(kStreamId));
   CreateStream();
 }
 
 // Full test with output only.
 TEST_P(AudioOutputDeviceTest, CreateStream) {
-  Initialize();
   StartAudioDevice();
   ExpectRenderCallback();
   CreateStream();