Fix the issue of ShellAcceleratorFilter sending accelerators twice for character keys.

ui/aura_shell/shell_accelerator_controller_unittest.cc

 // Copyright (c) 2011 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 "ui/aura/root_window.h"
+#include "ui/aura/test/test_window_delegate.h"
+#include "ui/aura/test/test_windows.h"
 #include "ui/aura_shell/shell.h"
 #include "ui/aura_shell/shell_accelerator_controller.h"
+#include "ui/aura_shell/shell_accelerator_filter.h"
+#include "ui/aura_shell/shell_window_ids.h"
 #include "ui/aura_shell/test/aura_shell_test_base.h"
 
+#if defined(USE_X11)
+#include <X11/Xlib.h>
+#include "base/memory/scoped_ptr.h"
+#include "ui/base/keycodes/keyboard_code_conversion_x.h"
+#endif
+
 namespace aura_shell {
 namespace test {
 
 namespace {
 class TestTarget : public ui::AcceleratorTarget {
  public:
-  TestTarget() : accelerator_pressed_(false) {};
+  TestTarget() : accelerator_pressed_count_(0) {};
   virtual ~TestTarget() {};
 
   bool accelerator_pressed() const {

[Daniel Erat, 2011/12/07 16:58:07]

this method seems unnecessary now -- can't callers just use
accelerator_pressed_count() instead?

[mazda, 2011/12/08 05:39:24]

Replaced accelerator_pressed with accelerator_pressed_count.

-    return accelerator_pressed_;
+    return accelerator_pressed_count_ > 0;
   }
 
-  void set_accelerator_pressed(bool accelerator_pressed) {
-    accelerator_pressed_ = accelerator_pressed;
+  int accelerator_pressed_count() const {
+    return accelerator_pressed_count_;
+  }
+
+  void set_accelerator_pressed_count(int accelerator_pressed_count) {
+    accelerator_pressed_count_ = accelerator_pressed_count;
   }
 
   // Overridden from ui::AcceleratorTarget:
   virtual bool AcceleratorPressed(const ui::Accelerator& accelerator) OVERRIDE;
 
  private:
-  bool accelerator_pressed_;
+  int accelerator_pressed_count_;
 };
 
 bool TestTarget::AcceleratorPressed(const ui::Accelerator& accelerator) {
-  set_accelerator_pressed(true);
+  ++accelerator_pressed_count_;
   return true;
 }
 
 }  // namespace
 
 class ShellAcceleratorControllerTest : public AuraShellTestBase {
  public:
   ShellAcceleratorControllerTest() {};
   virtual ~ShellAcceleratorControllerTest() {};
 
   static ShellAcceleratorController* GetController();
 
-  // testing::Test:
-  // virtual void SetUp() OVERRIDE;
-  // virtual void TearDown() OVERRIDE;
+ private:
+  // Overridden from testing::Test:
+  virtual void SetUp() OVERRIDE;
+  virtual void TearDown() OVERRIDE;
+
+  scoped_ptr<aura_shell::internal::ShellAcceleratorFilter> accelerator_filter_;
 };
 
+void ShellAcceleratorControllerTest::SetUp() {
+  AuraShellTestBase::SetUp();
+  aura_shell::Shell::GetInstance()->AddRootWindowEventFilter(
+      accelerator_filter_.get());
+  // A focused window must exist for accelerators to be processed.
+  aura::Window* default_container =
+      aura_shell::Shell::GetInstance()->GetContainer(
+          internal::kShellWindowId_DefaultContainer);
+  aura::Window* window = aura::test::CreateTestWindowWithDelegate(
+      new aura::test::TestWindowDelegate,
+      -1,
+      gfx::Rect(),
+      default_container);
+  window->Activate();
+}
+
+void ShellAcceleratorControllerTest::TearDown() {
+  aura_shell::Shell::GetInstance()->RemoveRootWindowEventFilter(
+      accelerator_filter_.get());
+  AuraShellTestBase::TearDown();
+}
+
 ShellAcceleratorController* ShellAcceleratorControllerTest::GetController() {
   return Shell::GetInstance()->accelerator_controller();
 }
 
-// void ShellAcceleratorControllerTest::SetUp() {
-//   AuraShellTestBase::SetUp();
-// }
-
-// void ShellAcceleratorControllerTest::TearDown() {
-//   AuraShellTestBase::TearDown();
-// }
-
 TEST_F(ShellAcceleratorControllerTest, Register) {
   const ui::Accelerator accelerator_a(ui::VKEY_A, false, false, false);
   TestTarget target;
   GetController()->Register(accelerator_a, &target);
 
   // The registered accelerator is processed.
   EXPECT_TRUE(GetController()->Process(accelerator_a));
   EXPECT_TRUE(target.accelerator_pressed());
 }
 
@@ skipping 18 matching lines @@
   const ui::Accelerator accelerator_b(ui::VKEY_B, false, false, false);
   GetController()->Register(accelerator_b, &target);
 
   // Unregistering a different accelerator does not affect the other
   // accelerator.
   GetController()->Unregister(accelerator_b, &target);
   EXPECT_TRUE(GetController()->Process(accelerator_a));
   EXPECT_TRUE(target.accelerator_pressed());
 
   // The unregistered accelerator is no longer processed.
-  target.set_accelerator_pressed(false);
+  target.set_accelerator_pressed_count(0);
   GetController()->Unregister(accelerator_a, &target);
   EXPECT_FALSE(GetController()->Process(accelerator_a));
   EXPECT_FALSE(target.accelerator_pressed());
 }
 
 TEST_F(ShellAcceleratorControllerTest, UnregisterAll) {
   const ui::Accelerator accelerator_a(ui::VKEY_A, false, false, false);
   TestTarget target1;
   GetController()->Register(accelerator_a, &target1);
   const ui::Accelerator accelerator_b(ui::VKEY_B, false, false, false);
@@ skipping 20 matching lines @@
 
   // The registered accelerator is processed.
   EXPECT_TRUE(GetController()->Process(accelerator_a));
   EXPECT_TRUE(target1.accelerator_pressed());
 
   // The non-registered accelerator is not processed.
   const ui::Accelerator accelerator_b(ui::VKEY_B, false, false, false);
   EXPECT_FALSE(GetController()->Process(accelerator_b));
 }
 
+#if defined(OS_WIN) || defined(USE_X11)
+TEST_F(ShellAcceleratorControllerTest, ProcessOnce) {
+  const ui::Accelerator accelerator_a(ui::VKEY_A, false, false, false);
+  TestTarget target1;

[Daniel Erat, 2011/12/07 16:58:07]

nit: naming it 'target1' in tests that only have a single target seems strange

[mazda, 2011/12/08 05:39:24]

Done.

+  GetController()->Register(accelerator_a, &target1);
+
+  // The accelerator is processed only once.
+#if defined(OS_WIN)
+  MSG msg1 = { NULL, WM_KEYDOWN, VKEY_A, 0 };
+  EXPECT_TRUE(aura::RootWindow::GetInstance()->GetDispatcher()->Dispatch(msg1));
+  MSG msg2 = { NULL, WM_CHAR, L'A', 0 };
+  EXPECT_TRUE(aura::RootWindow::GetInstance()->GetDispatcher()->Dispatch(msg2));
+#elif defined(USE_X11)
+  scoped_ptr<XEvent> key_event(ui::CreateXEventForTesting(ui::ET_KEY_PRESSED,
+                                                          ui::VKEY_A,
+                                                          0));
+  EXPECT_TRUE(aura::RootWindow::GetInstance()->GetDispatcher()->Dispatch(
+      key_event.get()));
+#endif
+  EXPECT_EQ(1, target1.accelerator_pressed_count());
+}
+#endif
+
 TEST_F(ShellAcceleratorControllerTest, GlobalAccelerators) {
   // TODO(mazda): Uncomment the followings once they are implemented.
   // CycleBackward
   // EXPECT_TRUE(GetController()->Process(
   //     ui::Accelerator(ui::VKEY_TAB, true, false, true)));
   // CycleForwrard
   // EXPECT_TRUE(GetController()->Process(
   //     ui::Accelerator(ui::VKEY_TAB, false, false, true)));
   // TakeScreenshot
   // EXPECT_TRUE(GetController()->Process(
   //     ui::Accelerator(ui::VKEY_F5, false, true, false)));
   // EXPECT_TRUE(GetController()->Process(
   //     ui::Accelerator(ui::VKEY_PRINT, false, false, false)));
 #if !defined(NDEBUG)
   // TODO(mazda): Callig RotateScreen in unit test causes a crash because of
   // "pure virtual method called" for some reasons. Need to investigate.
   // RotateScreen
   // EXPECT_TRUE(GetController()->Process(
   //     ui::Accelerator(ui::VKEY_HOME, false, true, false)));
 #if !defined(OS_LINUX)
   // ToggleDesktopFullScreen (not implemented yet on Linux)
   EXPECT_TRUE(GetController()->Process(
       ui::Accelerator(ui::VKEY_F11, false, true, false)));
 #endif
 #endif
 }
 
 }  // namespace test
 }  // namespace aura_shell