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

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',

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_;
 

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();

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);

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

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_

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() {}
 };

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

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_;

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);
 }

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);
 }

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: