Support for showing/hiding status area volume controls in desktop devices

chrome/browser/chromeos/audio/audio_handler.h

Index: chrome/browser/chromeos/audio/audio_handler.h
===================================================================
--- chrome/browser/chromeos/audio/audio_handler.h	(revision 119043)
+++ chrome/browser/chromeos/audio/audio_handler.h	(working copy)
@@ -18,17 +18,26 @@
 
 class AudioHandler {
  public:
+  class VolumeObserver {
+   public:
+    VolumeObserver();

[xiyuan, 2012/01/26 18:07:10]

nit: move this into protected as we don't expect people to use this class
directly.

[achuithb, 2012/01/27 01:48:05]

Done.

+    virtual void OnVolumeChanged() = 0;
+   protected:
+    virtual ~VolumeObserver();
+  };
+
   static void Initialize();
   static void Shutdown();
   // GetInstance returns NULL if not initialized or if already shutdown.
   static AudioHandler* GetInstance();
 
-  // Is the mixer initialized?
+  // GetInitialized returns NULL if GetInstance returns NULL or if the
+  // mixer has not finished initializing.
   // TODO(derat): All of the volume-percent methods will produce "interesting"
   // results before the mixer is initialized, since the driver's volume range
   // isn't known at that point.  This could be avoided if AudioMixer objects
   // instead took percentages and did their own conversions to decibels.
-  bool IsInitialized();
+  static AudioHandler* GetInitialized();

[Daniel Erat, 2012/01/26 17:55:21]

chrome os code includes too many static methods.  remove this, please.

[xiyuan, 2012/01/26 18:07:10]

Yeah, let's reuse the GetInstance() above.

[achuithb, 2012/01/26 23:19:34]

Both SystemKeyEventListener and VolumeMenuBotton have need for a
static/anonymous function of the nature:
AudioHandler* GetAudioHandler() {
  return AudioHandler::GetInstance() &&
AudioHandler::GetInstance()->IsMixerInitialized() ? AudioHandler::GetInstance()
: NULL;
}

GetInstance() does not suffice if you want to read/write volume, since we have
to wait for the mixer to be initialized as well.

I was trying to avoid code duplication by putting this here. So your preference
would be to have the two copies of the anonymous namespace functions in the cc
files instead?

[xiyuan, 2012/01/26 23:41:58]

Okay. In this case, I am okay to have it here so that the logic could be shared.

[Daniel Erat, 2012/01/26 23:53:28]

Well, my preference would be for the above TODO to be fixed so that AudioHandler
is usable immediately. :-/

In the meantime, I'm okay with this if you rename it to
GetInstanceIfInitialized() or GetInstanceIfReady() or something along those
lines.

[achuithb, 2012/01/27 01:48:05]

Done.

 
   // Gets volume level in our internal 0-100% range, 0 being pure silence.
   double GetVolumePercent();
@@ -45,6 +54,10 @@
   // Mutes or unmutes all audio.
   void SetMuted(bool do_mute);
 
+  // Adds/Removes a VolumeObserver.

[Daniel Erat, 2012/01/26 17:55:21]

nit: remove unnecessary comment

+  void AddVolumeObserver(VolumeObserver* observer);
+  void RemoveVolumeObserver(VolumeObserver* observer);
+
  private:
   // Defines the delete on exit Singleton traits we like.  Best to have this
   // and constructor/destructor private as recommended for Singletons.
@@ -53,12 +66,18 @@
   AudioHandler();
   virtual ~AudioHandler();
 
+  bool IsMixerInitialized();
+
   // Conversion between our internal scaling (0-100%) and decibels.
   double VolumeDbToPercent(double volume_db) const;
   double PercentToVolumeDb(double volume_percent) const;
 
+  void VolumeChanged();
+
   scoped_ptr<AudioMixer> mixer_;
 
+  ObserverList<VolumeObserver> volume_observers_;
+
   DISALLOW_COPY_AND_ASSIGN(AudioHandler);
 };