Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(53)

Issue 1374993003: Re-land attempt 2: Add localized default audio device names. (Closed)

Created:
5 years, 2 months ago by ajm
Modified:
5 years, 2 months ago
Reviewers:
DaleCurtis
CC:
chromium-reviews, imcheng+watch_chromium.org, posciak+watch_chromium.org, avayvod+watch_chromium.org, jam, mcasas+watch_chromium.org, feature-media-reviews_chromium.org, darin-cc_chromium.org, jasonroberts+watch_google.com, xjz+watch_chromium.org, isheriff+watch_chromium.org, mlamouri+watch-media_chromium.org, miu+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Re-land attempt 2: Add localized default audio device names. Original CL here: https://codereview.chromium.org/1339183002/ First re-land attempt here: https://codereview.chromium.org/1362093002 The first attempt failed due to a an audio_manager test that expects the default device name to not be reused. This was failing on platforms with more than one class of default device (e.g. the communications device on Windows). Fix in PS#2 courtesy of guidou. PS#1 is identical to the first re-land attempt. TBR=thakis BUG=497001, 537107 Committed: https://crrev.com/3cb903b207cb69689366bf1cd5ff9e11128151dc Cr-Commit-Position: refs/heads/master@{#351395}

Patch Set 1 #

Patch Set 2 : Fix Windows test. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+322 lines, -91 lines) Patch
M chrome/app/generated_resources.grd View 1 chunk +18 lines, -0 lines 0 comments Download
M chrome/browser/chrome_browser_main.cc View 2 chunks +6 lines, -0 lines 0 comments Download
M chrome/chrome_common.gypi View 3 chunks +4 lines, -0 lines 0 comments Download
M chrome/chrome_renderer.gypi View 1 chunk +1 line, -0 lines 0 comments Download
M chrome/common/BUILD.gn View 1 chunk +1 line, -0 lines 0 comments Download
A chrome/common/media/media_resource_provider.h View 1 chunk +18 lines, -0 lines 0 comments Download
A chrome/common/media/media_resource_provider.cc View 1 chunk +36 lines, -0 lines 0 comments Download
M chrome/renderer/BUILD.gn View 1 chunk +4 lines, -1 line 0 comments Download
M chrome/renderer/DEPS View 1 chunk +1 line, -0 lines 0 comments Download
M chrome/renderer/chrome_render_process_observer.cc View 2 chunks +9 lines, -0 lines 0 comments Download
M content/browser/renderer_host/media/audio_output_device_enumerator.cc View 1 chunk +1 line, -1 line 0 comments Download
M media/audio/alsa/audio_manager_alsa.cc View 1 chunk +3 lines, -3 lines 0 comments Download
M media/audio/android/audio_android_unittest.cc View 7 chunks +18 lines, -18 lines 0 comments Download
M media/audio/android/audio_manager_android.cc View 1 chunk +9 lines, -7 lines 0 comments Download
M media/audio/audio_manager.h View 1 chunk +7 lines, -0 lines 0 comments Download
M media/audio/audio_manager.cc View 2 chunks +16 lines, -0 lines 0 comments Download
M media/audio/audio_manager_base.h View 1 chunk +6 lines, -9 lines 0 comments Download
M media/audio/audio_manager_base.cc View 1 chunk +7 lines, -6 lines 0 comments Download
M media/audio/audio_manager_unittest.cc View 2 chunks +2 lines, -4 lines 0 comments Download
M media/audio/audio_output_controller_unittest.cc View 1 chunk +1 line, -1 line 0 comments Download
M media/audio/cras/audio_manager_cras.h View 1 chunk +0 lines, -4 lines 0 comments Download
M media/audio/cras/audio_manager_cras.cc View 5 chunks +22 lines, -33 lines 0 comments Download
M media/audio/mac/audio_manager_mac.cc View 1 chunk +1 line, -1 line 0 comments Download
M media/audio/pulse/audio_manager_pulse.cc View 1 chunk +1 line, -1 line 0 comments Download
M media/audio/win/audio_manager_win.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M media/base/BUILD.gn View 2 chunks +4 lines, -0 lines 0 comments Download
A media/base/fake_media_resources.h View 1 chunk +15 lines, -0 lines 0 comments Download
A media/base/fake_media_resources.cc View 1 1 chunk +21 lines, -0 lines 0 comments Download
A media/base/media_resources.h View 1 chunk +52 lines, -0 lines 0 comments Download
A media/base/media_resources.cc View 1 chunk +29 lines, -0 lines 0 comments Download
M media/base/run_all_unittests.cc View 2 chunks +2 lines, -0 lines 0 comments Download
M media/cast/cast_testing.gypi View 1 chunk +1 line, -0 lines 0 comments Download
M media/media.gyp View 2 chunks +4 lines, -0 lines 0 comments Download

Messages

Total messages: 11 (2 generated)
ajm
Dale could have a quick glance at PS#2? Earlier approved by Tommi here: https://codereview.chromium.org/1376533003/ TBR-ing ...
5 years, 2 months ago (2015-09-29 18:07:19 UTC) #2
DaleCurtis
lgtm
5 years, 2 months ago (2015-09-29 18:43:22 UTC) #3
DaleCurtis
Did you verify this locally?
5 years, 2 months ago (2015-09-29 18:43:41 UTC) #4
ajm
On 2015/09/29 18:43:41, DaleCurtis wrote: > Did you verify this locally? I wanted to, but ...
5 years, 2 months ago (2015-09-29 18:59:02 UTC) #5
DaleCurtis
I patched it in locally for you and media_unittests passes. Is there another test I ...
5 years, 2 months ago (2015-09-29 19:40:30 UTC) #6
ajm
On 2015/09/29 19:40:30, DaleCurtis wrote: > I patched it in locally for you and media_unittests ...
5 years, 2 months ago (2015-09-29 20:51:06 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1374993003/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1374993003/20001
5 years, 2 months ago (2015-09-29 20:52:07 UTC) #9
commit-bot: I haz the power
Committed patchset #2 (id:20001)
5 years, 2 months ago (2015-09-29 21:00:07 UTC) #10
commit-bot: I haz the power
5 years, 2 months ago (2015-09-29 21:00:51 UTC) #11
Message was sent while issue was closed.
Patchset 2 (id:??) landed as
https://crrev.com/3cb903b207cb69689366bf1cd5ff9e11128151dc
Cr-Commit-Position: refs/heads/master@{#351395}

Powered by Google App Engine
This is Rietveld 408576698