Fix accelerator history tracking

ui/base/accelerators/accelerator_history.cc

Index: ui/base/accelerators/accelerator_history.cc
diff --git a/ui/base/accelerators/accelerator_history.cc b/ui/base/accelerators/accelerator_history.cc
index d356e3209146c77f2ca603cd4734abda3924a184..94e78da26927d3cca60b718d5c2ca74fefcfd1cd 100644
--- a/ui/base/accelerators/accelerator_history.cc
+++ b/ui/base/accelerators/accelerator_history.cc
@@ -4,24 +4,27 @@
 
 #include "ui/base/accelerators/accelerator_history.h"
 
-#include "ui/events/event_constants.h"
-
 namespace ui {
 
-// ----------------------------------------------------------------------
-// Public Methods
-// ----------------------------------------------------------------------
-
-AcceleratorHistory::AcceleratorHistory()
-  : current_accelerator_(),
-    previous_accelerator_() {
-}
+AcceleratorHistory::AcceleratorHistory() {}
 
-AcceleratorHistory::~AcceleratorHistory() {
-}
+AcceleratorHistory::~AcceleratorHistory() {}
 
 void AcceleratorHistory::StoreCurrentAccelerator(
-                            const Accelerator& accelerator) {
+    const Accelerator& accelerator) {
+  // Track the currently pressed keys so that we don't mistakenly store an
+  // already pressed key as a new keypress after another key has been released.
+  // As an example, when the user presses and holds Alt+Search, then releases
+  // Alt but keeps holding the Search key down, at this point no new Search
+  // presses should be stored in the history after the Alt release, since Search
+  // was never released in the first place. crbug.com/704280.
+  if (accelerator.key_state() == Accelerator::KeyState::PRESSED) {
+    if (!currently_pressed_keys_.emplace(accelerator.key_code()).second)

[sadrul, 2017/04/07 05:43:39]

What code is generating the second Accelerator? Is it the repeat key-press event
for the search key? Does that event have the REPEAT flag turned on? Can we
filter based on that flag?

[afakhry, 2017/04/07 15:42:57]

Unfortunately, REPEAT is unreliable in this case. When we press and hold Alt,
Search, then release Alt (while still holding Search) a new key_event is
generated for Search right after releasing Alt, which is not marked as REPEAT.

The way that REPEAT flag is set is broken in this case since it only compares
the current event with the previous event:
https://cs.chromium.org/chromium/src/ui/events/event.cc?l=1123.
The Alt_Release event in the middle of the Search repeats breaks this. We need
to track the currently pressed events.

+      return;
+  } else {
+    currently_pressed_keys_.erase(accelerator.key_code());
+  }
+
   if (accelerator != current_accelerator_) {
     previous_accelerator_ = current_accelerator_;
     current_accelerator_ = accelerator;