Make remaining tests pass when running audio service OOP. |
|||||
Issue descriptionWebRtcGetUserMediaBrowserTest.GetUserMediaFailToAccessAudioDevice, some muting test WebRtcGetUserMediaBrowserTest.GetAudioStreamAndCheckMutingInitiallyUnmuted and friends, and some cast browser tests aren't passing.
,
May 17 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/432064159b1332d60a56bb92a40ef9d6e6b1af19 commit 432064159b1332d60a56bb92a40ef9d6e6b1af19 Author: Max Morin <maxmorin@chromium.org> Date: Thu May 17 09:17:17 2018 Fix GetUserMediaFailToAccessAudioDevice with out of process audio This tests tells the audio manager to fail creating an audio stream, but accessing the audio manager directly isn't possible when running out of process. Thus, a command line switch is introduced. I ran the test 100 times for each combination of (in content streams, in process audio service streams, out of process audio service streams) and (tsan, release) to ensure it's not flaky in any way, but note that running it out of process requires updating the flag whitelist in content::UtilityProcessHost (next CL, since we'll need a few flags). Bug: 835742, 843103 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 Change-Id: I8a6a659523f39a1c66c3a9ed9d37de9eedbc8265 Reviewed-on: https://chromium-review.googlesource.com/1059594 Reviewed-by: Dale Curtis <dalecurtis@chromium.org> Reviewed-by: Max Morin <maxmorin@chromium.org> Reviewed-by: Olga Sharonova <olka@chromium.org> Reviewed-by: Patrik Höglund <phoglund@chromium.org> Commit-Queue: Max Morin <maxmorin@chromium.org> Cr-Commit-Position: refs/heads/master@{#559466} [modify] https://crrev.com/432064159b1332d60a56bb92a40ef9d6e6b1af19/content/browser/webrtc/webrtc_getusermedia_browsertest.cc [modify] https://crrev.com/432064159b1332d60a56bb92a40ef9d6e6b1af19/media/audio/audio_manager_base.cc [modify] https://crrev.com/432064159b1332d60a56bb92a40ef9d6e6b1af19/media/audio/audio_manager_base.h [modify] https://crrev.com/432064159b1332d60a56bb92a40ef9d6e6b1af19/media/audio/audio_output_resampler.cc [modify] https://crrev.com/432064159b1332d60a56bb92a40ef9d6e6b1af19/media/base/media_switches.cc [modify] https://crrev.com/432064159b1332d60a56bb92a40ef9d6e6b1af19/media/base/media_switches.h
,
May 17 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/62d1b5d08068c7310f809243cd7a7e5c4d1c30b7 commit 62d1b5d08068c7310f809243cd7a7e5c4d1c30b7 Author: Max Morin <maxmorin@chromium.org> Date: Thu May 17 09:42:05 2018 Add audio switches to utility process whitelist. This CL adds the audio related switches which are used by the audio service to the whitelisted switches for utility processes. This is needed when running the audio service out of process. Also moves all the ifdefed switches to the bottom. Bug: 843103 Change-Id: I2172d02b28f5011a915c89d9509dd15ccb166950 Reviewed-on: https://chromium-review.googlesource.com/1059624 Commit-Queue: Max Morin <maxmorin@chromium.org> Reviewed-by: Kinuko Yasuda <kinuko@chromium.org> Cr-Commit-Position: refs/heads/master@{#559472} [modify] https://crrev.com/62d1b5d08068c7310f809243cd7a7e5c4d1c30b7/content/browser/utility_process_host.cc
,
May 17 2018
There are a couple of test fixes in review at https://chromium-review.googlesource.com/c/chromium/src/+/1062038. The only remaining test after that is TabCaptureApiTest.ApiTests, which Guido is currently looking at.
,
May 17 2018
In TabCaptureApiTest.ApiTests the test that specifically fails is tabIsUnmutedWhenTabCaptured() [1], which checks that when tab capture starts, the tab is unmuted.
I observed that, when run in-process, this is acomplished by the sequence:
AudioInputController::DoRecord() [2], which invokes
WebContentsAudioInputStream::Impl::Start() [3], which invokes
WebContentsAudioInputStream::Impl::UnmuteWebContentsAudio()
WebContents::SetAudioMuted()
When I run the test with the OOP service, AudioInputController::DoRecord() is never called.
[1] https://cs.chromium.org/chromium/src/chrome/test/data/extensions/api_test/tab_capture/api_tests.js?type=cs&q=api_tests.js&sq=package:chromium&g=0&l=118
,
May 17 2018
The tab is unmuted as a result of the renderer process invoking content::MojoAudioInputIPC::RecordStream()
AudioInputStreamPtr::Record()
When the audio service runs in process, the tab is unmuted in the following sequence:
media::AudioInputController::Record()
content::AudioInputDelegateImpl::OnRecordStream()
media::MojoAudioInputStream::Record()
content::AudioInputDelegateImpl::OnRecordStream()
media::AudioInputController::Record()
AudioInputController::DoRecord()
content::WebContentsAudioInputStream::Impl::Start()
WebContentsAudioInputStream::Impl::UnmuteWebContentsAudio()
content::WebContentsImpl::SetAudioMuted()
When the service runs OOP, this is handled by
audio::LoopbackStream::Record()
but it does not result in a call to content::WebContentsImpl::SetAudioMuted().
This happens in all tests in TabCaptureApiTest.ApiTests, but it only results in failure in tabIsUnmutedWhenTabCaptured(), which specifically checks that the tab is unmuted.
miu@: Can you take a look?
,
May 17 2018
,
May 17 2018
Failing tab capture test runs can be found here https://chromium-review.googlesource.com/c/chromium/src/+/1057807 (The other failures there are taken care of, CLs awaiting review)
,
May 17 2018
Looks like some of the sound/muting unit_tests fail too. Looking into it...
,
May 17 2018
Thanks! Note there is an unlanded CL with a fix for some tests https://chromium-review.googlesource.com/c/chromium/src/+/1062038
,
May 17 2018
That CL fixes SoundContentSettingObserverTest and the other observer one. So only TabCaptureApi remains (Any failures on Android/ChromeOS can be ignored)
,
May 17 2018
,
May 17 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a3bd958dc2cf2a726126eaa5420bbdb3afd281aa commit a3bd958dc2cf2a726126eaa5420bbdb3afd281aa Author: Yuri Wiitala <miu@chromium.org> Date: Thu May 17 23:14:41 2018 Fix 'forced tab unmute' behavior when using OOP Audio Service The new Audio Service uses separate infrastructure for muting and tab capture. The legacy implementation, however, did not; and there was some logic in chrome/browser/ui/tab_utils.cc mitigating that. This change conditionally allows the mute state of a tab to be toggled/updated independently of tab capture. It also adds TODOs to mark the logic that should be deleted once the Audio Service launches. Bug: 843103 , 824019 ,672469 Change-Id: Ie21f4895717b5769808b87676f723a9ed5a30af4 Reviewed-on: https://chromium-review.googlesource.com/1065065 Reviewed-by: Xiangjun Zhang <xjz@chromium.org> Commit-Queue: Yuri Wiitala <miu@chromium.org> Cr-Commit-Position: refs/heads/master@{#559727} [modify] https://crrev.com/a3bd958dc2cf2a726126eaa5420bbdb3afd281aa/chrome/browser/extensions/api/tab_capture/tab_capture_apitest.cc [modify] https://crrev.com/a3bd958dc2cf2a726126eaa5420bbdb3afd281aa/chrome/browser/ui/tabs/tab_utils.cc [modify] https://crrev.com/a3bd958dc2cf2a726126eaa5420bbdb3afd281aa/chrome/test/data/extensions/api_test/tab_capture/api_tests.js
,
May 17 2018
,
May 18 2018
\o/
,
May 18 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d5dcd11bcb43052b089561a443c8f677ceed692d commit d5dcd11bcb43052b089561a443c8f677ceed692d Author: Max Morin <maxmorin@chromium.org> Date: Fri May 18 11:23:59 2018 Fix Chrome unit tests when using the audio service. WebContentsImpl accesses ServiceManagerConnection when we're toggling muting using the audio service. This is always set in production, but in tests we need to make sure that it is initialized. Tbr: markusheintz Bug: 843103 Change-Id: Ief7819edbeed6616e9e7defb5860b422f6862f9b Reviewed-on: https://chromium-review.googlesource.com/1062038 Commit-Queue: Max Morin <maxmorin@chromium.org> Reviewed-by: Becca Hughes <beccahughes@chromium.org> Cr-Commit-Position: refs/heads/master@{#559860} [modify] https://crrev.com/d5dcd11bcb43052b089561a443c8f677ceed692d/chrome/browser/content_settings/sound_content_setting_observer_unittest.cc [modify] https://crrev.com/d5dcd11bcb43052b089561a443c8f677ceed692d/chrome/browser/media/media_engagement_contents_observer_unittest.cc
,
Jun 7 2018
[bulk-edit: disregard if N/A] Can the owner please set milestone to this bug if applicable? |
|||||
►
Sign in to add a comment |
|||||
Comment 1 by bugdroid1@chromium.org
, May 16 2018