New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 738219 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Dec 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug

Blocking:
issue 738210



Sign in to add a comment

Remove InputMethodManager access from ash/system/tray_caps_lock.cc

Project Member Reported by jamescook@chromium.org, Jun 29 2017

Issue description

TrayCapsLock shows an icon in the system tray when caps lock is enabled. (You'll need to remap the search key to caps lock in settings to see this behavior.)

InputMethodManager is not available in the ash process under chrome --mash.

This may need to delegate back to Chrome via ImeController and ime_controller.mojom

Fixing this may interact with the SystemTrayDelegate::IsSearchKeyMappedToCapsLock() cleanup in  issue 665569 . It might be possible to do both at the same time.

 
Owner: blakeo@chromium.org
Project Member

Comment 2 by bugdroid1@chromium.org, Aug 30 2017

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

commit ae0ca643c24ea05201b4d253ef8d177f274f9a3b
Author: Blake O'Hare <blakeo@chromium.org>
Date: Wed Aug 30 05:42:50 2017

mash: Report caps lock state changes via Mojo ImeController

ImeKeyboard is in Chrome
TrayCapsLock is in Ash
...and they both talk to each other synchronously
For Mus, these need to run in separate processes without references to each
other.

How it was:
- TrayCapsLock was an ImeKeyboard Observer and added itself to the ImeKeyboard
  directly.
- TrayCapsLock would query ImeKeyboard directly for current state

How it is now:
- TrayCapsLock is an ImeController Observer
- ImeController can now be observed
- ImeControllerClient is now an ImeKeyboard Observer
- When ImeControllerClient receives state changes from the ImeKeyboard, it
  reports it to ImeController, which then calls its observers.
- ImeController tracks the caps lock state from the last state change
  notification. All synchronous calls that used to come from TrayCapsLock to
  ImeKeyboard, now simply check this value instead.

Bug:  738219 
Change-Id: I5eb3231e0039499b03e6912436b07b29635429bc
Reviewed-on: https://chromium-review.googlesource.com/625508
Commit-Queue: Blake O'Hare <blakeo@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#498375}
[modify] https://crrev.com/ae0ca643c24ea05201b4d253ef8d177f274f9a3b/ash/ime/ime_controller.cc
[modify] https://crrev.com/ae0ca643c24ea05201b4d253ef8d177f274f9a3b/ash/ime/ime_controller.h
[modify] https://crrev.com/ae0ca643c24ea05201b4d253ef8d177f274f9a3b/ash/ime/ime_controller_unittest.cc
[modify] https://crrev.com/ae0ca643c24ea05201b4d253ef8d177f274f9a3b/ash/public/interfaces/ime_controller.mojom
[modify] https://crrev.com/ae0ca643c24ea05201b4d253ef8d177f274f9a3b/ash/system/tray_caps_lock.cc
[modify] https://crrev.com/ae0ca643c24ea05201b4d253ef8d177f274f9a3b/ash/system/tray_caps_lock.h
[modify] https://crrev.com/ae0ca643c24ea05201b4d253ef8d177f274f9a3b/ash/system/tray_caps_lock_unittest.cc
[modify] https://crrev.com/ae0ca643c24ea05201b4d253ef8d177f274f9a3b/chrome/browser/chromeos/input_method/input_method_manager_impl_unittest.cc
[modify] https://crrev.com/ae0ca643c24ea05201b4d253ef8d177f274f9a3b/chrome/browser/ui/ash/ime_controller_client.cc
[modify] https://crrev.com/ae0ca643c24ea05201b4d253ef8d177f274f9a3b/chrome/browser/ui/ash/ime_controller_client.h
[modify] https://crrev.com/ae0ca643c24ea05201b4d253ef8d177f274f9a3b/chrome/browser/ui/ash/ime_controller_client_unittest.cc

Comment 3 by blakeo@chromium.org, Aug 30 2017

The above change fixes about 80% of the dependency. There's still a constant that needs to be moved before the dependency can be removed outright. 
Project Member

Comment 4 by bugdroid1@chromium.org, Oct 6 2017

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

commit d00831e79d9a92a31510379961b1cfa862a0bf7e
Author: Blake O'Hare <blakeo@chromium.org>
Date: Fri Oct 06 12:37:25 2017

mash: Move modifier keys constants into a common file

Bug:  738219 
Change-Id: I7fa5158fe0f1dbb00298eb2ebf8d710cce16f3d1
Reviewed-on: https://chromium-review.googlesource.com/683574
Commit-Queue: Blake O'Hare <blakeo@chromium.org>
Reviewed-by: Satoru Takabayashi <satorux@chromium.org>
Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#507038}
[modify] https://crrev.com/d00831e79d9a92a31510379961b1cfa862a0bf7e/ash/system/tray_caps_lock.cc
[modify] https://crrev.com/d00831e79d9a92a31510379961b1cfa862a0bf7e/chrome/browser/chromeos/events/event_rewriter_unittest.cc
[modify] https://crrev.com/d00831e79d9a92a31510379961b1cfa862a0bf7e/chrome/browser/chromeos/preferences.cc
[modify] https://crrev.com/d00831e79d9a92a31510379961b1cfa862a0bf7e/chrome/browser/ui/webui/chromeos/keyboard_overlay_ui.cc
[modify] https://crrev.com/d00831e79d9a92a31510379961b1cfa862a0bf7e/ui/base/ime/chromeos/ime_keyboard.h
[modify] https://crrev.com/d00831e79d9a92a31510379961b1cfa862a0bf7e/ui/chromeos/events/BUILD.gn
[modify] https://crrev.com/d00831e79d9a92a31510379961b1cfa862a0bf7e/ui/chromeos/events/event_rewriter_chromeos.cc
[add] https://crrev.com/d00831e79d9a92a31510379961b1cfa862a0bf7e/ui/chromeos/events/modifier_key.h

Comment 5 by blakeo@chromium.org, Dec 26 2017

Status: Fixed (was: Assigned)
This appears to be complete. 
Components: -Internals>MUS Internals>Services>WindowService

Sign in to add a comment