Commit 6b683a83 authored by Max Morin's avatar Max Morin Committed by Commit Bot

Reland "Refactor flow of audio platform errors to renderer".

Original description, (original code is PS 1):

Currently, errors from the platform (e.g. device disconnected), are sent
directly to the renderer (AudioOutputStreamClient). When moving to the audio
service, this becomes suboptimal. It is tricky to signal an error occurring
before the ClientPtr is set up, and sometimes we may not want it torn down due
to a platform error (in case we want to try some sort of recovery operation
such as restarting the audio service). This CL instead signals the error to the
browser (AudioOutputStreamObserver), which signals it to the client through the
new AudioOutputStreamProviderClient interface. That interface also gets the
"stream created" callback as a method rather than a mojo response, so that we
will be able to call it multiple times in the future.

Diff PS 1 -> PS 2 removes a call to a sequence checker that was accidentally
added when rebasing the original CL.

Diff PS 2 -> last PS fixes an issue when pepper would call Start before the
stream is created. This is solved by storing the state that the client
expects in the MojoAudioOutputIPC object. That object then initializes the
state when the stream has been created. As an aside, it is a great preparation
for restarting stream, since MojoAudioOutputIPC will be able to restore the
volume and playing state without involvement from AudioOutputDevice.

Bug: 787806,830493,834242
Change-Id: Iee3a3902e9086cc9df4c5d210e43e65c162273fe
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Reviewed-on: https://chromium-review.googlesource.com/1023397
Commit-Queue: Max Morin <maxmorin@chromium.org>
Reviewed-by: default avatarYuri Wiitala <miu@chromium.org>
Reviewed-by: default avatarDale Curtis <dalecurtis@chromium.org>
Reviewed-by: default avatarKinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#553965}
parent a69dac0d
......@@ -37,10 +37,12 @@ using AudioOutputStreamFactoryRequest =
using AudioOutputStream = media::mojom::AudioOutputStream;
using AudioOutputStreamPtr = mojo::InterfacePtr<AudioOutputStream>;
using AudioOutputStreamRequest = mojo::InterfaceRequest<AudioOutputStream>;
using AudioOutputStreamClient = media::mojom::AudioOutputStreamClient;
using AudioOutputStreamClientPtr = mojo::InterfacePtr<AudioOutputStreamClient>;
using AudioOutputStreamClientRequest =
mojo::InterfaceRequest<AudioOutputStreamClient>;
using AudioOutputStreamProviderClient =
media::mojom::AudioOutputStreamProviderClient;
using AudioOutputStreamProviderClientPtr =
mojo::InterfacePtr<AudioOutputStreamProviderClient>;
using AudioOutputStreamProviderClientRequest =
mojo::InterfaceRequest<AudioOutputStreamProviderClient>;
using AudioOutputStreamProvider = media::mojom::AudioOutputStreamProvider;
using AudioOutputStreamProviderPtr =
mojo::InterfacePtr<AudioOutputStreamProvider>;
......@@ -166,13 +168,21 @@ class MockContext : public RendererAudioOutputStreamFactoryContext {
DISALLOW_COPY_AND_ASSIGN(MockContext);
};
class MockClient : public AudioOutputStreamClient {
class MockClient : public AudioOutputStreamProviderClient {
public:
MockClient() {}
MockClient() : provider_client_binding_(this) {}
~MockClient() override {}
void StreamCreated(media::mojom::AudioDataPipePtr data_pipe) {
AudioOutputStreamProviderClientPtr MakeProviderClientPtr() {
AudioOutputStreamProviderClientPtr p;
provider_client_binding_.Bind(mojo::MakeRequest(&p));
return p;
}
void Created(AudioOutputStreamPtr stream,
media::mojom::AudioDataPipePtr data_pipe) {
was_called_ = true;
stream_ = std::move(stream);
}
bool was_called() { return was_called_; }
......@@ -180,6 +190,8 @@ class MockClient : public AudioOutputStreamClient {
MOCK_METHOD0(OnError, void());
private:
mojo::Binding<AudioOutputStreamProviderClient> provider_client_binding_;
AudioOutputStreamPtr stream_;
bool was_called_ = false;
DISALLOW_COPY_AND_ASSIGN(MockClient);
......@@ -199,17 +211,13 @@ void AuthCallback(media::OutputDeviceStatus* status_out,
} // namespace
// This test authorizes and creates a stream, and checks that
// 1. the authorization callback is called with appropriate parameters.
// 1. the ProviderClient callback is called with appropriate parameters.
// 2. the AudioOutputDelegate is created.
// 3. when the delegate calls OnStreamCreated, this is propagated to the client.
TEST(RenderFrameAudioOutputStreamFactoryTest, CreateStream) {
content::TestBrowserThreadBundle thread_bundle;
AudioOutputStreamProviderPtr provider;
AudioOutputStreamPtr output_stream;
MockClient client;
AudioOutputStreamClientPtr client_ptr;
mojo::Binding<AudioOutputStreamClient> client_binding(
&client, mojo::MakeRequest(&client_ptr));
media::AudioOutputDelegate::EventHandler* event_handler = nullptr;
auto factory_context = std::make_unique<MockContext>(true);
factory_context->PrepareDelegateForCreation(
......@@ -229,9 +237,7 @@ TEST(RenderFrameAudioOutputStreamFactoryTest, CreateStream) {
GetTestAudioParameters().AsHumanReadableString());
EXPECT_TRUE(id.empty());
provider->Acquire(
mojo::MakeRequest(&output_stream), std::move(client_ptr), params,
base::BindOnce(&MockClient::StreamCreated, base::Unretained(&client)));
provider->Acquire(params, client.MakeProviderClientPtr());
base::RunLoop().RunUntilIdle();
ASSERT_NE(event_handler, nullptr);
......@@ -270,11 +276,7 @@ TEST(RenderFrameAudioOutputStreamFactoryTest, NotAuthorized_Denied) {
TEST(RenderFrameAudioOutputStreamFactoryTest, ConnectionError_DeletesStream) {
content::TestBrowserThreadBundle thread_bundle;
AudioOutputStreamProviderPtr provider;
AudioOutputStreamPtr output_stream;
MockClient client;
AudioOutputStreamClientPtr client_ptr;
mojo::Binding<AudioOutputStreamClient> client_binding(
&client, mojo::MakeRequest(&client_ptr));
bool delegate_is_destructed = false;
media::AudioOutputDelegate::EventHandler* event_handler = nullptr;
auto factory_context = std::make_unique<MockContext>(true);
......@@ -292,14 +294,11 @@ TEST(RenderFrameAudioOutputStreamFactoryTest, ConnectionError_DeletesStream) {
const std::string& id) {}));
base::RunLoop().RunUntilIdle();
provider->Acquire(
mojo::MakeRequest(&output_stream), std::move(client_ptr),
GetTestAudioParameters(),
base::BindOnce(&MockClient::StreamCreated, base::Unretained(&client)));
provider->Acquire(GetTestAudioParameters(), client.MakeProviderClientPtr());
base::RunLoop().RunUntilIdle();
ASSERT_NE(event_handler, nullptr);
EXPECT_FALSE(delegate_is_destructed);
output_stream.reset();
provider.reset();
base::RunLoop().RunUntilIdle();
EXPECT_TRUE(delegate_is_destructed);
}
......@@ -307,11 +306,7 @@ TEST(RenderFrameAudioOutputStreamFactoryTest, ConnectionError_DeletesStream) {
TEST(RenderFrameAudioOutputStreamFactoryTest, DelegateError_DeletesStream) {
content::TestBrowserThreadBundle thread_bundle;
AudioOutputStreamProviderPtr provider;
AudioOutputStreamPtr output_stream;
MockClient client;
AudioOutputStreamClientPtr client_ptr;
mojo::Binding<AudioOutputStreamClient> client_binding(
&client, mojo::MakeRequest(&client_ptr));
bool delegate_is_destructed = false;
media::AudioOutputDelegate::EventHandler* event_handler = nullptr;
auto factory_context = std::make_unique<MockContext>(true);
......@@ -329,10 +324,7 @@ TEST(RenderFrameAudioOutputStreamFactoryTest, DelegateError_DeletesStream) {
const std::string& id) {}));
base::RunLoop().RunUntilIdle();
provider->Acquire(
mojo::MakeRequest(&output_stream), std::move(client_ptr),
GetTestAudioParameters(),
base::BindOnce(&MockClient::StreamCreated, base::Unretained(&client)));
provider->Acquire(GetTestAudioParameters(), client.MakeProviderClientPtr());
base::RunLoop().RunUntilIdle();
ASSERT_NE(event_handler, nullptr);
EXPECT_FALSE(delegate_is_destructed);
......
......@@ -73,7 +73,8 @@ class FakeAudioOutputIPCDelegate : public media::AudioOutputIPCDelegate {
const media::AudioParameters& output_params,
const std::string& matched_device_id) override {}
void OnStreamCreated(base::SharedMemoryHandle handle,
base::SyncSocket::Handle socket_handle) override {}
base::SyncSocket::Handle socket_handle,
bool playing_automatically) override {}
void OnIPCClosed() override {}
};
......
......@@ -32,9 +32,9 @@ MojoAudioOutputIPC::MojoAudioOutputIPC(
MojoAudioOutputIPC::~MojoAudioOutputIPC() {
DCHECK(!AuthorizationRequested() && !StreamCreationRequested())
<< "CloseStream must be called before destructing the AudioOutputIPC";
// No thread check.
// Destructing |weak_factory_| on any thread is safe since it's not used after
// the final call to CloseStream, where its pointers are invalidated.
// No sequence check.
// Destructing |weak_factory_| on any sequence is safe since it's not used
// after the final call to CloseStream, where its pointers are invalidated.
}
void MojoAudioOutputIPC::RequestDeviceAuthorization(
......@@ -81,28 +81,26 @@ void MojoAudioOutputIPC::CreateStream(media::AudioOutputIPCDelegate* delegate,
// Since the creation callback won't fire if the provider binding is gone
// and |this| owns |stream_provider_|, unretained is safe.
stream_creation_start_time_ = base::TimeTicks::Now();
media::mojom::AudioOutputStreamClientPtr client_ptr;
media::mojom::AudioOutputStreamProviderClientPtr client_ptr;
binding_.Bind(mojo::MakeRequest(&client_ptr));
stream_provider_->Acquire(mojo::MakeRequest(&stream_), std::move(client_ptr),
params,
base::BindOnce(&MojoAudioOutputIPC::StreamCreated,
// Unretained is safe because |this| owns |binding_|.
binding_.set_connection_error_with_reason_handler(
base::BindOnce(&MojoAudioOutputIPC::ProviderClientBindingDisconnected,
base::Unretained(this)));
// Don't set a connection error handler. Either an error has already been
// signaled through the AudioOutputStreamClient interface, or the connection
// is broken because the frame owning |this| was destroyed, in which
// case |this| will soon be cleaned up anyways.
stream_provider_->Acquire(params, std::move(client_ptr));
}
void MojoAudioOutputIPC::PlayStream() {
DCHECK(io_task_runner_->RunsTasksInCurrentSequence());
DCHECK(stream_.is_bound());
expected_state_ = kPlaying;
if (stream_.is_bound())
stream_->Play();
}
void MojoAudioOutputIPC::PauseStream() {
DCHECK(io_task_runner_->RunsTasksInCurrentSequence());
DCHECK(stream_.is_bound());
expected_state_ = kPaused;
if (stream_.is_bound())
stream_->Pause();
}
......@@ -112,6 +110,8 @@ void MojoAudioOutputIPC::CloseStream() {
stream_.reset();
binding_.Close();
delegate_ = nullptr;
expected_state_ = kPaused;
volume_ = base::nullopt;
// Cancel any pending callbacks for this stream.
weak_factory_.InvalidateWeakPtrs();
......@@ -119,22 +119,31 @@ void MojoAudioOutputIPC::CloseStream() {
void MojoAudioOutputIPC::SetVolume(double volume) {
DCHECK(io_task_runner_->RunsTasksInCurrentSequence());
DCHECK(stream_.is_bound());
volume_ = volume;
if (stream_.is_bound())
stream_->SetVolume(volume);
// else volume is set when the stream is created.
}
void MojoAudioOutputIPC::OnError() {
void MojoAudioOutputIPC::ProviderClientBindingDisconnected(
uint32_t disconnect_reason,
const std::string& description) {
DCHECK(io_task_runner_->RunsTasksInCurrentSequence());
DCHECK(delegate_);
if (disconnect_reason == kPlatformErrorDisconnectReason) {
delegate_->OnError();
}
// Otherwise, disconnection was due to the frame owning |this| being
// destructed or having a navigation. In this case, |this| will soon be
// cleaned up.
}
bool MojoAudioOutputIPC::AuthorizationRequested() {
bool MojoAudioOutputIPC::AuthorizationRequested() const {
return stream_provider_.is_bound();
}
bool MojoAudioOutputIPC::StreamCreationRequested() {
return stream_.is_bound();
bool MojoAudioOutputIPC::StreamCreationRequested() const {
return binding_.is_bound();
}
media::mojom::AudioOutputStreamProviderRequest
......@@ -203,13 +212,14 @@ void MojoAudioOutputIPC::ReceivedDeviceAuthorization(
delegate_->OnDeviceAuthorized(status, params, device_id);
}
void MojoAudioOutputIPC::StreamCreated(
void MojoAudioOutputIPC::Created(media::mojom::AudioOutputStreamPtr stream,
media::mojom::AudioDataPipePtr data_pipe) {
DCHECK(io_task_runner_->RunsTasksInCurrentSequence());
DCHECK(delegate_);
UMA_HISTOGRAM_TIMES("Media.Audio.Render.OutputDeviceStreamCreationTime",
base::TimeTicks::Now() - stream_creation_start_time_);
stream_ = std::move(stream);
base::PlatformFile socket_handle;
auto result =
......@@ -226,7 +236,13 @@ void MojoAudioOutputIPC::StreamCreated(
DCHECK_EQ(protection,
mojo::UnwrappedSharedMemoryHandleProtection::kReadWrite);
delegate_->OnStreamCreated(memory_handle, socket_handle);
delegate_->OnStreamCreated(memory_handle, socket_handle,
expected_state_ == kPlaying);
if (volume_)
stream_->SetVolume(*volume_);
if (expected_state_ == kPlaying)
stream_->Play();
}
} // namespace content
......@@ -10,6 +10,7 @@
#include "base/callback_helpers.h"
#include "base/macros.h"
#include "base/memory/weak_ptr.h"
#include "base/optional.h"
#include "base/time/time.h"
#include "content/common/content_export.h"
#include "content/common/media/renderer_audio_output_stream_factory.mojom.h"
......@@ -24,7 +25,7 @@ namespace content {
// thread.
class CONTENT_EXPORT MojoAudioOutputIPC
: public media::AudioOutputIPC,
public media::mojom::AudioOutputStreamClient {
public media::mojom::AudioOutputStreamProviderClient {
public:
using FactoryAccessorCB =
base::RepeatingCallback<mojom::RendererAudioOutputStreamFactory*()>;
......@@ -48,15 +49,22 @@ class CONTENT_EXPORT MojoAudioOutputIPC
void CloseStream() override;
void SetVolume(double volume) override;
// media::mojom::AudioOutputStreamClient implementation.
void OnError() override;
// media::mojom::AudioOutputStreamProviderClient implementation.
void Created(media::mojom::AudioOutputStreamPtr stream,
media::mojom::AudioDataPipePtr data_pipe) override;
private:
static constexpr double kDefaultVolume = 1.0;
using AuthorizationCB = mojom::RendererAudioOutputStreamFactory::
RequestDeviceAuthorizationCallback;
bool AuthorizationRequested();
bool StreamCreationRequested();
bool AuthorizationRequested() const;
bool StreamCreationRequested() const;
void ProviderClientBindingDisconnected(uint32_t disconnect_reason,
const std::string& description);
media::mojom::AudioOutputStreamProviderRequest MakeProviderRequest();
// Tries to acquire a RendererAudioOutputStreamFactory and requests device
......@@ -71,11 +79,14 @@ class CONTENT_EXPORT MojoAudioOutputIPC
const media::AudioParameters& params,
const std::string& device_id) const;
void StreamCreated(media::mojom::AudioDataPipePtr data_pipe);
const FactoryAccessorCB factory_accessor_;
mojo::Binding<media::mojom::AudioOutputStreamClient> binding_;
// This is the state that |delegate_| expects the stream to be in. It is
// maintained for when the stream is created.
enum { kPaused, kPlaying } expected_state_ = kPaused;
base::Optional<double> volume_;
mojo::Binding<media::mojom::AudioOutputStreamProviderClient> binding_;
media::mojom::AudioOutputStreamProviderPtr stream_provider_;
media::mojom::AudioOutputStreamPtr stream_;
media::AudioOutputIPCDelegate* delegate_ = nullptr;
......
......@@ -57,30 +57,33 @@ class TestStreamProvider : public media::mojom::AudioOutputStreamProvider {
EXPECT_TRUE(binding_);
}
void Acquire(media::mojom::AudioOutputStreamRequest stream_request,
media::mojom::AudioOutputStreamClientPtr client_ptr,
const media::AudioParameters& params,
AcquireCallback callback) override {
void Acquire(const media::AudioParameters& params,
media::mojom::AudioOutputStreamProviderClientPtr provider_client)
override {
EXPECT_EQ(binding_, base::nullopt);
EXPECT_NE(stream_, nullptr);
std::swap(client_, client_ptr);
binding_.emplace(stream_, std::move(stream_request));
std::swap(provider_client, provider_client_);
media::mojom::AudioOutputStreamPtr stream_ptr;
binding_.emplace(stream_, mojo::MakeRequest(&stream_ptr));
base::CancelableSyncSocket foreign_socket;
EXPECT_TRUE(
base::CancelableSyncSocket::CreatePair(&socket_, &foreign_socket));
std::move(callback).Run({base::in_place,
mojo::SharedBufferHandle::Create(kMemoryLength),
provider_client_->Created(
std::move(stream_ptr),
{base::in_place, mojo::SharedBufferHandle::Create(kMemoryLength),
mojo::WrapPlatformFile(foreign_socket.Release())});
}
media::mojom::AudioOutputStreamClient* client() {
DCHECK(client_.get());
return client_.get();
void SignalErrorToProviderClient() {
provider_client_.ResetWithReason(
media::mojom::AudioOutputStreamProviderClient::
kPlatformErrorDisconnectReason,
std::string());
}
private:
media::mojom::AudioOutputStream* stream_;
media::mojom::AudioOutputStreamClientPtr client_;
media::mojom::AudioOutputStreamProviderClientPtr provider_client_;
base::Optional<mojo::Binding<media::mojom::AudioOutputStream>> binding_;
base::CancelableSyncSocket socket_;
};
......@@ -134,6 +137,10 @@ class TestRemoteFactory : public mojom::RendererAudioOutputStreamFactory {
expected_device_id_ = device_id;
}
void SignalErrorToProviderClient() {
provider_->SignalErrorToProviderClient();
}
void Disconnect() {
binding_.Close();
this_proxy_.reset();
......@@ -143,10 +150,6 @@ class TestRemoteFactory : public mojom::RendererAudioOutputStreamFactory {
expect_request_ = false;
}
media::mojom::AudioOutputStreamClient* client() {
return provider_->client();
}
MojoAudioOutputIPC::FactoryAccessorCB GetAccessor() {
return base::BindRepeating(&TestRemoteFactory::get, base::Unretained(this));
}
......@@ -178,7 +181,8 @@ class MockDelegate : public media::AudioOutputIPCDelegate {
~MockDelegate() override {}
void OnStreamCreated(base::SharedMemoryHandle mem_handle,
base::SyncSocket::Handle socket_handle) {
base::SyncSocket::Handle socket_handle,
bool playing_automatically) {
base::SharedMemory sh_mem(
mem_handle, /*read_only*/ false); // Releases the shared memory handle.
base::SyncSocket socket(socket_handle); // Releases the socket descriptor.
......@@ -386,7 +390,7 @@ TEST(MojoAudioOutputIPC, IsReusableAfterError) {
Mock::VerifyAndClearExpectations(&delegate);
EXPECT_CALL(delegate, OnError());
stream_factory.client()->OnError();
stream_factory.SignalErrorToProviderClient();
base::RunLoop().RunUntilIdle();
Mock::VerifyAndClearExpectations(&delegate);
......@@ -530,6 +534,12 @@ TEST(MojoAudioOutputIPC, Play_Plays) {
StrictMock<MockStream> stream;
StrictMock<MockDelegate> delegate;
EXPECT_CALL(delegate, OnDeviceAuthorized(
media::OutputDeviceStatus::OUTPUT_DEVICE_STATUS_OK,
_, std::string(kReturnedDeviceId)));
EXPECT_CALL(delegate, GotOnStreamCreated());
EXPECT_CALL(stream, Play());
const std::unique_ptr<media::AudioOutputIPC> ipc =
std::make_unique<MojoAudioOutputIPC>(
stream_factory.GetAccessor(),
......@@ -539,15 +549,9 @@ TEST(MojoAudioOutputIPC, Play_Plays) {
ipc->RequestDeviceAuthorization(&delegate, kSessionId, kDeviceId);
ipc->CreateStream(&delegate, Params());
base::RunLoop().RunUntilIdle();
ipc->PlayStream();
EXPECT_CALL(delegate, OnDeviceAuthorized(
media::OutputDeviceStatus::OUTPUT_DEVICE_STATUS_OK,
_, std::string(kReturnedDeviceId)));
EXPECT_CALL(delegate, GotOnStreamCreated());
EXPECT_CALL(stream, Play());
base::RunLoop().RunUntilIdle();
ipc->CloseStream();
base::RunLoop().RunUntilIdle();
}
......@@ -558,6 +562,12 @@ TEST(MojoAudioOutputIPC, Pause_Pauses) {
StrictMock<MockStream> stream;
StrictMock<MockDelegate> delegate;
EXPECT_CALL(delegate, OnDeviceAuthorized(
media::OutputDeviceStatus::OUTPUT_DEVICE_STATUS_OK,
_, std::string(kReturnedDeviceId)));
EXPECT_CALL(delegate, GotOnStreamCreated());
EXPECT_CALL(stream, Pause());
const std::unique_ptr<media::AudioOutputIPC> ipc =
std::make_unique<MojoAudioOutputIPC>(
stream_factory.GetAccessor(),
......@@ -567,15 +577,9 @@ TEST(MojoAudioOutputIPC, Pause_Pauses) {
ipc->RequestDeviceAuthorization(&delegate, kSessionId, kDeviceId);
ipc->CreateStream(&delegate, Params());
base::RunLoop().RunUntilIdle();
ipc->PauseStream();
EXPECT_CALL(delegate, OnDeviceAuthorized(
media::OutputDeviceStatus::OUTPUT_DEVICE_STATUS_OK,
_, std::string(kReturnedDeviceId)));
EXPECT_CALL(delegate, GotOnStreamCreated());
EXPECT_CALL(stream, Pause());
base::RunLoop().RunUntilIdle();
ipc->CloseStream();
base::RunLoop().RunUntilIdle();
}
......@@ -586,6 +590,12 @@ TEST(MojoAudioOutputIPC, SetVolume_SetsVolume) {
StrictMock<MockStream> stream;
StrictMock<MockDelegate> delegate;
EXPECT_CALL(delegate, OnDeviceAuthorized(
media::OutputDeviceStatus::OUTPUT_DEVICE_STATUS_OK,
_, std::string(kReturnedDeviceId)));
EXPECT_CALL(delegate, GotOnStreamCreated());
EXPECT_CALL(stream, SetVolume(kNewVolume));
const std::unique_ptr<media::AudioOutputIPC> ipc =
std::make_unique<MojoAudioOutputIPC>(
stream_factory.GetAccessor(),
......@@ -595,15 +605,9 @@ TEST(MojoAudioOutputIPC, SetVolume_SetsVolume) {
ipc->RequestDeviceAuthorization(&delegate, kSessionId, kDeviceId);
ipc->CreateStream(&delegate, Params());
base::RunLoop().RunUntilIdle();
ipc->SetVolume(kNewVolume);
EXPECT_CALL(delegate, OnDeviceAuthorized(
media::OutputDeviceStatus::OUTPUT_DEVICE_STATUS_OK,
_, std::string(kReturnedDeviceId)));
EXPECT_CALL(delegate, GotOnStreamCreated());
EXPECT_CALL(stream, SetVolume(kNewVolume));
base::RunLoop().RunUntilIdle();
ipc->CloseStream();
base::RunLoop().RunUntilIdle();
}
......
......@@ -90,7 +90,8 @@ void PepperPlatformAudioOutput::OnDeviceAuthorized(
void PepperPlatformAudioOutput::OnStreamCreated(
base::SharedMemoryHandle handle,
base::SyncSocket::Handle socket_handle) {
base::SyncSocket::Handle socket_handle,
bool playing_automatically) {
DCHECK(handle.IsValid());
#if defined(OS_WIN)
DCHECK(socket_handle);
......@@ -106,8 +107,9 @@ void PepperPlatformAudioOutput::OnStreamCreated(
client_->StreamCreated(handle, handle.GetSize(), socket_handle);
} else {
main_task_runner_->PostTask(
FROM_HERE, base::BindOnce(&PepperPlatformAudioOutput::OnStreamCreated,
this, handle, socket_handle));
FROM_HERE,
base::BindOnce(&PepperPlatformAudioOutput::OnStreamCreated, this,
handle, socket_handle, playing_automatically));
}
}
......
......@@ -58,7 +58,8 @@ class PepperPlatformAudioOutput
const media::AudioParameters& output_params,
const std::string& matched_device_id) override;
void OnStreamCreated(base::SharedMemoryHandle handle,
base::SyncSocket::Handle socket_handle) override;
base::SyncSocket::Handle socket_handle,
bool playing_automatically) override;
void OnIPCClosed() override;
protected:
......
......@@ -181,7 +181,8 @@ void PepperPlatformAudioOutputDev::OnDeviceAuthorized(
void PepperPlatformAudioOutputDev::OnStreamCreated(
base::SharedMemoryHandle handle,
base::SyncSocket::Handle socket_handle) {
base::SyncSocket::Handle socket_handle,
bool playing_automatically) {
DCHECK(handle.IsValid());
#if defined(OS_WIN)
DCHECK(socket_handle);
......@@ -207,7 +208,7 @@ void PepperPlatformAudioOutputDev::OnStreamCreated(
main_task_runner_->PostTask(
FROM_HERE,
base::BindOnce(&PepperPlatformAudioOutputDev::OnStreamCreated, this,
handle, socket_handle));
handle, socket_handle, playing_automatically));
}
}
......
......@@ -63,7 +63,8 @@ class PepperPlatformAudioOutputDev
const media::AudioParameters& output_params,
const std::string& matched_device_id) override;
void OnStreamCreated(base::SharedMemoryHandle handle,
base::SyncSocket::Handle socket_handle) override;
base::SyncSocket::Handle socket_handle,
bool playing_automatically) override;
void OnIPCClosed() override;
protected:
......
This diff is collapsed.
......@@ -72,7 +72,6 @@
#include "base/time/time.h"
#include "media/audio/audio_device_thread.h"
#include "media/audio/audio_output_ipc.h"
#include "media/audio/scoped_task_runner_observer.h"
#include "media/base/audio_parameters.h"
#include "media/base/audio_renderer_sink.h"
#include "media/base/media_export.h"
......@@ -80,13 +79,13 @@
namespace base {
class OneShotTimer;
class SingleThreadTaskRunner;
}
namespace media {
class MEDIA_EXPORT AudioOutputDevice : public AudioRendererSink,
public AudioOutputIPCDelegate,
public ScopedTaskRunnerObserver {
public AudioOutputIPCDelegate {
public:
// NOTE: Clients must call Initialize() before using.
AudioOutputDevice(
......@@ -118,7 +117,8 @@ class MEDIA_EXPORT AudioOutputDevice : public AudioRendererSink,
const media::AudioParameters& output_params,
const std::string& matched_device_id) override;
void OnStreamCreated(base::SharedMemoryHandle handle,
base::SyncSocket::Handle socket_handle) override;
base::SyncSocket::Handle socket_handle,
bool play_automatically) override;
void OnIPCClosed() override;
protected:
......@@ -128,15 +128,12 @@ class MEDIA_EXPORT AudioOutputDevice : public AudioRendererSink,
~AudioOutputDevice() override;
private:
// Note: The ordering of members in this enum is critical to correct behavior!
enum State {
IPC_CLOSED, // No more IPCs can take place.
IDLE, // Not started.
AUTHORIZING, // Sent device authorization request, waiting for reply.
AUTHORIZED, // Successful device authorization received.
CREATING_STREAM, // Waiting for OnStreamCreated() to be called back.
PAUSED, // Paused. OnStreamCreated() has been called. Can Play()/Stop().
PLAYING, // Playing back. Can Pause()/Stop().
enum StartupState {
IDLE, // Authorization not requested.
AUTHORIZATION_REQUESTED, // Sent (possibly completed) device
// authorization request.
STREAM_CREATION_REQUESTED, // Sent (possibly completed) device creation
// request. Can Play()/Pause()/Stop().
};
// Methods called on IO thread ----------------------------------------------
......@@ -161,32 +158,25 @@ class MEDIA_EXPORT AudioOutputDevice : public AudioRendererSink,
void NotifyRenderCallbackOfError();
// base::MessageLoopCurrent::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;
const scoped_refptr<base::SingleThreadTaskRunner> io_task_runner_;
AudioParameters audio_parameters_;
RenderCallback* callback_;
// A pointer to the IPC layer that takes care of sending requests over to
// the implementation. Only valid when state_ != IPC_CLOSED and must only
// be accessed on the IO thread.
// the implementation. May be set to nullptr after errors.
std::unique_ptr<AudioOutputIPC> ipc_;
// Current state (must only be accessed from the IO thread). See comments for
// State enum above.
State state_;
// State of Start() calls before OnDeviceAuthorized() is called.
bool start_on_authorized_;
StartupState state_;
// For UMA stats. May only be accessed on the IO thread.
bool had_callback_error_ = false;
// State of Play() / Pause() calls before OnStreamCreated() is called.
bool play_on_start_;
// Last set volume.
double volume_ = 1.0;
// The media session ID used to identify which input device to be started.
// Only used by Unified IO.
......
This diff is collapsed.
......@@ -32,9 +32,12 @@ class MEDIA_EXPORT AudioOutputIPCDelegate {
// Called when an audio stream has been created.
// See media/mojo/interfaces/audio_data_pipe.mojom for documentation of
// |handle| and |socket_handle|.
// |handle| and |socket_handle|. |playing_automatically| indicates if the
// AudioOutputIPCDelegate is playing right away due to an earlier call to
// Play();
virtual void OnStreamCreated(base::SharedMemoryHandle handle,
base::SyncSocket::Handle socket_handle) = 0;
base::SyncSocket::Handle socket_handle,
bool playing_automatically) = 0;
// Called when the AudioOutputIPC object is going away and/or when the IPC
// channel has been closed and no more ipc requests can be made.
......
......@@ -25,21 +25,6 @@ interface AudioOutputStream {
SetVolume(double volume);
};
interface AudioOutputStreamClient {
// Called if the stream has an error such as failing to open/losing access to
// a device. This renders the stream unusable.
OnError();
};
interface AudioOutputStreamProvider {
// Creates a new AudioOutputStream using |params|. |data_pipe| is used to
// transfer audio data.
// Can only be called once.
Acquire(AudioOutputStream& output_stream, AudioOutputStreamClient client,
AudioParameters params)
=> (AudioDataPipe data_pipe);
};
// An AudioOutputStreamObserver gets notifications about events related to an
// AudioOutputStream. DidStartPlaying() is invoked when the stream starts
// playing and it is eventually followed by a DidStopPlaying() call. A stream
......@@ -48,6 +33,13 @@ interface AudioOutputStreamProvider {
// Note: It is possible that DidStopPlaying() is not called in shutdown
// situations.
interface AudioOutputStreamObserver {
// This code is used as disconnect reason when stream ended or failed to
// start due to an unrecoverable platform error, e.g. the hardware device is
// busy or disconnected.
// If the error code isn't used, the stream ended due to being terminated by
// the client.
const uint32 kPlatformErrorDisconnectReason = 1;
// This notification indicates that the stream started playing. The stream
// should be considered non-audible until a DidChangeAudibleState() call says
// otherwise.
......@@ -63,3 +55,27 @@ interface AudioOutputStreamObserver {
// DidStartPlaying() and before DidStopPlaying().
DidChangeAudibleState(bool is_audible);
};
interface AudioOutputStreamProvider {
// Creates a new AudioOutputStream using |params|. |client| is notified when
// the stream is ready. The stream lifetime is bound by the lifetime of
// |client|. On error, the |client| will have a disconnect reason among the
// specified ones in AudioOutputStreamProviderClient.
// Can only be called once.
Acquire(AudioParameters params, AudioOutputStreamProviderClient client);
};
interface AudioOutputStreamProviderClient {
// This code is used as disconnect reason when stream ended or failed to
// start due to an unrecoverable platform error, e.g. the hardware device is
// busy or disconnected.
// If the error code isn't used, the stream ended/wasn't started due to the
// stream becoming unnecessary or unwanted, or the request was dropped.
const uint32 kPlatformErrorDisconnectReason = 1;
// |stream| is used to pass commands to the stream, and |data_pipe| is used
// to transfer the audio data.
// TODO(https://crbug.com/787806): Currently, this will be called at most
// once. In the future, it may be called several times.
Created(AudioOutputStream stream, AudioDataPipe data_pipe);
};
......@@ -15,3 +15,6 @@ per-file test_manifest.json=file://ipc/SECURITY_OWNERS
per-file pipeline_apptest_manifest.json=set noparent
per-file pipeline_apptest_manifest.json=file://ipc/SECURITY_OWNERS
per-file mojo_audio_output*=file://media/audio/OWNERS
per-file mojo_audio_input*=file://media/audio/OWNERS
......@@ -15,29 +15,20 @@
namespace media {
MojoAudioOutputStream::MojoAudioOutputStream(
mojom::AudioOutputStreamRequest request,
mojom::AudioOutputStreamClientPtr client,
CreateDelegateCallback create_delegate_callback,
StreamCreatedCallback stream_created_callback,
base::OnceClosure deleter_callback)
DeleterCallback deleter_callback)
: stream_created_callback_(std::move(stream_created_callback)),
deleter_callback_(std::move(deleter_callback)),
binding_(this, std::move(request)),
client_(std::move(client)),
binding_(this),
weak_factory_(this) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
DCHECK(stream_created_callback_);
DCHECK(deleter_callback_);
// |this| owns |binding_|, so unretained is safe.
binding_.set_connection_error_handler(
base::BindOnce(&MojoAudioOutputStream::OnError, base::Unretained(this)));
client_.set_connection_error_handler(
base::BindOnce(&MojoAudioOutputStream::OnError, base::Unretained(this)));
delegate_ = std::move(create_delegate_callback).Run(this);
if (!delegate_) {
// Failed to initialize the stream. We cannot call |deleter_callback_| yet,
// since construction isn't done.
binding_.Close();
base::ThreadTaskRunnerHandle::Get()->PostTask(
FROM_HERE,
base::BindOnce(&MojoAudioOutputStream::OnStreamError,
......@@ -95,21 +86,27 @@ void MojoAudioOutputStream::OnStreamCreated(
DCHECK(buffer_handle.is_valid());
DCHECK(socket_handle.is_valid());
base::ResetAndReturn(&stream_created_callback_)
.Run(
{base::in_place, std::move(buffer_handle), std::move(socket_handle)});
mojom::AudioOutputStreamPtr stream;
binding_.Bind(mojo::MakeRequest(&stream));
// |this| owns |binding_| so unretained is safe.
binding_.set_connection_error_handler(base::BindOnce(
&MojoAudioOutputStream::StreamConnectionLost, base::Unretained(this)));
std::move(stream_created_callback_)
.Run(std::move(stream), {base::in_place, std::move(buffer_handle),
std::move(socket_handle)});
}
void MojoAudioOutputStream::OnStreamError(int stream_id) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
client_->OnError();
OnError();
DCHECK(deleter_callback_);
std::move(deleter_callback_).Run(/*had_error*/ true); // Deletes |this|.
}
void MojoAudioOutputStream::OnError() {
void MojoAudioOutputStream::StreamConnectionLost() {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
DCHECK(deleter_callback_);
std::move(deleter_callback_).Run(); // Deletes |this|.
std::move(deleter_callback_).Run(/*had_error*/ false); // Deletes |this|.
}
} // namespace media
......@@ -23,21 +23,22 @@ class MEDIA_MOJO_EXPORT MojoAudioOutputStream
public AudioOutputDelegate::EventHandler {
public:
using StreamCreatedCallback =
mojom::AudioOutputStreamProvider::AcquireCallback;
base::OnceCallback<void(mojom::AudioOutputStreamPtr,
media::mojom::AudioDataPipePtr)>;
using CreateDelegateCallback =
base::OnceCallback<std::unique_ptr<AudioOutputDelegate>(
AudioOutputDelegate::EventHandler*)>;
using DeleterCallback = base::OnceCallback<void(bool)>;
// |create_delegate_callback| is used to obtain an AudioOutputDelegate for the
// stream in the constructor. |stream_created_callback| is called when the
// stream has been initialized. |deleter_callback| is called when this class
// should be removed (stream ended/error). |deleter_callback| is required to
// destroy |this| synchronously.
MojoAudioOutputStream(mojom::AudioOutputStreamRequest request,
mojom::AudioOutputStreamClientPtr client,
CreateDelegateCallback create_delegate_callback,
// should be removed (stream ended/error). Its argument indicates if an error
// was encountered (false indicates that the remote end closed the stream).
// |deleter_callback| is required to destroy |this| synchronously.
MojoAudioOutputStream(CreateDelegateCallback create_delegate_callback,
StreamCreatedCallback stream_created_callback,
base::OnceClosure deleter_callback);
DeleterCallback deleter_callback);
~MojoAudioOutputStream() override;
......@@ -54,15 +55,13 @@ class MEDIA_MOJO_EXPORT MojoAudioOutputStream
std::unique_ptr<base::CancelableSyncSocket> foreign_socket) override;
void OnStreamError(int stream_id) override;
// Closes connection to client and notifies owner.
void OnError();
void StreamConnectionLost();
SEQUENCE_CHECKER(sequence_checker_);
StreamCreatedCallback stream_created_callback_;
base::OnceClosure deleter_callback_;
DeleterCallback deleter_callback_;
mojo::Binding<AudioOutputStream> binding_;
mojom::AudioOutputStreamClientPtr client_;
std::unique_ptr<AudioOutputDelegate> delegate_;
base::WeakPtrFactory<MojoAudioOutputStream> weak_factory_;
......
......@@ -23,8 +23,9 @@ MojoAudioOutputStreamProvider::MojoAudioOutputStreamProvider(
observer_binding_(observer_.get()) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
// Unretained is safe since |this| owns |binding_|.
binding_.set_connection_error_handler(base::Bind(
&MojoAudioOutputStreamProvider::OnError, base::Unretained(this)));
binding_.set_connection_error_handler(
base::BindOnce(&MojoAudioOutputStreamProvider::CleanUp,
base::Unretained(this), /*had_error*/ false));
DCHECK(create_delegate_callback_);
DCHECK(deleter_callback_);
}
......@@ -34,10 +35,8 @@ MojoAudioOutputStreamProvider::~MojoAudioOutputStreamProvider() {
}
void MojoAudioOutputStreamProvider::Acquire(
mojom::AudioOutputStreamRequest stream_request,
mojom::AudioOutputStreamClientPtr client,
const AudioParameters& params,
AcquireCallback callback) {
mojom::AudioOutputStreamProviderClientPtr provider_client) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
#if !defined(OS_ANDROID)
if (params.IsBitstreamFormat()) {
......@@ -53,29 +52,31 @@ void MojoAudioOutputStreamProvider::Acquire(
return;
}
provider_client_ = std::move(provider_client);
mojom::AudioOutputStreamObserverPtr observer_ptr;
observer_binding_.Bind(mojo::MakeRequest(&observer_ptr));
// Unretained is safe since |this| owns |audio_output_|.
audio_output_.emplace(std::move(stream_request), std::move(client),
base::BindOnce(std::move(create_delegate_callback_),
params, std::move(observer_ptr)),
std::move(callback),
base::BindOnce(&MojoAudioOutputStreamProvider::OnError,
audio_output_.emplace(
base::BindOnce(std::move(create_delegate_callback_), params,
std::move(observer_ptr)),
base::BindOnce(&mojom::AudioOutputStreamProviderClient::Created,
base::Unretained(provider_client_.get())),
base::BindOnce(&MojoAudioOutputStreamProvider::CleanUp,
base::Unretained(this)));
}
void MojoAudioOutputStreamProvider::OnError() {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
// Deletes |this|:
void MojoAudioOutputStreamProvider::CleanUp(bool had_error) {
if (had_error) {
provider_client_.ResetWithReason(
mojom::AudioOutputStreamProviderClient::kPlatformErrorDisconnectReason,
std::string());
}
std::move(deleter_callback_).Run(this);
}
void MojoAudioOutputStreamProvider::BadMessage(const std::string& error) {
mojo::ReportBadMessage(error);
if (binding_.is_bound())
binding_.Unbind();
if (observer_binding_.is_bound())
observer_binding_.Unbind();
std::move(deleter_callback_).Run(this); // deletes |this|.
}
......
......@@ -42,25 +42,25 @@ class MEDIA_MOJO_EXPORT MojoAudioOutputStreamProvider
private:
// mojom::AudioOutputStreamProvider implementation.
void Acquire(mojom::AudioOutputStreamRequest stream_request,
mojom::AudioOutputStreamClientPtr client,
void Acquire(
const AudioParameters& params,
AcquireCallback acquire_callback) override;
mojom::AudioOutputStreamProviderClientPtr provider_client) override;
// Called when |audio_output_| had an error.
void OnError();
void CleanUp(bool had_error);
// Closes mojo connections, reports a bad message, and self-destructs.
void BadMessage(const std::string& error);
SEQUENCE_CHECKER(sequence_checker_);
base::Optional<MojoAudioOutputStream> audio_output_;
mojo::Binding<AudioOutputStreamProvider> binding_;
CreateDelegateCallback create_delegate_callback_;
DeleterCallback deleter_callback_;
std::unique_ptr<mojom::AudioOutputStreamObserver> observer_;
mojo::Binding<mojom::AudioOutputStreamObserver> observer_binding_;
base::Optional<MojoAudioOutputStream> audio_output_;
mojom::AudioOutputStreamProviderClientPtr provider_client_;
DISALLOW_COPY_AND_ASSIGN(MojoAudioOutputStreamProvider);
};
......
......@@ -31,8 +31,6 @@ using testing::StrictMock;
using MockDeleter = base::MockCallback<
base::OnceCallback<void(mojom::AudioOutputStreamProvider*)>>;
void FakeAcquireCallback(mojom::AudioDataPipePtr data_pipe) {}
class FakeObserver : public mojom::AudioOutputStreamObserver {
public:
FakeObserver() = default;
......@@ -83,26 +81,22 @@ TEST(MojoAudioOutputStreamProviderTest, AcquireTwice_BadMessage) {
mojo::MakeRequest(&provider_ptr), base::BindOnce(&CreateFakeDelegate),
deleter.Get(), std::make_unique<FakeObserver>());
mojom::AudioOutputStreamPtr stream_1;
mojom::AudioOutputStreamClientPtr client_1;
mojom::AudioOutputStreamClientRequest client_request_1 =
mojom::AudioOutputStreamProviderClientPtr client_1;
mojo::MakeRequest(&client_1);
provider_ptr->Acquire(media::AudioParameters::UnavailableDeviceParams(),
std::move(client_1));
mojom::AudioOutputStreamPtr stream_2;
mojom::AudioOutputStreamClientPtr client_2;
mojom::AudioOutputStreamClientRequest client_request_2 =
mojom::AudioOutputStreamProviderClientPtr client_2;
mojo::MakeRequest(&client_2);
provider_ptr->Acquire(mojo::MakeRequest(&stream_1), std::move(client_1),
media::AudioParameters::UnavailableDeviceParams(),
base::BindOnce(&FakeAcquireCallback));
provider_ptr->Acquire(mojo::MakeRequest(&stream_2), std::move(client_2),
media::AudioParameters::UnavailableDeviceParams(),
base::BindOnce(&FakeAcquireCallback));
provider_ptr->Acquire(media::AudioParameters::UnavailableDeviceParams(),
std::move(client_2));
EXPECT_CALL(deleter, Run(provider)).WillOnce(DeleteArg<0>());
base::RunLoop().RunUntilIdle();
EXPECT_TRUE(got_bad_message);
Mock::VerifyAndClear(&deleter);
mojo::edk::SetDefaultProcessErrorCallback(mojo::edk::ProcessErrorCallback());
}
TEST(MojoAudioOutputStreamProviderTest,
......@@ -124,12 +118,9 @@ TEST(MojoAudioOutputStreamProviderTest,
mojo::MakeRequest(&provider_ptr), base::BindOnce(&CreateFakeDelegate),
deleter.Get(), std::make_unique<FakeObserver>());
mojom::AudioOutputStreamPtr stream;
mojom::AudioOutputStreamClientPtr client;
mojom::AudioOutputStreamClientRequest client_request =
mojom::AudioOutputStreamProviderClientPtr client;
mojo::MakeRequest(&client);
provider_ptr->Acquire(mojo::MakeRequest(&stream), std::move(client), params,
base::BindOnce(&FakeAcquireCallback));
provider_ptr->Acquire(params, std::move(client));
#if defined(OS_ANDROID)
base::RunLoop().RunUntilIdle();
......@@ -144,6 +135,7 @@ TEST(MojoAudioOutputStreamProviderTest,
EXPECT_TRUE(got_bad_message);
Mock::VerifyAndClear(&deleter);
#endif
mojo::edk::SetDefaultProcessErrorCallback(mojo::edk::ProcessErrorCallback());
}
} // namespace media
......@@ -11,6 +11,7 @@
#include "base/run_loop.h"
#include "base/sync_socket.h"
#include "media/audio/audio_output_controller.h"
#include "mojo/public/cpp/system/message_pipe.h"
#include "mojo/public/cpp/system/platform_handle.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
......@@ -88,14 +89,14 @@ class MockDelegateFactory {
class MockDeleter {
public:
MOCK_METHOD0(Finished, void());
MOCK_METHOD1(Finished, void(bool));
};
class MockClient : public mojom::AudioOutputStreamClient {
class MockClient {
public:
MockClient() = default;
void Initialized(mojom::AudioDataPipePtr data_pipe) {
void Initialize(mojom::AudioDataPipePtr data_pipe) {
ASSERT_TRUE(data_pipe->shared_memory.is_valid());
ASSERT_TRUE(data_pipe->socket.is_valid());
......@@ -121,8 +122,6 @@ class MockClient : public mojom::AudioOutputStreamClient {
MOCK_METHOD0(GotNotification, void());
MOCK_METHOD0(OnError, void());
private:
std::unique_ptr<base::SharedMemory> buffer_;
std::unique_ptr<base::CancelableSyncSocket> socket_;
......@@ -133,9 +132,9 @@ std::unique_ptr<AudioOutputDelegate> CreateNoDelegate(
return nullptr;
}
void NotCalled(mojom::AudioDataPipePtr data_pipe) {
EXPECT_TRUE(false) << "The StreamCreated callback was called despite the "
"test expecting it not to.";
void NotCalled(mojom::AudioOutputStreamPtr, mojom::AudioDataPipePtr) {
ADD_FAILURE() << "The StreamCreated callback was called despite the test "
"expecting it not to.";
}
} // namespace
......@@ -143,23 +142,30 @@ void NotCalled(mojom::AudioDataPipePtr data_pipe) {
class MojoAudioOutputStreamTest : public Test {
public:
MojoAudioOutputStreamTest()
: foreign_socket_(std::make_unique<TestCancelableSyncSocket>()),
client_binding_(&client_, mojo::MakeRequest(&client_ptr_)) {}
: foreign_socket_(std::make_unique<TestCancelableSyncSocket>()) {}
AudioOutputStreamPtr CreateAudioOutput() {
AudioOutputStreamPtr p;
mojom::AudioOutputStreamPtr p;
pending_stream_request_ = mojo::MakeRequest(&p);
ExpectDelegateCreation();
impl_ = std::make_unique<MojoAudioOutputStream>(
mojo::MakeRequest(&p), std::move(client_ptr_),
base::BindOnce(&MockDelegateFactory::CreateDelegate,
base::Unretained(&mock_delegate_factory_)),
base::BindOnce(&MockClient::Initialized, base::Unretained(&client_)),
base::BindOnce(&MojoAudioOutputStreamTest::CreatedStream,
base::Unretained(this)),
base::BindOnce(&MockDeleter::Finished, base::Unretained(&deleter_)));
EXPECT_TRUE(p.is_bound());
return p;
}
protected:
void CreatedStream(mojom::AudioOutputStreamPtr stream,
mojom::AudioDataPipePtr data_pipe) {
EXPECT_EQ(mojo::FuseMessagePipes(pending_stream_request_.PassMessagePipe(),
stream.PassInterface().PassHandle()),
MOJO_RESULT_OK);
client_.Initialize(std::move(data_pipe));
}
void ExpectDelegateCreation() {
delegate_ = new StrictMock<MockDelegate>();
mock_delegate_factory_.PrepareDelegateForCreation(
......@@ -180,45 +186,51 @@ class MojoAudioOutputStreamTest : public Test {
StrictMock<MockDelegateFactory> mock_delegate_factory_;
StrictMock<MockDeleter> deleter_;
StrictMock<MockClient> client_;
media::mojom::AudioOutputStreamClientPtr client_ptr_;
mojo::Binding<media::mojom::AudioOutputStreamClient> client_binding_;
mojom::AudioOutputStreamRequest pending_stream_request_;
std::unique_ptr<MojoAudioOutputStream> impl_;
};
TEST_F(MojoAudioOutputStreamTest, NoDelegate_SignalsError) {
bool deleter_called = false;
EXPECT_CALL(client_, OnError()).Times(1);
mojom::AudioOutputStreamPtr stream_ptr;
MojoAudioOutputStream stream(
mojo::MakeRequest(&stream_ptr), std::move(client_ptr_),
base::BindOnce(&CreateNoDelegate), base::BindOnce(&NotCalled),
base::BindOnce([](bool* p) { *p = true; }, &deleter_called));
EXPECT_FALSE(deleter_called)
<< "Stream shouldn't call the deleter from its constructor.";
base::BindOnce(&MockDeleter::Finished, base::Unretained(&deleter_)));
EXPECT_CALL(deleter_, Finished(true));
base::RunLoop().RunUntilIdle();
EXPECT_TRUE(deleter_called);
}
TEST_F(MojoAudioOutputStreamTest, Play_Plays) {
AudioOutputStreamPtr audio_output_ptr = CreateAudioOutput();
EXPECT_CALL(client_, GotNotification());
EXPECT_CALL(*delegate_, OnPlayStream());
delegate_event_handler_->OnStreamCreated(kStreamId, &mem_,
std::move(foreign_socket_));
audio_output_ptr->Play();
base::RunLoop().RunUntilIdle();
}
TEST_F(MojoAudioOutputStreamTest, Pause_Pauses) {
AudioOutputStreamPtr audio_output_ptr = CreateAudioOutput();
EXPECT_CALL(client_, GotNotification());
EXPECT_CALL(*delegate_, OnPauseStream());
delegate_event_handler_->OnStreamCreated(kStreamId, &mem_,
std::move(foreign_socket_));
audio_output_ptr->Pause();
base::RunLoop().RunUntilIdle();
}
TEST_F(MojoAudioOutputStreamTest, SetVolume_SetsVolume) {
AudioOutputStreamPtr audio_output_ptr = CreateAudioOutput();
EXPECT_CALL(client_, GotNotification());
EXPECT_CALL(*delegate_, OnSetVolume(kNewVolume));
delegate_event_handler_->OnStreamCreated(kStreamId, &mem_,
std::move(foreign_socket_));
audio_output_ptr->SetVolume(kNewVolume);
base::RunLoop().RunUntilIdle();
}
......@@ -253,9 +265,11 @@ TEST_F(MojoAudioOutputStreamTest, Created_NotifiesClient) {
TEST_F(MojoAudioOutputStreamTest, SetVolumeTooLarge_Error) {
AudioOutputStreamPtr audio_output_ptr = CreateAudioOutput();
EXPECT_CALL(deleter_, Finished());
EXPECT_CALL(client_, OnError()).Times(1);
EXPECT_CALL(deleter_, Finished(true));
EXPECT_CALL(client_, GotNotification());
delegate_event_handler_->OnStreamCreated(kStreamId, &mem_,
std::move(foreign_socket_));
audio_output_ptr->SetVolume(15);
base::RunLoop().RunUntilIdle();
Mock::VerifyAndClear(&deleter_);
......@@ -263,9 +277,11 @@ TEST_F(MojoAudioOutputStreamTest, SetVolumeTooLarge_Error) {
TEST_F(MojoAudioOutputStreamTest, SetVolumeNegative_Error) {
AudioOutputStreamPtr audio_output_ptr = CreateAudioOutput();
EXPECT_CALL(deleter_, Finished());
EXPECT_CALL(client_, OnError()).Times(1);
EXPECT_CALL(deleter_, Finished(true));
EXPECT_CALL(client_, GotNotification());
delegate_event_handler_->OnStreamCreated(kStreamId, &mem_,
std::move(foreign_socket_));
audio_output_ptr->SetVolume(-0.5);
base::RunLoop().RunUntilIdle();
Mock::VerifyAndClear(&deleter_);
......@@ -273,8 +289,7 @@ TEST_F(MojoAudioOutputStreamTest, SetVolumeNegative_Error) {
TEST_F(MojoAudioOutputStreamTest, DelegateErrorBeforeCreated_PropagatesError) {
AudioOutputStreamPtr audio_output_ptr = CreateAudioOutput();
EXPECT_CALL(deleter_, Finished());
EXPECT_CALL(client_, OnError()).Times(1);
EXPECT_CALL(deleter_, Finished(true));
ASSERT_NE(nullptr, delegate_event_handler_);
delegate_event_handler_->OnStreamError(kStreamId);
......@@ -286,8 +301,7 @@ TEST_F(MojoAudioOutputStreamTest, DelegateErrorBeforeCreated_PropagatesError) {
TEST_F(MojoAudioOutputStreamTest, DelegateErrorAfterCreated_PropagatesError) {
AudioOutputStreamPtr audio_output_ptr = CreateAudioOutput();
EXPECT_CALL(client_, GotNotification());
EXPECT_CALL(deleter_, Finished());
EXPECT_CALL(client_, OnError()).Times(1);
EXPECT_CALL(deleter_, Finished(true));
base::RunLoop().RunUntilIdle();
ASSERT_NE(nullptr, delegate_event_handler_);
......@@ -300,9 +314,14 @@ TEST_F(MojoAudioOutputStreamTest, DelegateErrorAfterCreated_PropagatesError) {
Mock::VerifyAndClear(&deleter_);
}
TEST_F(MojoAudioOutputStreamTest, RemoteEndGone_Error) {
TEST_F(MojoAudioOutputStreamTest, RemoteEndGone_CallsDeleter) {
AudioOutputStreamPtr audio_output_ptr = CreateAudioOutput();
EXPECT_CALL(deleter_, Finished());
EXPECT_CALL(client_, GotNotification());
EXPECT_CALL(deleter_, Finished(false));
delegate_event_handler_->OnStreamCreated(kStreamId, &mem_,
std::move(foreign_socket_));
audio_output_ptr.reset();
base::RunLoop().RunUntilIdle();
Mock::VerifyAndClear(&deleter_);
......
......@@ -22,7 +22,6 @@ OutputStream::OutputStream(
CreatedCallback created_callback,
DeleteCallback delete_callback,
media::mojom::AudioOutputStreamRequest stream_request,
media::mojom::AudioOutputStreamClientPtr client,
media::mojom::AudioOutputStreamObserverAssociatedPtr observer,
media::mojom::AudioLogPtr log,
media::AudioManager* audio_manager,
......@@ -33,7 +32,6 @@ OutputStream::OutputStream(
: foreign_socket_(),
delete_callback_(std::move(delete_callback)),
binding_(this, std::move(stream_request)),
client_(std::move(client)),
observer_(std::move(observer)),
log_(media::mojom::ThreadSafeAudioLogPtr::Create(std::move(log))),
coordinator_(coordinator),
......@@ -50,7 +48,6 @@ OutputStream::OutputStream(
&reader_),
weak_factory_(this) {
DCHECK(binding_.is_bound());
DCHECK(client_.is_bound());
DCHECK(observer_.is_bound());
DCHECK(created_callback);
DCHECK(delete_callback_);
......@@ -60,7 +57,6 @@ OutputStream::OutputStream(
base::RepeatingClosure error_handler =
base::BindRepeating(&OutputStream::OnError, base::Unretained(this));
binding_.set_connection_error_handler(error_handler);
client_.set_connection_error_handler(error_handler);
// We allow the observer to terminate the stream by closing the message pipe.
observer_.set_connection_error_handler(std::move(error_handler));
......@@ -182,8 +178,10 @@ void OutputStream::OnControllerPaused() {
void OutputStream::OnControllerError() {
DCHECK_CALLED_ON_VALID_SEQUENCE(owning_sequence_);
// Only propagate platform errors to the renderer.
client_->OnError();
// Only propagate platform errors to the observer.
observer_.ResetWithReason(
media::mojom::AudioOutputStreamObserver::kPlatformErrorDisconnectReason,
std::string());
log_->get()->OnError();
OnError();
}
......
......@@ -48,7 +48,6 @@ class OutputStream final : public media::mojom::AudioOutputStream,
OutputStream(CreatedCallback created_callback,
DeleteCallback delete_callback,
media::mojom::AudioOutputStreamRequest stream_request,
media::mojom::AudioOutputStreamClientPtr client,
media::mojom::AudioOutputStreamObserverAssociatedPtr observer,
media::mojom::AudioLogPtr log,
media::AudioManager* audio_manager,
......@@ -82,7 +81,6 @@ class OutputStream final : public media::mojom::AudioOutputStream,
base::CancelableSyncSocket foreign_socket_;
DeleteCallback delete_callback_;
mojo::Binding<AudioOutputStream> binding_;
media::mojom::AudioOutputStreamClientPtr client_;
media::mojom::AudioOutputStreamObserverAssociatedPtr observer_;
const scoped_refptr<media::mojom::ThreadSafeAudioLogPtr> log_;
GroupCoordinator* const coordinator_;
......
......@@ -52,30 +52,7 @@ class MockStream : public media::AudioOutputStream {
DISALLOW_COPY_AND_ASSIGN(MockStream);
};
class MockClient : public media::mojom::AudioOutputStreamClient {
public:
MockClient() : binding_(this) {}
media::mojom::AudioOutputStreamClientPtr MakePtr() {
DCHECK(!binding_.is_bound());
media::mojom::AudioOutputStreamClientPtr ptr;
binding_.Bind(mojo::MakeRequest(&ptr));
binding_.set_connection_error_handler(base::BindOnce(
&MockClient::BindingConnectionError, base::Unretained(this)));
return ptr;
}
void CloseBinding() { binding_.Close(); }
MOCK_METHOD0(OnError, void());
MOCK_METHOD0(BindingConnectionError, void());
private:
mojo::Binding<media::mojom::AudioOutputStreamClient> binding_;
DISALLOW_COPY_AND_ASSIGN(MockClient);
};
const uint32_t kNoDisconnectReason = 0;
class MockObserver : public media::mojom::AudioOutputStreamObserver {
public:
......@@ -85,7 +62,7 @@ class MockObserver : public media::mojom::AudioOutputStreamObserver {
DCHECK(!binding_.is_bound());
media::mojom::AudioOutputStreamObserverAssociatedPtrInfo ptr_info;
binding_.Bind(mojo::MakeRequest(&ptr_info));
binding_.set_connection_error_handler(base::BindOnce(
binding_.set_connection_error_with_reason_handler(base::BindOnce(
&MockObserver::BindingConnectionError, base::Unretained(this)));
return ptr_info;
}
......@@ -96,7 +73,8 @@ class MockObserver : public media::mojom::AudioOutputStreamObserver {
MOCK_METHOD0(DidStopPlaying, void());
MOCK_METHOD1(DidChangeAudibleState, void(bool));
MOCK_METHOD0(BindingConnectionError, void());
MOCK_METHOD2(BindingConnectionError,
void(uint32_t /*disconnect_reason*/, const std::string&));
private:
mojo::AssociatedBinding<media::mojom::AudioOutputStreamObserver> binding_;
......@@ -147,17 +125,14 @@ class TestEnvironment {
media::mojom::AudioOutputStreamPtr CreateStream() {
media::mojom::AudioOutputStreamPtr stream_ptr;
stream_factory_ptr_->CreateOutputStream(
mojo::MakeRequest(&stream_ptr), client_.MakePtr(),
observer_.MakePtrInfo(), log_.MakePtr(), "",
media::AudioParameters::UnavailableDeviceParams(),
mojo::MakeRequest(&stream_ptr), observer_.MakePtrInfo(), log_.MakePtr(),
"", media::AudioParameters::UnavailableDeviceParams(),
base::UnguessableToken::Create(), created_callback_.Get());
return stream_ptr;
}
media::MockAudioManager& audio_manager() { return audio_manager_; }
MockClient& client() { return client_; }
MockObserver& observer() { return observer_; }
MockLog& log() { return log_; }
......@@ -174,7 +149,6 @@ class TestEnvironment {
StreamFactory stream_factory_;
mojom::StreamFactoryPtr stream_factory_ptr_;
mojo::Binding<mojom::StreamFactory> stream_factory_binding_;
StrictMock<MockClient> client_;
StrictMock<MockObserver> observer_;
NiceMock<MockLog> log_;
StrictMock<MockCreatedCallback> created_callback_;
......@@ -203,8 +177,7 @@ TEST(AudioServiceOutputStreamTest, ConstructDestruct) {
EXPECT_CALL(env.log(), OnClosed());
EXPECT_CALL(mock_stream, Close());
EXPECT_CALL(env.observer(), BindingConnectionError());
EXPECT_CALL(env.client(), BindingConnectionError());
EXPECT_CALL(env.observer(), BindingConnectionError(kNoDisconnectReason, _));
stream_ptr.reset();
base::RunLoop().RunUntilIdle();
}
......@@ -228,41 +201,11 @@ TEST(AudioServiceOutputStreamTest,
Mock::VerifyAndClear(&env.created_callback());
EXPECT_CALL(mock_stream, Close());
EXPECT_CALL(env.client(), BindingConnectionError());
env.observer().CloseBinding();
base::RunLoop().RunUntilIdle();
Mock::VerifyAndClear(&mock_stream);
Mock::VerifyAndClear(&env.client());
}
TEST(AudioServiceOutputStreamTest,
ConstructStreamAndDestructClient_DestructsStream) {
TestEnvironment env;
MockStream mock_stream;
env.audio_manager().SetMakeOutputStreamCB(base::BindRepeating(
[](media::AudioOutputStream* stream, const media::AudioParameters& params,
const std::string& device_id) { return stream; },
&mock_stream));
EXPECT_CALL(env.created_callback(), Created(successfully_));
EXPECT_CALL(mock_stream, Open()).WillOnce(Return(true));
EXPECT_CALL(mock_stream, SetVolume(1));
media::mojom::AudioOutputStreamPtr stream_ptr = env.CreateStream();
base::RunLoop().RunUntilIdle();
Mock::VerifyAndClear(&mock_stream);
Mock::VerifyAndClear(&env.created_callback());
EXPECT_CALL(mock_stream, Close());
EXPECT_CALL(env.observer(), BindingConnectionError());
env.client().CloseBinding();
base::RunLoop().RunUntilIdle();
Mock::VerifyAndClear(&mock_stream);
Mock::VerifyAndClear(&env.observer());
}
TEST(AudioServiceOutputStreamTest,
......@@ -284,14 +227,12 @@ TEST(AudioServiceOutputStreamTest,
Mock::VerifyAndClear(&env.created_callback());
EXPECT_CALL(mock_stream, Close());
EXPECT_CALL(env.observer(), BindingConnectionError());
EXPECT_CALL(env.client(), BindingConnectionError());
EXPECT_CALL(env.observer(), BindingConnectionError(kNoDisconnectReason, _));
stream_ptr.reset();
base::RunLoop().RunUntilIdle();
Mock::VerifyAndClear(&mock_stream);
Mock::VerifyAndClear(&env.client());
Mock::VerifyAndClear(&env.observer());
}
......@@ -327,8 +268,7 @@ TEST(AudioServiceOutputStreamTest, Play_Plays) {
EXPECT_CALL(mock_stream, Close());
EXPECT_CALL(env.observer(), DidChangeAudibleState(false)).Times(AtMost(1));
EXPECT_CALL(env.observer(), DidStopPlaying()).Times(AtMost(1));
EXPECT_CALL(env.observer(), BindingConnectionError());
EXPECT_CALL(env.client(), BindingConnectionError());
EXPECT_CALL(env.observer(), BindingConnectionError(kNoDisconnectReason, _));
stream_ptr.reset();
base::RunLoop().RunUntilIdle();
}
......@@ -370,8 +310,7 @@ TEST(AudioServiceOutputStreamTest, PlayAndPause_PlaysAndStops) {
Mock::VerifyAndClear(&env.observer());
EXPECT_CALL(mock_stream, Close());
EXPECT_CALL(env.observer(), BindingConnectionError());
EXPECT_CALL(env.client(), BindingConnectionError());
EXPECT_CALL(env.observer(), BindingConnectionError(kNoDisconnectReason, _));
stream_ptr.reset();
base::RunLoop().RunUntilIdle();
}
......@@ -401,8 +340,7 @@ TEST(AudioServiceOutputStreamTest, SetVolume_SetsVolume) {
Mock::VerifyAndClear(&mock_stream);
EXPECT_CALL(mock_stream, Close());
EXPECT_CALL(env.observer(), BindingConnectionError());
EXPECT_CALL(env.client(), BindingConnectionError());
EXPECT_CALL(env.observer(), BindingConnectionError(kNoDisconnectReason, _));
stream_ptr.reset();
base::RunLoop().RunUntilIdle();
}
......@@ -425,8 +363,7 @@ TEST(AudioServiceOutputStreamTest, SetNegativeVolume_BadMessage) {
Mock::VerifyAndClear(&env.created_callback());
EXPECT_CALL(mock_stream, Close());
EXPECT_CALL(env.observer(), BindingConnectionError());
EXPECT_CALL(env.client(), BindingConnectionError());
EXPECT_CALL(env.observer(), BindingConnectionError(kNoDisconnectReason, _));
EXPECT_CALL(env.bad_message_callback(), Run(_));
stream_ptr->SetVolume(-0.1);
base::RunLoop().RunUntilIdle();
......@@ -450,8 +387,7 @@ TEST(AudioServiceOutputStreamTest, SetVolumeGreaterThanOne_BadMessage) {
Mock::VerifyAndClear(&env.created_callback());
EXPECT_CALL(mock_stream, Close());
EXPECT_CALL(env.observer(), BindingConnectionError());
EXPECT_CALL(env.client(), BindingConnectionError());
EXPECT_CALL(env.observer(), BindingConnectionError(kNoDisconnectReason, _));
EXPECT_CALL(env.bad_message_callback(), Run(_));
stream_ptr->SetVolume(1.1);
base::RunLoop().RunUntilIdle();
......@@ -466,12 +402,12 @@ TEST(AudioServiceOutputStreamTest,
media::mojom::AudioOutputStreamPtr stream_ptr = env.CreateStream();
EXPECT_CALL(env.created_callback(), Created(unsuccessfully_));
EXPECT_CALL(env.observer(), BindingConnectionError());
EXPECT_CALL(env.observer(),
BindingConnectionError(media::mojom::AudioOutputStreamObserver::
kPlatformErrorDisconnectReason,
_));
EXPECT_CALL(env.log(), OnError());
EXPECT_CALL(env.client(), OnError());
EXPECT_CALL(env.client(), BindingConnectionError());
base::RunLoop().RunUntilIdle();
Mock::VerifyAndClear(&env.client());
Mock::VerifyAndClear(&env.observer());
}
......@@ -486,12 +422,12 @@ TEST(AudioServiceOutputStreamTest,
media::mojom::AudioOutputStreamPtr stream_ptr = env.CreateStream();
EXPECT_CALL(env.created_callback(), Created(unsuccessfully_));
EXPECT_CALL(env.observer(), BindingConnectionError());
EXPECT_CALL(env.client(), OnError());
EXPECT_CALL(env.client(), BindingConnectionError());
EXPECT_CALL(env.observer(),
BindingConnectionError(media::mojom::AudioOutputStreamObserver::
kPlatformErrorDisconnectReason,
_));
base::RunLoop().RunUntilIdle();
Mock::VerifyAndClear(&env.client());
Mock::VerifyAndClear(&env.observer());
}
......
......@@ -50,7 +50,6 @@ interface StreamFactory {
// will later be used for muting streams or capturing them for loopback.
CreateOutputStream(
media.mojom.AudioOutputStream& stream,
media.mojom.AudioOutputStreamClient client,
associated media.mojom.AudioOutputStreamObserver observer,
media.mojom.AudioLog log,
string device_id, media.mojom.AudioParameters params,
......
......@@ -54,7 +54,6 @@ void StreamFactory::CreateInputStream(
void StreamFactory::CreateOutputStream(
media::mojom::AudioOutputStreamRequest stream_request,
media::mojom::AudioOutputStreamClientPtr client,
media::mojom::AudioOutputStreamObserverAssociatedPtrInfo observer_info,
media::mojom::AudioLogPtr log,
const std::string& output_device_id,
......@@ -72,9 +71,8 @@ void StreamFactory::CreateOutputStream(
output_streams_.insert(std::make_unique<OutputStream>(
std::move(created_callback), std::move(deleter_callback),
std::move(stream_request), std::move(client), std::move(observer),
std::move(log), audio_manager_, output_device_id, params, &coordinator_,
group_id));
std::move(stream_request), std::move(observer), std::move(log),
audio_manager_, output_device_id, params, &coordinator_, group_id));
}
void StreamFactory::BindMuter(mojom::LocalMuterAssociatedRequest request,
......
......@@ -65,7 +65,6 @@ class StreamFactory final : public mojom::StreamFactory {
void CreateOutputStream(
media::mojom::AudioOutputStreamRequest stream_request,
media::mojom::AudioOutputStreamClientPtr client,
media::mojom::AudioOutputStreamObserverAssociatedPtrInfo observer_info,
media::mojom::AudioLogPtr log,
const std::string& output_device_id,
......
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