More AudioMessageFilter cleanup. Fix crash.

Removes the |lock_| used by AudioMessageFilter for adding and removing
delegates in favor of ensuring those calls happen on the IO thread.

Also, currently message filters are expected to take care of cleaning
up delegates when the IPC closes unexpectedly. I broke this in my last
patch set, http://crrev.com/184478

BUG=178499
TEST=|delegates_| is empty after OnChannelClosing() completes. |stream_id_|
is correctly associated on various audio pathways.

Review URL: https://codereview.chromium.org/12342029

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@185073 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 7 insertions, 9 deletions

diff --git a/media/audio/audio_output_device.cc b/media/audio/audio_output_device.cc
index e95d21b..fd62dc11f 100644
--- a/media/audio/audio_output_device.cc
+++ b/media/audio/audio_output_device.cc
@@ -45,11 +45,11 @@ AudioOutputDevice::AudioOutputDevice(
     : ScopedLoopObserver(io_loop),
       callback_(NULL),
       ipc_(ipc),
+      stream_id_(0),
       state_(IDLE),
       play_on_start_(true),
       stopping_hack_(false) {
   CHECK(ipc_);
-  stream_id_ = ipc_->AddDelegate(this);
 }
 
 void AudioOutputDevice::Initialize(const AudioParameters& params,
@@ -64,9 +64,6 @@ AudioOutputDevice::~AudioOutputDevice() {
   // The current design requires that the user calls Stop() before deleting
   // this class.
   DCHECK(audio_thread_.IsStopped());
-
-  if (ipc_)
-    ipc_->RemoveDelegate(stream_id_);
 }
 
 void AudioOutputDevice::Start() {
@@ -112,6 +109,7 @@ bool AudioOutputDevice::SetVolume(double volume) {
 void AudioOutputDevice::CreateStreamOnIOThread(const AudioParameters& params) {
   DCHECK(message_loop()->BelongsToCurrentThread());
   if (state_ == IDLE) {
+    stream_id_ = ipc_->AddDelegate(this);
     state_ = CREATING_STREAM;
     ipc_->CreateStream(stream_id_, params);
   }
@@ -148,7 +146,9 @@ void AudioOutputDevice::ShutDownOnIOThread() {
   // Make sure we don't call shutdown more than once.
   if (state_ >= CREATING_STREAM) {
     ipc_->CloseStream(stream_id_);
+    ipc_->RemoveDelegate(stream_id_);
     state_ = IDLE;
+    stream_id_ = 0;
   }
 
   // We can run into an issue where ShutDownOnIOThread is called right after
diff --git a/media/audio/audio_output_device_unittest.cc b/media/audio/audio_output_device_unittest.cc
index 07a752d..5628f66 100644
--- a/media/audio/audio_output_device_unittest.cc
+++ b/media/audio/audio_output_device_unittest.cc
@@ -155,9 +155,6 @@ AudioOutputDeviceTest::AudioOutputDeviceTest()
       CHANNEL_LAYOUT_STEREO, input_channels_,
       48000, 16, 1024);
 
-  EXPECT_CALL(audio_output_ipc_, AddDelegate(_))
-      .WillOnce(Return(kStreamId));
-
   audio_device_ = new AudioOutputDevice(
       &audio_output_ipc_, io_loop_.message_loop_proxy());
 
@@ -168,14 +165,13 @@ AudioOutputDeviceTest::AudioOutputDeviceTest()
 }
 
 AudioOutputDeviceTest::~AudioOutputDeviceTest() {
-  EXPECT_CALL(audio_output_ipc_, RemoveDelegate(kStreamId));
-
   audio_device_ = NULL;
 }
 
 void AudioOutputDeviceTest::StartAudioDevice() {
   audio_device_->Start();
 
+  EXPECT_CALL(audio_output_ipc_, AddDelegate(_)).WillOnce(Return(kStreamId));
   EXPECT_CALL(audio_output_ipc_, CreateStream(kStreamId, _));
 
   io_loop_.RunUntilIdle();
@@ -247,6 +243,7 @@ void AudioOutputDeviceTest::StopAudioDevice() {
   audio_device_->Stop();
 
   EXPECT_CALL(audio_output_ipc_, CloseStream(kStreamId));
+  EXPECT_CALL(audio_output_ipc_, RemoveDelegate(kStreamId));
 
   io_loop_.RunUntilIdle();
 }
@@ -282,6 +279,7 @@ 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();
 }