Thanks for the pointer to accelerator_controller.cc. It looks like that code is calling SetCapsLockEnabled on ImeKeyboard, while we are calling it on the InputController. The former appears to notify keyboard observers, while the latter is lower level and does not. I'll try changing are code to call ImeKeyboard::SetCapsLockEnabled and see if it resolves our issue.

Some more detail on what we're trying to do:

When using Chrome Remote Desktop, it's possible for the local and remote caps lock states to desynchronize, such as if the user on the client toggled caps lock while the CRD windows doesn't have focus, and then returns to it, or if only one of the ends is ChromeOS and the user taps shift (in which case, caps lock will be canceled on the ChromeOS end but not the other).

To make things more intuitive, we want to keep the caps lock state consistent between the client and the host. Specifically, when a user on the client presses a key, we want the character generated on the host to match what they would expect based on their local caps lock state. To facilitate this, we send the client's caps lock state along with each key press event.

In https://codereview.chromium.org/2852813002/, I used InputController::SetCapsLockEnabled to attempt to set the host caps lock state to match the client when a key press event is received by the a ChromeOS host.

Using that API, I noticed that if I connected to a ChromeOS host from a Linux client, turned caps lock on on the host (but not on the client), and pressed an alphabetic key, then the key would properly appear lower case, but the onscreen indicator would stay on.

Again, it sounds like this is probably a result of using the lower level API by mistake, so I'll change it to the higher level one and report back.

Thanks.

IMHO, for non caps lock key, if the lock states of client and host does not match, it works well by toggling the states of host to match that of client.

For caps lock key, the host (Chrome OS) triggers toggling on release. Let assume that the client triggers 'on' on press and triggers 'off' on release. If the initial caps lock state of host and client matches and are both turned off, the caps lock bit in the modifier of 4 caps lock key event will be:
       Press Release Press Release
Client OFF   ON      ON    ON
Host   OFF   OFF     ON    ON

If the initial state of host is on and that of client is off, the caps lock bit in the modifier of 4 caps lock key event will be:
       Press Release Press Release
Client OFF   ON      ON    ON
Host   ON    ON      OFF   OFF

So, notice that for caps lock key event, we could SetLockStates() on key press while do nothing if it's key release.

The code will be like:
  if (event.has_lock_states() &&
      (dom_code != ui::DomCode::CAPS_LOCK || event.pressed())) {
    SetLockStates(event.lock_states());
  }

However, there's a problem with this strategy. The key (ESC, ALT, etc.) is possibly a key to be rewritten. As key event goes through InputInjectorChromeos and then EventRewriter. We have no idea if the key is a remapped key in the former. So the above strategy works fine for real caps lock key, but does not work for keys remapped to caps lock.

 rkjnsn@, I uploaded a CL for what I thought in the above #5. It works well for raw caps lock key, but does not work for remapped keys. Do you have any idea how to get access to EventRewriter in InputInjectorChromeos? If we could get the remapping info, we could make it work for remapped keys as well.

rkjnsn@, Any updates? Can we revert the CL https://codereview.chromium.org/2852813002/ ?

Sorry for the delayed response! An office move plus some vacation meant I haven't had much time to look at it. Since using ImeKeyboard::SetCapsLockEnabled instead of InputController::SetCapsLockEnabled properly sets the onscreen indicator, the issue as reported here was simply a matter of me using the wrong API. Thus, I believe this bug can be closed.

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

commit 64dd0e1fab7201713af95e7293e428074ef8536c
Author: weidongg <weidongg@chromium.org>
Date: Mon Jun 05 22:48:45 2017

Remove duplicate Caps Lock handling in CRD

In Chrome Remote Desktop, original code tries to match the client's caps
lock state with host's (Chrome OS) by toggling Caps Lock twice. But caps
lock state switch is triggered in different ways in different systems.
For example, Chrome OS triggers both ON and OFF on key release while
Linux triggers ON on key press and OFF on key release.

Let's assume client is Linux while host is Chrome OS. The caps lock
state of both systems is initialized to OFF. If you tap caps lock key in
client twice, the caps lock state at the time the key event is generated
(not the time the key event takes effect) is:
Press   Release Press   Release
Client  OFF     ON      ON      ON
Host    OFF     OFF     ON      ON

If the caps lock state of client is OFF while that of host is ON, the
state table will be:
Press   Release Press   Release
Client  OFF     ON      ON      ON
Host    ON      ON      OFF     OFF

So, if we trigger state toggling whenever state mismatches in client and
host, then we may toggle duplicately. So the solution is to trigger
toggling on key press when state mismatches. Also, we should not
trigger toggling for key that is possibly mapped to caps lock in event
rewriter. Otherwise, the toggling could occur duplicately.

BUG= 719996 
TEST=NONE

Review-Url: https://codereview.chromium.org/2881293002
Cr-Commit-Position: refs/heads/master@{#477110}

[modify] https://crrev.com/64dd0e1fab7201713af95e7293e428074ef8536c/remoting/host/input_injector_chromeos.cc

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

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

Withdrawing merge request.