Associate audio streams with their source/destination RenderView.

media/audio/audio_output_device_unittest.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include <vector>
 
 #include "base/at_exit.h"
 #include "base/message_loop.h"
 #include "base/process_util.h"
 #include "base/shared_memory.h"
@@ skipping 82 matching lines @@
 }  // namespace.
 
 class AudioOutputDeviceTest
     : public testing::Test,
       public testing::WithParamInterface<bool> {
  public:
   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();
   void ExpectRenderCallback();
   void WaitUntilRenderCallback();
   void StopAudioDevice();
 
  protected:
   // Used to clean up TLS pointers that the test(s) will initialize.
   // Must remain the first member of this class.
@@ skipping 29 matching lines @@
 
   int io_buffer_size = output_memory_size + input_memory_size;
 
   // This is where it gets a bit hacky.  The shared memory contract between
   // AudioOutputDevice and its browser side counter part includes a bit more
   // than just the audio data, so we must call TotalSharedMemorySizeInBytes()
   // to get the actual size needed to fit the audio data plus the extra data.
   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_,
                                 input_channels_,
                                 &callback_);
   } else {
     audio_device_->Initialize(default_audio_parameters_,
                               &callback_);
   }
   io_loop_.RunUntilIdle();
 }
 
 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();
 }
 
 void AudioOutputDeviceTest::CreateStream() {
   const int kMemorySize = CalculateMemorySize();
 
   ASSERT_TRUE(shared_memory_.CreateAndMapAnonymous(kMemorySize));
   memset(shared_memory_.memory(), 0xff, kMemorySize);
@@ skipping 51 matching lines @@
   // Don't hang the test if we never get the Render() callback.
   io_loop_.PostDelayedTask(FROM_HERE, MessageLoop::QuitClosure(),
                            TestTimeouts::action_timeout());
   io_loop_.Run();
 }
 
 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();
   StopAudioDevice();
 }
 
 // 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.
   audio_device_->Stop();
 
   // 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();
   WaitUntilRenderCallback();
   StopAudioDevice();
 }
 
 INSTANTIATE_TEST_CASE_P(Render, AudioOutputDeviceTest, Values(false));
 INSTANTIATE_TEST_CASE_P(RenderIO, AudioOutputDeviceTest, Values(true));
 
 }  // namespace media.