Commit d356b167 authored by Mario Sanchez Prada's avatar Mario Sanchez Prada Committed by Commit Bot

Migrate references of media::mojom::AudioOutputStream to new Mojo types

Convert both the implementation and clients in the browser and renderer
processes for the media.mojom.AudioOutputStream interface, and adapt
unit tests.

Bug: 955171
Change-Id: I2a5fb08b9ff0b156385551d9a936e68834215276
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1803146
Commit-Queue: Mario Sanchez Prada <mario@igalia.com>
Reviewed-by: default avatarTommi <tommi@chromium.org>
Reviewed-by: default avatarOksana Zhuravlova <oksamyt@chromium.org>
Reviewed-by: default avatarYuri Wiitala <miu@chromium.org>
Reviewed-by: default avatarDominick Ng <dominickn@chromium.org>
Cr-Commit-Position: refs/heads/master@{#697132}
parent 4810ece7
......@@ -175,8 +175,7 @@ void AudioOutputStreamBroker::StreamCreated(
return;
}
client_->Created(media::mojom::AudioOutputStreamPtr(std::move(stream)),
std::move(data_pipe));
client_->Created(std::move(stream), std::move(data_pipe));
}
void AudioOutputStreamBroker::ObserverBindingLost(
......
......@@ -19,6 +19,8 @@
#include "media/mojo/mojom/audio_output_stream.mojom.h"
#include "mojo/public/cpp/bindings/binding.h"
#include "mojo/public/cpp/bindings/interface_request.h"
#include "mojo/public/cpp/bindings/pending_receiver.h"
#include "mojo/public/cpp/bindings/pending_remote.h"
#include "mojo/public/cpp/bindings/remote.h"
#include "mojo/public/cpp/system/buffer.h"
#include "mojo/public/cpp/system/platform_handle.h"
......@@ -54,7 +56,7 @@ class MockAudioOutputStreamProviderClient
MockAudioOutputStreamProviderClient() : binding_(this) {}
~MockAudioOutputStreamProviderClient() override {}
void Created(media::mojom::AudioOutputStreamPtr,
void Created(mojo::PendingRemote<media::mojom::AudioOutputStream>,
media::mojom::ReadWriteAudioDataPipePtr) override {
OnCreated();
}
......@@ -96,7 +98,7 @@ class MockStreamFactory : public audio::FakeStreamFactory {
group_id(group_id) {}
bool requested = false;
media::mojom::AudioOutputStreamRequest stream_request;
mojo::PendingReceiver<media::mojom::AudioOutputStream> stream_receiver;
media::mojom::AudioOutputStreamObserverAssociatedPtrInfo observer_info;
media::mojom::AudioLogPtr log;
const std::string output_device_id;
......@@ -126,7 +128,7 @@ class MockStreamFactory : public audio::FakeStreamFactory {
EXPECT_TRUE(stream_request_data_->params.Equals(params));
EXPECT_EQ(stream_request_data_->group_id, group_id);
stream_request_data_->requested = true;
stream_request_data_->stream_request = std::move(stream_receiver);
stream_request_data_->stream_receiver = std::move(stream_receiver);
stream_request_data_->observer_info = std::move(observer);
stream_request_data_->log.Bind(std ::move(log));
stream_request_data_->created_callback = std::move(created_callback);
......
......@@ -20,7 +20,9 @@
#include "media/base/audio_parameters.h"
#include "media/mojo/mojom/audio_data_pipe.mojom.h"
#include "mojo/public/cpp/bindings/binding.h"
#include "mojo/public/cpp/bindings/pending_remote.h"
#include "mojo/public/cpp/bindings/receiver.h"
#include "mojo/public/cpp/bindings/remote.h"
#include "mojo/public/cpp/system/platform_handle.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
......@@ -30,9 +32,6 @@ namespace content {
namespace {
using testing::Test;
using AudioOutputStream = media::mojom::AudioOutputStream;
using AudioOutputStreamPtr = mojo::InterfacePtr<AudioOutputStream>;
using AudioOutputStreamRequest = mojo::InterfaceRequest<AudioOutputStream>;
using AudioOutputStreamProviderClient =
media::mojom::AudioOutputStreamProviderClient;
using AudioOutputStreamProviderClientPtr =
......@@ -174,10 +173,11 @@ class MockClient : public AudioOutputStreamProviderClient {
return p;
}
void Created(AudioOutputStreamPtr stream,
void Created(mojo::PendingRemote<media::mojom::AudioOutputStream> stream,
media::mojom::ReadWriteAudioDataPipePtr data_pipe) override {
was_called_ = true;
stream_ = std::move(stream);
stream_.reset();
stream_.Bind(std::move(stream));
}
bool was_called() { return was_called_; }
......@@ -186,7 +186,7 @@ class MockClient : public AudioOutputStreamProviderClient {
private:
mojo::Binding<AudioOutputStreamProviderClient> provider_client_binding_;
AudioOutputStreamPtr stream_;
mojo::Remote<media::mojom::AudioOutputStream> stream_;
bool was_called_ = false;
DISALLOW_COPY_AND_ASSIGN(MockClient);
......
......@@ -228,14 +228,15 @@ void MojoAudioOutputIPC::ReceivedDeviceAuthorization(
}
void MojoAudioOutputIPC::Created(
media::mojom::AudioOutputStreamPtr stream,
mojo::PendingRemote<media::mojom::AudioOutputStream> pending_stream,
media::mojom::ReadWriteAudioDataPipePtr 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);
stream_.reset();
stream_.Bind(std::move(pending_stream));
base::PlatformFile socket_handle;
auto result =
......
......@@ -19,6 +19,8 @@
#include "media/mojo/mojom/audio_data_pipe.mojom.h"
#include "media/mojo/mojom/audio_output_stream.mojom.h"
#include "mojo/public/cpp/bindings/binding.h"
#include "mojo/public/cpp/bindings/pending_remote.h"
#include "mojo/public/cpp/bindings/remote.h"
namespace content {
......@@ -55,7 +57,7 @@ class CONTENT_EXPORT MojoAudioOutputIPC
void SetVolume(double volume) override;
// media::mojom::AudioOutputStreamProviderClient implementation.
void Created(media::mojom::AudioOutputStreamPtr stream,
void Created(mojo::PendingRemote<media::mojom::AudioOutputStream> stream,
media::mojom::ReadWriteAudioDataPipePtr data_pipe) override;
private:
......@@ -93,7 +95,7 @@ class CONTENT_EXPORT MojoAudioOutputIPC
mojo::Binding<media::mojom::AudioOutputStreamProviderClient> binding_;
media::mojom::AudioOutputStreamProviderPtr stream_provider_;
media::mojom::AudioOutputStreamPtr stream_;
mojo::Remote<media::mojom::AudioOutputStream> stream_;
media::AudioOutputIPCDelegate* delegate_ = nullptr;
scoped_refptr<base::SingleThreadTaskRunner> io_task_runner_;
......
......@@ -17,6 +17,7 @@
#include "media/audio/audio_device_description.h"
#include "media/base/audio_parameters.h"
#include "mojo/public/cpp/bindings/binding.h"
#include "mojo/public/cpp/bindings/pending_remote.h"
#include "mojo/public/cpp/bindings/receiver.h"
#include "mojo/public/cpp/bindings/remote.h"
#include "mojo/public/cpp/system/platform_handle.h"
......@@ -56,23 +57,24 @@ class TestStreamProvider : public media::mojom::AudioOutputStreamProvider {
~TestStreamProvider() override {
// If we expected a stream to be acquired, make sure it is so.
if (stream_)
EXPECT_TRUE(binding_);
EXPECT_TRUE(receiver_);
}
void Acquire(
const media::AudioParameters& params,
media::mojom::AudioOutputStreamProviderClientPtr provider_client,
const base::Optional<base::UnguessableToken>& processing_id) override {
EXPECT_EQ(binding_, base::nullopt);
EXPECT_EQ(receiver_, base::nullopt);
EXPECT_NE(stream_, nullptr);
std::swap(provider_client, provider_client_);
media::mojom::AudioOutputStreamPtr stream_ptr;
binding_.emplace(stream_, mojo::MakeRequest(&stream_ptr));
mojo::PendingRemote<media::mojom::AudioOutputStream> stream_pending_remote;
receiver_.emplace(stream_,
stream_pending_remote.InitWithNewPipeAndPassReceiver());
base::CancelableSyncSocket foreign_socket;
EXPECT_TRUE(
base::CancelableSyncSocket::CreatePair(&socket_, &foreign_socket));
provider_client_->Created(
std::move(stream_ptr),
std::move(stream_pending_remote),
{base::in_place, base::UnsafeSharedMemoryRegion::Create(kMemoryLength),
mojo::WrapPlatformFile(foreign_socket.Release())});
}
......@@ -87,7 +89,7 @@ class TestStreamProvider : public media::mojom::AudioOutputStreamProvider {
private:
media::mojom::AudioOutputStream* stream_;
media::mojom::AudioOutputStreamProviderClientPtr provider_client_;
base::Optional<mojo::Binding<media::mojom::AudioOutputStream>> binding_;
base::Optional<mojo::Receiver<media::mojom::AudioOutputStream>> receiver_;
base::CancelableSyncSocket socket_;
};
......
......@@ -85,5 +85,6 @@ interface AudioOutputStreamProviderClient {
// 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, ReadWriteAudioDataPipe data_pipe);
Created(pending_remote<AudioOutputStream> stream,
ReadWriteAudioDataPipe data_pipe);
};
......@@ -21,8 +21,7 @@ MojoAudioOutputStream::MojoAudioOutputStream(
StreamCreatedCallback stream_created_callback,
DeleterCallback deleter_callback)
: stream_created_callback_(std::move(stream_created_callback)),
deleter_callback_(std::move(deleter_callback)),
binding_(this) {
deleter_callback_(std::move(deleter_callback)) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
DCHECK(stream_created_callback_);
DCHECK(deleter_callback_);
......@@ -85,15 +84,16 @@ void MojoAudioOutputStream::OnStreamCreated(
DCHECK(socket_handle.is_valid());
mojom::AudioOutputStreamPtr stream;
binding_.Bind(mojo::MakeRequest(&stream));
// |this| owns |binding_| so unretained is safe.
binding_.set_connection_error_handler(base::BindOnce(
mojo::PendingRemote<mojom::AudioOutputStream> pending_stream;
receiver_.Bind(pending_stream.InitWithNewPipeAndPassReceiver());
// |this| owns |receiver_| so unretained is safe.
receiver_.set_disconnect_handler(base::BindOnce(
&MojoAudioOutputStream::StreamConnectionLost, base::Unretained(this)));
std::move(stream_created_callback_)
.Run(std::move(stream), {base::in_place, std::move(shared_memory_region),
std::move(socket_handle)});
.Run(std::move(pending_stream),
{base::in_place, std::move(shared_memory_region),
std::move(socket_handle)});
}
void MojoAudioOutputStream::OnStreamError(int stream_id) {
......
......@@ -12,7 +12,8 @@
#include "media/audio/audio_output_delegate.h"
#include "media/mojo/mojom/audio_output_stream.mojom.h"
#include "media/mojo/services/media_mojo_export.h"
#include "mojo/public/cpp/bindings/binding.h"
#include "mojo/public/cpp/bindings/pending_remote.h"
#include "mojo/public/cpp/bindings/receiver.h"
namespace media {
......@@ -23,7 +24,7 @@ class MEDIA_MOJO_EXPORT MojoAudioOutputStream
public AudioOutputDelegate::EventHandler {
public:
using StreamCreatedCallback =
base::OnceCallback<void(mojom::AudioOutputStreamPtr,
base::OnceCallback<void(mojo::PendingRemote<mojom::AudioOutputStream>,
media::mojom::ReadWriteAudioDataPipePtr)>;
using CreateDelegateCallback =
base::OnceCallback<std::unique_ptr<AudioOutputDelegate>(
......@@ -62,7 +63,7 @@ class MEDIA_MOJO_EXPORT MojoAudioOutputStream
StreamCreatedCallback stream_created_callback_;
DeleterCallback deleter_callback_;
mojo::Binding<AudioOutputStream> binding_;
mojo::Receiver<AudioOutputStream> receiver_{this};
std::unique_ptr<AudioOutputDelegate> delegate_;
base::WeakPtrFactory<MojoAudioOutputStream> weak_factory_{this};
......
......@@ -13,6 +13,9 @@
#include "base/test/task_environment.h"
#include "media/audio/audio_output_controller.h"
#include "media/mojo/mojom/audio_data_pipe.mojom.h"
#include "mojo/public/cpp/bindings/pending_receiver.h"
#include "mojo/public/cpp/bindings/pending_remote.h"
#include "mojo/public/cpp/bindings/remote.h"
#include "mojo/public/cpp/system/message_pipe.h"
#include "mojo/public/cpp/system/platform_handle.h"
#include "testing/gmock/include/gmock/gmock.h"
......@@ -34,8 +37,6 @@ using testing::Return;
using testing::SaveArg;
using testing::StrictMock;
using testing::Test;
using AudioOutputStream = mojom::AudioOutputStream;
using AudioOutputStreamPtr = mojo::InterfacePtr<AudioOutputStream>;
class TestCancelableSyncSocket : public base::CancelableSyncSocket {
public:
......@@ -125,7 +126,8 @@ std::unique_ptr<AudioOutputDelegate> CreateNoDelegate(
return nullptr;
}
void NotCalled(mojom::AudioOutputStreamPtr, mojom::ReadWriteAudioDataPipePtr) {
void NotCalled(mojo::PendingRemote<mojom::AudioOutputStream>,
mojom::ReadWriteAudioDataPipePtr) {
ADD_FAILURE() << "The StreamCreated callback was called despite the test "
"expecting it not to.";
}
......@@ -137,9 +139,9 @@ class MojoAudioOutputStreamTest : public Test {
MojoAudioOutputStreamTest()
: foreign_socket_(std::make_unique<TestCancelableSyncSocket>()) {}
AudioOutputStreamPtr CreateAudioOutput() {
mojom::AudioOutputStreamPtr p;
pending_stream_request_ = mojo::MakeRequest(&p);
mojo::Remote<mojom::AudioOutputStream> CreateAudioOutput() {
mojo::Remote<mojom::AudioOutputStream> remote;
pending_stream_receiver = remote.BindNewPipeAndPassReceiver();
ExpectDelegateCreation();
impl_ = std::make_unique<MojoAudioOutputStream>(
base::BindOnce(&MockDelegateFactory::CreateDelegate,
......@@ -147,14 +149,14 @@ class MojoAudioOutputStreamTest : public Test {
base::BindOnce(&MojoAudioOutputStreamTest::CreatedStream,
base::Unretained(this)),
base::BindOnce(&MockDeleter::Finished, base::Unretained(&deleter_)));
return p;
return remote;
}
protected:
void CreatedStream(mojom::AudioOutputStreamPtr stream,
void CreatedStream(mojo::PendingRemote<mojom::AudioOutputStream> stream,
mojom::ReadWriteAudioDataPipePtr data_pipe) {
EXPECT_EQ(mojo::FuseMessagePipes(pending_stream_request_.PassMessagePipe(),
stream.PassInterface().PassHandle()),
EXPECT_EQ(mojo::FuseMessagePipes(pending_stream_receiver.PassPipe(),
stream.PassPipe()),
MOJO_RESULT_OK);
client_.Initialize(std::move(data_pipe));
}
......@@ -180,12 +182,11 @@ class MojoAudioOutputStreamTest : public Test {
StrictMock<MockDelegateFactory> mock_delegate_factory_;
StrictMock<MockDeleter> deleter_;
StrictMock<MockClient> client_;
mojom::AudioOutputStreamRequest pending_stream_request_;
mojo::PendingReceiver<mojom::AudioOutputStream> pending_stream_receiver;
std::unique_ptr<MojoAudioOutputStream> impl_;
};
TEST_F(MojoAudioOutputStreamTest, NoDelegate_SignalsError) {
mojom::AudioOutputStreamPtr stream_ptr;
MojoAudioOutputStream stream(
base::BindOnce(&CreateNoDelegate), base::BindOnce(&NotCalled),
base::BindOnce(&MockDeleter::Finished, base::Unretained(&deleter_)));
......@@ -194,54 +195,59 @@ TEST_F(MojoAudioOutputStreamTest, NoDelegate_SignalsError) {
}
TEST_F(MojoAudioOutputStreamTest, Play_Plays) {
AudioOutputStreamPtr audio_output_ptr = CreateAudioOutput();
mojo::Remote<mojom::AudioOutputStream> audio_output_remote =
CreateAudioOutput();
EXPECT_CALL(client_, GotNotification());
EXPECT_CALL(*delegate_, OnPlayStream());
delegate_event_handler_->OnStreamCreated(kStreamId, std::move(mem_),
std::move(foreign_socket_));
audio_output_ptr->Play();
audio_output_remote->Play();
base::RunLoop().RunUntilIdle();
}
TEST_F(MojoAudioOutputStreamTest, Pause_Pauses) {
AudioOutputStreamPtr audio_output_ptr = CreateAudioOutput();
mojo::Remote<mojom::AudioOutputStream> audio_output_remote =
CreateAudioOutput();
EXPECT_CALL(client_, GotNotification());
EXPECT_CALL(*delegate_, OnPauseStream());
delegate_event_handler_->OnStreamCreated(kStreamId, std::move(mem_),
std::move(foreign_socket_));
audio_output_ptr->Pause();
audio_output_remote->Pause();
base::RunLoop().RunUntilIdle();
}
TEST_F(MojoAudioOutputStreamTest, SetVolume_SetsVolume) {
AudioOutputStreamPtr audio_output_ptr = CreateAudioOutput();
mojo::Remote<mojom::AudioOutputStream> audio_output_remote =
CreateAudioOutput();
EXPECT_CALL(client_, GotNotification());
EXPECT_CALL(*delegate_, OnSetVolume(kNewVolume));
delegate_event_handler_->OnStreamCreated(kStreamId, std::move(mem_),
std::move(foreign_socket_));
audio_output_ptr->SetVolume(kNewVolume);
audio_output_remote->SetVolume(kNewVolume);
base::RunLoop().RunUntilIdle();
}
TEST_F(MojoAudioOutputStreamTest, Flush_FlushesStream) {
AudioOutputStreamPtr audio_output_ptr = CreateAudioOutput();
mojo::Remote<mojom::AudioOutputStream> audio_output_remote =
CreateAudioOutput();
EXPECT_CALL(client_, GotNotification());
EXPECT_CALL(*delegate_, OnFlushStream());
delegate_event_handler_->OnStreamCreated(kStreamId, std::move(mem_),
std::move(foreign_socket_));
audio_output_ptr->Flush();
audio_output_remote->Flush();
base::RunLoop().RunUntilIdle();
}
TEST_F(MojoAudioOutputStreamTest, DestructWithCallPending_Safe) {
AudioOutputStreamPtr audio_output_ptr = CreateAudioOutput();
mojo::Remote<mojom::AudioOutputStream> audio_output_remote =
CreateAudioOutput();
EXPECT_CALL(client_, GotNotification());
base::RunLoop().RunUntilIdle();
......@@ -249,13 +255,14 @@ TEST_F(MojoAudioOutputStreamTest, DestructWithCallPending_Safe) {
foreign_socket_->ExpectOwnershipTransfer();
delegate_event_handler_->OnStreamCreated(kStreamId, std::move(mem_),
std::move(foreign_socket_));
audio_output_ptr->Play();
audio_output_remote->Play();
impl_.reset();
base::RunLoop().RunUntilIdle();
}
TEST_F(MojoAudioOutputStreamTest, Created_NotifiesClient) {
AudioOutputStreamPtr audio_output_ptr = CreateAudioOutput();
mojo::Remote<mojom::AudioOutputStream> audio_output_remote =
CreateAudioOutput();
base::RunLoop().RunUntilIdle();
EXPECT_CALL(client_, GotNotification());
......@@ -269,31 +276,34 @@ TEST_F(MojoAudioOutputStreamTest, Created_NotifiesClient) {
}
TEST_F(MojoAudioOutputStreamTest, SetVolumeTooLarge_Error) {
AudioOutputStreamPtr audio_output_ptr = CreateAudioOutput();
mojo::Remote<mojom::AudioOutputStream> audio_output_remote =
CreateAudioOutput();
EXPECT_CALL(deleter_, Finished(true));
EXPECT_CALL(client_, GotNotification());
delegate_event_handler_->OnStreamCreated(kStreamId, std::move(mem_),
std::move(foreign_socket_));
audio_output_ptr->SetVolume(15);
audio_output_remote->SetVolume(15);
base::RunLoop().RunUntilIdle();
Mock::VerifyAndClear(&deleter_);
}
TEST_F(MojoAudioOutputStreamTest, SetVolumeNegative_Error) {
AudioOutputStreamPtr audio_output_ptr = CreateAudioOutput();
mojo::Remote<mojom::AudioOutputStream> audio_output_remote =
CreateAudioOutput();
EXPECT_CALL(deleter_, Finished(true));
EXPECT_CALL(client_, GotNotification());
delegate_event_handler_->OnStreamCreated(kStreamId, std::move(mem_),
std::move(foreign_socket_));
audio_output_ptr->SetVolume(-0.5);
audio_output_remote->SetVolume(-0.5);
base::RunLoop().RunUntilIdle();
Mock::VerifyAndClear(&deleter_);
}
TEST_F(MojoAudioOutputStreamTest, DelegateErrorBeforeCreated_PropagatesError) {
AudioOutputStreamPtr audio_output_ptr = CreateAudioOutput();
mojo::Remote<mojom::AudioOutputStream> audio_output_remote =
CreateAudioOutput();
EXPECT_CALL(deleter_, Finished(true));
ASSERT_NE(nullptr, delegate_event_handler_);
......@@ -304,7 +314,8 @@ TEST_F(MojoAudioOutputStreamTest, DelegateErrorBeforeCreated_PropagatesError) {
}
TEST_F(MojoAudioOutputStreamTest, DelegateErrorAfterCreated_PropagatesError) {
AudioOutputStreamPtr audio_output_ptr = CreateAudioOutput();
mojo::Remote<mojom::AudioOutputStream> audio_output_remote =
CreateAudioOutput();
EXPECT_CALL(client_, GotNotification());
EXPECT_CALL(deleter_, Finished(true));
base::RunLoop().RunUntilIdle();
......@@ -320,14 +331,15 @@ TEST_F(MojoAudioOutputStreamTest, DelegateErrorAfterCreated_PropagatesError) {
}
TEST_F(MojoAudioOutputStreamTest, RemoteEndGone_CallsDeleter) {
AudioOutputStreamPtr audio_output_ptr = CreateAudioOutput();
mojo::Remote<mojom::AudioOutputStream> audio_output_remote =
CreateAudioOutput();
EXPECT_CALL(client_, GotNotification());
EXPECT_CALL(deleter_, Finished(false));
delegate_event_handler_->OnStreamCreated(kStreamId, std::move(mem_),
std::move(foreign_socket_));
audio_output_ptr.reset();
audio_output_remote.reset();
base::RunLoop().RunUntilIdle();
Mock::VerifyAndClear(&deleter_);
}
......
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