CBC-RS/TC-watchlist

Adding a bunch of folks to take a look. This should def not happen

robby.n.payne@ does the device start with the correct layout and then revert to the old? Or does it come up with the wrong layout on boot/wake and recover on a reboot.  Can you file a feedback report via alt+shift+i if this happens again.

afakhry@, I'm looking at the code in event_rewriter_chromeos.cc and it looks like we're reading the keyboard type from udev on every function key press. If anything fails there we'll end up falling back to the older layout. We should be able to figure out a way to make this more robust.

Yes, this was introduced in this CL: https://chromium-review.googlesource.com/c/chromium/src/+/571652.
If we fail to get the proper layout from udev for any reason, we'll default to Layout1 (non-eve layout), and we'll store this keyboard device in `device_id_to_info_` with that default layout (even though it might be wrong due to the earlier failure). On subsequent key events, we'll use that value we stored, and won't attempt to re-read from udev.
That's why rebooting is the only way to recover from this bad state.

I suggest observing those potential failures, and not storing the device info when they happen. This gives us a chance to retry.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/aa02b4e9e62729e8570796fd5de6597fba1ceb51

commit aa02b4e9e62729e8570796fd5de6597fba1ceb51
Author: Ahmed Fakhry <afakhry@google.com>
Date: Mon Nov 13 21:43:32 2017

Don't store the keyboard info when reading the layout property fails

Reading the keyboard device layout property from udev might fail. When
this happens, we used to store the keyboard info with a default layout
value = Layout1.

This CL avoid storing the keyboard info when a failure in udev is detected,
we will still fallback to the default layout for the current key event, but
we will attempt to re-read the layout property from udev on subsequent keys
until a valid layout is returned. This eliminates the need to reboot the
device to exit the bad state.

BUG= 783166 
TEST=added new test.

Change-Id: I94d6771ef955e20cee831ad0a93e31dcaaf8f610
Reviewed-on: https://chromium-review.googlesource.com/764444
Reviewed-by: Dan Erat <derat@chromium.org>
Commit-Queue: Ahmed Fakhry <afakhry@chromium.org>
Cr-Commit-Position: refs/heads/master@{#516063}
[modify] https://crrev.com/aa02b4e9e62729e8570796fd5de6597fba1ceb51/chrome/browser/chromeos/events/event_rewriter_unittest.cc
[modify] https://crrev.com/aa02b4e9e62729e8570796fd5de6597fba1ceb51/ui/chromeos/events/event_rewriter_chromeos.cc
[modify] https://crrev.com/aa02b4e9e62729e8570796fd5de6597fba1ceb51/ui/chromeos/events/event_rewriter_chromeos.h
[modify] https://crrev.com/aa02b4e9e62729e8570796fd5de6597fba1ceb51/ui/chromeos/events/keyboard_layout_util.cc

Thanks.  Should we merge to 63?

Yes.

This bug requires manual review: M63 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), gkihumba@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Will wait for this to be verified on tot first. Please mark as fixed for TE to have it on their radar.

Re#13: Done.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/df5824ae3976152e922652d5fc2e72f0fb03c189

commit df5824ae3976152e922652d5fc2e72f0fb03c189
Author: Ahmed Fakhry <afakhry@google.com>
Date: Wed Nov 29 03:26:33 2017

[Merge to M-63] Don't store the keyboard info when reading the layout property fails

Reading the keyboard device layout property from udev might fail. When
this happens, we used to store the keyboard info with a default layout
value = Layout1.

This CL avoid storing the keyboard info when a failure in udev is detected,
we will still fallback to the default layout for the current key event, but
we will attempt to re-read the layout property from udev on subsequent keys
until a valid layout is returned. This eliminates the need to reboot the
device to exit the bad state.

TBR=derat@chromium.org, kpschoedel@chromium.org
BUG= 783166 
TEST=added new test.

(cherry picked from commit aa02b4e9e62729e8570796fd5de6597fba1ceb51)

Change-Id: I94d6771ef955e20cee831ad0a93e31dcaaf8f610
Reviewed-on: https://chromium-review.googlesource.com/764444
Reviewed-by: Dan Erat <derat@chromium.org>
Commit-Queue: Ahmed Fakhry <afakhry@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#516063}
Reviewed-on: https://chromium-review.googlesource.com/795180
Reviewed-by: Ahmed Fakhry <afakhry@chromium.org>
Cr-Commit-Position: refs/branch-heads/3239@{#599}
Cr-Branched-From: adb61db19020ed8ecee5e91b1a0ea4c924ae2988-refs/heads/master@{#508578}
[modify] https://crrev.com/df5824ae3976152e922652d5fc2e72f0fb03c189/chrome/browser/chromeos/events/event_rewriter_unittest.cc
[modify] https://crrev.com/df5824ae3976152e922652d5fc2e72f0fb03c189/ui/chromeos/events/event_rewriter_chromeos.cc
[modify] https://crrev.com/df5824ae3976152e922652d5fc2e72f0fb03c189/ui/chromeos/events/event_rewriter_chromeos.h
[modify] https://crrev.com/df5824ae3976152e922652d5fc2e72f0fb03c189/ui/chromeos/events/keyboard_layout_util.cc