Regression: Search+Key pops up app launcher

ui/base/accelerators/accelerator_history.cc

+// 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 "ui/base/accelerators/accelerator_history.h"
+#include "ui/events/event_constants.h"
+
+namespace ui {
+
+namespace {
+
+inline int CalculateModifiers(const ui::KeyEvent& event) {
+  int modifiers = ui::EF_NONE;
+  if (event.IsShiftDown())
+    modifiers |= ui::EF_SHIFT_DOWN;
+  if (event.IsControlDown())
+    modifiers |= ui::EF_CONTROL_DOWN;
+  if (event.IsAltDown())
+    modifiers |= ui::EF_ALT_DOWN;
+#if defined(OS_MACOSX) || defined(OS_CHROMEOS)
+  if (event.IsCommandDown())
+  modifiers |= ui::EF_COMMAND_DOWN;
+#endif
+  return modifiers;
+}
+
+};
+
+// ----------------------------------------------------------------------
+// Public Methods
+// ----------------------------------------------------------------------
+
+AcceleratorHistory* AcceleratorHistory::GetInstance() {
+  return Singleton<AcceleratorHistory>::get();
+}
+
+void AcceleratorHistory::BuildCurrentAccelerator(const ui::KeyEvent& event) {
+  // Only build accelerators for 'pressed' or 'released' key events.
+  if (event.type() != ui::ET_KEY_PRESSED && event.type() != ui::ET_KEY_RELEASED)

[Jun Mukai, 2014/11/14 02:09:48]

Can this line be IsKeyEvent() instead?

[afakhry, 2014/11/14 17:30:57]

No, because we want to avoid types like ET_TRANSLATED_KEY_PRESS, and
ET_TRANSLATED_KEY_RELEASE.

+    return;
+
+  // Record the new accelerator only if the new key event is different from the
+  // previous one.
+  int modifiers = CalculateModifiers(event);

[Jun Mukai, 2014/11/14 02:09:48]

why not directly using event.flags()?

[afakhry, 2014/11/14 17:30:57]

We are only interested in the flags used by the Accelerator. However, I think we
can do something more clever than if/elses.. Thanks for pointing this out!

+  if (ShouldBuildCurrentAccelerator(event, modifiers)) {
+    ui::Accelerator accelerator(event.key_code(), modifiers);
+    accelerator.set_type(event.type());
+    accelerator.set_is_repeat(event.IsRepeat());
+
+    previous_accelerator_ = current_accelerator_;
+    current_accelerator_ = AcceleratorEvent(accelerator);
+  }
+}
+
+const AcceleratorEvent& AcceleratorHistory::GetCurrentAccelerator() const {
+  return current_accelerator_;
+}
+
+const AcceleratorEvent& AcceleratorHistory::GetPreviousAccelerator() const {
+  return previous_accelerator_;
+}
+
+// ----------------------------------------------------------------------
+// Private Methods
+// ----------------------------------------------------------------------
+
+AcceleratorHistory::AcceleratorHistory()
+  : current_accelerator_(),
+    previous_accelerator_() {
+}
+
+AcceleratorHistory::~AcceleratorHistory() {
+}
+
+bool AcceleratorHistory::ShouldBuildCurrentAccelerator(
+                                const ui::KeyEvent& event,
+                                const int event_modifiers) const {
+  return !(event.type() == current_accelerator_.accelerator.type() &&
+          event.key_code() == current_accelerator_.accelerator.key_code() &&
+          event.IsRepeat() == current_accelerator_.accelerator.IsRepeat() &&
+          event_modifiers == current_accelerator_.accelerator.modifiers());
+}
+
+}; // namespace ui
+
+