mash: Have chrome set itself as a controller interface for changing volume

mash: Have chrome set itself as a controller interface for changing volume

This eliminates a case where ash was directly connecting out to Chrome.

* Add ash::mojom::AcceleratorController interface and use to set the
ash::mojom::VolumeController
* Rename volume controller files to match class names. chrome/browser/ui/ash
is chromeos-only these days so the _chromeos filenames aren't needed.
* Add ash_util::GetAshServiceName() to return "ash" vs. "content_browser"
based on mash vs. classic ash

BUG=670789
TEST=ash_unittests, browser_tests, manually change volume with F9/F10
TBR=xiyuan@chromium.org

Committed: https://crrev.com/7e0a04e949287e6ea20e5d2be9fe8a168ac0ae7b
Cr-Commit-Position: refs/heads/master@{#436766}

[James Cook, 2016-12-02 23:46:16]

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

[James Cook, 2016-12-02 23:46:27]

jamescook@chromium.org changed reviewers:
+ msw@chromium.org

[James Cook, 2016-12-02 23:46:28]

msw, please take a look.

[commit-bot: I haz the power, 2016-12-02 23:46:48]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[James Cook, 2016-12-02 23:51:12]

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

[commit-bot: I haz the power, 2016-12-02 23:51:58]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-03 00:21:24]

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

[commit-bot: I haz the power, 2016-12-03 00:21:25]

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

[msw, 2016-12-03 00:26:43]

One comment worth sharing before I review the rest.

https://codereview.chromium.org/2552483002/diff/40001/ash/common/accelerators...
File ash/common/accelerators/accelerator_controller.h (right):

