Fix yet another AudioOutputDevice crash.

Bug is identical to the last :/. No idea why it's much more frequent with mojo,
possibly a race is likely to happen when destroying a frame: |callback_| is
destroyed due to frame being destroyed, and authorization is failed for the
same reason. With the per process message filter, authorization wouldn't fail
due to the frame being destructed.

Bug: 819277
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: Ic17dc96d2c83a08732b43094bac2f2c3fa7035ad
Reviewed-on: https://chromium-review.googlesource.com/951774
Commit-Queue: Max Morin <maxmorin@chromium.org>
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#541801}

media/audio/audio_output_device.cc

@@ -217,11 +217,7 @@ void AudioOutputDevice::CreateStreamOnIOThread() {
     case IPC_CLOSED:
       // We must make sure to not access |callback_| in case Stop() has already
       // been called.
-      {
-        base::AutoLock auto_lock_(audio_thread_lock_);
-        if (!stopping_hack_)
-          callback_->OnRenderError();
-      }
+      NotifyRenderCallbackOfError();
       break;
 
     case IDLE:
@@ -330,11 +326,7 @@ void AudioOutputDevice::OnError() {
   // TODO(tommi): Add an explicit contract for clearing the callback
   // object.  Possibly require calling Initialize again or provide
   // a callback object via Start() and clear it in Stop().
-  {
-    base::AutoLock auto_lock_(audio_thread_lock_);
-    if (audio_thread_)
-      callback_->OnRenderError();
-  }
+  NotifyRenderCallbackOfError();
 }
 
 void AudioOutputDevice::OnDeviceAuthorized(
@@ -407,8 +399,8 @@ void AudioOutputDevice::OnDeviceAuthorized(
     // indefinitely after this method returns.
     ipc_->CloseStream();
     OnIPCClosed();
-    if (callback_)
-      callback_->OnRenderError();
+
+    NotifyRenderCallbackOfError();
   }
 }
 
@@ -473,6 +465,19 @@ void AudioOutputDevice::OnIPCClosed() {
   did_receive_auth_.Signal();
 }
 
+void AudioOutputDevice::NotifyRenderCallbackOfError() {
+  TRACE_EVENT0("audio", "AudioOutputDevice::NotifyRenderCallbackOfError");
+  DCHECK(task_runner()->BelongsToCurrentThread());
+
+  {
+    base::AutoLock auto_lock(audio_thread_lock_);
+    // Avoid signaling error if Initialize() hasn't been called yet, or if
+    // Stop() has already been called.
+    if (callback_ && !stopping_hack_)
+      callback_->OnRenderError();
+  }
+}
+
 void AudioOutputDevice::WillDestroyCurrentMessageLoop() {
   LOG(ERROR) << "IO loop going away before the audio device has been stopped";
   ShutDownOnIOThread();

media/audio/audio_output_device.h

@@ -160,6 +160,8 @@ class MEDIA_EXPORT AudioOutputDevice : public AudioRendererSink,
       const std::string& matched_device_id,
       bool timed_out);
 
+  void NotifyRenderCallbackOfError();
+
   // base::MessageLoop::DestructionObserver implementation for the IO loop.
   // If the IO loop dies before we do, we shut down the audio thread from here.
   void WillDestroyCurrentMessageLoop() override;

media/audio/audio_output_device_unittest.cc

@@ -392,6 +392,31 @@ TEST_F(AudioOutputDeviceTest,
   StopAudioDevice();
 }
 
+TEST_F(AudioOutputDeviceTest, AuthorizationFailsBeforeInitialize_NoError) {
+  // Clear audio device set by fixture.
+  StopAudioDevice();
+  audio_output_ipc_ = new MockAudioOutputIPC();
+  audio_device_ = new AudioOutputDevice(
+      base::WrapUnique(audio_output_ipc_), io_loop_.task_runner(), 0,
+      kDefaultDeviceId, url::Origin(),
+      base::TimeDelta::FromMilliseconds(kAuthTimeoutForTestingMs));
+  EXPECT_CALL(
+      *audio_output_ipc_,
+      RequestDeviceAuthorization(audio_device_.get(), 0, kDefaultDeviceId, _));
+
+  audio_device_->RequestDeviceAuthorization();
+  audio_device_->Initialize(default_audio_parameters_, &callback_);
+  base::RunLoop().RunUntilIdle();
+  audio_device_->Stop();
+
+  // We've stopped, so accessing |callback_| isn't ok.
+  EXPECT_CALL(callback_, OnRenderError()).Times(0);
+  audio_device_->OnDeviceAuthorized(OUTPUT_DEVICE_STATUS_ERROR_NOT_AUTHORIZED,
+                                    default_audio_parameters_,
+                                    kDefaultDeviceId);
+  base::RunLoop().RunUntilIdle();
+}
+
 TEST_F(AudioOutputDeviceTest, AuthorizationTimedOut) {
   base::Thread thread("DeviceInfo");
   thread.Start();