summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorchcunningham <chcunningham@chromium.org>2016-03-04 15:22:01 -0800
committerCommit bot <commit-bot@chromium.org>2016-03-04 23:23:54 +0000
commit6b206d7b4963be6e8e7ff269dcf0015eb7d8bffa (patch)
treeea7cfc69d2a9172ddd43cfbff9131061e3deeda5
parentb5421b5647a0627226b8b2f48e6143b9d0cf3647 (diff)
downloadchromium_src-6b206d7b4963be6e8e7ff269dcf0015eb7d8bffa.zip
chromium_src-6b206d7b4963be6e8e7ff269dcf0015eb7d8bffa.tar.gz
chromium_src-6b206d7b4963be6e8e7ff269dcf0015eb7d8bffa.tar.bz2
Revert of Add lock to fix race in AudioRendererMixerInput. (patchset #5 id:80001 of https://codereview.chromium.org/1748183006/ )
Reason for revert: This seems to cause failures in LayoutTests/http/tests/security/media. Not sure why this wasn't caught by CQ... Tracking CQ weirdness here: https://bugs.chromium.org/p/chromium/issues/detail?id=592079 Original issue's description: > Add lock to fix race in AudioRendererMixerInput. > > Clusterfuzz identified a race between the media thread calling SetVolume > and the audio device thread calling ProvideInput. Add a lock to > synchronize access to |volume_| between threads. > > Also adds some thread_checkers to just firm up the contract that the > majority of these methods are called only by the media thread. > > BUG=588992 > > Committed: https://crrev.com/57c6958a99f2c62a4408d2a9d262a22129309106 > Cr-Commit-Position: refs/heads/master@{#379349} TBR=dalecurtis@chromium.org,olka@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=588992 Review URL: https://codereview.chromium.org/1771463002 Cr-Commit-Position: refs/heads/master@{#379388}
-rw-r--r--media/base/audio_renderer_mixer_input.cc35
-rw-r--r--media/base/audio_renderer_mixer_input.h10
2 files changed, 2 insertions, 43 deletions
diff --git a/media/base/audio_renderer_mixer_input.cc b/media/base/audio_renderer_mixer_input.cc
index f990538..5743700 100644
--- a/media/base/audio_renderer_mixer_input.cc
+++ b/media/base/audio_renderer_mixer_input.cc
@@ -29,22 +29,15 @@ AudioRendererMixerInput::AudioRendererMixerInput(
mixer_(nullptr),
callback_(nullptr),
error_cb_(base::Bind(&AudioRendererMixerInput::OnRenderError,
- base::Unretained(this))) {
- // Can be constructed on any thread, but sink operations should all occur
- // on same thread.
- thread_checker_.DetachFromThread();
-}
+ base::Unretained(this))) {}
AudioRendererMixerInput::~AudioRendererMixerInput() {
- DCHECK(!started_);
DCHECK(!mixer_);
}
void AudioRendererMixerInput::Initialize(
const AudioParameters& params,
AudioRendererSink::RenderCallback* callback) {
- DCHECK(thread_checker_.CalledOnValidThread());
- DCHECK(!started_);
DCHECK(!mixer_);
DCHECK(callback);
@@ -53,7 +46,6 @@ void AudioRendererMixerInput::Initialize(
}
void AudioRendererMixerInput::Start() {
- DCHECK(thread_checker_.CalledOnValidThread());
DCHECK(!started_);
DCHECK(!mixer_);
DCHECK(callback_); // Initialized.
@@ -76,8 +68,6 @@ void AudioRendererMixerInput::Start() {
}
void AudioRendererMixerInput::Stop() {
- DCHECK(thread_checker_.CalledOnValidThread());
-
// Stop() may be called at any time, if Pause() hasn't been called we need to
// remove our mixer input before shutdown.
Pause();
@@ -100,8 +90,6 @@ void AudioRendererMixerInput::Stop() {
}
void AudioRendererMixerInput::Play() {
- DCHECK(thread_checker_.CalledOnValidThread());
-
if (playing_ || !mixer_)
return;
@@ -110,8 +98,6 @@ void AudioRendererMixerInput::Play() {
}
void AudioRendererMixerInput::Pause() {
- DCHECK(thread_checker_.CalledOnValidThread());
-
if (!playing_ || !mixer_)
return;
@@ -120,8 +106,6 @@ void AudioRendererMixerInput::Pause() {
}
bool AudioRendererMixerInput::SetVolume(double volume) {
- DCHECK(thread_checker_.CalledOnValidThread());
- base::AutoLock auto_lock(volume_lock_);
volume_ = volume;
return true;
}
@@ -134,8 +118,6 @@ void AudioRendererMixerInput::SwitchOutputDevice(
const std::string& device_id,
const url::Origin& security_origin,
const SwitchOutputDeviceCB& callback) {
- DCHECK(thread_checker_.CalledOnValidThread());
-
if (!mixer_) {
if (pending_switch_callback_.is_null()) {
pending_switch_callback_ = callback;
@@ -177,16 +159,12 @@ void AudioRendererMixerInput::SwitchOutputDevice(
}
AudioParameters AudioRendererMixerInput::GetOutputParameters() {
- DCHECK(thread_checker_.CalledOnValidThread());
-
if (mixer_)
return mixer_->GetOutputDevice()->GetOutputParameters();
return get_hardware_params_cb_.Run(device_id_, security_origin_);
}
OutputDeviceStatus AudioRendererMixerInput::GetDeviceStatus() {
- DCHECK(thread_checker_.CalledOnValidThread());
-
if (mixer_)
return mixer_->GetOutputDevice()->GetDeviceStatus();
@@ -198,11 +176,6 @@ OutputDeviceStatus AudioRendererMixerInput::GetDeviceStatus() {
double AudioRendererMixerInput::ProvideInput(AudioBus* audio_bus,
base::TimeDelta buffer_delay) {
- // No thread checker here. This method is called on a different thread as part
- // of audio rendering. AudioRendererMixer has locks that protect us from
- // things like attempting to ProvideInput while simultaneously removing
- // ourselves from mixer inputs (see Pause()).
-
// TODO(chcunningham): Delete this conversion and change ProvideInput to more
// precisely describe delay as a count of frames delayed instead of TimeDelta.
// See http://crbug.com/587522.
@@ -217,11 +190,7 @@ double AudioRendererMixerInput::ProvideInput(AudioBus* audio_bus,
frames_filled, audio_bus->frames() - frames_filled);
}
- // Synchronize access to |volume_| with SetVolume().
- {
- base::AutoLock auto_lock(volume_lock_);
- return frames_filled > 0 ? volume_ : 0;
- }
+ return frames_filled > 0 ? volume_ : 0;
}
void AudioRendererMixerInput::OnRenderError() {
diff --git a/media/base/audio_renderer_mixer_input.h b/media/base/audio_renderer_mixer_input.h
index 6ae5a4bc..34f37c5 100644
--- a/media/base/audio_renderer_mixer_input.h
+++ b/media/base/audio_renderer_mixer_input.h
@@ -9,8 +9,6 @@
#include "base/callback.h"
#include "base/macros.h"
-#include "base/synchronization/lock.h"
-#include "base/threading/thread_checker.h"
#include "media/base/audio_converter.h"
#include "media/base/audio_renderer_sink.h"
#include "media/base/output_device.h"
@@ -71,14 +69,6 @@ class MEDIA_EXPORT AudioRendererMixerInput
private:
friend class AudioRendererMixerInputTest;
- // Used to DCHECK that control methods (Start/Stop/Switch...) are called from
- // the same thread.
- base::ThreadChecker thread_checker_;
-
- // Protect |volume_|, accessed by separate threads in ProvideInput() and
- // SetVolume().
- base::Lock volume_lock_;
-
bool started_;
bool playing_;
double volume_;