Commit b305c700 authored by Max Morin's avatar Max Morin Committed by Commit Bot

Revert "Fix yet another AudioOutputDevice crash."

This reverts commit a7c1c185.

Reason for revert: Clusterfuzz reports...

Original change's description:
> 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}

TBR=dalecurtis@chromium.org,maxmorin@chromium.org

Change-Id: I9bf3d357bd09ebd1fa60b4324d204da44b23b72a
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 819277,820276,820341
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
Reviewed-on: https://chromium-review.googlesource.com/957042Reviewed-by: default avatarMax Morin <maxmorin@chromium.org>
Commit-Queue: Max Morin <maxmorin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542065}
parent 4f3b6063
...@@ -217,7 +217,11 @@ void AudioOutputDevice::CreateStreamOnIOThread() { ...@@ -217,7 +217,11 @@ void AudioOutputDevice::CreateStreamOnIOThread() {
case IPC_CLOSED: case IPC_CLOSED:
// We must make sure to not access |callback_| in case Stop() has already // We must make sure to not access |callback_| in case Stop() has already
// been called. // been called.
NotifyRenderCallbackOfError(); {
base::AutoLock auto_lock_(audio_thread_lock_);
if (!stopping_hack_)
callback_->OnRenderError();
}
break; break;
case IDLE: case IDLE:
...@@ -326,7 +330,11 @@ void AudioOutputDevice::OnError() { ...@@ -326,7 +330,11 @@ void AudioOutputDevice::OnError() {
// TODO(tommi): Add an explicit contract for clearing the callback // TODO(tommi): Add an explicit contract for clearing the callback
// object. Possibly require calling Initialize again or provide // object. Possibly require calling Initialize again or provide
// a callback object via Start() and clear it in Stop(). // a callback object via Start() and clear it in Stop().
NotifyRenderCallbackOfError(); {
base::AutoLock auto_lock_(audio_thread_lock_);
if (audio_thread_)
callback_->OnRenderError();
}
} }
void AudioOutputDevice::OnDeviceAuthorized( void AudioOutputDevice::OnDeviceAuthorized(
...@@ -399,8 +407,8 @@ void AudioOutputDevice::OnDeviceAuthorized( ...@@ -399,8 +407,8 @@ void AudioOutputDevice::OnDeviceAuthorized(
// indefinitely after this method returns. // indefinitely after this method returns.
ipc_->CloseStream(); ipc_->CloseStream();
OnIPCClosed(); OnIPCClosed();
if (callback_)
NotifyRenderCallbackOfError(); callback_->OnRenderError();
} }
} }
...@@ -465,19 +473,6 @@ void AudioOutputDevice::OnIPCClosed() { ...@@ -465,19 +473,6 @@ void AudioOutputDevice::OnIPCClosed() {
did_receive_auth_.Signal(); 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() { void AudioOutputDevice::WillDestroyCurrentMessageLoop() {
LOG(ERROR) << "IO loop going away before the audio device has been stopped"; LOG(ERROR) << "IO loop going away before the audio device has been stopped";
ShutDownOnIOThread(); ShutDownOnIOThread();
......
...@@ -160,8 +160,6 @@ class MEDIA_EXPORT AudioOutputDevice : public AudioRendererSink, ...@@ -160,8 +160,6 @@ class MEDIA_EXPORT AudioOutputDevice : public AudioRendererSink,
const std::string& matched_device_id, const std::string& matched_device_id,
bool timed_out); bool timed_out);
void NotifyRenderCallbackOfError();
// base::MessageLoop::DestructionObserver implementation for the IO loop. // base::MessageLoop::DestructionObserver implementation for the IO loop.
// If the IO loop dies before we do, we shut down the audio thread from here. // If the IO loop dies before we do, we shut down the audio thread from here.
void WillDestroyCurrentMessageLoop() override; void WillDestroyCurrentMessageLoop() override;
......
...@@ -392,31 +392,6 @@ TEST_F(AudioOutputDeviceTest, ...@@ -392,31 +392,6 @@ TEST_F(AudioOutputDeviceTest,
StopAudioDevice(); 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) { TEST_F(AudioOutputDeviceTest, AuthorizationTimedOut) {
base::Thread thread("DeviceInfo"); base::Thread thread("DeviceInfo");
thread.Start(); thread.Start();
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment