A11y setting: mono audio UI

A11y setting: mono audio
mainly two parts: 1) a11y checkbox 2) dbus method call and an a11y controller class

BUG=404701
TEST=accessibility_manager_browsertest.cc
Youtube speaker headphones test left and right

There is still a bug when playing reverse polarity stereo audio sound, which I will open a bug to track and discuss with cras side members. crbug/598744

Committed: https://crrev.com/533c8f6025861232be19af95fff93faff3b5fdb1
Cr-Commit-Position: refs/heads/master@{#386543}

[Qiang(Joe) Xu, 2016-03-29 00:45:04]

Description was changed from

==========
A11y setting: mono audio

BUG=404701
TEST=accessibility_manager_browsertest.cc
Youtube speaker headphones test left and right

There is still a bug when playing reverse polarity stereo audio sound.
==========

to

==========
A11y setting: mono audio
mainly two parts: 1) a11y checkbox 2) dbus method call and an a11y controller
class

BUG=404701
TEST=accessibility_manager_browsertest.cc
Youtube speaker headphones test left and right

There is still a bug when playing reverse polarity stereo audio sound, which I
will open a bug to track and discuss with cras side members.
==========

[Qiang(Joe) Xu, 2016-03-29 00:45:04]

warx@chromium.org changed reviewers:
+ jbudorick@chromium.org, mpearson@google.com, oshima@chromium.org,
stevenjb@chromium.org

[stevenjb, 2016-03-29 00:54:00]

https://codereview.chromium.org/1840913002/diff/1/DEPS
File DEPS (right):

https://codereview.chromium.org/1840913002/diff/1/DEPS#newcode390
DEPS:390: Var('chromium_git') + '/chromiumos/platform/system_api.git' + '@' +
'452060bb03235f1241f5822f326f2abbc936e656',
This should be done as a separate CL.

[stevenjb, 2016-03-29 01:05:19]

Could you separate out the DBus support (i.e. the chromeos/ and dbus/ changes)
from the UI changes? Thanks!

https://codereview.chromium.org/1840913002/diff/1/build/android/test_runner.p...
File build/android/test_runner.pydeps (left):

https://codereview.chromium.org/1840913002/diff/1/build/android/test_runner.p...
build/android/test_runner.pydeps:138:
pylib/remote/device/remote_device_uirobot_test_run.py
Intentional? If so, these changes should be in a separate CL.

https://codereview.chromium.org/1840913002/diff/1/dbus/message.cc
File dbus/message.cc (right):

https://codereview.chromium.org/1840913002/diff/1/dbus/message.cc#newcode848
dbus/message.cc:848: *doubles = NULL;
nullptr

https://codereview.chromium.org/1840913002/diff/1/dbus/message.h
File dbus/message.h (right):

https://codereview.chromium.org/1840913002/diff/1/dbus/message.h#newcode315
dbus/message.h:315: void AppendArrayOfDoubles(const double* values, size_t
length);
Add a comment here (or add a single comment for this group of similar methods
and group them together).

https://codereview.chromium.org/1840913002/diff/1/dbus/message.h#newcode420
dbus/message.h:420: bool PopArrayOfDoubles(const double** doubles, size_t*
length);
Ditto

[Qiang(Joe) Xu, 2016-03-29 01:35:10]

On 2016/03/29 01:05:19, stevenjb wrote:
> Could you separate out the DBus support (i.e. the chromeos/ and dbus/ changes)
> from the UI changes? Thanks!
> 
>
https://codereview.chromium.org/1840913002/diff/1/build/android/test_runner.p...
> File build/android/test_runner.pydeps (left):
> 
>
https://codereview.chromium.org/1840913002/diff/1/build/android/test_runner.p...
> build/android/test_runner.pydeps:138:
> pylib/remote/device/remote_device_uirobot_test_run.py
> Intentional? If so, these changes should be in a separate CL.
> 
> https://codereview.chromium.org/1840913002/diff/1/dbus/message.cc
> File dbus/message.cc (right):
> 
> https://codereview.chromium.org/1840913002/diff/1/dbus/message.cc#newcode848
> dbus/message.cc:848: *doubles = NULL;
> nullptr
> 
> https://codereview.chromium.org/1840913002/diff/1/dbus/message.h
> File dbus/message.h (right):
> 
> https://codereview.chromium.org/1840913002/diff/1/dbus/message.h#newcode315
> dbus/message.h:315: void AppendArrayOfDoubles(const double* values, size_t
> length);
> Add a comment here (or add a single comment for this group of similar methods
> and group them together).
> 
> https://codereview.chromium.org/1840913002/diff/1/dbus/message.h#newcode420
> dbus/message.h:420: bool PopArrayOfDoubles(const double** doubles, size_t*
> length);
> Ditto

OK. I can separate them. How about this one? leave it or delete?

[Qiang(Joe) Xu, 2016-03-29 01:36:51]

https://codereview.chromium.org/1840913002/diff/1/build/android/test_runner.p...
File build/android/test_runner.pydeps (left):

https://codereview.chromium.org/1840913002/diff/1/build/android/test_runner.p...
build/android/test_runner.pydeps:138:
pylib/remote/device/remote_device_uirobot_test_run.py
On 2016/03/29 01:05:19, stevenjb wrote:
> Intentional? If so, these changes should be in a separate CL.

oops, I didn't notice it contained this.
It is a presubmit error and generated by the suggested script. I don't know why
git pull cannot get it, and need me to generate it.

[stevenjb, 2016-03-29 01:41:05]

I would extract the DBus changes into a separate branch / CL, then use 'git
cl upstream' to rebase this CL on that one.


On Mon, Mar 28, 2016 at 6:35 PM, <warx@chromium.org> wrote:

> On 2016/03/29 01:05:19, stevenjb wrote:
> > Could you separate out the DBus support (i.e. the chromeos/ and dbus/
> changes)
> > from the UI changes? Thanks!
> >
> >
>
>
https://codereview.chromium.org/1840913002/diff/1/build/android/test_runner.p...
> > File build/android/test_runner.pydeps (left):
> >
> >
>
>
https://codereview.chromium.org/1840913002/diff/1/build/android/test_runner.p...
> > build/android/test_runner.pydeps:138:
> > pylib/remote/device/remote_device_uirobot_test_run.py
> > Intentional? If so, these changes should be in a separate CL.
> >
> > https://codereview.chromium.org/1840913002/diff/1/dbus/message.cc
> > File dbus/message.cc (right):
> >
> >
> https://codereview.chromium.org/1840913002/diff/1/dbus/message.cc#newcode848
> > dbus/message.cc:848: *doubles = NULL;
> > nullptr
> >
> > https://codereview.chromium.org/1840913002/diff/1/dbus/message.h
> > File dbus/message.h (right):
> >
> >
> https://codereview.chromium.org/1840913002/diff/1/dbus/message.h#newcode315
> > dbus/message.h:315: void AppendArrayOfDoubles(const double* values,
> size_t
> > length);
> > Add a comment here (or add a single comment for this group of similar
> methods
> > and group them together).
> >
> >
> https://codereview.chromium.org/1840913002/diff/1/dbus/message.h#newcode420
> > dbus/message.h:420: bool PopArrayOfDoubles(const double** doubles,
> size_t*
> > length);
> > Ditto
>
> OK. I can separate them. How about this one? leave it or delete?
>
> https://codereview.chromium.org/1840913002/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[jbudorick, 2016-03-29 13:06:54]

jbudorick@chromium.org changed reviewers:
+ agrieve@chromium.org

[jbudorick, 2016-03-29 13:08:20]

https://codereview.chromium.org/1840913002/diff/1/build/android/test_runner.p...
File build/android/test_runner.pydeps (left):

https://codereview.chromium.org/1840913002/diff/1/build/android/test_runner.p...
build/android/test_runner.pydeps:138:
pylib/remote/device/remote_device_uirobot_test_run.py
On 2016/03/29 01:36:51, Qiang (Joe) Xu wrote:
> On 2016/03/29 01:05:19, stevenjb wrote:
> > Intentional? If so, these changes should be in a separate CL.
> 
> oops, I didn't notice it contained this.
> It is a presubmit error and generated by the suggested script. I don't know
why
> git pull cannot get it, and need me to generate it.

+agrieve

These changes shouldn't be here, but the fact that the script is suggesting them
might mean the presubmit check is bugged. I'm guessing that you're not working
with an Android checkout?

[agrieve, 2016-03-29 15:09:56]

https://codereview.chromium.org/1840913002/diff/1/build/android/test_runner.p...
File build/android/test_runner.pydeps (left):

https://codereview.chromium.org/1840913002/diff/1/build/android/test_runner.p...
build/android/test_runner.pydeps:138:
pylib/remote/device/remote_device_uirobot_test_run.py
On 2016/03/29 13:08:20, jbudorick wrote:
> On 2016/03/29 01:36:51, Qiang (Joe) Xu wrote:
> > On 2016/03/29 01:05:19, stevenjb wrote:
> > > Intentional? If so, these changes should be in a separate CL.
> > 
> > oops, I didn't notice it contained this.
> > It is a presubmit error and generated by the suggested script. I don't know
> why
> > git pull cannot get it, and need me to generate it.
> 
> +agrieve
> 
> These changes shouldn't be here, but the fact that the script is suggesting
them
> might mean the presubmit check is bugged. I'm guessing that you're not working
> with an Android checkout?

Reverting the offending CL:
https://codereview.chromium.org/1839103002/

Sorry about that!

[Mark P, 2016-03-29 16:42:33]

mpearson@chromium.org changed reviewers:
+ mpearson@chromium.org

[Mark P, 2016-03-29 16:42:33]

actions.xml lgtm

[Qiang(Joe) Xu, 2016-03-29 16:43:17]

Description was changed from

==========
A11y setting: mono audio
mainly two parts: 1) a11y checkbox 2) dbus method call and an a11y controller
class

BUG=404701
TEST=accessibility_manager_browsertest.cc
Youtube speaker headphones test left and right

There is still a bug when playing reverse polarity stereo audio sound, which I
will open a bug to track and discuss with cras side members.
==========

to

==========
A11y setting: mono audio
mainly two parts: 1) a11y checkbox 2) dbus method call and an a11y controller
class

BUG=404701
TEST=accessibility_manager_browsertest.cc
Youtube speaker headphones test left and right

There is still a bug when playing reverse polarity stereo audio sound, which I
will open a bug to track and discuss with cras side members. crbug/598744
==========

[Qiang(Joe) Xu, 2016-03-29 16:43:17]

warx@chromium.org changed reviewers:
- mpearson@chromium.org

[Qiang(Joe) Xu, 2016-03-31 02:02:21]

dbus changes moved in:
https://chromium.googlesource.com/chromium/src/+/a377c049175591928ea1507ef762...

cras_audio_handler changes in:
https://chromium.googlesource.com/chromium/src/+/745246282c486ab7a03e40def351...

This issue is updated to UI checkbox only due to merge.

[Qiang(Joe) Xu, 2016-04-07 22:25:21]

warx@chromium.org changed reviewers:
- agrieve@chromium.org, jbudorick@chromium.org

[Qiang(Joe) Xu, 2016-04-07 22:25:48]

warx@chromium.org changed reviewers:
- mpearson@google.com

[Qiang(Joe) Xu, 2016-04-07 22:26:19]

The CQ bit was checked by warx@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-04-07 22:26:50]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1840913002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1840913002/40001

[commit-bot: I haz the power, 2016-04-07 23:52:26]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-04-07 23:52:27]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Qiang(Joe) Xu, 2016-04-08 16:31:56]

Patchset #4 (id:60001) has been deleted

[Qiang(Joe) Xu, 2016-04-08 16:39:44]

The CQ bit was checked by warx@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-04-08 16:40:13]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1840913002/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1840913002/100001

[commit-bot: I haz the power, 2016-04-08 16:43:14]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-04-08 16:43:16]

Dry run: Try jobs failed on following builders:
  linux_chromium_compile_dbg_32_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  ios_dbg_simulator_gn on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_dbg_simulator_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device_gn on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_gn...)
  ios_rel_device_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)
  mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_gn_rel on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_gn_r...)
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Qiang(Joe) Xu, 2016-04-08 18:14:36]

The CQ bit was checked by warx@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-04-08 18:15:04]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1840913002/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1840913002/100001

[commit-bot: I haz the power, 2016-04-08 18:18:27]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-04-08 18:18:28]

Dry run: Try jobs failed on following builders:
  ios_dbg_simulator_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device_gn on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_gn...)
  mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_gn_rel on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_gn_r...)
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Qiang(Joe) Xu, 2016-04-08 18:42:12]

The CQ bit was checked by warx@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-04-08 18:42:56]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1840913002/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1840913002/120001

[commit-bot: I haz the power, 2016-04-08 19:57:24]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-04-08 19:57:25]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[stevenjb, 2016-04-08 20:53:43]

chromeos/ lgtm

[Qiang(Joe) Xu, 2016-04-08 21:08:33]

warx@chromium.org changed reviewers:
+ sky@chromium.org

[Qiang(Joe) Xu, 2016-04-09 00:17:53]

The CQ bit was checked by warx@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-04-09 00:18:09]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1840913002/140001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1840913002/140001

[commit-bot: I haz the power, 2016-04-09 01:25:46]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-04-09 01:25:47]

Dry run: This issue passed the CQ dry run.

[Qiang(Joe) Xu, 2016-04-09 01:40:07]

On 2016/04/09 01:25:47, commit-bot: I haz the power wrote:
> Dry run: This issue passed the CQ dry run.

Fix dry-run browser_test failures. oshima/sky could you help review on ash/ and
chrome/ ? tks

