Fix KeyboardOverlayView not closing by pressing any cancelling key.

KeyboardOverlayView::IsCancelingKeyEvent() is given a KeyEvent instead of
an accelerator, and the comparison is done expecting exact equality of the
KeyEvent's flags and the cancelling shortcut flags. Any non-modifier flags
generated on the KeyEvent will make the comparison fail and hence the Keyboard
Overlay gets stuck.

We fix this by masking out all non-key-modifier flags before we do the comparison.

BUG=535008
TEST=ash_unittests --gtest_filter=KeyboardOverlayViewTest.*

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

Cr-Commit-Position: refs/heads/master@{#359926}

3 files changed, 28 insertions, 2 deletions

diff --git a/ash/content/keyboard_overlay/keyboard_overlay_view.cc b/ash/content/keyboard_overlay/keyboard_overlay_view.cc
index 9673df2..e0a547d 100644
--- a/ash/content/keyboard_overlay/keyboard_overlay_view.cc
+++ b/ash/content/keyboard_overlay/keyboard_overlay_view.cc
@@ -26,6 +26,7 @@ const ash::KeyboardOverlayView::KeyEventData kCancelKeys[] = {
     {ui::VKEY_HELP, ui::EF_NONE},
     {ui::VKEY_F14, ui::EF_NONE},
 };
+
 }
 
 namespace ash {
@@ -47,8 +48,10 @@ void KeyboardOverlayView::Cancel() {
 bool KeyboardOverlayView::IsCancelingKeyEvent(ui::KeyEvent* event) {
   if (event->type() != ui::ET_KEY_PRESSED)
     return false;
-  // Ignore the caps lock state.
-  const int flags = (event->flags() & ~ui::EF_CAPS_LOCK_DOWN);
+
+  // Ignore the non-modifiers flags in order to treat it the same way
+  // Accelerators are generated and compared. crbug.com/535008.
+  const int flags = ui::Accelerator::MaskOutKeyEventFlags(event->flags());
   for (size_t i = 0; i < arraysize(kCancelKeys); ++i) {
     if ((kCancelKeys[i].key_code == event->key_code()) &&
         (kCancelKeys[i].flags == flags))
diff --git a/ash/content/keyboard_overlay/keyboard_overlay_view.h b/ash/content/keyboard_overlay/keyboard_overlay_view.h
index 99d0354..5d8c1af 100644
--- a/ash/content/keyboard_overlay/keyboard_overlay_view.h
+++ b/ash/content/keyboard_overlay/keyboard_overlay_view.h
@@ -52,6 +52,8 @@ class ASH_WITH_CONTENT_EXPORT KeyboardOverlayView
 
  private:
   FRIEND_TEST_ALL_PREFIXES(KeyboardOverlayViewTest, OpenAcceleratorsClose);
+  FRIEND_TEST_ALL_PREFIXES(KeyboardOverlayViewTest,
+                           TestCancelingKeysWithNonModifierFlags);
   FRIEND_TEST_ALL_PREFIXES(KeyboardOverlayViewTest, NoRedundantCancelingKeys);
 
   // Overridden from views::WidgetDelegate:
diff --git a/ash/content/keyboard_overlay/keyboard_overlay_view_unittest.cc b/ash/content/keyboard_overlay/keyboard_overlay_view_unittest.cc
index 62cdde7..ef5159e 100644
--- a/ash/content/keyboard_overlay/keyboard_overlay_view_unittest.cc
+++ b/ash/content/keyboard_overlay/keyboard_overlay_view_unittest.cc
@@ -41,6 +41,27 @@ TEST_F(KeyboardOverlayViewTest, OpenAcceleratorsClose) {
   }
 }
 
+// Test modifiers that might exist in a KeyEvent but they shouldn't be
+// considered in an accelerator comparison to determine if a KeyEvent is a
+// canceling key.
+TEST_F(KeyboardOverlayViewTest, TestCancelingKeysWithNonModifierFlags) {
+  ui::test::TestWebDialogDelegate delegate(GURL("chrome://keyboardoverlay"));
+  KeyboardOverlayView view(
+      ShellContentState::GetInstance()->GetActiveBrowserContext(), &delegate,
+      new ui::test::TestWebContentsHandler);
+
+  const int kNonModifierFlags = ui::EF_IS_REPEAT | ui::EF_IME_FABRICATED_KEY |
+                                ui::EF_NUM_LOCK_DOWN | ui::EF_IS_SYNTHESIZED;
+
+  std::vector<KeyboardOverlayView::KeyEventData> canceling_keys;
+  KeyboardOverlayView::GetCancelingKeysForTesting(&canceling_keys);
+  for (const auto& key_data : canceling_keys) {
+    ui::KeyEvent key_event(ui::ET_KEY_PRESSED, key_data.key_code,
+                           key_data.flags | kNonModifierFlags);
+    EXPECT_TRUE(view.IsCancelingKeyEvent(&key_event));
+  }
+}
+
 // Verifies that there are no redunduant keys in the canceling keys.
 TEST_F(KeyboardOverlayViewTest, NoRedundantCancelingKeys) {
   std::vector<KeyboardOverlayView::KeyEventData> open_keys;