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

chrome/browser/chromeos/audio/audio_handler.cc

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/chromeos/audio/audio_handler.h"
 
 #include <algorithm>
 #include <cmath>
 
 #include "base/logging.h"
 #include "base/memory/singleton.h"
 #include "chrome/browser/chromeos/audio/audio_mixer_alsa.h"
+#include "chrome/browser/chromeos/system/runtime_environment.h"
 #include "content/public/browser/browser_thread.h"
 
 using std::max;
 using std::min;
 
 namespace chromeos {
 
 namespace {
 
 // A value of less than one adjusts quieter volumes in larger steps (giving
@@ skipping 19 matching lines @@
   }
 }
 
 // static
 AudioHandler* AudioHandler::GetInstance() {
   VLOG_IF(1, !g_audio_handler)
       << "AudioHandler::GetInstance() called with NULL global instance.";
   return g_audio_handler;
 }
 
-bool AudioHandler::IsInitialized() {
-  return mixer_->IsInitialized();
+// static
+AudioHandler* AudioHandler::GetInstanceIfInitialized() {
+  return g_audio_handler && g_audio_handler->IsMixerInitialized() ?
+         g_audio_handler : NULL;
 }
 
 double AudioHandler::GetVolumePercent() {
   return VolumeDbToPercent(mixer_->GetVolumeDb());
 }
 
 void AudioHandler::SetVolumePercent(double volume_percent) {
   volume_percent = min(max(volume_percent, 0.0), 100.0);
   mixer_->SetVolumeDb(PercentToVolumeDb(volume_percent));
+  FOR_EACH_OBSERVER(VolumeObserver, volume_observers_, OnVolumeChanged());
 }
 
 void AudioHandler::AdjustVolumeByPercent(double adjust_by_percent) {
   const double old_volume_db = mixer_->GetVolumeDb();
   const double old_percent = VolumeDbToPercent(old_volume_db);
   SetVolumePercent(old_percent + adjust_by_percent);
 }
 
 bool AudioHandler::IsMuted() {
   return mixer_->IsMuted();
 }
 
 void AudioHandler::SetMuted(bool mute) {
   mixer_->SetMuted(mute);
+  FOR_EACH_OBSERVER(VolumeObserver, volume_observers_, OnVolumeChanged());
 }
 
 AudioHandler::AudioHandler()
     : mixer_(new AudioMixerAlsa()) {
   mixer_->Init();
 }
 
 AudioHandler::~AudioHandler() {
   mixer_.reset();
 };
 
+bool AudioHandler::IsMixerInitialized() {
+  return mixer_->IsInitialized();
+}
+
 // VolumeDbToPercent() and PercentToVolumeDb() conversion functions allow us
 // complete control over how the 0 to 100% range is mapped to actual loudness.
 //
 // The mapping is confined to these two functions to make it easy to adjust and
 // have everything else just work.  The range is biased to give finer resolution
 // in the higher volumes if kVolumeBias is less than 1.0.
 
 double AudioHandler::VolumeDbToPercent(double volume_db) const {
   double min_volume_db, max_volume_db;
   mixer_->GetVolumeLimits(&min_volume_db, &max_volume_db);
 
   if (volume_db < min_volume_db)
     return 0.0;
   // TODO(derat): Choose a better mapping between percent and decibels.  The
   // bottom twenty-five percent or so is useless on a CR-48's internal speakers;
   // it's all inaudible.
   return 100.0 * pow((volume_db - min_volume_db) /
       (max_volume_db - min_volume_db), 1/kVolumeBias);
 }
 
 double AudioHandler::PercentToVolumeDb(double volume_percent) const {
   double min_volume_db, max_volume_db;
   mixer_->GetVolumeLimits(&min_volume_db, &max_volume_db);
 
   return pow(volume_percent / 100.0, kVolumeBias) *
       (max_volume_db - min_volume_db) + min_volume_db;
 }
 
+void AudioHandler::AddVolumeObserver(VolumeObserver* observer) {
+  volume_observers_.AddObserver(observer);
+}
+
+void AudioHandler::RemoveVolumeObserver(VolumeObserver* observer) {
+  volume_observers_.RemoveObserver(observer);
+}
+
+AudioHandler::VolumeObserver::VolumeObserver() {
+  // AudioHandler should be initialized here if we are on the device, though
+  // the mixer may not have finished initializing yet.
+  DCHECK(AudioHandler::GetInstance() ||
+      !system::runtime_environment::IsRunningOnChromeOS());
+  if (AudioHandler::GetInstance())

[Daniel Erat, 2012/01/27 18:03:57]

remove the if statement; you already have a DCHECK()

[achuithb, 2012/01/27 18:53:44]

AudioHandler::GetInstance() is NULL on chrome-chromeos for linux. I need the
NULL check (or I could alternately use IsRunningOnChromeOS, but this is
clearer).

[stevenjb, 2012/01/27 19:45:02]

I'm not sure that this is clearer; how about at the top of the file:
if (!system::runtime_environment::IsRunningOnChromeOS())
  return;
Or, if it's possible that AudioHandler::GetInstance() might be true on non
chromeos:
if (!system::runtime_environment::IsRunningOnChromeOS() &&
!AudioHandler::GetInstance())
  return;
This way it's clear that when not running on chromeos, we expect that the audio
handler may not be initialized and just want to exit. Then we do the DCHECK and
call AddVolimeObserver unconditionally.

[Daniel Erat, 2012/01/27 23:06:54]

yeah, i'd also prefer:

  if (system::runtime_environment::IsRunningOnChromeOS() {
    DCHECK(AudioHandler::GetInstance());
    AudioHandler::GetInstance()->AddVolumeObserver(this);
  }

DCHECK() is more-or-less optional at this point, since we're going to crash on a
null deref otherwise.  the code in the d'tor should be similar.

[achuithb, 2012/01/31 00:58:43]

Well, I was following the pattern in:
http://code.google.com/codesearch#OAMlx_jo-ck/src/chrome/browser/chromeos/sta...
Steven - since you wrote that code, I'd like to understand why your opinion has
changed? I like the way it's written and I copied it exactly here.

If we add a mock audio handler for chromeos-chrome for linux (something that we
should do), testing for runtime_environment will no longer be correct. 
Null-checking a pointer that could be NULL will always be correct. 

I'll switch this to using the runtime_environment check for now, using Dan's
arrangement (leaving out the DCHECK since we're about to crash anyway).

+    AudioHandler::GetInstance()->AddVolumeObserver(this);
+}
+
+AudioHandler::VolumeObserver::~VolumeObserver() {
+  if (AudioHandler::GetInstance())
+    AudioHandler::GetInstance()->RemoveVolumeObserver(this);
+}
+
 }  // namespace chromeos