If the user clicks on a key on the accessibility virtual keyboard the cursor

disappears, which makes typing the next key difficult. This patch disables the
hide cursor whenever the accessibility keyboard is enabled.

Added class ash::ash_cursor_manager which extends wm::cursor_manager, and moved
the logic of whether or not to hide the cursor on key events to here.

Follow up bug to add unittest: crbug.com/359324

BUG=355094

Review URL: https://codereview.chromium.org/213743013

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@262299 0039d316-1c4b-4281-b951-d872f2087c98

12 files changed, 150 insertions, 63 deletions

diff --git a/ash/ash.gyp b/ash/ash.gyp
index a1b4038..0d62c87 100644
--- a/ash/ash.gyp
+++ b/ash/ash.gyp
@@ -518,6 +518,8 @@
         'wm/ash_focus_rules.h',
         'wm/boot_splash_screen_chromeos.cc',
         'wm/boot_splash_screen_chromeos.h',
+        'wm/cursor_manager_chromeos.cc',
+        'wm/cursor_manager_chromeos.h',
         'wm/coordinate_conversion.cc',
         'wm/coordinate_conversion.h',
         'wm/default_state.cc',
diff --git a/ash/shell.h b/ash/shell.h
index 0692bc1..e8ce236 100644
--- a/ash/shell.h
+++ b/ash/shell.h
@@ -12,6 +12,7 @@
 #include "ash/metrics/user_metrics_recorder.h"
 #include "ash/shelf/shelf_types.h"
 #include "ash/system/user/login_status.h"
+#include "ash/wm/cursor_manager_chromeos.h"
 #include "ash/wm/system_modal_container_event_filter_delegate.h"
 #include "base/basictypes.h"
 #include "base/compiler_specific.h"
@@ -720,7 +721,14 @@ class ASH_EXPORT Shell : public SystemModalContainerEventFilterDelegate,
   // |native_cursor_manager_| is owned by |cursor_manager_|, but we keep a
   // pointer to vend to test code.
   AshNativeCursorManager* native_cursor_manager_;
+
+// Cursor may be hidden on certain key events in ChromeOS, whereas we never hide
+// the cursor on Windows.
+#if defined(OS_CHROMEOS)
+  CursorManager cursor_manager_;
+#else  // !defined(OS_CHROMEOS)
   ::wm::CursorManager cursor_manager_;
+#endif  // defined(OS_CHROMEOS)
 
   ObserverList<ShellObserver> observers_;
 
