Commit 4d7e06ce authored by Guido Urdaneta's avatar Guido Urdaneta Committed by Commit Bot

Add extra checks when creating a new sink in WebRTCAudioRenderer.

These changes are intended to address a crash that occurs
in WebRtcAudioRenderer::SwitchOutputDevice.
Android behavior left unmodified due to issues with the test version
of frames, but this is not an issue because setSinkId() is not supported
on Android.

Bug: 1116409
Change-Id: Icb88c32b29cb8450fed48e6961e123b3c25917af
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2355926
Commit-Queue: Guido Urdaneta <guidou@chromium.org>
Reviewed-by: default avatarMarina Ciocea <marinaciocea@chromium.org>
Cr-Commit-Position: refs/heads/master@{#800087}
parent dcd07902
......@@ -59,6 +59,7 @@ scoped_refptr<media::AudioOutputDevice> NewOutputDevice(
const base::UnguessableToken& frame_token,
const media::AudioSinkParameters& params,
base::TimeDelta auth_timeout) {
CHECK(AudioOutputIPCFactory::get());
auto device = base::MakeRefCounted<media::AudioOutputDevice>(
AudioOutputIPCFactory::get()->CreateAudioOutputIPC(frame_token),
AudioOutputIPCFactory::get()->io_task_runner(), params, auth_timeout);
......@@ -123,6 +124,10 @@ AudioDeviceFactory::NewAudioRendererSink(
blink::WebAudioDeviceSourceType source_type,
const base::UnguessableToken& frame_token,
const media::AudioSinkParameters& params) {
// Can be empty in tests on Android.
#if !defined(OS_ANDROID)
CHECK(!frame_token.is_empty());
#endif
if (factory_) {
scoped_refptr<media::AudioRendererSink> device =
factory_->CreateAudioRendererSink(source_type, frame_token, params);
......
......@@ -24,6 +24,8 @@
#include "third_party/blink/public/platform/web_string.h"
#include "third_party/blink/public/web/web_heap.h"
#include "third_party/blink/public/web/web_local_frame.h"
#include "third_party/blink/public/web/web_local_frame_client.h"
#include "third_party/blink/public/web/web_view.h"
#include "third_party/blink/renderer/platform/mediastream/media_stream_component.h"
#include "third_party/blink/renderer/platform/mediastream/media_stream_descriptor.h"
#include "third_party/blink/renderer/platform/testing/testing_platform_support.h"
......@@ -120,7 +122,24 @@ class WebRtcAudioRendererTest : public testing::Test {
}
protected:
WebRtcAudioRendererTest() : source_(new MockAudioRendererSource()) {
WebRtcAudioRendererTest()
: source_(new MockAudioRendererSource())
// Tests crash on Android if these are defined. https://crbug.com/1119689
#if !defined(OS_ANDROID)
,
web_view_(blink::WebView::Create(/*client=*/nullptr,
/*is_hidden=*/false,
/*is_inside_portal=*/false,
/*compositing_enabled=*/false,
nullptr,
mojo::NullAssociatedReceiver())),
web_local_frame_(blink::WebLocalFrame::CreateMainFrame(
web_view_,
&web_local_frame_client_,
nullptr,
base::UnguessableToken::Create()))
#endif
{
MediaStreamSourceVector dummy_components;
stream_descriptor_ = MakeGarbageCollected<MediaStreamDescriptor>(
String::FromUTF8("new stream"), dummy_components, dummy_components);
......@@ -131,14 +150,14 @@ class WebRtcAudioRendererTest : public testing::Test {
void SetupRenderer(const String& device_id) {
renderer_ = new blink::WebRtcAudioRenderer(
scheduler::GetSingleThreadTaskRunnerForTesting(), stream_descriptor_,
nullptr, base::UnguessableToken::Create(), device_id,
web_local_frame_, base::UnguessableToken::Create(), device_id,
base::RepeatingCallback<void()>());
media::AudioSinkParameters params;
EXPECT_CALL(
*audio_device_factory_platform_,
MockNewAudioRendererSink(blink::WebAudioDeviceSourceType::kWebRtc,
nullptr /*blink::WebLocalFrame*/, _))
web_local_frame_, _))
.Times(testing::AtLeast(1))
.WillRepeatedly(DoAll(SaveArg<2>(&params), InvokeWithoutArgs([&]() {
EXPECT_EQ(params.device_id, device_id.Utf8());
......@@ -189,6 +208,9 @@ class WebRtcAudioRendererTest : public testing::Test {
base::UnguessableToken::Create();
std::unique_ptr<MockAudioRendererSource> source_;
Persistent<MediaStreamDescriptor> stream_descriptor_;
WebView* web_view_ = nullptr;
WebLocalFrameClient web_local_frame_client_;
WebLocalFrame* web_local_frame_ = nullptr;
scoped_refptr<blink::WebRtcAudioRenderer> renderer_;
scoped_refptr<blink::WebMediaStreamAudioRenderer> renderer_proxy_;
};
......
......@@ -563,12 +563,23 @@ void WebRtcAudioRenderer::SwitchOutputDevice(
DCHECK_NE(state_, UNINITIALIZED);
}
auto* web_frame = source_internal_frame_->web_frame();
#if !defined(OS_ANDROID)
// Frames are allowed to be null in Android due to an issue in tests.
// In practice, this is not an issue, since Android does not support
// setSinkId(). https://crbug.com/1119689
if (!web_frame) {
SendLogMessage(String::Format("%s => (ERROR: No Frame)", __func__));
std::move(callback).Run(media::OUTPUT_DEVICE_STATUS_ERROR_INTERNAL);
return;
}
#endif
media::AudioSinkParameters sink_params(session_id_, device_id);
sink_params.processing_id = source_->GetAudioProcessingId();
scoped_refptr<media::AudioRendererSink> new_sink =
Platform::Current()->NewAudioRendererSink(
WebAudioDeviceSourceType::kWebRtc,
source_internal_frame_->web_frame(), sink_params);
WebAudioDeviceSourceType::kWebRtc, web_frame, sink_params);
media::OutputDeviceStatus status =
new_sink->GetOutputDeviceInfo().device_status();
UMA_HISTOGRAM_ENUMERATION(
......
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