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

ash/public/interfaces/volume.mojom

Index: ash/public/interfaces/volume.mojom
diff --git a/ash/public/interfaces/volume.mojom b/ash/public/interfaces/volume.mojom
index 85bd93928bbbbece6849930d9c3662d92313f4af..d54035204624e59ac29d570d065060dffc4cadc6 100644
--- a/ash/public/interfaces/volume.mojom
+++ b/ash/public/interfaces/volume.mojom
@@ -4,10 +4,18 @@
 
 module ash.mojom;
 
+// Implemented by ash. For now, used to delegate volume changes back to Chrome.

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

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();
};

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

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?

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

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]

sgtm

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

Done.

+// In the future volume may be owned by ash, in which case this interface will
+// implement VolumeUp/VolumeDown/etc.
+interface Volume {
+  // Sets the client interface.
+  SetClient(VolumeClient client);
+};
+
 // Implemented by Chrome and used by ash to request audio volume changes.
 // Implemented by Chrome because it's currently the only client of audio dbus
 // interfaces (via CrasAudioHandler); this may be simplified if that changes.
-interface VolumeController {
+interface VolumeClient {
   // Mute the audio volume.
   VolumeMute();
   // Decrease the audio volume.