diff --git a/ash/wm/ash_native_cursor_manager_interactive_uitest.cc b/ash/wm/ash_native_cursor_manager_interactive_uitest.cc
index 48d2af5..5084e7c 100644
--- a/ash/wm/ash_native_cursor_manager_interactive_uitest.cc
+++ b/ash/wm/ash_native_cursor_manager_interactive_uitest.cc
@@ -25,8 +25,6 @@
 
 namespace ash {
 
-using ::wm::CursorManager;
-
 class AshNativeCursorManagerTest : public test::AshTestBase {
  public:
   AshNativeCursorManagerTest() {}
@@ -90,7 +88,7 @@ void MoveMouseSync(aura::Window* window, int x, int y) {
 #endif
 
 TEST_F(AshNativeCursorManagerTest, MAYBE_CursorChangeOnEnterNotify) {
-  CursorManager* cursor_manager = Shell::GetInstance()->cursor_manager();
+  ::wm::CursorManager* cursor_manager = Shell::GetInstance()->cursor_manager();
   test::CursorManagerTestApi test_api(cursor_manager);
 
   DisplayManager* display_manager = Shell::GetInstance()->display_manager();
diff --git a/ash/wm/ash_native_cursor_manager_unittest.cc b/ash/wm/ash_native_cursor_manager_unittest.cc
index 5d9ab1e..141e8a8 100644
--- a/ash/wm/ash_native_cursor_manager_unittest.cc
+++ b/ash/wm/ash_native_cursor_manager_unittest.cc
@@ -19,8 +19,6 @@
 #include "ui/base/cursor/cursor_loader_win.h"
 #endif
 
-using ::wm::CursorManager;
-
 namespace ash {
 namespace test {
 
@@ -54,7 +52,7 @@ class MouseEventLocationDelegate : public aura::test::TestWindowDelegate {
 typedef test::AshTestBase AshNativeCursorManagerTest;
 
 TEST_F(AshNativeCursorManagerTest, LockCursor) {
-  CursorManager* cursor_manager = Shell::GetInstance()->cursor_manager();
+  ::wm::CursorManager* cursor_manager = Shell::GetInstance()->cursor_manager();
   CursorManagerTestApi test_api(cursor_manager);
   gfx::Display display(0);
 #if defined(OS_WIN)
@@ -113,7 +111,7 @@ TEST_F(AshNativeCursorManagerTest, LockCursor) {
 }
 
 TEST_F(AshNativeCursorManagerTest, SetCursor) {
-  CursorManager* cursor_manager = Shell::GetInstance()->cursor_manager();
+  ::wm::CursorManager* cursor_manager = Shell::GetInstance()->cursor_manager();
   CursorManagerTestApi test_api(cursor_manager);
 #if defined(OS_WIN)
   ui::CursorLoaderWin::SetCursorResourceModule(L"ash_unittests.exe");
@@ -127,7 +125,7 @@ TEST_F(AshNativeCursorManagerTest, SetCursor) {
 }
 
 TEST_F(AshNativeCursorManagerTest, SetCursorSet) {
-  CursorManager* cursor_manager = Shell::GetInstance()->cursor_manager();
+  ::wm::CursorManager* cursor_manager = Shell::GetInstance()->cursor_manager();
   CursorManagerTestApi test_api(cursor_manager);
 
   EXPECT_EQ(ui::CURSOR_SET_NORMAL, test_api.GetCurrentCursorSet());
@@ -143,7 +141,7 @@ TEST_F(AshNativeCursorManagerTest, SetCursorSet) {
 }
 
 TEST_F(AshNativeCursorManagerTest, SetScale) {
-  CursorManager* cursor_manager = Shell::GetInstance()->cursor_manager();
+  ::wm::CursorManager* cursor_manager = Shell::GetInstance()->cursor_manager();
   CursorManagerTestApi test_api(cursor_manager);
 
   EXPECT_EQ(1.f, test_api.GetCurrentScale());
@@ -156,7 +154,7 @@ TEST_F(AshNativeCursorManagerTest, SetScale) {
 }
 
 TEST_F(AshNativeCursorManagerTest, SetDeviceScaleFactorAndRotation) {
-  CursorManager* cursor_manager = Shell::GetInstance()->cursor_manager();
+  ::wm::CursorManager* cursor_manager = Shell::GetInstance()->cursor_manager();
   CursorManagerTestApi test_api(cursor_manager);
 
   gfx::Display display(0);
diff --git a/ash/wm/cursor_manager_chromeos.cc b/ash/wm/cursor_manager_chromeos.cc
new file mode 100644
index 0000000..39a74d5
--- /dev/null
+++ b/ash/wm/cursor_manager_chromeos.cc
@@ -0,0 +1,55 @@
+// Copyright 2014 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 "ash/wm/cursor_manager_chromeos.h"
+
+#include "base/logging.h"
+#include "ui/keyboard/keyboard_util.h"
+#include "ui/wm/core/cursor_manager.h"
+#include "ui/wm/core/native_cursor_manager.h"
+
+namespace ash {
+
+CursorManager::CursorManager(
+    scoped_ptr< ::wm::NativeCursorManager> delegate)
+        : ::wm::CursorManager(delegate.Pass()) {
+}
+
+CursorManager::~CursorManager() {
+}
+
+bool CursorManager::ShouldHideCursorOnKeyEvent(
+    const ui::KeyEvent& event) const {
+  // Clicking on a key when the accessibility virtual keyboard is enabled should
+  // not hide the cursor.
+  if (keyboard::GetAccessibilityKeyboardEnabled())
+    return false;
+  // All alt and control key commands are ignored.
+  if (event.IsAltDown() || event.IsControlDown())
+    return false;
+
+  ui::KeyboardCode code = event.key_code();
+  if (code >= ui::VKEY_F1 && code <= ui::VKEY_F24)
+    return false;
+  if (code >= ui::VKEY_BROWSER_BACK && code <= ui::VKEY_MEDIA_LAUNCH_APP2)
+    return false;
+  switch (code) {
+    // Modifiers.
+    case ui::VKEY_SHIFT:
+    case ui::VKEY_CONTROL:
+    case ui::VKEY_MENU:
+    // Search key == VKEY_LWIN.
+    case ui::VKEY_LWIN:
+    case ui::VKEY_WLAN:
+    case ui::VKEY_POWER:
+    case ui::VKEY_BRIGHTNESS_DOWN:
+    case ui::VKEY_BRIGHTNESS_UP:
+    case ui::VKEY_KBD_BRIGHTNESS_UP:
+    case ui::VKEY_KBD_BRIGHTNESS_DOWN:
+      return false;
+    default:
+      return true;
+  }
+}
+}  // namespace ash
diff --git a/ash/wm/cursor_manager_chromeos.h b/ash/wm/cursor_manager_chromeos.h
new file mode 100644
index 0000000..70cfa1e
--- /dev/null
+++ b/ash/wm/cursor_manager_chromeos.h
@@ -0,0 +1,43 @@
+// Copyright 2014 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.
+
+#ifndef ASH_WM_CURSOR_MANAGER_CHROMEOS_H_
+#define ASH_WM_CURSOR_MANAGER_CHROMEOS_H_
+
+#include "ash/ash_export.h"
+#include "base/basictypes.h"
+#include "base/compiler_specific.h"
+#include "base/memory/scoped_ptr.h"
+#include "ui/wm/core/cursor_manager.h"
+#include "ui/wm/core/native_cursor_manager_delegate.h"
+
+namespace ui {
+class KeyEvent;
+}
+
+namespace wm {
+class NativeCursorManager;
+}
+
+namespace ash {
+
+// This class overrides the cursor hiding behaviour on ChromeOS. The cursor is
+// hidden on certain key events only if the accessibility keyboard is not
+// enabled.
+class ASH_EXPORT CursorManager : public ::wm::CursorManager {
+ public:
+  explicit CursorManager(
+      scoped_ptr< ::wm::NativeCursorManager> delegate);
+  virtual ~CursorManager();
+
+  // aura::client::CursorClient:
+  virtual bool ShouldHideCursorOnKeyEvent(
+      const ui::KeyEvent& event) const OVERRIDE;
+ private:
+  DISALLOW_COPY_AND_ASSIGN(CursorManager);
+};
+
+}  // namespace ash
+
+#endif  // ASH_WM_CURSOR_MANAGER_CHROMEOS_H_
diff --git a/ui/aura/client/cursor_client.h b/ui/aura/client/cursor_client.h
index 99d9b6a..8f2872e 100644
--- a/ui/aura/client/cursor_client.h
+++ b/ui/aura/client/cursor_client.h
@@ -14,6 +14,10 @@ namespace gfx {
 class Display;
 }
 
+namespace ui {
+class KeyEvent;
+}
+
 namespace aura {
 class Window;
 namespace client {
@@ -80,6 +84,9 @@ class AURA_EXPORT CursorClient {
   virtual void AddObserver(CursorClientObserver* observer) = 0;
   virtual void RemoveObserver(CursorClientObserver* observer) = 0;
 
+  // Returns true if the mouse cursor should be hidden on |event|.
+  virtual bool ShouldHideCursorOnKeyEvent(const ui::KeyEvent& event) const = 0;
+
  protected:
   virtual ~CursorClient() {}
 };
diff --git a/ui/aura/test/test_cursor_client.cc b/ui/aura/test/test_cursor_client.cc
index f0411ef..5662555 100644
--- a/ui/aura/test/test_cursor_client.cc
+++ b/ui/aura/test/test_cursor_client.cc
@@ -99,5 +99,10 @@ void TestCursorClient::RemoveObserver(
   observers_.RemoveObserver(observer);
 }
 
+bool TestCursorClient::ShouldHideCursorOnKeyEvent(
+    const ui::KeyEvent& event) const {
+  return true;
+}
+
 }  // namespace test
 }  // namespace aura
diff --git a/ui/aura/test/test_cursor_client.h b/ui/aura/test/test_cursor_client.h
index 2d152de..10452f1 100644
--- a/ui/aura/test/test_cursor_client.h
+++ b/ui/aura/test/test_cursor_client.h
@@ -10,6 +10,10 @@
 #include "base/observer_list.h"
 #include "ui/aura/client/cursor_client.h"
 
+namespace ui {
+class KeyEvent;
+}
+
 namespace aura {
 namespace test {
 
@@ -43,6 +47,8 @@ class TestCursorClient : public aura::client::CursorClient {
       aura::client::CursorClientObserver* observer) OVERRIDE;
   virtual void RemoveObserver(
       aura::client::CursorClientObserver* observer) OVERRIDE;
+  virtual bool ShouldHideCursorOnKeyEvent(
+      const ui::KeyEvent& event) const OVERRIDE;
 
  private:
   bool visible_;
diff --git a/ui/wm/core/compound_event_filter.cc b/ui/wm/core/compound_event_filter.cc
index 6664f72..a49e5d5 100644
--- a/ui/wm/core/compound_event_filter.cc
+++ b/ui/wm/core/compound_event_filter.cc
@@ -25,55 +25,8 @@ namespace wm {
 
 namespace {
 
-bool ShouldHideCursorOnKeyEvent(const ui::KeyEvent& event) {
-#if defined(OS_CHROMEOS)
-  // All alt and control key commands are ignored.
-  if (event.IsAltDown() || event.IsControlDown())
-    return false;
-
-  static bool inited = false;
-  static base::hash_set<int32> ignored_keys;
-  if (!inited) {
-    // Modifiers.
-    ignored_keys.insert(ui::VKEY_SHIFT);
-    ignored_keys.insert(ui::VKEY_CONTROL);
-    ignored_keys.insert(ui::VKEY_MENU);
-
-    // Search key == VKEY_LWIN.
-    ignored_keys.insert(ui::VKEY_LWIN);
-
-    // Function keys.
-    for (int key = ui::VKEY_F1; key <= ui::VKEY_F24; ++key)
-      ignored_keys.insert(key);
-
-    // Media keys.
-    for (int key = ui::VKEY_BROWSER_BACK; key <= ui::VKEY_MEDIA_LAUNCH_APP2;
-         ++key) {
-      ignored_keys.insert(key);
-    }
-
-#if defined(OS_POSIX)
-    ignored_keys.insert(ui::VKEY_WLAN);
-    ignored_keys.insert(ui::VKEY_POWER);
-    ignored_keys.insert(ui::VKEY_BRIGHTNESS_DOWN);
-    ignored_keys.insert(ui::VKEY_BRIGHTNESS_UP);
-    ignored_keys.insert(ui::VKEY_KBD_BRIGHTNESS_DOWN);
-    ignored_keys.insert(ui::VKEY_KBD_BRIGHTNESS_UP);
-#endif
-
-    inited = true;
-  }
-
-  if (ignored_keys.count(event.key_code()) > 0)
-    return false;
-
-  return true;
-#else  // !defined(OS_CHROMEOS)
-  return false;
-#endif  // defined(OS_CHROMEOS)
-}
-
 // Returns true if the cursor should be hidden on touch events.
+// TODO(tdanderson|rsadam): Move this function into CursorClient.
 bool ShouldHideCursorOnTouch(const ui::TouchEvent& event) {
 #if defined(OS_WIN)
   return true;
@@ -240,10 +193,11 @@ void CompoundEventFilter::SetMouseEventsEnableStateOnEvent(aura::Window* target,
 // CompoundEventFilter, ui::EventHandler implementation:
 
 void CompoundEventFilter::OnKeyEvent(ui::KeyEvent* event) {
-  if (ShouldHideCursorOnKeyEvent(*event)) {
-    SetCursorVisibilityOnEvent(
-        static_cast<aura::Window*>(event->target()), event, false);
-  }
+  aura::Window* target = static_cast<aura::Window*>(event->target());
+  aura::client::CursorClient* client =
+      aura::client::GetCursorClient(target->GetRootWindow());
+  if (client && client->ShouldHideCursorOnKeyEvent(*event))
+    SetCursorVisibilityOnEvent(target, event, false);
 
   FilterKeyEvent(event);
 }
diff --git a/ui/wm/core/cursor_manager.cc b/ui/wm/core/cursor_manager.cc
index 1a95c9f..736ef6f 100644
--- a/ui/wm/core/cursor_manager.cc
+++ b/ui/wm/core/cursor_manager.cc
@@ -206,6 +206,11 @@ void CursorManager::RemoveObserver(
   observers_.RemoveObserver(observer);
 }
 
+bool CursorManager::ShouldHideCursorOnKeyEvent(
+    const ui::KeyEvent& event) const {
+  return false;
+}
+
 void CursorManager::CommitCursor(gfx::NativeCursor cursor) {
   current_state_->set_cursor(cursor);
 }
diff --git a/ui/wm/core/cursor_manager.h b/ui/wm/core/cursor_manager.h
index 3640c7e..a6b226e 100644
--- a/ui/wm/core/cursor_manager.h
+++ b/ui/wm/core/cursor_manager.h
@@ -20,6 +20,10 @@ namespace gfx {
 class Display;
 }
 
+namespace ui {
+class KeyEvent;
+}
+
 namespace wm {
 
 namespace internal {
@@ -35,7 +39,7 @@ class NativeCursorManager;
 class WM_CORE_EXPORT CursorManager : public aura::client::CursorClient,
                                      public NativeCursorManagerDelegate {
  public:
-  CursorManager(scoped_ptr<NativeCursorManager> delegate);
+  explicit CursorManager(scoped_ptr<NativeCursorManager> delegate);
   virtual ~CursorManager();
 
   // Overridden from aura::client::CursorClient:
@@ -59,6 +63,8 @@ class WM_CORE_EXPORT CursorManager : public aura::client::CursorClient,
       aura::client::CursorClientObserver* observer) OVERRIDE;
   virtual void RemoveObserver(
       aura::client::CursorClientObserver* observer) OVERRIDE;
+  virtual bool ShouldHideCursorOnKeyEvent(
+      const ui::KeyEvent& event) const OVERRIDE;
 
  private:
   // Overridden from NativeCursorManagerDelegate: