Skip to content

Commit

Permalink
Revert of Pass task runners to AudioManager constructor. (patchset ch…
Browse files Browse the repository at this point in the history 
…romium#50 id:980001 of https://codereview.chromium.org/1806313003/ )

Reason for revert:
Broke TSan: https://build.chromium.org/p/chromium.memory.fyi/builders/Linux%20TSan%20Tests/builds/19362

WARNING: ThreadSanitizer: data race (pid=25863)
  Write of size 8 at 0x0000081b8060 by main thread:
    #0 media::AudioManagerDeleter::operator()(media::AudioManager const*) const media/audio/audio_manager.cc:264:20 (content_browsertests+0x000001e77078)
    chromium#1 reset buildtools/third_party/libc++/trunk/include/memory:2735:13 (content_browsertests+0x000004e5f2af)
    chromium#2 ~unique_ptr buildtools/third_party/libc++/trunk/include/memory:2703 (content_browsertests+0x000004e5f2af)
    chromium#3 content::BrowserMainLoop::~BrowserMainLoop() content/browser/browser_main_loop.cc:428 (content_browsertests+0x000004e5f2af)
    chromium#4 content::BrowserMainLoop::~BrowserMainLoop() content/browser/browser_main_loop.cc:424:37 (content_browsertests+0x000004e5f779)
    chromium#5 operator() buildtools/third_party/libc++/trunk/include/memory:2529:13 (content_browsertests+0x000004e68120)
    chromium#6 reset buildtools/third_party/libc++/trunk/include/memory:2735 (content_browsertests+0x000004e68120)
    chromium#7 content::BrowserMainRunnerImpl::Shutdown() content/browser/browser_main_runner.cc:223 (content_browsertests+0x000004e68120)
    chromium#8 ShellBrowserMain(content::MainFunctionParams const&, std::__1::unique_ptr<content::BrowserMainRunner, std::__1::default_delete<content::BrowserMainRunner> > const&) content/shell/browser/shell_browser_main.cc:33:3 (content_browsertests+0x000000c97ce7)
    chromium#9 content::ShellMainDelegate::RunProcess(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, content::MainFunctionParams const&) content/shell/app/shell_main_delegate.cc:288:16 (content_browsertests+0x000000c8bec4)
    chromium#10 content::RunNamedProcessTypeMain(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, content::MainFunctionParams const&, content::ContentMainDelegate*) content/app/content_main_runner.cc:368:25 (content_browsertests+0x000005558812)
    chromium#11 content::ContentMainRunnerImpl::Run() content/app/content_main_runner.cc:742:12 (content_browsertests+0x000005559511)
    chromium#12 content::ContentMain(content::ContentMainParams const&) content/app/content_main.cc:20:15 (content_browsertests+0x00000555797e)
    chromium#13 content::BrowserTestBase::SetUp() content/public/test/browser_test_base.cc:287:3 (content_browsertests+0x000000c55601)
    chromium#14 content::ContentBrowserTest::SetUp() content/public/test/content_browser_test.cc:92:3 (content_browsertests+0x000000c5e8e6)
    chromium#15 SetUp content/browser/renderer_host/render_widget_host_view_browsertest.cc:221:5 (content_browsertests+0x000000739618)
    chromium#16 content::(anonymous namespace)::CompositingRenderWidgetHostViewBrowserTestTabCapture::SetUp() content/browser/renderer_host/render_widget_host_view_browsertest.cc:403 (content_browsertests+0x000000739618)
    chromium#17 HandleExceptionsInMethodIfSupported<testing::Test, void> testing/gtest/src/gtest.cc:2458:12 (content_browsertests+0x000000f3168d)
    chromium#18 testing::Test::Run() testing/gtest/src/gtest.cc:2470 (content_browsertests+0x000000f3168d)
    chromium#19 testing::TestInfo::Run() testing/gtest/src/gtest.cc:2656:5 (content_browsertests+0x000000f32833)
    chromium#20 testing::TestCase::Run() testing/gtest/src/gtest.cc:2774:5 (content_browsertests+0x000000f33118)
    chromium#21 testing::internal::UnitTestImpl::RunAllTests() testing/gtest/src/gtest.cc:4647:11 (content_browsertests+0x000000f3c562)
    chromium#22 HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool> testing/gtest/src/gtest.cc:2458:12 (content_browsertests+0x000000f3bfc6)
    chromium#23 testing::UnitTest::Run() testing/gtest/src/gtest.cc:4255 (content_browsertests+0x000000f3bfc6)
    chromium#24 RUN_ALL_TESTS testing/gtest/include/gtest/gtest.h:2237:10 (content_browsertests+0x000000d209eb)
    chromium#25 base::TestSuite::Run() base/test/test_suite.cc:230 (content_browsertests+0x000000d209eb)
    chromium#26 content::ContentTestLauncherDelegate::RunTestSuite(int, char**) content/test/content_test_launcher.cc:105:12 (content_browsertests+0x000000c677db)
    chromium#27 content::LaunchTests(content::TestLauncherDelegate*, int, int, char**) content/public/test/test_launcher.cc:517:12 (content_browsertests+0x000000cfb2fc)
    chromium#28 main content/test/content_test_launcher.cc:131:10 (content_browsertests+0x000000c67762)

  Previous read of size 8 at 0x0000081b8060 by thread T15:
    #0 media::(anonymous namespace)::AudioManagerHelper::UpdateLastAudioThreadTimeTick() media/audio/audio_manager.cc:179:5 (content_browsertests+0x000001e77986)
    chromium#1 Run<> base/bind_internal.h:181:12 (content_browsertests+0x000001e77df2)
    chromium#2 MakeItSo<media::(anonymous namespace)::AudioManagerHelper *> base/bind_internal.h:321 (content_browsertests+0x000001e77df2)
    chromium#3 base::internal::Invoker<base::IndexSequence<0ul>, base::internal::BindState<base::internal::RunnableAdapter<void (media::(anonymous namespace)::AudioManagerHelper::*)()>, void (media::(anonymous namespace)::AudioManagerHelper*), base::internal::UnretainedWrapper<media::(anonymous namespace)::AudioManagerHelper> >, base::internal::InvokeHelper<false, void, base::internal::RunnableAdapter<void (media::(anonymous namespace)::AudioManagerHelper::*)()> >, void ()>::Run(base::internal::BindStateBase*) base/bind_internal.h:372 (content_browsertests+0x000001e77df2)
    chromium#4 Run base/callback.h:397:12 (content_browsertests+0x00000111aa7d)
    chromium#5 base::debug::TaskAnnotator::RunTask(char const*, base::PendingTask const&) base/debug/task_annotator.cc:51 (content_browsertests+0x00000111aa7d)
    chromium#6 base::MessageLoop::RunTask(base::PendingTask const&) base/message_loop/message_loop.cc:479:3 (content_browsertests+0x0000010958e6)
    chromium#7 base::MessageLoop::DeferOrRunPendingTask(base::PendingTask const&) base/message_loop/message_loop.cc:488:5 (content_browsertests+0x000001095ecd)
    chromium#8 base::MessageLoop::DoDelayedWork(base::TimeTicks*) base/message_loop/message_loop.cc:638:10 (content_browsertests+0x00000109662a)
    chromium#9 base::MessagePumpDefault::Run(base::MessagePump::Delegate*) base/message_loop/message_pump_default.cc:37:17 (content_browsertests+0x000001099875)
    chromium#10 base::MessageLoop::RunHandler() base/message_loop/message_loop.cc:443:3 (content_browsertests+0x00000109528b)
    chromium#11 base::RunLoop::Run() base/run_loop.cc:35:3 (content_browsertests+0x0000010b5cc6)
    chromium#12 base::MessageLoop::Run() base/message_loop/message_loop.cc:295:3 (content_browsertests+0x000001094a95)
    chromium#13 base::Thread::Run(base::MessageLoop*) base/threading/thread.cc:202:3 (content_browsertests+0x0000010de279)
    chromium#14 base::Thread::ThreadMain() base/threading/thread.cc:254:3 (content_browsertests+0x0000010de449)
    chromium#15 base::(anonymous namespace)::ThreadFunc(void*) base/threading/platform_thread_posix.cc:70:3 (content_browsertests+0x0000010d88bd)

  Location is global 'media::(anonymous namespace)::g_last_created' of size 8 at 0x0000081b8060 (content_browsertests+0x0000081b8060)

  Thread T15 'AudioThread' (tid=25884, running) created by main thread at:
    #0 pthread_create <null> (content_browsertests+0x0000004a15c5)
    chromium#1 base::(anonymous namespace)::CreateThread(unsigned long, bool, base::PlatformThread::Delegate*, base::PlatformThreadHandle*, base::ThreadPriority) base/threading/platform_thread_posix.cc:109:13 (content_browsertests+0x0000010d860a)
    chromium#2 base::PlatformThread::CreateWithPriority(unsigned long, base::PlatformThread::Delegate*, base::PlatformThreadHandle*, base::ThreadPriority) base/threading/platform_thread_posix.cc:190:10 (content_browsertests+0x0000010d8515)
    chromium#3 base::Thread::StartWithOptions(base::Thread::Options const&) base/threading/thread.cc:116:10 (content_browsertests+0x0000010ddfa0)
    chromium#4 base::Thread::Start() base/threading/thread.cc:86:10 (content_browsertests+0x0000010ddde4)
    chromium#5 content::BrowserMainLoop::CreateAudioManager() content/browser/browser_main_loop.cc:1468:3 (content_browsertests+0x000004e665c6)
    chromium#6 content::BrowserMainLoop::BrowserThreadsStarted() content/browser/browser_main_loop.cc:1228:5 (content_browsertests+0x000004e633b5)
    chromium#7 Run<> base/bind_internal.h:181:12 (content_browsertests+0x000004e67612)
    chromium#8 MakeItSo<content::BrowserMainLoop *> base/bind_internal.h:313 (content_browsertests+0x000004e67612)
    chromium#9 base::internal::Invoker<base::IndexSequence<0ul>, base::internal::BindState<base::internal::RunnableAdapter<int (content::BrowserMainLoop::*)()>, int (content::BrowserMainLoop*), base::internal::UnretainedWrapper<content::BrowserMainLoop> >, base::internal::InvokeHelper<false, int, base::internal::RunnableAdapter<int (content::BrowserMainLoop::*)()> >, int ()>::Run(base::internal::BindStateBase*) base/bind_internal.h:372 (content_browsertests+0x000004e67612)
    chromium#10 Run base/callback.h:397:12 (content_browsertests+0x0000050dd941)
    chromium#11 content::StartupTaskRunner::RunAllTasksNow() content/browser/startup_task_runner.cc:45 (content_browsertests+0x0000050dd941)
    chromium#12 content::BrowserMainLoop::CreateStartupTasks() content/browser/browser_main_loop.cc:803:3 (content_browsertests+0x000004e622db)
    chromium#13 content::BrowserMainRunnerImpl::Initialize(content::MainFunctionParams const&) content/browser/browser_main_runner.cc:139:5 (content_browsertests+0x000004e67c63)
    chromium#14 ShellBrowserMain(content::MainFunctionParams const&, std::__1::unique_ptr<content::BrowserMainRunner, std::__1::default_delete<content::BrowserMainRunner> > const&) content/shell/browser/shell_browser_main.cc:23:19 (content_browsertests+0x000000c97ca7)
    chromium#15 content::ShellMainDelegate::RunProcess(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, content::MainFunctionParams const&) content/shell/app/shell_main_delegate.cc:288:16 (content_browsertests+0x000000c8bec4)
    chromium#16 content::RunNamedProcessTypeMain(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, content::MainFunctionParams const&, content::ContentMainDelegate*) content/app/content_main_runner.cc:368:25 (content_browsertests+0x000005558812)
    chromium#17 content::ContentMainRunnerImpl::Run() content/app/content_main_runner.cc:742:12 (content_browsertests+0x000005559511)
    chromium#18 content::ContentMain(content::ContentMainParams const&) content/app/content_main.cc:20:15 (content_browsertests+0x00000555797e)
    chromium#19 content::BrowserTestBase::SetUp() content/public/test/browser_test_base.cc:287:3 (content_browsertests+0x000000c55601)
    chromium#20 content::ContentBrowserTest::SetUp() content/public/test/content_browser_test.cc:92:3 (content_browsertests+0x000000c5e8e6)
    chromium#21 SetUp content/browser/renderer_host/render_widget_host_view_browsertest.cc:221:5 (content_browsertests+0x000000739618)
    chromium#22 content::(anonymous namespace)::CompositingRenderWidgetHostViewBrowserTestTabCapture::SetUp() content/browser/renderer_host/render_widget_host_view_browsertest.cc:403 (content_browsertests+0x000000739618)
    chromium#23 HandleExceptionsInMethodIfSupported<testing::Test, void> testing/gtest/src/gtest.cc:2458:12 (content_browsertests+0x000000f3168d)
    chromium#24 testing::Test::Run() testing/gtest/src/gtest.cc:2470 (content_browsertests+0x000000f3168d)
    chromium#25 testing::TestInfo::Run() testing/gtest/src/gtest.cc:2656:5 (content_browsertests+0x000000f32833)
    chromium#26 testing::TestCase::Run() testing/gtest/src/gtest.cc:2774:5 (content_browsertests+0x000000f33118)
    chromium#27 testing::internal::UnitTestImpl::RunAllTests() testing/gtest/src/gtest.cc:4647:11 (content_browsertests+0x000000f3c562)
    chromium#28 HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool> testing/gtest/src/gtest.cc:2458:12 (content_browsertests+0x000000f3bfc6)
    chromium#29 testing::UnitTest::Run() testing/gtest/src/gtest.cc:4255 (content_browsertests+0x000000f3bfc6)
    chromium#30 RUN_ALL_TESTS testing/gtest/include/gtest/gtest.h:2237:10 (content_browsertests+0x000000d209eb)
    chromium#31 base::TestSuite::Run() base/test/test_suite.cc:230 (content_browsertests+0x000000d209eb)
    chromium#32 content::ContentTestLauncherDelegate::RunTestSuite(int, char**) content/test/content_test_launcher.cc:105:12 (content_browsertests+0x000000c677db)
    chromium#33 content::LaunchTests(content::TestLauncherDelegate*, int, int, char**) content/public/test/test_launcher.cc:517:12 (content_browsertests+0x000000cfb2fc)
    chromium#34 main content/test/content_test_launcher.cc:131:10 (content_browsertests+0x000000c67762)

SUMMARY: ThreadSanitizer: data race media/audio/audio_manager.cc:264:20 in media::AudioManagerDeleter::operator()(media::AudioManager const*) const

Original issue's description:
> Pass task runners to AudioManager constructor.
>
> This patch replaces the internal AudioManagerBase::audio_thread with
> an external task runner. The old implementation made it harder to test
> and override the internal audio thread.
>
> AudioManagerBase::Shutdown had to complete on the audio thread before
> the AudioManager instance could be deleted. It relied on Thread::Stop
> on the main thread to wait for the audio thread to shutdown. A subclass
> using a different thread shared with other modules could not do that.
>
> Passing a task runner into AudioManager and making GetTaskRunner
> non-virtual ensures that the task runner will not change for the
> lifetime of AudioManager instance.
>
> BUG=594234
>
> Committed: https://crrev.com/e05981bd06b4694b426990acd41be8ab6d9a1659
> Cr-Commit-Position: refs/heads/master@{#388048}

TBR=dalecurtis@chromium.org,tommi@chromium.org,xhwang@chromium.org,rkc@chromium.org,jam@chromium.org,ckehoe@chromium.org,alokp@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=594234

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

Cr-Commit-Position: refs/heads/master@{#388096}
  • Loading branch information
@jyasskin
jyasskin authored and Commit bot committed Apr 19, 2016
1 parent fd1b5cf commit 78655cc
Show file tree
Hide file tree
Showing 72 changed files with 1,040 additions and 1,027 deletions.
13 changes: 4 additions & 9 deletions chromecast/media/audio/cast_audio_manager.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,10 @@ static const int kDefaultOutputBufferSize = 2048;
namespace chromecast {
namespace media {

CastAudioManager::CastAudioManager(
scoped_refptr<base::SingleThreadTaskRunner> task_runner,
scoped_refptr<base::SingleThreadTaskRunner> worker_task_runner,
::media::AudioLogFactory* audio_log_factory,
MediaPipelineBackendManager* backend_manager)
: AudioManagerBase(std::move(task_runner),
std::move(worker_task_runner),
audio_log_factory),
backend_manager_(backend_manager) {}
CastAudioManager::CastAudioManager(::media::AudioLogFactory* audio_log_factory,
MediaPipelineBackendManager* backend_manager)
: AudioManagerBase(audio_log_factory), backend_manager_(backend_manager) {
}

CastAudioManager::~CastAudioManager() {
Shutdown();
Expand Down
11 changes: 3 additions & 8 deletions chromecast/media/audio/cast_audio_manager.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,9 @@ struct MediaPipelineDeviceParams;

class CastAudioManager : public ::media::AudioManagerBase {
public:
CastAudioManager(
scoped_refptr<base::SingleThreadTaskRunner> task_runner,
scoped_refptr<base::SingleThreadTaskRunner> worker_task_runner,
::media::AudioLogFactory* audio_log_factory,
MediaPipelineBackendManager* backend_manager);
CastAudioManager(::media::AudioLogFactory* audio_log_factory,
MediaPipelineBackendManager* backend_manager);
~CastAudioManager() override;

// AudioManager implementation.
bool HasAudioOutputDevices() override;
Expand All @@ -40,9 +38,6 @@ class CastAudioManager : public ::media::AudioManagerBase {
virtual std::unique_ptr<MediaPipelineBackend> CreateMediaPipelineBackend(
const MediaPipelineDeviceParams& params);

protected:
~CastAudioManager() override;

private:
// AudioManagerBase implementation.
::media::AudioOutputStream* MakeLinearOutputStream(
Expand Down
10 changes: 2 additions & 8 deletions chromecast/media/audio/cast_audio_manager_factory.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,9 @@ CastAudioManagerFactory::CastAudioManagerFactory(

CastAudioManagerFactory::~CastAudioManagerFactory() {}

scoped_ptr<::media::AudioManager, ::media::AudioManagerDeleter>
CastAudioManagerFactory::CreateInstance(
scoped_refptr<base::SingleThreadTaskRunner> task_runner,
scoped_refptr<base::SingleThreadTaskRunner> worker_task_runner,
::media::AudioManager* CastAudioManagerFactory::CreateInstance(
::media::AudioLogFactory* audio_log_factory) {
return scoped_ptr<::media::AudioManager, ::media::AudioManagerDeleter>(
new CastAudioManager(std::move(task_runner),
std::move(worker_task_runner), audio_log_factory,
backend_manager_));
return new CastAudioManager(audio_log_factory, backend_manager_);
}

} // namespace media
Expand Down
6 changes: 2 additions & 4 deletions chromecast/media/audio/cast_audio_manager_factory.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,8 @@ class CastAudioManagerFactory : public ::media::AudioManagerFactory {
~CastAudioManagerFactory() override;

// ::media::AudioManagerFactory overrides.
scoped_ptr<::media::AudioManager, ::media::AudioManagerDeleter>
CreateInstance(scoped_refptr<base::SingleThreadTaskRunner> task_runner,
scoped_refptr<base::SingleThreadTaskRunner> worker_task_runner,
::media::AudioLogFactory* audio_log_factory) override;
::media::AudioManager* CreateInstance(
::media::AudioLogFactory* audio_log_factory) override;

private:
MediaPipelineBackendManager* const backend_manager_;
Expand Down
121 changes: 82 additions & 39 deletions chromecast/media/audio/cast_audio_output_stream_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,7 @@

#include "base/bind.h"
#include "base/macros.h"
#include "base/run_loop.h"
#include "base/synchronization/waitable_event.h"
#include "base/thread_task_runner_handle.h"
#include "chromecast/base/metrics/cast_metrics_test_helper.h"
#include "chromecast/media/audio/cast_audio_manager.h"
#include "chromecast/media/base/media_message_loop.h"
Expand Down Expand Up @@ -186,9 +184,8 @@ class FakeAudioSourceCallback

class FakeAudioManager : public CastAudioManager {
public:
FakeAudioManager(scoped_refptr<base::SingleThreadTaskRunner> task_runner)
: CastAudioManager(task_runner, task_runner, nullptr, nullptr),
media_pipeline_backend_(nullptr) {}
FakeAudioManager()
: CastAudioManager(nullptr, nullptr), media_pipeline_backend_(nullptr) {}
~FakeAudioManager() override {}

// CastAudioManager overrides.
Expand Down Expand Up @@ -234,9 +231,9 @@ class CastAudioOutputStreamTest : public ::testing::Test {
void SetUp() override {
metrics::InitializeMetricsHelperForTesting();

audio_task_runner_ = base::ThreadTaskRunnerHandle::Get();
audio_manager_.reset(new FakeAudioManager);
audio_task_runner_ = audio_manager_->GetTaskRunner();
backend_task_runner_ = media::MediaMessageLoop::GetTaskRunner();
audio_manager_.reset(new FakeAudioManager(audio_task_runner_));
}

void TearDown() override {
Expand All @@ -259,68 +256,107 @@ class CastAudioOutputStreamTest : public ::testing::Test {

// Synchronous utility functions.
::media::AudioOutputStream* CreateStream() {
return audio_manager_->MakeAudioOutputStream(GetAudioParams(),
kDefaultDeviceId);
::media::AudioOutputStream* stream = nullptr;

base::WaitableEvent completion_event(false, false);
audio_task_runner_->PostTask(
FROM_HERE,
base::Bind(&CastAudioOutputStreamTest::CreateStreamOnAudioThread,
base::Unretained(this), GetAudioParams(), &stream,
&completion_event));
completion_event.Wait();

return stream;
}
bool OpenStream(::media::AudioOutputStream* stream) {
bool success = stream->Open();
DCHECK(stream);

bool success = false;
base::WaitableEvent completion_event(false, false);
audio_task_runner_->PostTask(
FROM_HERE,
base::Bind(&CastAudioOutputStreamTest::OpenStreamOnAudioThread,
base::Unretained(this), stream, &success,
&completion_event));
completion_event.Wait();

// Drain the backend task runner so that appropriate states are set on
// the backend pipeline devices.
RunUntilIdle(backend_task_runner_.get());
return success;
}
void CloseStream(::media::AudioOutputStream* stream) {
stream->Close();
audio_task_runner_->PostTask(FROM_HERE,
base::Bind(&::media::AudioOutputStream::Close,
base::Unretained(stream)));
RunUntilIdle(audio_task_runner_.get());
RunUntilIdle(backend_task_runner_.get());
// Backend task runner may have posted more tasks to the audio task runner.
// So we need to drain it once more.
message_loop_.RunUntilIdle();
RunUntilIdle(audio_task_runner_.get());
}
void StartStream(
::media::AudioOutputStream* stream,
::media::AudioOutputStream::AudioSourceCallback* source_callback) {
stream->Start(source_callback);
// Drain the audio task runner so that tasks posted by
audio_task_runner_->PostTask(
FROM_HERE, base::Bind(&::media::AudioOutputStream::Start,
base::Unretained(stream), source_callback));
// Drain the audio task runner twice so that tasks posted by
// media::AudioOutputStream::Start are run as well.
message_loop_.RunUntilIdle();
RunUntilIdle(audio_task_runner_.get());
RunUntilIdle(audio_task_runner_.get());
// Drain the backend task runner so that appropriate states are set on
// the backend pipeline devices.
RunUntilIdle(backend_task_runner_.get());
// Drain the audio task runner again to run the tasks posted by the
// backend on audio task runner.
message_loop_.RunUntilIdle();
RunUntilIdle(audio_task_runner_.get());
}
void StopStream(::media::AudioOutputStream* stream) {
stream->Stop();
audio_task_runner_->PostTask(FROM_HERE,
base::Bind(&::media::AudioOutputStream::Stop,
base::Unretained(stream)));
RunUntilIdle(audio_task_runner_.get());
// Drain the backend task runner so that appropriate states are set on
// the backend pipeline devices.
RunUntilIdle(backend_task_runner_.get());
}
void SetStreamVolume(::media::AudioOutputStream* stream, double volume) {
stream->SetVolume(volume);
audio_task_runner_->PostTask(
FROM_HERE, base::Bind(&::media::AudioOutputStream::SetVolume,
base::Unretained(stream), volume));
RunUntilIdle(audio_task_runner_.get());
// Drain the backend task runner so that appropriate states are set on
// the backend pipeline devices.
RunUntilIdle(backend_task_runner_.get());
}
double GetStreamVolume(::media::AudioOutputStream* stream) {
double volume = 0.0;
stream->GetVolume(&volume);
audio_task_runner_->PostTask(
FROM_HERE, base::Bind(&::media::AudioOutputStream::GetVolume,
base::Unretained(stream), &volume));
RunUntilIdle(audio_task_runner_.get());
// No need to drain the backend task runner because getting the volume
// does not involve posting any task to the backend.
return volume;
}

void RunAudioLoopFor(int frames) {
::media::AudioParameters audio_params = GetAudioParams();
base::TimeDelta duration = audio_params.GetBufferDuration() * frames;

base::RunLoop run_loop;
message_loop_.task_runner()->PostDelayedTask(
FROM_HERE, run_loop.QuitClosure(), duration);
run_loop.Run();
void CreateStreamOnAudioThread(const ::media::AudioParameters& audio_params,
::media::AudioOutputStream** stream,
base::WaitableEvent* completion_event) {
DCHECK(audio_task_runner_->BelongsToCurrentThread());
*stream = audio_manager_->MakeAudioOutputStream(GetAudioParams(),
kDefaultDeviceId);
completion_event->Signal();
}
void OpenStreamOnAudioThread(::media::AudioOutputStream* stream,
bool* success,
base::WaitableEvent* completion_event) {
DCHECK(audio_task_runner_->BelongsToCurrentThread());
*success = stream->Open();
completion_event->Signal();
}

base::MessageLoop message_loop_;
std::unique_ptr<FakeAudioManager> audio_manager_;
scoped_refptr<base::SingleThreadTaskRunner> audio_task_runner_;
scoped_refptr<base::SingleThreadTaskRunner> backend_task_runner_;
Expand Down Expand Up @@ -440,7 +476,6 @@ TEST_F(CastAudioOutputStreamTest, PushFrame) {
std::unique_ptr<FakeAudioSourceCallback> source_callback(
new FakeAudioSourceCallback);
StartStream(stream, source_callback.get());
RunAudioLoopFor(2);
StopStream(stream);

// Verify that the stream pushed frames to the backend.
Expand Down Expand Up @@ -483,7 +518,10 @@ TEST_F(CastAudioOutputStreamTest, DeviceBusy) {

// Sleep for a few frames and verify that more frames were not pushed
// because the backend device was busy.
RunAudioLoopFor(5);
::media::AudioParameters audio_params = GetAudioParams();
base::TimeDelta pause = audio_params.GetBufferDuration() * 5;
base::PlatformThread::Sleep(pause);
RunUntilIdle(audio_task_runner_.get());
RunUntilIdle(backend_task_runner_.get());
EXPECT_EQ(1u, audio_decoder->pushed_buffer_count());

Expand All @@ -494,7 +532,9 @@ TEST_F(CastAudioOutputStreamTest, DeviceBusy) {
base::Bind(&FakeAudioDecoder::set_pipeline_status,
base::Unretained(audio_decoder),
FakeAudioDecoder::PIPELINE_STATUS_OK));
RunAudioLoopFor(5);

base::PlatformThread::Sleep(pause);
RunUntilIdle(audio_task_runner_.get());
RunUntilIdle(backend_task_runner_.get());
EXPECT_LT(1u, audio_decoder->pushed_buffer_count());
EXPECT_FALSE(source_callback->error());
Expand All @@ -515,7 +555,6 @@ TEST_F(CastAudioOutputStreamTest, DeviceError) {
std::unique_ptr<FakeAudioSourceCallback> source_callback(
new FakeAudioSourceCallback);
StartStream(stream, source_callback.get());
RunAudioLoopFor(2);

// Make sure that AudioOutputStream attempted to push the initial frame.
EXPECT_LT(0u, audio_decoder->pushed_buffer_count());
Expand All @@ -539,7 +578,6 @@ TEST_F(CastAudioOutputStreamTest, DeviceAsyncError) {
std::unique_ptr<FakeAudioSourceCallback> source_callback(
new FakeAudioSourceCallback);
StartStream(stream, source_callback.get());
RunAudioLoopFor(5);

// Make sure that one frame was pushed.
EXPECT_EQ(1u, audio_decoder->pushed_buffer_count());
Expand All @@ -552,7 +590,7 @@ TEST_F(CastAudioOutputStreamTest, DeviceAsyncError) {
base::Unretained(audio_decoder),
FakeAudioDecoder::PIPELINE_STATUS_OK));

RunAudioLoopFor(5);
RunUntilIdle(audio_task_runner_.get());
RunUntilIdle(backend_task_runner_.get());
// AudioOutputStream must report error to source callback.
EXPECT_TRUE(source_callback->error());
Expand Down Expand Up @@ -587,11 +625,16 @@ TEST_F(CastAudioOutputStreamTest, StartStopStart) {

std::unique_ptr<FakeAudioSourceCallback> source_callback(
new FakeAudioSourceCallback);
stream->Start(source_callback.get());
RunAudioLoopFor(2);
stream->Stop();
stream->Start(source_callback.get());
RunAudioLoopFor(2);
audio_task_runner_->PostTask(
FROM_HERE, base::Bind(&::media::AudioOutputStream::Start,
base::Unretained(stream), source_callback.get()));
audio_task_runner_->PostTask(
FROM_HERE,
base::Bind(&::media::AudioOutputStream::Stop, base::Unretained(stream)));
audio_task_runner_->PostTask(
FROM_HERE, base::Bind(&::media::AudioOutputStream::Start,
base::Unretained(stream), source_callback.get()));
RunUntilIdle(audio_task_runner_.get());
RunUntilIdle(backend_task_runner_.get());

FakeAudioDecoder* audio_device = GetAudio();
Expand Down
1 change: 0 additions & 1 deletion components/audio_modem/BUILD.gn
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,6 @@ source_set("unit_tests") {
deps = [
":test_support",
"//base",
"//base/test:test_support",
"//content/test:test_support",
"//media",
"//media:shared_memory_support",
Expand Down
19 changes: 18 additions & 1 deletion components/audio_modem/audio_player_impl.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,11 @@
#include <string>

#include "base/bind.h"
#include "base/location.h"
#include "base/bind_helpers.h"
#include "base/logging.h"
#include "base/run_loop.h"
#include "components/audio_modem/public/audio_modem_types.h"
#include "content/public/browser/browser_thread.h"
#include "media/audio/audio_manager.h"
#include "media/audio/audio_parameters.h"
#include "media/base/audio_bus.h"
Expand Down Expand Up @@ -160,4 +162,19 @@ void AudioPlayerImpl::OnError(media::AudioOutputStream* /* stream */) {
base::Unretained(this)));
}

void AudioPlayerImpl::FlushAudioLoopForTesting() {
if (media::AudioManager::Get()->GetTaskRunner()->BelongsToCurrentThread())
return;

// Queue task on the audio thread, when it is executed, that means we've
// successfully executed all the tasks before us.
base::RunLoop rl;
media::AudioManager::Get()->GetTaskRunner()->PostTaskAndReply(
FROM_HERE,
base::Bind(base::IgnoreResult(&AudioPlayerImpl::FlushAudioLoopForTesting),
base::Unretained(this)),
rl.QuitClosure());
rl.Run();
}

} // namespace audio_modem
4 changes: 4 additions & 0 deletions components/audio_modem/audio_player_impl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,10 @@ class AudioPlayerImpl final
uint32_t frames_skipped) override;
void OnError(media::AudioOutputStream* stream) override;

// Flushes the audio loop, making sure that any queued operations are
// performed.
void FlushAudioLoopForTesting();

bool is_playing_;

// Self-deleting object.
Expand Down
Loading

0 comments on commit 78655cc

Please sign in to comment.