[sky, 2016-04-11 15:11:08]

chrome LGTM I did not look at ash

[oshima, 2016-04-11 17:05:30]

https://codereview.chromium.org/1840913002/diff/140001/ash/shell.cc
File ash/shell.cc (right):

https://codereview.chromium.org/1840913002/diff/140001/ash/shell.cc#newcode769
ash/shell.cc:769: #endif  // defined(OS_CHROMEOS)
since you initialized very early on, can you destroy later, 
after touch_transformer_controller ?

https://codereview.chromium.org/1840913002/diff/140001/ash/system/tray_access...
File ash/system/tray_accessibility.cc (right):

https://codereview.chromium.org/1840913002/diff/140001/ash/system/tray_access...
ash/system/tray_accessibility.cc:147: mono_audio_enabled_(false),
is this used?

[Qiang(Joe) Xu, 2016-04-11 20:59:10]

https://codereview.chromium.org/1840913002/diff/140001/ash/shell.cc
File ash/shell.cc (right):

https://codereview.chromium.org/1840913002/diff/140001/ash/shell.cc#newcode769
ash/shell.cc:769: #endif  // defined(OS_CHROMEOS)
On 2016/04/11 17:05:30, oshima wrote:
> since you initialized very early on, can you destroy later, 
> after touch_transformer_controller ?

Done.

https://codereview.chromium.org/1840913002/diff/140001/ash/system/tray_access...
File ash/system/tray_accessibility.cc (right):

https://codereview.chromium.org/1840913002/diff/140001/ash/system/tray_access...
ash/system/tray_accessibility.cc:147: mono_audio_enabled_(false),
On 2016/04/11 17:05:30, oshima wrote:
> is this used?

Not used. It is now updated, deleting both mono_audio_view_ and
mono_audio_enabled_. This makes not showing a11y hand icon in tray when enabling
mono audio.

[oshima, 2016-04-11 21:03:46]

ash lgtm

[Qiang(Joe) Xu, 2016-04-11 21:29:44]

The CQ bit was checked by warx@chromium.org

[Qiang(Joe) Xu, 2016-04-11 21:29:44]

The patchset sent to the CQ was uploaded after l-g-t-m from
mpearson@chromium.org, stevenjb@chromium.org, sky@chromium.org
Link to the patchset: https://codereview.chromium.org/1840913002/#ps160001
(title: "delete tray a11y hand icon view")

[commit-bot: I haz the power, 2016-04-11 21:30:57]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1840913002/160001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1840913002/160001

[commit-bot: I haz the power, 2016-04-12 01:19:52]

Description was changed from

==========
A11y setting: mono audio
mainly two parts: 1) a11y checkbox 2) dbus method call and an a11y controller
class

BUG=404701
TEST=accessibility_manager_browsertest.cc
Youtube speaker headphones test left and right

There is still a bug when playing reverse polarity stereo audio sound, which I
will open a bug to track and discuss with cras side members. crbug/598744
==========

to

==========
A11y setting: mono audio
mainly two parts: 1) a11y checkbox 2) dbus method call and an a11y controller
class

BUG=404701
TEST=accessibility_manager_browsertest.cc
Youtube speaker headphones test left and right

There is still a bug when playing reverse polarity stereo audio sound, which I
will open a bug to track and discuss with cras side members. crbug/598744
==========

[commit-bot: I haz the power, 2016-04-12 01:19:54]

Committed patchset #8 (id:160001)

[commit-bot: I haz the power, 2016-04-12 01:21:02]

Description was changed from

==========
A11y setting: mono audio
mainly two parts: 1) a11y checkbox 2) dbus method call and an a11y controller
class

BUG=404701
TEST=accessibility_manager_browsertest.cc
Youtube speaker headphones test left and right

There is still a bug when playing reverse polarity stereo audio sound, which I
will open a bug to track and discuss with cras side members. crbug/598744
==========

to

==========
A11y setting: mono audio
mainly two parts: 1) a11y checkbox 2) dbus method call and an a11y controller
class

BUG=404701
TEST=accessibility_manager_browsertest.cc
Youtube speaker headphones test left and right

There is still a bug when playing reverse polarity stereo audio sound, which I
will open a bug to track and discuss with cras side members. crbug/598744

Committed: https://crrev.com/533c8f6025861232be19af95fff93faff3b5fdb1
Cr-Commit-Position: refs/heads/master@{#386543}
==========

[commit-bot: I haz the power, 2016-04-12 01:21:04]

Patchset 8 (id:??) landed as
https://crrev.com/533c8f6025861232be19af95fff93faff3b5fdb1
Cr-Commit-Position: refs/heads/master@{#386543}