summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rwxr-xr-xchrome/chrome.gyp1
-rw-r--r--chrome/renderer/audio_message_filter.h3
-rw-r--r--chrome/renderer/media/audio_renderer_impl.cc27
-rw-r--r--chrome/renderer/media/audio_renderer_impl.h12
-rw-r--r--chrome/renderer/media/audio_renderer_impl_unittest.cc129
-rw-r--r--media/filters/audio_renderer_base.cc7
6 files changed, 177 insertions, 2 deletions
diff --git a/chrome/chrome.gyp b/chrome/chrome.gyp
index 0f7c302..c81b650 100755
--- a/chrome/chrome.gyp
+++ b/chrome/chrome.gyp
@@ -4715,6 +4715,7 @@
'common/worker_thread_ticker_unittest.cc',
'common/zip_unittest.cc',
'renderer/audio_message_filter_unittest.cc',
+ 'renderer/media/audio_renderer_impl_unittest.cc',
'renderer/extensions/extension_api_client_unittest.cc',
'renderer/extensions/json_schema_unittest.cc',
'renderer/net/render_dns_master_unittest.cc',
diff --git a/chrome/renderer/audio_message_filter.h b/chrome/renderer/audio_message_filter.h
index 7516a68..9a21046 100644
--- a/chrome/renderer/audio_message_filter.h
+++ b/chrome/renderer/audio_message_filter.h
@@ -49,6 +49,9 @@ class AudioMessageFilter : public IPC::ChannelProxy::MessageFilter {
MessageLoop* message_loop() { return message_loop_; }
private:
+ // For access to |message_loop_|.
+ friend class AudioRendererImplTest;
+
FRIEND_TEST(AudioMessageFilterTest, Basic);
FRIEND_TEST(AudioMessageFilterTest, Delegates);
diff --git a/chrome/renderer/media/audio_renderer_impl.cc b/chrome/renderer/media/audio_renderer_impl.cc
index dfca1be..f10ff37 100644
--- a/chrome/renderer/media/audio_renderer_impl.cc
+++ b/chrome/renderer/media/audio_renderer_impl.cc
@@ -95,6 +95,8 @@ void AudioRendererImpl::OnStop() {
return;
stopped_ = true;
+ // We should never touch |io_loop_| after being stopped, so post our final
+ // task to clean up.
io_loop_->PostTask(FROM_HERE,
NewRunnableMethod(this, &AudioRendererImpl::OnDestroy));
}
@@ -117,10 +119,16 @@ void AudioRendererImpl::OnReadComplete(media::Buffer* buffer_in) {
void AudioRendererImpl::SetPlaybackRate(float rate) {
DCHECK(rate >= 0.0f);
+ AutoLock auto_lock(lock_);
+ // Handle the case where we stopped due to |io_loop_| dying.
+ if (stopped_) {
+ AudioRendererBase::SetPlaybackRate(rate);
+ return;
+ }
+
// We have two cases here:
// Play: GetPlaybackRate() == 0.0 && rate != 0.0
// Pause: GetPlaybackRate() != 0.0 && rate == 0.0
- AutoLock auto_lock(lock_);
if (GetPlaybackRate() == 0.0f && rate != 0.0f) {
// Play is a bit tricky, we can only play if we have done prerolling.
// TODO(hclam): I should check for end of streams status here.
@@ -227,6 +235,7 @@ void AudioRendererImpl::OnCreateStream(
// Make sure we don't call create more than once.
DCHECK_EQ(0, stream_id_);
stream_id_ = filter_->AddDelegate(this);
+ io_loop_->AddDestructionObserver(this);
ViewHostMsg_Audio_CreateStream params;
params.format = format;
@@ -254,8 +263,24 @@ void AudioRendererImpl::OnPause() {
void AudioRendererImpl::OnDestroy() {
DCHECK(MessageLoop::current() == io_loop_);
+ // Make sure we don't call destroy more than once.
+ DCHECK_NE(0, stream_id_);
filter_->RemoveDelegate(stream_id_);
filter_->Send(new ViewHostMsg_CloseAudioStream(0, stream_id_));
+ io_loop_->RemoveDestructionObserver(this);
+ stream_id_ = 0;
+}
+
+void AudioRendererImpl::WillDestroyCurrentMessageLoop() {
+ DCHECK(MessageLoop::current() == io_loop_);
+
+ // We treat the IO loop going away the same as stopping.
+ AutoLock auto_lock(lock_);
+ if (stopped_)
+ return;
+
+ stopped_ = true;
+ OnDestroy();
}
void AudioRendererImpl::OnSetVolume(double volume) {
diff --git a/chrome/renderer/media/audio_renderer_impl.h b/chrome/renderer/media/audio_renderer_impl.h
index aec60e7..01885be 100644
--- a/chrome/renderer/media/audio_renderer_impl.h
+++ b/chrome/renderer/media/audio_renderer_impl.h
@@ -100,11 +100,13 @@
#include "media/base/factory.h"
#include "media/base/filters.h"
#include "media/filters/audio_renderer_base.h"
+#include "testing/gtest/include/gtest/gtest_prod.h"
class AudioMessageFilter;
class AudioRendererImpl : public media::AudioRendererBase,
- public AudioMessageFilter::Delegate {
+ public AudioMessageFilter::Delegate,
+ public MessageLoop::DestructionObserver {
public:
// Methods called on render thread ------------------------------------------
// Methods called during construction.
@@ -142,6 +144,11 @@ class AudioRendererImpl : public media::AudioRendererBase,
friend class media::FilterFactoryImpl1<AudioRendererImpl,
AudioMessageFilter*>;
+ // For access to constructor and IO thread methods.
+ friend class AudioRendererImplTest;
+ FRIEND_TEST(AudioRendererImplTest, Stop);
+ FRIEND_TEST(AudioRendererImplTest, DestroyedMessageLoop_OnReadComplete);
+
explicit AudioRendererImpl(AudioMessageFilter* filter);
virtual ~AudioRendererImpl();
@@ -163,6 +170,9 @@ class AudioRendererImpl : public media::AudioRendererBase,
void OnNotifyPacketReady();
void OnDestroy();
+ // Called on IO thread when message loop is dying.
+ virtual void WillDestroyCurrentMessageLoop();
+
// Information about the audio stream.
int channels_;
int sample_rate_;
diff --git a/chrome/renderer/media/audio_renderer_impl_unittest.cc b/chrome/renderer/media/audio_renderer_impl_unittest.cc
new file mode 100644
index 0000000..a3b3420
--- /dev/null
+++ b/chrome/renderer/media/audio_renderer_impl_unittest.cc
@@ -0,0 +1,129 @@
+// Copyright (c) 2009 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 "chrome/common/render_messages.h"
+#include "chrome/renderer/media/audio_renderer_impl.h"
+#include "media/base/data_buffer.h"
+#include "media/base/mock_filter_host.h"
+#include "media/base/mock_filters.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+class AudioRendererImplTest : public ::testing::Test {
+ public:
+ static const int kRouteId = 0;
+ static const int kSize = 1024;
+
+ AudioRendererImplTest() {
+ message_loop_.reset(new MessageLoop(MessageLoop::TYPE_IO));
+
+ // TODO(scherkus): use gmock with AudioMessageFilter to verify
+ // AudioRendererImpl calls or doesn't call Send().
+ filter_ = new AudioMessageFilter(kRouteId);
+ filter_->message_loop_ = message_loop_.get();
+
+ // Create temporary shared memory.
+ CHECK(shared_mem_.Create(L"", false, false, kSize));
+
+ // Setup expectations for initialization.
+ EXPECT_CALL(callback_, OnFilterCallback());
+ EXPECT_CALL(callback_, OnCallbackDestroyed());
+ decoder_ = new media::MockAudioDecoder(2, 48000, 16);
+
+ // Create and initialize audio renderer.
+ renderer_ = new AudioRendererImpl(filter_);
+ renderer_->set_host(&host_);
+ renderer_->set_message_loop(message_loop_.get());
+ renderer_->Initialize(decoder_, callback_.NewCallback());
+
+ // Run pending tasks and simulate responding with a created audio stream.
+ message_loop_->RunAllPending();
+ renderer_->OnCreated(shared_mem_.handle(), kSize);
+ }
+
+ virtual ~AudioRendererImplTest() {
+ }
+
+ protected:
+ // Fixtures.
+ scoped_ptr<MessageLoop> message_loop_;
+ scoped_refptr<AudioMessageFilter> filter_;
+ base::SharedMemory shared_mem_;
+ media::MockFilterHost host_;
+ media::MockFilterCallback callback_;
+ scoped_refptr<media::MockAudioDecoder> decoder_;
+ scoped_refptr<AudioRendererImpl> renderer_;
+
+ private:
+ DISALLOW_COPY_AND_ASSIGN(AudioRendererImplTest);
+};
+
+TEST_F(AudioRendererImplTest, SetPlaybackRate) {
+ // Execute SetPlaybackRate() codepath to create an IPC message.
+
+ // Toggle play/pause to generate some IPC messages.
+ renderer_->SetPlaybackRate(0.0f);
+ renderer_->SetPlaybackRate(1.0f);
+ renderer_->SetPlaybackRate(0.0f);
+
+ message_loop_->RunAllPending();
+ renderer_->Stop();
+}
+
+TEST_F(AudioRendererImplTest, SetVolume) {
+ // Execute SetVolume() codepath to create an IPC message.
+ renderer_->SetVolume(0.5f);
+ message_loop_->RunAllPending();
+ renderer_->Stop();
+}
+
+TEST_F(AudioRendererImplTest, Stop) {
+ // Declare some state messages.
+ const ViewMsg_AudioStreamState kError =
+ { ViewMsg_AudioStreamState::kError };
+ const ViewMsg_AudioStreamState kPlaying =
+ { ViewMsg_AudioStreamState::kPlaying };
+ const ViewMsg_AudioStreamState kPaused =
+ { ViewMsg_AudioStreamState::kPaused };
+
+ // Execute Stop() codepath to create an IPC message.
+ renderer_->Stop();
+ message_loop_->RunAllPending();
+
+ // Run AudioMessageFilter::Delegate methods, which can be executed after being
+ // stopped. AudioRendererImpl shouldn't create any messages.
+ renderer_->OnRequestPacket(kSize, base::Time());
+ renderer_->OnStateChanged(kError);
+ renderer_->OnStateChanged(kPlaying);
+ renderer_->OnStateChanged(kPaused);
+ renderer_->OnCreated(shared_mem_.handle(), kSize);
+ renderer_->OnVolume(0.5);
+
+ // It's possible that the upstream decoder replies right after being stopped.
+ scoped_refptr<media::Buffer> buffer = new media::DataBuffer(kSize);
+ renderer_->OnReadComplete(buffer);
+}
+
+TEST_F(AudioRendererImplTest, DestroyedMessageLoop_SetPlaybackRate) {
+ // Kill the message loop and verify SetPlaybackRate() still works.
+ message_loop_.reset();
+ renderer_->SetPlaybackRate(0.0f);
+ renderer_->SetPlaybackRate(1.0f);
+ renderer_->SetPlaybackRate(0.0f);
+ renderer_->Stop();
+}
+
+TEST_F(AudioRendererImplTest, DestroyedMessageLoop_SetVolume) {
+ // Kill the message loop and verify SetVolume() still works.
+ message_loop_.reset();
+ renderer_->SetVolume(0.5f);
+ renderer_->Stop();
+}
+
+TEST_F(AudioRendererImplTest, DestroyedMessageLoop_OnReadComplete) {
+ // Kill the message loop and verify OnReadComplete() still works.
+ message_loop_.reset();
+ scoped_refptr<media::Buffer> buffer = new media::DataBuffer(kSize);
+ renderer_->OnReadComplete(buffer);
+ renderer_->Stop();
+}
diff --git a/media/filters/audio_renderer_base.cc b/media/filters/audio_renderer_base.cc
index 65829f6..4ec7403 100644
--- a/media/filters/audio_renderer_base.cc
+++ b/media/filters/audio_renderer_base.cc
@@ -131,6 +131,13 @@ void AudioRendererBase::OnReadComplete(Buffer* buffer_in) {
DCHECK_GT(pending_reads_, 0u);
--pending_reads_;
+ // TODO(scherkus): this happens due to a race, primarily because Stop() is a
+ // synchronous call when it should be asynchronous and accept a callback.
+ // Refer to http://crbug.com/16059
+ if (state_ == kStopped) {
+ return;
+ }
+
// Don't enqueue an end-of-stream buffer because it has no data.
if (buffer_in->IsEndOfStream()) {
recieved_end_of_stream_ = true;
generated by cgit v1.1 at 2025-01-16 23:33:37 +0000