Skip to content

Commit

Permalink
Set audio type to NO_SERVICE if the user deselect audio capture
Browse files Browse the repository at this point in the history 
Change-Id: I98988b33035bd15d407e88034b1a66cd08f703d4
Bug: 1378910
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3997116
Reviewed-by: Elad Alon <eladalon@chromium.org>
Commit-Queue: Tove Petersson <tovep@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1069082}
  • Loading branch information
@tovepet
tovepet authored and Chromium LUCI CQ committed Nov 9, 2022
1 parent 88d35cd commit 0c68101
Show file tree
Hide file tree
Showing 6 changed files with 164 additions and 49 deletions.
37 changes: 37 additions & 0 deletions chrome/browser/media/webrtc/webrtc_getdisplaymedia_browsertest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,13 @@ void RunGetDisplayMedia(content::WebContents* tab,
: "expected-error");
}

void StopAllTracks(content::WebContents* tab) {
std::string result;
EXPECT_TRUE(content::ExecuteScriptAndExtractString(
tab->GetPrimaryMainFrame(), "stopAllTracks();", &result));
EXPECT_EQ(result, "stopped");
}

void UpdateWebContentsTitle(content::WebContents* contents,
const std::u16string& title) {
content::NavigationEntry* entry =
Expand Down Expand Up @@ -1225,6 +1232,36 @@ IN_PROC_BROWSER_TEST_P(GetDisplayMediaChangeSourceBrowserTest, ChangeSource) {
url_formatter::SchemeDisplay::OMIT_HTTP_AND_HTTPS)));
}

IN_PROC_BROWSER_TEST_P(GetDisplayMediaChangeSourceBrowserTest,
ChangeSourceThenStopTracksRemovesIndicators) {
if (!ShouldShowShareThisTabInsteadButton()) {
GTEST_SKIP();
}

ASSERT_TRUE(embedded_test_server()->Start());
OpenTestPageInNewTab(kCapturedPageMain);
content::WebContents* other_tab = OpenTestPageInNewTab(kMainHtmlPage);
content::WebContents* capturing_tab = OpenTestPageInNewTab(kMainHtmlPage);

RunGetDisplayMedia(capturing_tab, GetConstraints(), /*is_fake_ui=*/false,
/*expect_success=*/true,
/*is_tab_capture=*/true);

// Click the secondary button, i.e., the "Share this tab instead" button
GetDelegate(other_tab)->Cancel();

// Wait until the capture of the other tab has started.
while (!other_tab->IsBeingCaptured()) {
base::RunLoop().RunUntilIdle();
}

ASSERT_EQ(GetInfoBarManager(capturing_tab)->infobar_count(), 1u);
StopAllTracks(capturing_tab);
do {
base::RunLoop().RunUntilIdle();
} while (GetInfoBarManager(capturing_tab)->infobar_count() > 0u);
}

IN_PROC_BROWSER_TEST_P(GetDisplayMediaChangeSourceBrowserTest,
ChangeSourceReject) {
ASSERT_TRUE(embedded_test_server()->Start());
Expand Down
10 changes: 10 additions & 0 deletions chrome/test/data/webrtc/webrtc_getdisplaymedia_test.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,16 @@
returnToTest("ended");
}
}

