Side Slide Gestures for Accessibility

ash/root_window_controller.cc

Index: ash/root_window_controller.cc
diff --git a/ash/root_window_controller.cc b/ash/root_window_controller.cc
index aafe0ba0c61f8dbeea3e3b61770b5191935adab0..189dd80114f1fe5188562ccbe5be7be02018084c 100644
--- a/ash/root_window_controller.cc
+++ b/ash/root_window_controller.cc
@@ -9,6 +9,7 @@
 
 #include "ash/ash_constants.h"
 #include "ash/ash_switches.h"
+#include "ash/audio/sounds.h"
 #include "ash/desktop_background/desktop_background_controller.h"
 #include "ash/desktop_background/desktop_background_widget_controller.h"
 #include "ash/desktop_background/user_wallpaper_delegate.h"
@@ -51,6 +52,9 @@
 #include "ash/wm/workspace_controller.h"
 #include "base/command_line.h"
 #include "base/time/time.h"
+#include "chromeos/audio/chromeos_sounds.h"

[James Cook, 2014/07/14 17:19:31]

Ash code is compile on non-ChromeOS platforms (e.g. Windows) so this and all
code that uses it needs to be in #if defined(OS_CHROMEOS).

[lisayin, 2014/07/14 22:38:38]

Done.

+#include "chromeos/audio/cras_audio_handler.h"
+#include "chromeos/chromeos_switches.h"
 #include "ui/aura/client/aura_constants.h"
 #include "ui/aura/client/screen_position_client.h"
 #include "ui/aura/window.h"
@@ -264,11 +268,14 @@ class EmptyWindowDelegate : public aura::WindowDelegate {
 #if defined(OS_CHROMEOS)
 // Responsible for initializing TouchExplorationController when spoken
 // feedback is on.
-class CrosAccessibilityObserver : public AccessibilityObserver {
+class CrosAccessibilityObserver

[James Cook, 2014/07/14 17:19:30]

Please extract this class into its own file.  It's a little too big for a
platform-specific local class. This will also let you use GYP to exclude it on
non-Chrome OS platforms and reduce the number of headers this file needs to
include.

This would also make it easier to add a small test for this, especially if you
inject your own mock CrasAudioHandler.

[dmazzoni, 2014/07/14 18:00:33]

That sounds good to me. I'd call it something like AshTouchExplorationManager or
something like that, since "accessibility observer" doesn't really describe its
primary purpose.

[lisayin, 2014/07/14 22:40:36]

Should the file be in the same folder as this file?

+    : public AccessibilityObserver,
+      public ui::TouchExplorationControllerDelegate {
  public:
   explicit CrosAccessibilityObserver(
       RootWindowController* root_window_controller)
-      : root_window_controller_(root_window_controller) {
+      : root_window_controller_(root_window_controller),
+        audio_handler_(chromeos::CrasAudioHandler::Get()) {
     Shell::GetInstance()->system_tray_notifier()->
         AddAccessibilityObserver(this);
     UpdateTouchExplorationState();
@@ -281,6 +288,34 @@ class CrosAccessibilityObserver : public AccessibilityObserver {
       system_tray_notifier->RemoveAccessibilityObserver(this);
   }
 
+  bool VolumeAdjustSoundEnabled() {

[James Cook, 2014/07/14 17:19:30]

const?

[lisayin, 2014/07/14 22:38:38]

Done.

+    return !CommandLine::ForCurrentProcess()->HasSwitch(
+        chromeos::switches::kDisableVolumeAdjustSound);
+  }
+
+  // Overridden from TouchExplorationControllerDelegate

[James Cook, 2014/07/14 17:19:30]

nit: end with colon

[lisayin, 2014/07/14 22:38:38]

Done.

+  virtual void PlayVolumeAdjustSound() OVERRIDE{
+    if (!VolumeAdjustSoundEnabled())
+      return;
+
+    if (!audio_handler_->IsOutputMuted() ||
+        !audio_handler_->GetOutputVolumePercent() == 100)
+      PlaySystemSoundIfSpokenFeedback(chromeos::SOUND_VOLUME_ADJUST);
+  }
+
+  // Overridden from TouchExplorationControllerDelegate

[James Cook, 2014/07/14 17:19:30]

nit: We usually only have one of these lines

[lisayin, 2014/07/14 22:38:38]

Done.

+  virtual void AdjustSound(float volume) OVERRIDE {
+    if (volume > 0) {
+      if (audio_handler_->IsOutputMuted()) {
+        audio_handler_->SetOutputMute(false);
+      }
+    }
+    audio_handler_->SetOutputVolumePercent(volume);
+    // Avoid negative volume.
+    if (audio_handler_->IsOutputVolumeBelowDefaultMuteLvel())

[James Cook, 2014/07/14 17:19:31]

Not your typo, but could you use this CL to change the name of this function to
"IsOutputVolumeBelowDefaultMuteLevel"?

+      audio_handler_->SetOutputMute(true);
+  }
+
  private:
   void UpdateTouchExplorationState() {
     AccessibilityDelegate* delegate =
@@ -288,9 +323,8 @@ class CrosAccessibilityObserver : public AccessibilityObserver {
     bool enabled = delegate->IsSpokenFeedbackEnabled();
 
     if (enabled && !touch_exploration_controller_.get()) {
-      touch_exploration_controller_.reset(
-          new ui::TouchExplorationController(
-              root_window_controller_->GetRootWindow()));
+      touch_exploration_controller_.reset(new ui::TouchExplorationController(
+          root_window_controller_->GetRootWindow(), this));
     } else if (!enabled) {
       touch_exploration_controller_.reset();
     }
@@ -304,6 +338,7 @@ class CrosAccessibilityObserver : public AccessibilityObserver {
 
   scoped_ptr<ui::TouchExplorationController> touch_exploration_controller_;
   RootWindowController* root_window_controller_;
+  chromeos::CrasAudioHandler* audio_handler_;
 
   DISALLOW_COPY_AND_ASSIGN(CrosAccessibilityObserver);
 };