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

chrome/browser/chromeos/system_key_event_listener.cc

Index: chrome/browser/chromeos/system_key_event_listener.cc
===================================================================
--- chrome/browser/chromeos/system_key_event_listener.cc	(revision 118753)
+++ chrome/browser/chromeos/system_key_event_listener.cc	(working copy)
@@ -9,6 +9,7 @@
 #include <X11/XF86keysym.h>
 #include <X11/XKBlib.h>
 
+#include "base/command_line.h"
 #include "chrome/browser/accessibility/accessibility_events.h"
 #include "chrome/browser/chromeos/audio/audio_handler.h"
 #include "chrome/browser/chromeos/dbus/dbus_thread_manager.h"
@@ -18,6 +19,7 @@
 #include "chrome/browser/chromeos/input_method/xkeyboard.h"
 #include "chrome/browser/chromeos/ui/brightness_bubble.h"
 #include "chrome/browser/chromeos/ui/volume_bubble.h"
+#include "chrome/common/chrome_switches.h"
 #include "content/public/browser/user_metrics.h"
 #include "third_party/cros_system_api/window_manager/chromeos_wm_ipc_enums.h"
 #include "ui/base/x/x11_util.h"
@@ -50,6 +52,11 @@
 
 static SystemKeyEventListener* g_system_key_event_listener = NULL;
 
+bool ShowVolumeStatus() {
+  return CommandLine::ForCurrentProcess()->
+      HasSwitch(switches::kShowVolumeStatus);
+}
+
 }  // namespace
 
 // static
@@ -76,6 +83,7 @@
 
 SystemKeyEventListener::SystemKeyEventListener()
     : stopped_(false),
+      show_status_area_volume_(ShowVolumeStatus()),
       num_lock_mask_(input_method::XKeyboard::GetNumLockMask()),
       xkb_event_base_(0) {
   input_method::XKeyboard::GetLockedModifiers(
@@ -165,6 +173,17 @@
   caps_lock_observers_.RemoveObserver(observer);
 }
 
+void SystemKeyEventListener::AddStatusAreaVolumeObserver(
+    StatusAreaVolumeObserver* observer) {
+  status_area_volume_observers_.AddObserver(observer);
+  ShowStatusAreaVolume(show_status_area_volume_);

[xiyuan, 2012/01/25 04:43:12]

I don't think this should be here.

Could we do something like this instead:
- Expose an accessor show_status_area_volume() in SystemKeyEventListener's
interface
- VolumeMenuButton's ctor based on the value of show_status_area_volume to
decide initial visibility state of itself and then AddStatusAreaVolumeObserver

[achuithb, 2012/01/25 10:46:06]

Done.

+}
+
+void SystemKeyEventListener::RemoveStatusAreaVolumeObserver(
+    StatusAreaVolumeObserver* observer) {
+  status_area_volume_observers_.RemoveObserver(observer);
+}
+
 #if defined(TOOLKIT_USES_GTK)
 // static
 GdkFilterReturn SystemKeyEventListener::GdkEventFilter(GdkXEvent* gxevent,
@@ -210,6 +229,9 @@
 }
 
 void SystemKeyEventListener::OnVolumeMute() {
+  if (show_status_area_volume_)
+    return;

[xiyuan, 2012/01/25 04:43:12]

Would those system volume keys still work after this change? Or there would be
no such keys for on a desktop system?

[achuithb, 2012/01/25 10:46:06]

The keys would no longer work. The idea is that there is only one way to control
the volume - either the F8-F10 keys, or the VolumeMenuButton. If the
VolumeMenuButton is visible, we don't want to use the keyboard volume keys. I'll
add a comment to this effect.

[Daniel Erat, 2012/01/25 15:58:51]

What's the motivation for making the keys not control the volume?  Do you plan
to disable Chrome OS functionality for all of the function keys when using
non-standard keyboards and send F1-12 to webpages instead?  This may be a
difficult change to do.

+
   AudioHandler* audio_handler = GetAudioHandler();
   if (!audio_handler)
     return;
@@ -226,6 +248,9 @@
 }
 
 void SystemKeyEventListener::OnVolumeDown() {
+  if (show_status_area_volume_)
+    return;
+
   AudioHandler* audio_handler = GetAudioHandler();
   if (!audio_handler)
     return;
@@ -243,6 +268,9 @@
 }
 
 void SystemKeyEventListener::OnVolumeUp() {
+  if (show_status_area_volume_)
+    return;
+
   AudioHandler* audio_handler = GetAudioHandler();
   if (!audio_handler)
     return;
@@ -263,10 +291,18 @@
 }
 
 void SystemKeyEventListener::OnCapsLock(bool enabled) {
-  FOR_EACH_OBSERVER(
-      CapsLockObserver, caps_lock_observers_, OnCapsLockChange(enabled));
+  FOR_EACH_OBSERVER(CapsLockObserver,
+                    caps_lock_observers_,
+                    OnCapsLockChange(enabled));
 }
 
+void SystemKeyEventListener::ShowStatusAreaVolume(bool show) {
+  show_status_area_volume_ = show;
+  FOR_EACH_OBSERVER(StatusAreaVolumeObserver,
+                    status_area_volume_observers_,
+                    ShowStatusAreaVolume(show));
+}
+
 void SystemKeyEventListener::ShowVolumeBubble() {
   AudioHandler* audio_handler = GetAudioHandler();
   if (audio_handler) {