Fix the toggle caps lock accelerator not always working.

ash/accelerators/accelerator_controller_unittest.cc

Index: ash/accelerators/accelerator_controller_unittest.cc
diff --git a/ash/accelerators/accelerator_controller_unittest.cc b/ash/accelerators/accelerator_controller_unittest.cc
index 9e6d3f3f0fc5528fe8dc15bd60f13cdf5e84895f..dcd74d8e0ca072984d690fea65afeac274345949 100644
--- a/ash/accelerators/accelerator_controller_unittest.cc
+++ b/ash/accelerators/accelerator_controller_unittest.cc
@@ -38,6 +38,10 @@
 #include "ui/aura/test/test_window_delegate.h"
 #include "ui/aura/test/test_windows.h"
 #include "ui/aura/window.h"
+#include "ui/base/ime/chromeos/fake_ime_keyboard.h"
+#include "ui/base/ime/chromeos/ime_keyboard.h"
+#include "ui/base/ime/chromeos/input_method_manager.h"
+#include "ui/base/ime/chromeos/mock_input_method_manager.h"
 #include "ui/display/manager/display_manager.h"
 #include "ui/display/screen.h"
 #include "ui/events/event.h"
@@ -1121,6 +1125,89 @@ TEST_F(AcceleratorControllerTest, PreferredReservedAccelerators) {
 
 namespace {
 
+class TestInputMethodManager
+    : public chromeos::input_method::MockInputMethodManager {
+ public:
+  TestInputMethodManager() = default;
+  ~TestInputMethodManager() override = default;
+
+  // MockInputMethodManager:
+  chromeos::input_method::ImeKeyboard* GetImeKeyboard() override {
+    return &keyboard_;
+  }
+
+ private:
+  chromeos::input_method::FakeImeKeyboard keyboard_;
+
+  DISALLOW_COPY_AND_ASSIGN(TestInputMethodManager);
+};
+
+class ToggleCapsLockTest : public AcceleratorControllerTest {
+ public:
+  ToggleCapsLockTest() = default;
+  ~ToggleCapsLockTest() override = default;
+
+  void SetUp() override {
+    AcceleratorControllerTest::SetUp();
+    chromeos::input_method::InputMethodManager::Initialize(
+        new TestInputMethodManager);
+  }
+
+  void TearDown() override {
+    chromeos::input_method::InputMethodManager::Shutdown();
+    AcceleratorControllerTest::TearDown();
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(ToggleCapsLockTest);
+};
+
+}  // namespace
+
+// Tests the four combinations of the TOGGLE_CAPS_LOCK accelerator.
+TEST_F(ToggleCapsLockTest, ToggleCapsLockAccelerators) {

[James Cook, 2017/02/08 22:39:08]

nit: Could the test itself live in the anonymous namespace? Or does it use
friends that need to be non-anonymous?

[afakhry, 2017/02/09 00:28:30]

The test can be placed in the anonymous namespace, but why is it better to do
so, especially that most other tests are not?

Done anyways.

[James Cook, 2017/02/09 01:22:21]

Putting it in an anonymous namespace reduces the amount of work the linker has
to do when resolving symbols, so linking ash_unittests might be a little faster.
I wish we did it for more tests. But you can't do it if you use FRIEND_TEST.

+  chromeos::input_method::InputMethodManager* input_method_manager =
+      chromeos::input_method::InputMethodManager::Get();
+  ASSERT_TRUE(input_method_manager);
+  EXPECT_FALSE(input_method_manager->GetImeKeyboard()->CapsLockIsEnabled());
+
+  // 1. Press Alt, Press Search, Release Search, Release Alt.
+  EXPECT_FALSE(
+      ProcessInController(ui::Accelerator(ui::VKEY_MENU, ui::EF_COMMAND_DOWN)));
+  EXPECT_TRUE(
+      ProcessInController(ReleaseAccelerator(ui::VKEY_LWIN, ui::EF_ALT_DOWN)));
+  EXPECT_TRUE(input_method_manager->GetImeKeyboard()->CapsLockIsEnabled());
+  input_method_manager->GetImeKeyboard()->SetCapsLockEnabled(false);
+  EXPECT_FALSE(input_method_manager->GetImeKeyboard()->CapsLockIsEnabled());

[James Cook, 2017/02/08 22:39:08]

nit: I don't think you need to check that caps-lock is disabled after you
explicitly disabled it. (and below)

[afakhry, 2017/02/09 00:28:30]

Me being extra cautious! Done.

+
+  // 2. Press Search, Press Alt, Release Search, Release Alt.
+  EXPECT_FALSE(
+      ProcessInController(ui::Accelerator(ui::VKEY_LWIN, ui::EF_ALT_DOWN)));
+  EXPECT_TRUE(
+      ProcessInController(ReleaseAccelerator(ui::VKEY_LWIN, ui::EF_ALT_DOWN)));
+  EXPECT_TRUE(input_method_manager->GetImeKeyboard()->CapsLockIsEnabled());
+  input_method_manager->GetImeKeyboard()->SetCapsLockEnabled(false);
+  EXPECT_FALSE(input_method_manager->GetImeKeyboard()->CapsLockIsEnabled());
+
+  // 3. Press Alt, Press Search, Release Alt, Release Search.
+  EXPECT_FALSE(
+      ProcessInController(ui::Accelerator(ui::VKEY_MENU, ui::EF_COMMAND_DOWN)));

[James Cook, 2017/02/08 22:39:08]

optional: I wonder if this test would read better with local vars like
press_alt_search and press_search_alt. It might make the line wrapping better,
and would help readers who don't have the command-means-alt mapping firmly in
their head.

[afakhry, 2017/02/09 00:28:30]

Great suggestion, actually it helped me fixed the order of the tests to match
the comments. Done.

+  EXPECT_TRUE(ProcessInController(
+      ReleaseAccelerator(ui::VKEY_MENU, ui::EF_COMMAND_DOWN)));
+  EXPECT_TRUE(input_method_manager->GetImeKeyboard()->CapsLockIsEnabled());
+  input_method_manager->GetImeKeyboard()->SetCapsLockEnabled(false);
+  EXPECT_FALSE(input_method_manager->GetImeKeyboard()->CapsLockIsEnabled());
+
+  // 4. Press Search, Press Alt, Release Alt, Release Search.
+  EXPECT_FALSE(
+      ProcessInController(ui::Accelerator(ui::VKEY_LWIN, ui::EF_ALT_DOWN)));
+  EXPECT_TRUE(ProcessInController(
+      ReleaseAccelerator(ui::VKEY_MENU, ui::EF_COMMAND_DOWN)));
+  EXPECT_TRUE(input_method_manager->GetImeKeyboard()->CapsLockIsEnabled());
+}

[James Cook, 2017/02/08 22:39:08]

Nice test. I like how you documented each block.

[afakhry, 2017/02/09 00:28:30]

Thanks.

+
+namespace {
+
 class PreferredReservedAcceleratorsTest : public test::AshTestBase {
  public:
   PreferredReservedAcceleratorsTest() {}