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

Issue 14678004: cros: Enable new cras audio handler by default (Closed)

Created:
7 years, 7 months ago by rkc
Modified:
7 years, 7 months ago
Reviewers:
James Cook
CC:
chromium-reviews, sadrul, nkostylev+watch_chromium.org, ben+watch_chromium.org, oshima+watch_chromium.org, stevenjb+watch_chromium.org, davemoore+watch_chromium.org, jennyz, stevenjb
Visibility:
Public.

Description

cros: Enable new cras audio handler by default Add separate flag for audio device switcher menu, off by default. Picking this up from https://codereview.chromium.org/14455012/; only fixed tests. R=jamescook@chromium.org TBR=stevenjb@chromium.org,jennyz@chromium.org BUG=160311 TEST=Audio input/output/mute still works Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=197644

Patch Set 1 #

Total comments: 18

Patch Set 2 : #

Patch Set 3 : #

Total comments: 8

Patch Set 4 : #

Patch Set 5 : #

Patch Set 6 : Added mock_cras_audio_handler.cc #

Patch Set 7 : gyp ordering. #

Patch Set 8 : clang fix. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+263 lines, -52 lines) Patch
M ash/ash_switches.h View 1 chunk +2 lines, -1 line 0 comments Download
M ash/ash_switches.cc View 3 chunks +8 lines, -3 lines 0 comments Download
M ash/system/chromeos/audio/tray_audio.cc View 3 chunks +11 lines, -4 lines 0 comments Download
M ash/test/ash_test_helper.cc View 1 chunk +4 lines, -0 lines 0 comments Download
M chrome/app/generated_resources.grd View 1 chunk +6 lines, -0 lines 0 comments Download
M chrome/browser/about_flags.cc View 1 chunk +9 lines, -2 lines 0 comments Download
M chrome/browser/chromeos/chrome_browser_main_chromeos.cc View 1 3 chunks +9 lines, -7 lines 0 comments Download
M chrome/browser/chromeos/login/chrome_restart_request.cc View 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/chromeos/system/ash_system_tray_delegate.cc View 3 chunks +7 lines, -5 lines 0 comments Download
M chrome/browser/policy/policy_browsertest.cc View 1 2 3 chunks +6 lines, -1 line 0 comments Download
M chrome/browser/ui/ash/volume_controller_browsertest_chromeos.cc View 1 2 3 3 chunks +16 lines, -6 lines 0 comments Download
M chrome/browser/ui/ash/volume_controller_chromeos.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/test/base/view_event_test_base.cc View 1 2 3 chunks +10 lines, -0 lines 0 comments Download
M chromeos/audio/cras_audio_handler.h View 1 2 3 4 5 6 7 2 chunks +22 lines, -18 lines 0 comments Download
M chromeos/audio/cras_audio_handler.cc View 1 2 3 4 5 6 7 4 chunks +29 lines, -2 lines 0 comments Download
A chromeos/audio/mock_cras_audio_handler.h View 1 1 chunk +42 lines, -0 lines 0 comments Download
A chromeos/audio/mock_cras_audio_handler.cc View 1 2 3 4 5 1 chunk +76 lines, -0 lines 0 comments Download
M chromeos/chromeos.gyp View 1 2 3 4 5 6 1 chunk +2 lines, -0 lines 0 comments Download

Messages

