Fix accelerator history tracking

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 {
 
-// ----------------------------------------------------------------------
-// Public Methods
-// ----------------------------------------------------------------------
+AcceleratorHistory::AcceleratorHistory() {}
 
-AcceleratorHistory::AcceleratorHistory()
-  : current_accelerator_(),
-    previous_accelerator_() {
-}
-
-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;
   }
 }
 
 } // namespace ui