https://codereview.chromium.org/2552483002/diff/40001/ash/common/accelerators...
ash/common/accelerators/accelerator_controller.h:44: NON_EXPORTED_BASE(public
mojom::Volume) {
Inheriting a Volume-specific interface here is somewhat odd. Consider instead
adding mojom:Accelerator[Controller] with SetVolumeClient(mojom::VolumeClientPtr
client). Renaming the interface doesn't commit us to anything more, it just
reflects the implementor. It'll be a little odd for Chrome to tell ash's
accelerator controller "here's your volume client", but it can all be scrapped
when we deprecate chrome's CrasAudioHandler/dbus use. WDYT?

[msw, 2016-12-05 20:15:09]

Yeah, the majority of my comments are about naming.

https://codereview.chromium.org/2552483002/diff/40001/ash/common/accelerators...
File ash/common/accelerators/accelerator_controller.h (right):

https://codereview.chromium.org/2552483002/diff/40001/ash/common/accelerators...
ash/common/accelerators/accelerator_controller.h:194: // Client interface in
chrome browser. May be null in tests.
optional nit: Mention here that it's used for [CrasAudioHandler] volume control
functions? Changing 'controller' -> 'client' makes the purpose of this member
less clear to me (I think "Why does the accelerator controller need to access
some random client of the volume interface?").

https://codereview.chromium.org/2552483002/diff/40001/ash/public/interfaces/v...
File ash/public/interfaces/volume.mojom (right):

https://codereview.chromium.org/2552483002/diff/40001/ash/public/interfaces/v...
ash/public/interfaces/volume.mojom:7: // Implemented by ash. For now, used to
delegate volume changes back to Chrome.
The new naming is confusing to me, the 'client' below is closer to the actual
implementer of volume controls than the implementation of this interface; and
the mute/down/up control functions don't actually live here yet, as you say. It
might make more sense to flip the naming until we actually change the access
patterns, eg: VolumeClient / Volume[Controls|Controller|Manager|Functions|Impl],
something? For example:

// TODO(crbug.com/#): Flip this pattern, ash should implement the volume control
interface or directly access [CrasAudioHandler] [dbus] volume control functions.

// Ash is a client of volume controls, implemented by Chrome.
interface VolumeClient {  // Or AcceleratorController...
  SetControls(VolumeControls controls); // Or SetVolumeControls...
};

// An interface for controlling the volume; implemented by Chrome.
interface VolumeControls {
  // Mute the audio volume.
  VolumeMute();
  // Decrease the audio volume.
  VolumeDown();
  // Increase the audio volume.
  VolumeUp();
};

https://codereview.chromium.org/2552483002/diff/40001/chrome/browser/ui/ash/v...
File chrome/browser/ui/ash/volume_controller.cc (right):

https://codereview.chromium.org/2552483002/diff/40001/chrome/browser/ui/ash/v...
chrome/browser/ui/ash/volume_controller.cc:46: // Register this object as the
client interface implementation.
nit: "client interface implementation" doesn't seem self-explanatory to me.

https://codereview.chromium.org/2552483002/diff/40001/chrome/browser/ui/ash/v...
File chrome/browser/ui/ash/volume_controller.h (right):

https://codereview.chromium.org/2552483002/diff/40001/chrome/browser/ui/ash/v...
chrome/browser/ui/ash/volume_controller.h:12: // Controls the volume when F8-10
or a multimedia key for volume is pressed.
optional nit: add a TODO about broken accelerators in mash? crbug.com/647781

https://codereview.chromium.org/2552483002/diff/40001/chrome/browser/ui/ash/v...
chrome/browser/ui/ash/volume_controller.h:13: class VolumeController : public
ash::mojom::VolumeClient {
Yeah, the controller implementing a client, just doesn't make sense...

[James Cook, 2016-12-05 20:25:49]

https://codereview.chromium.org/2552483002/diff/40001/ash/public/interfaces/v...
File ash/public/interfaces/volume.mojom (right):

https://codereview.chromium.org/2552483002/diff/40001/ash/public/interfaces/v...
ash/public/interfaces/volume.mojom:7: // Implemented by ash. For now, used to
delegate volume changes back to Chrome.
On 2016/12/05 20:15:09, msw wrote:
> The new naming is confusing to me, the 'client' below is closer to the actual
> implementer of volume controls than the implementation of this interface; and
> the mute/down/up control functions don't actually live here yet, as you say.
It
> might make more sense to flip the naming until we actually change the access
> patterns, eg: VolumeClient /
Volume[Controls|Controller|Manager|Functions|Impl],
> something? For example:
> 
> // TODO(crbug.com/#): Flip this pattern, ash should implement the volume
control
> interface or directly access [CrasAudioHandler] [dbus] volume control
functions.
> 
> // Ash is a client of volume controls, implemented by Chrome.
> interface VolumeClient {  // Or AcceleratorController...
>   SetControls(VolumeControls controls); // Or SetVolumeControls...
> };
> 
> // An interface for controlling the volume; implemented by Chrome.
> interface VolumeControls {
>   // Mute the audio volume.
>   VolumeMute();
>   // Decrease the audio volume.
>   VolumeDown();
>   // Increase the audio volume.
>   VolumeUp();
> };

It's going to be a couple days before I get back to this CL. However, what if I
went with:

New file accelerator_controller.mojom:

interface AcceleratorController {
  SetVolumeController(VolumeController controller);
};

Then in this file keep:

// TODO(crbug.com/#): Flip this pattern, ash should implement the volume control
interface...
interface VolumeController {
  ...same...
};

Then in src/chrome keep the class named VolumeController.

WDYT?

https://codereview.chromium.org/2552483002/diff/40001/ash/public/interfaces/v...
ash/public/interfaces/volume.mojom:7: // Implemented by ash. For now, used to
delegate volume changes back to Chrome.
On 2016/12/05 20:15:09, msw wrote:
> The new naming is confusing to me, the 'client' below is closer to the actual
> implementer of volume controls than the implementation of this interface; and
> the mute/down/up control functions don't actually live here yet, as you say.
It
> might make more sense to flip the naming until we actually change the access
> patterns, eg: VolumeClient /
Volume[Controls|Controller|Manager|Functions|Impl],
> something? For example:
> 
> // TODO(crbug.com/#): Flip this pattern, ash should implement the volume
control
> interface or directly access [CrasAudioHandler] [dbus] volume control
functions.
> 
> // Ash is a client of volume controls, implemented by Chrome.
> interface VolumeClient {  // Or AcceleratorController...
>   SetControls(VolumeControls controls); // Or SetVolumeControls...
> };
> 
> // An interface for controlling the volume; implemented by Chrome.
> interface VolumeControls {
>   // Mute the audio volume.
>   VolumeMute();
>   // Decrease the audio volume.
>   VolumeDown();
>   // Increase the audio volume.
>   VolumeUp();
> };

It's going to be a couple days before I get back to this CL. However, what if I
went with:

New file accelerator_controller.mojom:

interface AcceleratorController {
  SetVolumeController(VolumeController controller);
};

Then in this file keep:

// TODO(crbug.com/#): Flip this pattern, ash should implement the volume control
interface...
interface VolumeController {
  ...same...
};

Then in src/chrome keep the class named VolumeController.

WDYT?

[msw, 2016-12-05 20:27:00]

https://codereview.chromium.org/2552483002/diff/40001/ash/public/interfaces/v...
File ash/public/interfaces/volume.mojom (right):

https://codereview.chromium.org/2552483002/diff/40001/ash/public/interfaces/v...
ash/public/interfaces/volume.mojom:7: // Implemented by ash. For now, used to
delegate volume changes back to Chrome.
On 2016/12/05 20:25:49, James Cook wrote:
> On 2016/12/05 20:15:09, msw wrote:
> > The new naming is confusing to me, the 'client' below is closer to the
actual
> > implementer of volume controls than the implementation of this interface;
and
> > the mute/down/up control functions don't actually live here yet, as you say.
> It
> > might make more sense to flip the naming until we actually change the access
> > patterns, eg: VolumeClient /
> Volume[Controls|Controller|Manager|Functions|Impl],
> > something? For example:
> > 
> > // TODO(crbug.com/#): Flip this pattern, ash should implement the volume
> control
> > interface or directly access [CrasAudioHandler] [dbus] volume control
> functions.
> > 
> > // Ash is a client of volume controls, implemented by Chrome.
> > interface VolumeClient {  // Or AcceleratorController...
> >   SetControls(VolumeControls controls); // Or SetVolumeControls...
> > };
> > 
> > // An interface for controlling the volume; implemented by Chrome.
> > interface VolumeControls {
> >   // Mute the audio volume.
> >   VolumeMute();
> >   // Decrease the audio volume.
> >   VolumeDown();
> >   // Increase the audio volume.
> >   VolumeUp();
> > };
> 
> It's going to be a couple days before I get back to this CL. However, what if
I
> went with:
> 
> New file accelerator_controller.mojom:
> 
> interface AcceleratorController {
>   SetVolumeController(VolumeController controller);
> };
> 
> Then in this file keep:
> 
> // TODO(crbug.com/#): Flip this pattern, ash should implement the volume
control
> interface...
> interface VolumeController {
>   ...same...
> };
> 
> Then in src/chrome keep the class named VolumeController.
> 
> WDYT?

sgtm

[James Cook, 2016-12-06 19:22:44]

Description was changed from

==========
mash: Have chrome set itself as a client interface for changing volume

This eliminates a case where ash was directly connecting out to Chrome.

* Convert ash::mojom::VolumeController interface to Volume and VolumeClient
* Rename volume controller files to match class names. chrome/browser/ui/ash
is chromeos-only these days so the _chromeos filenames aren't needed.
* Add ash_util::GetAshServiceName() to return "ash" vs. "content_browser"
based on mash vs. classic ash

BUG=670789
TEST=ash_unittests, browser_tests, manually change volume with F9/F10
==========

to

==========
mash: Have chrome set itself as a controller interface for changing volume

This eliminates a case where ash was directly connecting out to Chrome.

* Add ash::mojom::AcceleratorController interface and use to set the
ash::mojom::VolumeController
* Rename volume controller files to match class names. chrome/browser/ui/ash
is chromeos-only these days so the _chromeos filenames aren't needed.
* Add ash_util::GetAshServiceName() to return "ash" vs. "content_browser"
based on mash vs. classic ash

BUG=670789
TEST=ash_unittests, browser_tests, manually change volume with F9/F10
==========

[James Cook, 2016-12-06 19:23:45]

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

[James Cook, 2016-12-06 19:23:57]

msw, please take another look.

https://codereview.chromium.org/2552483002/diff/40001/ash/common/accelerators...
File ash/common/accelerators/accelerator_controller.h (right):

https://codereview.chromium.org/2552483002/diff/40001/ash/common/accelerators...
ash/common/accelerators/accelerator_controller.h:44: NON_EXPORTED_BASE(public
mojom::Volume) {
On 2016/12/03 00:26:43, msw wrote:
> Inheriting a Volume-specific interface here is somewhat odd. Consider instead
> adding mojom:Accelerator[Controller] with
SetVolumeClient(mojom::VolumeClientPtr
> client). Renaming the interface doesn't commit us to anything more, it just
> reflects the implementor. It'll be a little odd for Chrome to tell ash's
> accelerator controller "here's your volume client", but it can all be scrapped
> when we deprecate chrome's CrasAudioHandler/dbus use. WDYT?

Done.

https://codereview.chromium.org/2552483002/diff/40001/ash/common/accelerators...
ash/common/accelerators/accelerator_controller.h:194: // Client interface in
chrome browser. May be null in tests.
On 2016/12/05 20:15:09, msw wrote:
> optional nit: Mention here that it's used for [CrasAudioHandler] volume
control
> functions? Changing 'controller' -> 'client' makes the purpose of this member
> less clear to me (I think "Why does the accelerator controller need to access
> some random client of the volume interface?").

Done.

https://codereview.chromium.org/2552483002/diff/40001/ash/public/interfaces/v...
File ash/public/interfaces/volume.mojom (right):

https://codereview.chromium.org/2552483002/diff/40001/ash/public/interfaces/v...
ash/public/interfaces/volume.mojom:7: // Implemented by ash. For now, used to
delegate volume changes back to Chrome.
On 2016/12/05 20:27:00, msw wrote:
> On 2016/12/05 20:25:49, James Cook wrote:
> > On 2016/12/05 20:15:09, msw wrote:
> > > The new naming is confusing to me, the 'client' below is closer to the
> actual
> > > implementer of volume controls than the implementation of this interface;
> and
> > > the mute/down/up control functions don't actually live here yet, as you
say.
> > It
> > > might make more sense to flip the naming until we actually change the
access
> > > patterns, eg: VolumeClient /
> > Volume[Controls|Controller|Manager|Functions|Impl],
> > > something? For example:
> > > 
> > > // TODO(crbug.com/#): Flip this pattern, ash should implement the volume
> > control
> > > interface or directly access [CrasAudioHandler] [dbus] volume control
> > functions.
> > > 
> > > // Ash is a client of volume controls, implemented by Chrome.
> > > interface VolumeClient {  // Or AcceleratorController...
> > >   SetControls(VolumeControls controls); // Or SetVolumeControls...
> > > };
> > > 
> > > // An interface for controlling the volume; implemented by Chrome.
> > > interface VolumeControls {
> > >   // Mute the audio volume.
> > >   VolumeMute();
> > >   // Decrease the audio volume.
> > >   VolumeDown();
> > >   // Increase the audio volume.
> > >   VolumeUp();
> > > };
> > 
> > It's going to be a couple days before I get back to this CL. However, what
if
> I
> > went with:
> > 
> > New file accelerator_controller.mojom:
> > 
> > interface AcceleratorController {
> >   SetVolumeController(VolumeController controller);
> > };
> > 
> > Then in this file keep:
> > 
> > // TODO(crbug.com/#): Flip this pattern, ash should implement the volume
> control
> > interface...
> > interface VolumeController {
> >   ...same...
> > };
> > 
> > Then in src/chrome keep the class named VolumeController.
> > 
> > WDYT?
> 
> sgtm

Done.

https://codereview.chromium.org/2552483002/diff/40001/chrome/browser/ui/ash/v...
File chrome/browser/ui/ash/volume_controller.cc (right):

https://codereview.chromium.org/2552483002/diff/40001/chrome/browser/ui/ash/v...
chrome/browser/ui/ash/volume_controller.cc:46: // Register this object as the
client interface implementation.
On 2016/12/05 20:15:09, msw wrote:
> nit: "client interface implementation" doesn't seem self-explanatory to me.

Done.

https://codereview.chromium.org/2552483002/diff/40001/chrome/browser/ui/ash/v...
File chrome/browser/ui/ash/volume_controller.h (right):

https://codereview.chromium.org/2552483002/diff/40001/chrome/browser/ui/ash/v...
chrome/browser/ui/ash/volume_controller.h:12: // Controls the volume when F8-10
or a multimedia key for volume is pressed.
On 2016/12/05 20:15:09, msw wrote:
> optional nit: add a TODO about broken accelerators in mash? crbug.com/647781 

Done.

https://codereview.chromium.org/2552483002/diff/40001/chrome/browser/ui/ash/v...
chrome/browser/ui/ash/volume_controller.h:13: class VolumeController : public
ash::mojom::VolumeClient {
On 2016/12/05 20:15:09, msw wrote:
> Yeah, the controller implementing a client, just doesn't make sense...

Done.

[commit-bot: I haz the power, 2016-12-06 19:24:12]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[James Cook, 2016-12-06 19:24:14]

Description was changed from

==========
mash: Have chrome set itself as a controller interface for changing volume

This eliminates a case where ash was directly connecting out to Chrome.

* Add ash::mojom::AcceleratorController interface and use to set the
ash::mojom::VolumeController
* Rename volume controller files to match class names. chrome/browser/ui/ash
is chromeos-only these days so the _chromeos filenames aren't needed.
* Add ash_util::GetAshServiceName() to return "ash" vs. "content_browser"
based on mash vs. classic ash

BUG=670789
TEST=ash_unittests, browser_tests, manually change volume with F9/F10
==========

to

==========
mash: Have chrome set itself as a controller interface for changing volume

This eliminates a case where ash was directly connecting out to Chrome.

* Add ash::mojom::AcceleratorController interface and use to set the
ash::mojom::VolumeController
* Rename volume controller files to match class names. chrome/browser/ui/ash
is chromeos-only these days so the _chromeos filenames aren't needed.
* Add ash_util::GetAshServiceName() to return "ash" vs. "content_browser"
based on mash vs. classic ash

BUG=670789
TEST=ash_unittests, browser_tests, manually change volume with F9/F10
==========

[msw, 2016-12-06 19:43:57]

lgtm with just one comment nit/q

https://codereview.chromium.org/2552483002/diff/80001/ash/mus/manifest.json
File ash/mus/manifest.json (right):

https://codereview.chromium.org/2552483002/diff/80001/ash/mus/manifest.json#n...
ash/mus/manifest.json:7: // See also
chrome_content_browser_manifest_overlay.json for classic ash
Hmm, I'm not sure how this comment is helpful? What are you hoping to convey?

[commit-bot: I haz the power, 2016-12-06 20:01:29]

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

[commit-bot: I haz the power, 2016-12-06 20:01:30]

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

[James Cook, 2016-12-06 20:11:53]

https://codereview.chromium.org/2552483002/diff/80001/ash/mus/manifest.json
File ash/mus/manifest.json (right):

https://codereview.chromium.org/2552483002/diff/80001/ash/mus/manifest.json#n...
ash/mus/manifest.json:7: // See also
chrome_content_browser_manifest_overlay.json for classic ash
On 2016/12/06 19:43:57, msw wrote:
> Hmm, I'm not sure how this comment is helpful? What are you hoping to convey?

I'm trying to convey the fact that when you add an interface in ash you need to
add it to two files.

Sadly, we don't have google3's "if you change this" annotation
https://drive.google.com/file/d/0B-KFDiTrQe9lenFxcXVFbURDeDA/view

[msw, 2016-12-06 20:13:29]

https://codereview.chromium.org/2552483002/diff/80001/ash/mus/manifest.json
File ash/mus/manifest.json (right):

https://codereview.chromium.org/2552483002/diff/80001/ash/mus/manifest.json#n...
ash/mus/manifest.json:7: // See also
chrome_content_browser_manifest_overlay.json for classic ash
On 2016/12/06 20:11:53, James Cook wrote:
> On 2016/12/06 19:43:57, msw wrote:
> > Hmm, I'm not sure how this comment is helpful? What are you hoping to
convey?
> 
> I'm trying to convey the fact that when you add an interface in ash you need
to
> add it to two files.
> 
> Sadly, we don't have google3's "if you change this" annotation
> https://drive.google.com/file/d/0B-KFDiTrQe9lenFxcXVFbURDeDA/view

Maybe explicitly say "Modifications here should correspond with changes to ..."
or similar.

[James Cook, 2016-12-06 20:14:41]

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

[commit-bot: I haz the power, 2016-12-06 20:15:17]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[James Cook, 2016-12-06 20:18:52]

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

[James Cook, 2016-12-06 20:19:14]

jamescook@chromium.org changed reviewers:
+ tsepez@chromium.org

[James Cook, 2016-12-06 20:19:15]

tsepez, can I get OWNERS for mojoms and manifests?

https://codereview.chromium.org/2552483002/diff/80001/ash/mus/manifest.json
File ash/mus/manifest.json (right):

https://codereview.chromium.org/2552483002/diff/80001/ash/mus/manifest.json#n...
ash/mus/manifest.json:7: // See also
chrome_content_browser_manifest_overlay.json for classic ash
On 2016/12/06 20:13:29, msw wrote:
> On 2016/12/06 20:11:53, James Cook wrote:
> > On 2016/12/06 19:43:57, msw wrote:
> > > Hmm, I'm not sure how this comment is helpful? What are you hoping to
> convey?
> > 
> > I'm trying to convey the fact that when you add an interface in ash you need
> to
> > add it to two files.
> > 
> > Sadly, we don't have google3's "if you change this" annotation
> > https://drive.google.com/file/d/0B-KFDiTrQe9lenFxcXVFbURDeDA/view
> 
> Maybe explicitly say "Modifications here should correspond with changes to
..."
> or similar.

Yeah, that's better. Done.

[commit-bot: I haz the power, 2016-12-06 20:19:30]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-06 21:26:16]

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

[commit-bot: I haz the power, 2016-12-06 21:26:18]

Dry run: This issue passed the CQ dry run.

[Tom Sepez, 2016-12-06 22:28:31]

lgtm

[James Cook, 2016-12-06 22:33:49]

Description was changed from

==========
mash: Have chrome set itself as a controller interface for changing volume

This eliminates a case where ash was directly connecting out to Chrome.

* Add ash::mojom::AcceleratorController interface and use to set the
ash::mojom::VolumeController
* Rename volume controller files to match class names. chrome/browser/ui/ash
is chromeos-only these days so the _chromeos filenames aren't needed.
* Add ash_util::GetAshServiceName() to return "ash" vs. "content_browser"
based on mash vs. classic ash

BUG=670789
TEST=ash_unittests, browser_tests, manually change volume with F9/F10
==========

to

==========
mash: Have chrome set itself as a controller interface for changing volume

This eliminates a case where ash was directly connecting out to Chrome.

* Add ash::mojom::AcceleratorController interface and use to set the
ash::mojom::VolumeController
* Rename volume controller files to match class names. chrome/browser/ui/ash
is chromeos-only these days so the _chromeos filenames aren't needed.
* Add ash_util::GetAshServiceName() to return "ash" vs. "content_browser"
based on mash vs. classic ash

BUG=670789
TEST=ash_unittests, browser_tests, manually change volume with F9/F10
TBR=xiyuan@chromium.org
==========

[James Cook, 2016-12-06 22:34:26]

jamescook@chromium.org changed reviewers:
+ xiyuan@chromium.org

[James Cook, 2016-12-06 22:34:26]

TBR xiyuan for deleting some code from
chrome/browser/chromeos/chrome_interface_factory.cc

[James Cook, 2016-12-06 22:34:47]

The CQ bit was checked by jamescook@chromium.org

[James Cook, 2016-12-06 22:34:48]

The patchset sent to the CQ was uploaded after l-g-t-m from msw@chromium.org
Link to the patchset: https://codereview.chromium.org/2552483002/#ps120001
(title: "comment")

[commit-bot: I haz the power, 2016-12-06 22:35:35]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-06 22:54:21]

CQ is committing da patch.

Bot data: {"patchset_id": 120001, "attempt_start_ts": 1481063687791170,
"parent_rev": "05a9953b8b82e3237eb2e09d54e49004393f0a06", "commit_rev":
"8ee65a245f44f96074cfda9cd2e9946b10cf469d"}

[commit-bot: I haz the power, 2016-12-06 22:54:45]

Description was changed from

==========
mash: Have chrome set itself as a controller interface for changing volume

This eliminates a case where ash was directly connecting out to Chrome.

* Add ash::mojom::AcceleratorController interface and use to set the
ash::mojom::VolumeController
* Rename volume controller files to match class names. chrome/browser/ui/ash
is chromeos-only these days so the _chromeos filenames aren't needed.
* Add ash_util::GetAshServiceName() to return "ash" vs. "content_browser"
based on mash vs. classic ash

BUG=670789
TEST=ash_unittests, browser_tests, manually change volume with F9/F10
TBR=xiyuan@chromium.org
==========

to

==========
mash: Have chrome set itself as a controller interface for changing volume

This eliminates a case where ash was directly connecting out to Chrome.

* Add ash::mojom::AcceleratorController interface and use to set the
ash::mojom::VolumeController
* Rename volume controller files to match class names. chrome/browser/ui/ash
is chromeos-only these days so the _chromeos filenames aren't needed.
* Add ash_util::GetAshServiceName() to return "ash" vs. "content_browser"
based on mash vs. classic ash

BUG=670789
TEST=ash_unittests, browser_tests, manually change volume with F9/F10
TBR=xiyuan@chromium.org
==========

[commit-bot: I haz the power, 2016-12-06 22:54:46]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2016-12-06 22:56:59]

Description was changed from

==========
mash: Have chrome set itself as a controller interface for changing volume

This eliminates a case where ash was directly connecting out to Chrome.

* Add ash::mojom::AcceleratorController interface and use to set the
ash::mojom::VolumeController
* Rename volume controller files to match class names. chrome/browser/ui/ash
is chromeos-only these days so the _chromeos filenames aren't needed.
* Add ash_util::GetAshServiceName() to return "ash" vs. "content_browser"
based on mash vs. classic ash

BUG=670789
TEST=ash_unittests, browser_tests, manually change volume with F9/F10
TBR=xiyuan@chromium.org
==========

to

==========
mash: Have chrome set itself as a controller interface for changing volume

This eliminates a case where ash was directly connecting out to Chrome.

* Add ash::mojom::AcceleratorController interface and use to set the
ash::mojom::VolumeController
* Rename volume controller files to match class names. chrome/browser/ui/ash
is chromeos-only these days so the _chromeos filenames aren't needed.
* Add ash_util::GetAshServiceName() to return "ash" vs. "content_browser"
based on mash vs. classic ash

BUG=670789
TEST=ash_unittests, browser_tests, manually change volume with F9/F10
TBR=xiyuan@chromium.org

Committed: https://crrev.com/7e0a04e949287e6ea20e5d2be9fe8a168ac0ae7b
Cr-Commit-Position: refs/heads/master@{#436766}
==========

[commit-bot: I haz the power, 2016-12-06 22:57:01]

Patchset 7 (id:??) landed as
https://crrev.com/7e0a04e949287e6ea20e5d2be9fe8a168ac0ae7b
Cr-Commit-Position: refs/heads/master@{#436766}