Total messages: 22 (0 generated)
rkc
7 years, 7 months ago (2013-04-30 20:43:26 UTC) #1
James Cook
https://codereview.chromium.org/14678004/diff/1/ash/test/ash_test_helper.cc File ash/test/ash_test_helper.cc (right): https://codereview.chromium.org/14678004/diff/1/ash/test/ash_test_helper.cc#newcode49 ash/test/ash_test_helper.cc:49: // an ash::Shell with the new CrasAudioHandler. crbug.com/233266 Can ...
7 years, 7 months ago (2013-04-30 20:55:05 UTC) #2
rkc
Fixed a bunch more tests and disabled one test. https://codereview.chromium.org/14678004/diff/1/ash/test/ash_test_helper.cc File ash/test/ash_test_helper.cc (right): https://codereview.chromium.org/14678004/diff/1/ash/test/ash_test_helper.cc#newcode49 ash/test/ash_test_helper.cc:49: ...
7 years, 7 months ago (2013-04-30 23:04:57 UTC) #3
James Cook
One more round. https://codereview.chromium.org/14678004/diff/12001/chrome/browser/policy/policy_browsertest.cc File chrome/browser/policy/policy_browsertest.cc (right): https://codereview.chromium.org/14678004/diff/12001/chrome/browser/policy/policy_browsertest.cc#newcode1650 chrome/browser/policy/policy_browsertest.cc:1650: chromeos::AudioHandler::Initialize( Do you still want this ...
7 years, 7 months ago (2013-04-30 23:31:21 UTC) #4
rkc
https://codereview.chromium.org/14678004/diff/12001/chrome/browser/policy/policy_browsertest.cc File chrome/browser/policy/policy_browsertest.cc (right): https://codereview.chromium.org/14678004/diff/12001/chrome/browser/policy/policy_browsertest.cc#newcode1650 chrome/browser/policy/policy_browsertest.cc:1650: chromeos::AudioHandler::Initialize( On 2013/04/30 23:31:21, James Cook (Chromium) wrote: > ...
7 years, 7 months ago (2013-04-30 23:36:37 UTC) #5
James Cook
LGTM assuming audio_pref_handler_ fix added OK to TBR stevenjb/jennyz if you need OWNERS, since they ...
7 years, 7 months ago (2013-04-30 23:50:33 UTC) #6
James Cook
On 2013/04/30 23:50:33, James Cook (Chromium) wrote: > LGTM assuming audio_pref_handler_ fix added > > ...
7 years, 7 months ago (2013-04-30 23:51:22 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rkc@chromium.org/14678004/25002
7 years, 7 months ago (2013-04-30 23:54:46 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rkc@chromium.org/14678004/31003
7 years, 7 months ago (2013-05-01 00:12:17 UTC) #9
commit-bot: I haz the power
Sorry for I got bad news for ya. Compile failed with a clobber build on ...
7 years, 7 months ago (2013-05-01 00:31:13 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rkc@chromium.org/14678004/41001
7 years, 7 months ago (2013-05-01 01:37:51 UTC) #11
commit-bot: I haz the power
Retried try job too often on linux_clang for step(s) compile http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_clang&number=103508
7 years, 7 months ago (2013-05-01 02:15:30 UTC) #12
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rkc@chromium.org/14678004/41001
7 years, 7 months ago (2013-05-01 02:59:48 UTC) #13
commit-bot: I haz the power
Retried try job too often on linux_clang for step(s) compile http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_clang&number=103529
7 years, 7 months ago (2013-05-01 03:27:49 UTC) #14
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rkc@chromium.org/14678004/41001
7 years, 7 months ago (2013-05-01 07:28:47 UTC) #15
commit-bot: I haz the power
Retried try job too often on win7_aura for step(s) content_browsertests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win7_aura&number=35346
7 years, 7 months ago (2013-05-01 09:12:37 UTC) #16
James Cook
On 2013/05/01 09:12:37, I haz the power (commit-bot) wrote: > Retried try job too often ...
7 years, 7 months ago (2013-05-01 15:31:10 UTC) #17
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rkc@chromium.org/14678004/41001
7 years, 7 months ago (2013-05-01 15:31:24 UTC) #18
commit-bot: I haz the power
Change committed as 197644
7 years, 7 months ago (2013-05-01 17:06:36 UTC) #19
bshe
On 2013/05/01 17:06:36, I haz the power (commit-bot) wrote: > Change committed as 197644 It ...
7 years, 7 months ago (2013-05-01 22:43:10 UTC) #20
James Cook
Please file a bug for rkc For now you can probably work around it by ...
7 years, 7 months ago (2013-05-01 23:12:15 UTC) #21
rkc
7 years, 7 months ago (2013-05-01 23:14:17 UTC) #22
I'll take it - will probably not be able to get to it before next week
though (since we already do have a workaround).


On Wed, May 1, 2013 at 4:12 PM, <jamescook@chromium.org> wrote:

> Please file a bug for rkc
>
> For now you can probably work around it by passing
> --ash-disable-new-audio-**handler to ash_shell
>
>
>
https://chromiumcodereview.**appspot.com/14678004/<https://chromiumcodereview...
>

Powered by Google App Engine
This is Rietveld 408576698