Detect and release stuck modifier keys.

We consider a modifier key to be "stuck" if we have sent a keydown event for it and subsequent input event indicates that it is no longer active.

BUG=464534

Review URL: https://codereview.chromium.org/984183002

Cr-Commit-Position: refs/heads/master@{#320220}

remoting/client/plugin/chromoting_instance.cc

@@ -189,6 +189,7 @@ ChromotingInstance::ChromotingInstance(PP_Instance pp_instance)
       input_tracker_(&mouse_input_filter_),
       touch_input_scaler_(&input_tracker_),
       key_mapper_(&touch_input_scaler_),
+      input_handler_(&input_tracker_),
       cursor_setter_(this),
       empty_cursor_filter_(&cursor_setter_),
       text_input_controller_(this),

remoting/client/plugin/pepper_input_handler.cc

@@ -13,6 +13,8 @@
 #include "ppapi/cpp/touch_point.h"
 #include "ppapi/cpp/var.h"
 #include "remoting/proto/event.pb.h"
+#include "remoting/protocol/input_event_tracker.h"
+#include "remoting/protocol/input_stub.h"
 #include "ui/events/keycodes/dom4/keycode_converter.h"
 
 namespace remoting {
@@ -106,8 +108,10 @@ protocol::MouseEvent MakeMouseEvent(const pp::MouseInputEvent& pp_mouse_event,
 
 }  // namespace
 
-PepperInputHandler::PepperInputHandler()
+PepperInputHandler::PepperInputHandler(
+    protocol::InputEventTracker* input_tracker)
     : input_stub_(nullptr),
+      input_tracker_(input_tracker),
       has_focus_(false),
       send_mouse_input_when_unfocused_(false),
       send_mouse_move_deltas_(false),
@@ -118,6 +122,8 @@ PepperInputHandler::PepperInputHandler()
 }
 
 bool PepperInputHandler::HandleInputEvent(const pp::InputEvent& event) {
+  ReleaseAllIfModifiersStuck(event);
+
   switch (event.GetType()) {
     // Touch input cases.
     case PP_INPUTEVENT_TYPE_TOUCHSTART:
@@ -259,4 +265,29 @@ void PepperInputHandler::DidChangeFocus(bool has_focus) {
   has_focus_ = has_focus;
 }
 
+void PepperInputHandler::ReleaseAllIfModifiersStuck(
+    const pp::InputEvent& event) {
+  switch (event.GetType()) {
+    case PP_INPUTEVENT_TYPE_MOUSEMOVE:
+    case PP_INPUTEVENT_TYPE_MOUSEENTER:
+    case PP_INPUTEVENT_TYPE_MOUSELEAVE:
+      // Don't check modifiers on every mouse move event.
+      break;
+
+    case PP_INPUTEVENT_TYPE_KEYUP:
+      // PPAPI doesn't always set modifiers correctly on KEYUP events. See
+      // crbug.com/464791 for details.
+      break;
+
+    default: {
+      uint32_t modifiers = event.GetModifiers();
+      input_tracker_->ReleaseAllIfModifiersStuck(
+          (modifiers & PP_INPUTEVENT_MODIFIER_ALTKEY) != 0,
+          (modifiers & PP_INPUTEVENT_MODIFIER_CONTROLKEY) != 0,
+          (modifiers & PP_INPUTEVENT_MODIFIER_METAKEY) != 0,
+          (modifiers & PP_INPUTEVENT_MODIFIER_SHIFTKEY) != 0);
+    }
+  }
+}
+
 }  // namespace remoting

remoting/client/plugin/pepper_input_handler.h

@@ -6,7 +6,6 @@
 #define REMOTING_CLIENT_PLUGIN_PEPPER_INPUT_HANDLER_H_
 
 #include "base/memory/scoped_ptr.h"
-#include "remoting/protocol/input_stub.h"
 
 namespace pp {
 class InputEvent;
@@ -16,11 +15,15 @@ namespace remoting {
 
 namespace protocol {
 class InputStub;
+class InputEventTracker;
 } // namespace protocol
 
 class PepperInputHandler {
  public:
-  PepperInputHandler();
+  // Create a PepperInputHandler using the specified InputEventTracker to
+  // handle auto-release. The InputEventTracker instance must remain valid
+  // for the lifetime of the PepperInputHandler.
+  explicit PepperInputHandler(protocol::InputEventTracker* input_tracker);
 
   // Sets the input stub to which processed events will be passed.
   void set_input_stub(protocol::InputStub* input_stub) {
@@ -44,9 +47,18 @@ class PepperInputHandler {
   void DidChangeFocus(bool has_focus);
 
  private:
+  // Check for any missed "keyup" events for modifiers. These can sometimes be
+  // missed due to OS-level keyboard shortcuts such as "lock screen" that cause
+  // focus to switch to another application. If any modifier keys are held that
+  // are not indicated as active on |event|, release all keys.
+  void ReleaseAllIfModifiersStuck(const pp::InputEvent& event);
+
   // Receives input events generated from PPAPI input.
   protocol::InputStub* input_stub_;
 
+  // Tracks input events to manage auto-release in ReleaseAllIfModifiersStuck.
+  protocol::InputEventTracker* input_tracker_;
+
   // True if the plugin has focus.
   bool has_focus_;
 

remoting/protocol/input_event_tracker.cc

@@ -67,6 +67,30 @@ void InputEventTracker::ReleaseAll() {
   DCHECK(touch_point_ids_.empty());
 }
 
+void InputEventTracker::ReleaseAllIfModifiersStuck(bool alt_expected,
+                                                   bool ctrl_expected,
+                                                   bool meta_expected,
+                                                   bool shift_expected) {
+  // See src/ui/events/keycodes/dom4/keycode_converter_data.h for these values.
+  bool alt_down =
+      pressed_keys_.find(0x0700e2) != pressed_keys_.end() ||  // Left
+      pressed_keys_.find(0x0700e6) != pressed_keys_.end();    // Right
+  bool ctrl_down =
+      pressed_keys_.find(0x0700e0) != pressed_keys_.end() ||  // Left
+      pressed_keys_.find(0x0700e4) != pressed_keys_.end();    // Right
+  bool meta_down =
+      pressed_keys_.find(0x0700e3) != pressed_keys_.end() ||  // Left
+      pressed_keys_.find(0x0700e7) != pressed_keys_.end();    // Right
+  bool shift_down =
+      pressed_keys_.find(0x0700e1) != pressed_keys_.end() ||  // Left
+      pressed_keys_.find(0x0700e5) != pressed_keys_.end();    // Right
+
+  if ((alt_down && !alt_expected) || (ctrl_down && !ctrl_expected) ||
+      (meta_down && !meta_expected) || (shift_down && !shift_expected)) {
+    ReleaseAll();
+  }
+}
+
 void InputEventTracker::InjectKeyEvent(const KeyEvent& event) {
   // We don't need to track the keyboard lock states of key down events.
   // Pressed keys will be released with |lock_states| set to 0.

remoting/protocol/input_event_tracker.h

@@ -33,6 +33,12 @@ class InputEventTracker : public InputStub {
   // touch points to the InputStub.
   void ReleaseAll();
 
+  // Similar to ReleaseAll, but conditional on a modifier key tracked by this
+  // class being pressed without the corresponding parameter indicating that it
+  // should be.
+  void ReleaseAllIfModifiersStuck(bool alt_expected, bool ctrl_expected,
+                                  bool meta_expected, bool shift_expected);
+
   // InputStub interface.
   void InjectKeyEvent(const KeyEvent& event) override;
   void InjectTextEvent(const TextEvent& event) override;