function stopAllTracks() {
if (audio_track) {
audio_track.stop();
}
if (video_track) {
video_track.stop();
}
returnToTest("stopped");
}
</script>
</head>
<body>
Expand Down
19 changes: 18 additions & 1 deletion content/browser/renderer_host/media/media_stream_manager.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -966,6 +966,15 @@ class MediaStreamManager::DeviceRequest {

void SetLabel(const std::string& label) { label_ = label; }

void DisableAudioSharing() {
SetAudioType(MediaStreamType::NO_SERVICE);
stream_controls_.audio.stream_type = MediaStreamType::NO_SERVICE;
stream_controls_.hotword_enabled = false;
stream_controls_.disable_local_echo = false;
stream_controls_.suppress_local_audio_playback = false;
stream_controls_.exclude_system_audio = false;
}

// The render process id that requested this stream to be generated and that
// will receive a handle to the MediaStream. This may be different from
// MediaStreamRequest::render_process_id which in the tab capture case
Expand Down Expand Up @@ -1085,7 +1094,7 @@ class MediaStreamManager::DeviceRequest {
// |MediaStreamType| type.
std::vector<TransferMap> transfer_status_map_;
MediaStreamRequestType request_type_;
const StreamControls stream_controls_;
StreamControls stream_controls_;
MediaStreamType audio_type_;
MediaStreamType video_type_;
int target_process_id_;
Expand Down Expand Up @@ -3184,6 +3193,14 @@ void MediaStreamManager::HandleAccessRequestResponse(
}
}

// If the user does not choose to share audio, the audio device is not
// added. In this case the audio type needs to be set to NO_SERVICE so that no
// audio device is added if a change-source is requested (i.e., if the
// share-this-tab-instead button is clicked). (Resolves crbug.com/1378910)
if (!found_audio) {
request->DisableAudioSharing();
}

// Check whether we've received all stream types requested.
if (!found_audio && blink::IsAudioInputMediaType(request->audio_type())) {
request->SetState(request->audio_type(), MEDIA_REQUEST_STATE_ERROR);
Expand Down
132 changes: 87 additions & 45 deletions content/browser/renderer_host/media/media_stream_manager_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@
#include "media/audio/fake_audio_manager.h"
#endif

using blink::mojom::MediaStreamType;
using blink::mojom::StreamSelectionInfo;
using blink::mojom::StreamSelectionInfoPtr;
using blink::mojom::StreamSelectionStrategy;
Expand Down Expand Up @@ -352,18 +353,24 @@ class MediaStreamManagerTest : public ::testing::Test {
base::RunLoop().RunUntilIdle();
}

void RequestAndStopGetDisplayMedia(bool request_audio) {
media_stream_manager_->UseFakeUIFactoryForTests(base::BindRepeating([]() {
return std::make_unique<FakeMediaStreamUIProxy>(
true
/*tests_use_fake_render_frame_hosts=*/);
}));

blink::StreamControls controls(request_audio /* request_audio */,
true /* request_video */);
void RequestAndStopGetDisplayMedia(bool app_requested_audio,
bool user_shared_audio) {
DCHECK(app_requested_audio || !user_shared_audio);
media_stream_manager_->UseFakeUIFactoryForTests(base::BindRepeating(
[](bool user_shared_audio) {
auto fake_ui = std::make_unique<FakeMediaStreamUIProxy>(
/*tests_use_fake_render_frame_hosts=*/true);
fake_ui->SetAudioShare(user_shared_audio);
return std::unique_ptr<FakeMediaStreamUIProxy>(std::move(fake_ui));
},
user_shared_audio));

blink::StreamControls controls(
app_requested_audio /* app_requested_audio */,
true /* request_video */);
controls.video.stream_type =
blink::mojom::MediaStreamType::DISPLAY_VIDEO_CAPTURE;
if (request_audio)
if (app_requested_audio)
controls.audio.stream_type =
blink::mojom::MediaStreamType::DISPLAY_AUDIO_CAPTURE;
const int render_process_id = 1;
Expand All @@ -374,8 +381,9 @@ class MediaStreamManagerTest : public ::testing::Test {
blink::MediaStreamDevice video_device;
blink::MediaStreamDevice audio_device;
MediaStreamManager::GenerateStreamsCallback generate_stream_callback =
base::BindOnce(GenerateStreamsCallback, &run_loop_, request_audio,
true /* request_video */, &audio_device, &video_device);
base::BindOnce(GenerateStreamsCallback, &run_loop_, app_requested_audio,
/*requst_video=*/true, &audio_device, &video_device,
user_shared_audio);
base::MockCallback<MediaStreamManager::DeviceStoppedCallback>
stopped_callback;
MediaStreamManager::DeviceChangedCallback changed_callback;
Expand All @@ -384,22 +392,33 @@ class MediaStreamManagerTest : public ::testing::Test {
MediaStreamManager::DeviceCaptureHandleChangeCallback
capture_handle_change_callback;

std::vector<blink::mojom::MediaStreamType> expected_types;
expected_types.push_back(
blink::mojom::MediaStreamType::DISPLAY_VIDEO_CAPTURE);
if (request_audio)
expected_types.push_back(
blink::mojom::MediaStreamType::DISPLAY_AUDIO_CAPTURE);
for (blink::mojom::MediaStreamType expected_type : expected_types) {
EXPECT_CALL(*media_observer_, OnMediaRequestStateChanged(
_, _, _, _, expected_type,
MEDIA_REQUEST_STATE_PENDING_APPROVAL));
EXPECT_CALL(*media_observer_,
OnMediaRequestStateChanged(_, _, _, _, expected_type,
MEDIA_REQUEST_STATE_OPENING));
EXPECT_CALL(*media_observer_,
OnMediaRequestStateChanged(
_, _, _, _, MediaStreamType::DISPLAY_VIDEO_CAPTURE,
MEDIA_REQUEST_STATE_PENDING_APPROVAL));
EXPECT_CALL(*media_observer_,
OnMediaRequestStateChanged(
_, _, _, _, MediaStreamType::DISPLAY_VIDEO_CAPTURE,
MEDIA_REQUEST_STATE_OPENING));
EXPECT_CALL(*media_observer_,
OnMediaRequestStateChanged(
_, _, _, _, MediaStreamType::DISPLAY_VIDEO_CAPTURE,
MEDIA_REQUEST_STATE_DONE));
if (app_requested_audio) {
EXPECT_CALL(*media_observer_,
OnMediaRequestStateChanged(_, _, _, _, expected_type,
MEDIA_REQUEST_STATE_DONE));
OnMediaRequestStateChanged(
_, _, _, _, MediaStreamType::DISPLAY_AUDIO_CAPTURE,
MEDIA_REQUEST_STATE_PENDING_APPROVAL));
if (user_shared_audio) {
EXPECT_CALL(*media_observer_,
OnMediaRequestStateChanged(
_, _, _, _, MediaStreamType::DISPLAY_AUDIO_CAPTURE,
MEDIA_REQUEST_STATE_OPENING));
EXPECT_CALL(*media_observer_,
OnMediaRequestStateChanged(
_, _, _, _, MediaStreamType::DISPLAY_AUDIO_CAPTURE,
MEDIA_REQUEST_STATE_DONE));
}
}
media_stream_manager_->GenerateStreams(
render_process_id, render_frame_id, requester_id, page_request_id,
Expand All @@ -414,9 +433,12 @@ class MediaStreamManagerTest : public ::testing::Test {

EXPECT_EQ(blink::mojom::MediaStreamType::DISPLAY_VIDEO_CAPTURE,
video_device.type);
if (request_audio)
if (app_requested_audio && user_shared_audio) {
EXPECT_EQ(blink::mojom::MediaStreamType::DISPLAY_AUDIO_CAPTURE,
audio_device.type);
} else {
EXPECT_EQ(blink::mojom::MediaStreamType::NO_SERVICE, audio_device.type);
}

EXPECT_CALL(
*media_observer_,
Expand All @@ -426,21 +448,23 @@ class MediaStreamManagerTest : public ::testing::Test {
media_stream_manager_->StopStreamDevice(render_process_id, render_frame_id,
requester_id, video_device.id,
video_device.session_id());
if (request_audio) {
blink::MediaStreamDevice device;
if (app_requested_audio && user_shared_audio) {
EXPECT_CALL(
*media_observer_,
OnMediaRequestStateChanged(
_, _, _, _, blink::mojom::MediaStreamType::DISPLAY_AUDIO_CAPTURE,
MEDIA_REQUEST_STATE_CLOSING));
blink::MediaStreamDevice device;
EXPECT_CALL(stopped_callback, Run(_, _))
.WillOnce(testing::SaveArg<1>(&device));
media_stream_manager_->StopStreamDevice(
render_process_id, render_frame_id, requester_id, audio_device.id,
audio_device.session_id());
EXPECT_EQ(device.type,
blink::mojom::MediaStreamType::DISPLAY_AUDIO_CAPTURE);
}
media_stream_manager_->StopStreamDevice(render_process_id, render_frame_id,
requester_id, audio_device.id,
audio_device.session_id());
EXPECT_EQ(device.type,
app_requested_audio && user_shared_audio
? blink::mojom::MediaStreamType::DISPLAY_AUDIO_CAPTURE
: blink::mojom::MediaStreamType::NO_SERVICE);
}

static void GenerateStreamsCallback(
Expand All @@ -449,13 +473,14 @@ class MediaStreamManagerTest : public ::testing::Test {
bool request_video,
blink::MediaStreamDevice* audio_device,
blink::MediaStreamDevice* video_device,
bool audio_share,
blink::mojom::MediaStreamRequestResult result,
const std::string& label,
blink::mojom::StreamDevicesSetPtr stream_devices_set,
bool pan_tilt_zoom_allowed) {
// TODO(crbug.com/1300883): Generalize to multiple streams.
DCHECK_EQ(stream_devices_set->stream_devices.size(), 1u);
if (request_audio) {
if (request_audio && audio_share) {
ASSERT_TRUE(
stream_devices_set->stream_devices[0]->audio_device.has_value());
*audio_device =
Expand Down Expand Up @@ -492,8 +517,11 @@ class MediaStreamManagerTest : public ::testing::Test {
blink::MediaStreamDevice audio_device;

MediaStreamManager::GenerateStreamsCallback generate_stream_callback =
base::BindOnce(GenerateStreamsCallback, &run_loop, true, false,
&audio_device, nullptr);
base::BindOnce(GenerateStreamsCallback, &run_loop,
/*request_audio=*/true,
/*request_video=*/false, &audio_device,
/*audio_device=*/nullptr,
/*audio_share=*/true);
MediaStreamManager::DeviceStoppedCallback stopped_callback;
MediaStreamManager::DeviceChangedCallback changed_callback;
MediaStreamManager::DeviceRequestStateChangeCallback
Expand Down Expand Up @@ -825,11 +853,20 @@ TEST_F(MediaStreamManagerTest, GenerateAndReuseStreamForAudioDevice) {
}

TEST_F(MediaStreamManagerTest, GetDisplayMediaRequestVideoOnly) {
RequestAndStopGetDisplayMedia(false /* request_audio */);
RequestAndStopGetDisplayMedia(/*app_requested_audio=*/false,
/*user_shared_audio=*/false);
}

TEST_F(MediaStreamManagerTest, GetDisplayMediaRequestAudioAndVideo) {
RequestAndStopGetDisplayMedia(true /* request_audio */);
RequestAndStopGetDisplayMedia(/*app_requested_audio=*/true,
/*user_shared_audio=*/true);
}

// The application requested audio, but the user deselected sharing of audio.
TEST_F(MediaStreamManagerTest,
GetDisplayMediaRequestAudioAndVideoNoAudioShare) {
RequestAndStopGetDisplayMedia(/*app_requested_audio=*/true,
/*user_shared_audio=*/false);
}

TEST_F(MediaStreamManagerTest, GetDisplayMediaRequestCallsUIProxy) {
Expand Down Expand Up @@ -903,8 +940,10 @@ TEST_F(MediaStreamManagerTest, DesktopCaptureDeviceStopped) {
blink::MediaStreamDevice video_device;
MediaStreamManager::GenerateStreamsCallback generate_stream_callback =
base::BindOnce(GenerateStreamsCallback, &run_loop_,
false /* request_audio */, true /* request_video */,
nullptr, &video_device);
/*request_audio=*/false,
/*request_video=*/true, /*audio_device=*/nullptr,
&video_device,
/*audio_share=*/true);
MediaStreamManager::DeviceStoppedCallback stopped_callback =
base::BindRepeating(
[](const std::string& label, const blink::MediaStreamDevice& device) {
Expand Down Expand Up @@ -962,8 +1001,10 @@ TEST_F(MediaStreamManagerTest, DesktopCaptureDeviceChanged) {
blink::MediaStreamDevice video_device;
MediaStreamManager::GenerateStreamsCallback generate_stream_callback =
base::BindOnce(GenerateStreamsCallback, &run_loop_,
false /* request_audio */, true /* request_video */,
nullptr, &video_device);
/*request_audio=*/false,
/*request_video=*/true, /*audio_device=*/nullptr,
&video_device,
/*audio_share=*/true);
MediaStreamManager::DeviceStoppedCallback stopped_callback;
MediaStreamManager::DeviceChangedCallback changed_callback =
base::BindRepeating(
Expand Down Expand Up @@ -1199,7 +1240,8 @@ class MediaStreamManagerTestForTransfers : public MediaStreamManagerTest {
blink::MediaStreamDevice audio_device;
MediaStreamManager::GenerateStreamsCallback generate_stream_callback =
base::BindOnce(GenerateStreamsCallback, &run_loop, request_audio,
request_video, &audio_device, &video_device);
request_video, &audio_device, &video_device,
/*audio_share=*/true);

media_stream_manager_->GenerateStreams(
render_process_id_, render_frame_id_, requester_id_,
Expand Down
9 changes: 8 additions & 1 deletion content/browser/renderer_host/media/media_stream_ui_proxy.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -487,7 +487,7 @@ void MediaStreamUIProxy::OnWindowId(WindowIdCallback window_id_callback,

FakeMediaStreamUIProxy::FakeMediaStreamUIProxy(
bool tests_use_fake_render_frame_hosts)
: MediaStreamUIProxy(nullptr), mic_access_(true), camera_access_(true) {
: MediaStreamUIProxy(nullptr) {
core_->tests_use_fake_render_frame_hosts_ = tests_use_fake_render_frame_hosts;
}

Expand All @@ -506,6 +506,10 @@ void FakeMediaStreamUIProxy::SetCameraAccess(bool access) {
camera_access_ = access;
}

void FakeMediaStreamUIProxy::SetAudioShare(bool audio_share) {
audio_share_ = audio_share;
}

void FakeMediaStreamUIProxy::RequestAccess(
std::unique_ptr<MediaStreamRequest> request,
ResponseCallback response_callback) {
Expand Down Expand Up @@ -559,6 +563,9 @@ void FakeMediaStreamUIProxy::RequestAccess(
devices_to_use = blink::mojom::StreamDevices();
}

if (!audio_share_) {
devices_to_use.audio_device = absl::nullopt;
}
const bool is_devices_empty = !devices_to_use.audio_device.has_value() &&
!devices_to_use.video_device.has_value();
if (is_devices_empty) {
Expand Down
6 changes: 4 additions & 2 deletions content/browser/renderer_host/media/media_stream_ui_proxy.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,7 @@ class CONTENT_EXPORT FakeMediaStreamUIProxy : public MediaStreamUIProxy {
void SetAvailableDevices(const blink::MediaStreamDevices& devices);
void SetMicAccess(bool access);
void SetCameraAccess(bool access);
void SetAudioShare(bool audio_share);

// MediaStreamUIProxy overrides.
void RequestAccess(std::unique_ptr<MediaStreamRequest> request,
Expand All @@ -175,8 +176,9 @@ class CONTENT_EXPORT FakeMediaStreamUIProxy : public MediaStreamUIProxy {
blink::MediaStreamDevices devices_;

// These are used for CheckAccess().
bool mic_access_;
bool camera_access_;
bool mic_access_ = true;
bool camera_access_ = true;
bool audio_share_ = true;
};

} // namespace content
Expand Down

0 comments on commit 0c68101

Please sign in to comment.