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

Issue 724305 link

Starred by 1 user

Issue metadata

Status: Archived
Owner:
Closed: Jun 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug

Blocking:
issue 647412



Sign in to add a comment

mash: Remove IME methods from ash::SystemTrayDelegate and ash::ImeControlDelegate

Project Member Reported by jamescook@chromium.org, May 18 2017

Issue description

For mustash (go/mustash) we're trying to eliminate direct ash -> chrome delegates.

The interfaces for chromeos IME live in //ui/base/ime/chromeos. Code in //ash is already using these interfaces today.

Replace this call pattern:
//ash/system - ImeListView
//ash/system - SystemTrayDelegate
//chrome/browser/ui/ash - SystemTrayDelegateChromeOS
//ui/base/ime/chromeos - InputMethodManager

With this:
//ash/system - ImeListView
//ui/base/ime/chromeos - InputMethodManager

Similar changes can happen in ash::AcceleratorController (which is already calling directly into InputMethodManager for caps-lock features). This will allow the elimination of ImeControlDelegate.

 
Cc: moshayedi@chromium.org
These are the specific methods:

  virtual void GetCurrentIME(IMEInfo* info);
  virtual void GetAvailableIMEList(IMEInfoList* list);
  virtual void GetCurrentIMEProperties(IMEPropertyInfoList* list);
  virtual base::string16 GetIMEManagedMessage();
  virtual void SwitchIME(const std::string& ime_id);
  virtual void ActivateIMEProperty(const std::string& key);

Long-term we will need a mojo service for these sorts of IME switching methods. That could be a chromeos::input_manager::InputMethodManager impl subclass that uses mojo to proxy to the browser. For now, though, just removing the methods simplifies the code quite a bit.


Project Member

Comment 2 by bugdroid1@chromium.org, May 24 2017

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

commit 1290776535cfda1fa1cd7867d5a2ae2ded7e92ec
Author: jamescook <jamescook@chromium.org>
Date: Wed May 24 19:07:05 2017

chromeos: Remove some IME methods from ash::SystemTrayDelegate

For the mustash project we're trying to eliminate delegates that call
from ash back into chrome browser.

The IME InputMethodManager interfaces live in //ui/base/ime/chromeos, and
//ash already uses them, so convert some of the system tray code to use
InputMethodManager directly.

* Remove InputMethodSwitchRecorder and move histogram recording to //ash
* Use InputMethodManager directly in the ash keyboard accelerator code
* Remove ash::ImeControlDelegate and chrome's ImeController.
* Use MockInputMethodManager in all AcceleratorControllerTests

BUG= 724305 
TEST=ash_unittests, manually switch IMEs when running chrome

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

[modify] https://crrev.com/1290776535cfda1fa1cd7867d5a2ae2ded7e92ec/ash/BUILD.gn
[modify] https://crrev.com/1290776535cfda1fa1cd7867d5a2ae2ded7e92ec/ash/accelerators/accelerator_controller.cc
[modify] https://crrev.com/1290776535cfda1fa1cd7867d5a2ae2ded7e92ec/ash/accelerators/accelerator_controller.h
[modify] https://crrev.com/1290776535cfda1fa1cd7867d5a2ae2ded7e92ec/ash/accelerators/accelerator_controller_unittest.cc
[add] https://crrev.com/1290776535cfda1fa1cd7867d5a2ae2ded7e92ec/ash/ime/ime_switch_type.h
[delete] https://crrev.com/526c7ebbc8501df55df5ecb0ee90e08368ee6852/ash/ime_control_delegate.h
[modify] https://crrev.com/1290776535cfda1fa1cd7867d5a2ae2ded7e92ec/ash/system/ime_menu/ime_list_view.cc
[modify] https://crrev.com/1290776535cfda1fa1cd7867d5a2ae2ded7e92ec/ash/system/tray/system_tray_delegate.cc
[modify] https://crrev.com/1290776535cfda1fa1cd7867d5a2ae2ded7e92ec/ash/system/tray/system_tray_delegate.h
[modify] https://crrev.com/1290776535cfda1fa1cd7867d5a2ae2ded7e92ec/chrome/browser/chromeos/BUILD.gn
[modify] https://crrev.com/1290776535cfda1fa1cd7867d5a2ae2ded7e92ec/chrome/browser/chromeos/input_method/input_method_manager_impl.cc
[delete] https://crrev.com/526c7ebbc8501df55df5ecb0ee90e08368ee6852/chrome/browser/chromeos/input_method/input_method_switch_recorder.cc
[delete] https://crrev.com/526c7ebbc8501df55df5ecb0ee90e08368ee6852/chrome/browser/chromeos/input_method/input_method_switch_recorder.h
[modify] https://crrev.com/1290776535cfda1fa1cd7867d5a2ae2ded7e92ec/chrome/browser/ui/BUILD.gn
[modify] https://crrev.com/1290776535cfda1fa1cd7867d5a2ae2ded7e92ec/chrome/browser/ui/ash/ash_init.cc
[delete] https://crrev.com/526c7ebbc8501df55df5ecb0ee90e08368ee6852/chrome/browser/ui/ash/ime_controller_chromeos.cc
[delete] https://crrev.com/526c7ebbc8501df55df5ecb0ee90e08368ee6852/chrome/browser/ui/ash/ime_controller_chromeos.h
[modify] https://crrev.com/1290776535cfda1fa1cd7867d5a2ae2ded7e92ec/chrome/browser/ui/ash/system_tray_delegate_chromeos.cc
[modify] https://crrev.com/1290776535cfda1fa1cd7867d5a2ae2ded7e92ec/chrome/browser/ui/ash/system_tray_delegate_chromeos.h
[modify] https://crrev.com/1290776535cfda1fa1cd7867d5a2ae2ded7e92ec/ui/base/ime/chromeos/mock_input_method_manager.cc
[modify] https://crrev.com/1290776535cfda1fa1cd7867d5a2ae2ded7e92ec/ui/base/ime/chromeos/mock_input_method_manager.h

Project Member

Comment 3 by bugdroid1@chromium.org, Jun 1 2017

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

commit 2d60f034473f06fd040407a2945f5b4a35b07df4
Author: jamescook <jamescook@chromium.org>
Date: Thu Jun 01 00:55:21 2017

chromeos: Move InputMethodUtil into //ui/base/ime/chromeos

For mustash we need to eliminate ash to chrome delegates. Moving this
class will allow it to be called directly from the system tray IME code
in ash, which will allow methods to be removed from SystemTrayDelegate.

Move the class into ui/base/ime/chromeos because that's where most of
the other utility code lives (like extension_ime_util).

* Move kBrailleIme constants to ui/base/ime/chromeos/extension_ime_util.h
* Fix input_method_whitelist.cc usage in BUILD rules for test targets
* Move Chrome OS IME strings to ui_chromeos_strings.grd

BUG= 724305 
TEST=ui_base_unittests, chrome unit_tests
TBR=rockot@chromium.org

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

[modify] https://crrev.com/2d60f034473f06fd040407a2945f5b4a35b07df4/chrome/app/chromeos_strings.grdp
[modify] https://crrev.com/2d60f034473f06fd040407a2945f5b4a35b07df4/chrome/browser/chromeos/BUILD.gn
[modify] https://crrev.com/2d60f034473f06fd040407a2945f5b4a35b07df4/chrome/browser/chromeos/accessibility/accessibility_manager.cc
[modify] https://crrev.com/2d60f034473f06fd040407a2945f5b4a35b07df4/chrome/browser/chromeos/accessibility/accessibility_manager_browsertest.cc
[modify] https://crrev.com/2d60f034473f06fd040407a2945f5b4a35b07df4/chrome/browser/chromeos/base/locale_util.cc
[modify] https://crrev.com/2d60f034473f06fd040407a2945f5b4a35b07df4/chrome/browser/chromeos/chrome_browser_main_chromeos.cc
[modify] https://crrev.com/2d60f034473f06fd040407a2945f5b4a35b07df4/chrome/browser/chromeos/extensions/input_method_api.cc
[modify] https://crrev.com/2d60f034473f06fd040407a2945f5b4a35b07df4/chrome/browser/chromeos/extensions/input_method_apitest_chromeos.cc
[modify] https://crrev.com/2d60f034473f06fd040407a2945f5b4a35b07df4/chrome/browser/chromeos/input_method/accessibility.cc
[modify] https://crrev.com/2d60f034473f06fd040407a2945f5b4a35b07df4/chrome/browser/chromeos/input_method/browser_state_monitor.cc
[modify] https://crrev.com/2d60f034473f06fd040407a2945f5b4a35b07df4/chrome/browser/chromeos/input_method/component_extension_ime_manager_impl.cc
[modify] https://crrev.com/2d60f034473f06fd040407a2945f5b4a35b07df4/chrome/browser/chromeos/input_method/input_method_manager_impl.h
[modify] https://crrev.com/2d60f034473f06fd040407a2945f5b4a35b07df4/chrome/browser/chromeos/input_method/input_method_persistence.cc
[modify] https://crrev.com/2d60f034473f06fd040407a2945f5b4a35b07df4/chrome/browser/chromeos/input_method/mock_input_method_manager_impl.cc
[modify] https://crrev.com/2d60f034473f06fd040407a2945f5b4a35b07df4/chrome/browser/chromeos/input_method/mode_indicator_browsertest.cc
[modify] https://crrev.com/2d60f034473f06fd040407a2945f5b4a35b07df4/chrome/browser/chromeos/input_method/mode_indicator_controller.cc
[modify] https://crrev.com/2d60f034473f06fd040407a2945f5b4a35b07df4/chrome/browser/chromeos/login/oobe_localization_browsertest.cc
[modify] https://crrev.com/2d60f034473f06fd040407a2945f5b4a35b07df4/chrome/browser/chromeos/login/session/user_session_manager.cc
[modify] https://crrev.com/2d60f034473f06fd040407a2945f5b4a35b07df4/chrome/browser/chromeos/login/ui/login_display_host_impl.cc
[modify] https://crrev.com/2d60f034473f06fd040407a2945f5b4a35b07df4/chrome/browser/chromeos/policy/device_local_account_browsertest.cc
[modify] https://crrev.com/2d60f034473f06fd040407a2945f5b4a35b07df4/chrome/browser/extensions/api/language_settings_private/language_settings_private_api.cc
[modify] https://crrev.com/2d60f034473f06fd040407a2945f5b4a35b07df4/chrome/browser/ui/ash/system_tray_delegate_chromeos.cc
[modify] https://crrev.com/2d60f034473f06fd040407a2945f5b4a35b07df4/chrome/browser/ui/webui/chromeos/login/gaia_screen_handler.cc
[modify] https://crrev.com/2d60f034473f06fd040407a2945f5b4a35b07df4/chrome/browser/ui/webui/chromeos/login/l10n_util.cc
[modify] https://crrev.com/2d60f034473f06fd040407a2945f5b4a35b07df4/chrome/browser/ui/webui/chromeos/login/signin_screen_handler.cc
[modify] https://crrev.com/2d60f034473f06fd040407a2945f5b4a35b07df4/chrome/browser/ui/webui/options/chromeos/cros_language_options_handler.cc
[modify] https://crrev.com/2d60f034473f06fd040407a2945f5b4a35b07df4/chrome/common/extensions/extension_constants.cc
[modify] https://crrev.com/2d60f034473f06fd040407a2945f5b4a35b07df4/chrome/common/extensions/extension_constants.h
[modify] https://crrev.com/2d60f034473f06fd040407a2945f5b4a35b07df4/chrome/test/BUILD.gn
[modify] https://crrev.com/2d60f034473f06fd040407a2945f5b4a35b07df4/ui/base/BUILD.gn
[modify] https://crrev.com/2d60f034473f06fd040407a2945f5b4a35b07df4/ui/base/ime/BUILD.gn
[modify] https://crrev.com/2d60f034473f06fd040407a2945f5b4a35b07df4/ui/base/ime/chromeos/DEPS
[modify] https://crrev.com/2d60f034473f06fd040407a2945f5b4a35b07df4/ui/base/ime/chromeos/extension_ime_util.cc
[modify] https://crrev.com/2d60f034473f06fd040407a2945f5b4a35b07df4/ui/base/ime/chromeos/extension_ime_util.h
[rename] https://crrev.com/2d60f034473f06fd040407a2945f5b4a35b07df4/ui/base/ime/chromeos/input_method_util.cc
[rename] https://crrev.com/2d60f034473f06fd040407a2945f5b4a35b07df4/ui/base/ime/chromeos/input_method_util.h
[rename] https://crrev.com/2d60f034473f06fd040407a2945f5b4a35b07df4/ui/base/ime/chromeos/input_method_util_unittest.cc
[modify] https://crrev.com/2d60f034473f06fd040407a2945f5b4a35b07df4/ui/chromeos/ui_chromeos_strings.grd

Project Member

Comment 4 by bugdroid1@chromium.org, Jun 1 2017

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

commit 800af5e6c1c96ce81b21cc615cad3aa76d250136
Author: jamescook <jamescook@chromium.org>
Date: Thu Jun 01 16:09:03 2017

chromeos: Remove SystemTrayDelegate::GetIMEManagedMessage()

For the mustash project we're eliminating ash-to-chrome delegates because
ash will run in its own process.

Instead of delegating back to chrome to look up a string, do the lookup
directly in ash.

Collapse the string with an existing page info string with almost-identical
text. This saves a little binary size and allows the string to be used in
both chromeos ash code and non-chromeos extensions webui code in chrome.

BUG= 724305 
TEST=ash_unittests, manual tests of IME menus

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

[modify] https://crrev.com/800af5e6c1c96ce81b21cc615cad3aa76d250136/ash/BUILD.gn
[modify] https://crrev.com/800af5e6c1c96ce81b21cc615cad3aa76d250136/ash/DEPS
[modify] https://crrev.com/800af5e6c1c96ce81b21cc615cad3aa76d250136/ash/strings/BUILD.gn
[modify] https://crrev.com/800af5e6c1c96ce81b21cc615cad3aa76d250136/ash/system/ime/tray_ime_chromeos.cc
[modify] https://crrev.com/800af5e6c1c96ce81b21cc615cad3aa76d250136/ash/system/ime_menu/ime_menu_tray_unittest.cc
[modify] https://crrev.com/800af5e6c1c96ce81b21cc615cad3aa76d250136/ash/system/tray/system_tray_delegate.cc
[modify] https://crrev.com/800af5e6c1c96ce81b21cc615cad3aa76d250136/ash/system/tray/system_tray_delegate.h
[modify] https://crrev.com/800af5e6c1c96ce81b21cc615cad3aa76d250136/chrome/app/generated_resources.grd
[modify] https://crrev.com/800af5e6c1c96ce81b21cc615cad3aa76d250136/chrome/browser/chromeos/arc/arc_support_host.cc
[modify] https://crrev.com/800af5e6c1c96ce81b21cc615cad3aa76d250136/chrome/browser/chromeos/options/network_config_view.cc
[modify] https://crrev.com/800af5e6c1c96ce81b21cc615cad3aa76d250136/chrome/browser/ui/ash/system_tray_delegate_chromeos.cc
[modify] https://crrev.com/800af5e6c1c96ce81b21cc615cad3aa76d250136/chrome/browser/ui/ash/system_tray_delegate_chromeos.h
[modify] https://crrev.com/800af5e6c1c96ce81b21cc615cad3aa76d250136/chrome/browser/ui/webui/policy_indicator_localized_strings_provider.cc
[modify] https://crrev.com/800af5e6c1c96ce81b21cc615cad3aa76d250136/components/policy_strings.grdp

I think I need to do something different here. Ash does not have an InputMethodManager under mustash and I don't think we want to add one. Its API surface is very large, and much bigger than what mustash needs. We should instead have a small IME-related mojo interface for ash, perhaps built into the existing one in services/ui.

Design doc:
https://docs.google.com/document/d/175R_hi-JEjzpN7aPOyMauqTTs7aP0w4B6SczBfgjjNU/edit?hl=en#

Project Member

Comment 6 by bugdroid1@chromium.org, Jun 5 2017

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

commit bcdc288b89a5645109f8a743fd65446aa0cf2ce6
Author: jamescook <jamescook@chromium.org>
Date: Mon Jun 05 17:27:08 2017

Revert of chromeos: Remove SystemTrayDelegate::GetIMEManagedMessage() (patchset #6 id:100001 of https://codereview.chromium.org/2911303003/ )

Reason for revert:
I'm going to take a different approach to the IME methods in the system tray delegate. There's a design doc linked from the bug.

Original issue's description:
> chromeos: Remove SystemTrayDelegate::GetIMEManagedMessage()
>
> For the mustash project we're eliminating ash-to-chrome delegates because
> ash will run in its own process.
>
> Instead of delegating back to chrome to look up a string, do the lookup
> directly in ash.
>
> Collapse the string with an existing page info string with almost-identical
> text. This saves a little binary size and allows the string to be used in
> both chromeos ash code and non-chromeos extensions webui code in chrome.
>
> BUG= 724305 
> TEST=ash_unittests, manual tests of IME menus
>
> Review-Url: https://codereview.chromium.org/2911303003
> Cr-Commit-Position: refs/heads/master@{#476314}
> Committed: https://chromium.googlesource.com/chromium/src/+/800af5e6c1c96ce81b21cc615cad3aa76d250136

TBR=msw@chromium.org,pastarmovj@chromium.org,blundell@chromium.org,stevenjb@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG= 724305 

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

[modify] https://crrev.com/bcdc288b89a5645109f8a743fd65446aa0cf2ce6/ash/BUILD.gn
[modify] https://crrev.com/bcdc288b89a5645109f8a743fd65446aa0cf2ce6/ash/DEPS
[modify] https://crrev.com/bcdc288b89a5645109f8a743fd65446aa0cf2ce6/ash/strings/BUILD.gn
[modify] https://crrev.com/bcdc288b89a5645109f8a743fd65446aa0cf2ce6/ash/system/ime/tray_ime_chromeos.cc
[modify] https://crrev.com/bcdc288b89a5645109f8a743fd65446aa0cf2ce6/ash/system/ime_menu/ime_menu_tray_unittest.cc
[modify] https://crrev.com/bcdc288b89a5645109f8a743fd65446aa0cf2ce6/ash/system/tray/system_tray_delegate.cc
[modify] https://crrev.com/bcdc288b89a5645109f8a743fd65446aa0cf2ce6/ash/system/tray/system_tray_delegate.h
[modify] https://crrev.com/bcdc288b89a5645109f8a743fd65446aa0cf2ce6/chrome/app/generated_resources.grd
[modify] https://crrev.com/bcdc288b89a5645109f8a743fd65446aa0cf2ce6/chrome/browser/chromeos/arc/arc_support_host.cc
[modify] https://crrev.com/bcdc288b89a5645109f8a743fd65446aa0cf2ce6/chrome/browser/chromeos/options/network_config_view.cc
[modify] https://crrev.com/bcdc288b89a5645109f8a743fd65446aa0cf2ce6/chrome/browser/ui/ash/system_tray_delegate_chromeos.cc
[modify] https://crrev.com/bcdc288b89a5645109f8a743fd65446aa0cf2ce6/chrome/browser/ui/ash/system_tray_delegate_chromeos.h
[modify] https://crrev.com/bcdc288b89a5645109f8a743fd65446aa0cf2ce6/chrome/browser/ui/webui/policy_indicator_localized_strings_provider.cc
[modify] https://crrev.com/bcdc288b89a5645109f8a743fd65446aa0cf2ce6/components/policy_strings.grdp

Project Member

Comment 7 by bugdroid1@chromium.org, Jun 10 2017

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

commit bb0267315083d4fbc1fbd9f308370e5573e79c44
Author: James Cook <jamescook@chromium.org>
Date: Sat Jun 10 00:45:18 2017

chromeos: Make ash system tray IME menus work under mustash, part 1

Introduce ash::ImeManager. It will eventually proxy IME-related requests
from ash to chrome browser over mojo. For now it is an interface
implemented in chrome browser by InputMethodManagerImpl.

Move SystemTrayDelegate IME interface methods to the ImeManager interface.

Move the SystemTrayDelegateChromeOS IME method implementations into
InputMethodManagerImpl, which is the class that actually does the work.

Add an explicit IsImeManaged() method to detect enterprise-managed IMEs,
rather than encoding that concept in a string being non-empty.

Use a more specific tooltip string for the IME enterprise-managed
system tray icon. Put the string in ash eliminates a chrome dependency.

Eliminate TestSystemTrayDelegate because it no longer has any useful
methods.

Bug:  724305 
Change-Id: I8545b00c2f0ad59cc276ca465d32a1952fedd4b6
Reviewed-on: https://chromium-review.googlesource.com/527555
Commit-Queue: James Cook <jamescook@chromium.org>
Reviewed-by: Zachary Kuznia <zork@chromium.org>
Reviewed-by: Hadi Moshayedi <moshayedi@chromium.org>
Reviewed-by: Terry Anderson <tdanderson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#478477}
[modify] https://crrev.com/bb0267315083d4fbc1fbd9f308370e5573e79c44/ash/BUILD.gn
[modify] https://crrev.com/bb0267315083d4fbc1fbd9f308370e5573e79c44/ash/ash_strings.grd
[modify] https://crrev.com/bb0267315083d4fbc1fbd9f308370e5573e79c44/ash/content/display/screen_orientation_controller_chromeos_unittest.cc
[add] https://crrev.com/bb0267315083d4fbc1fbd9f308370e5573e79c44/ash/ime/ime_controller.cc
[add] https://crrev.com/bb0267315083d4fbc1fbd9f308370e5573e79c44/ash/ime/ime_controller.h
[modify] https://crrev.com/bb0267315083d4fbc1fbd9f308370e5573e79c44/ash/mus/shell_delegate_mus.cc
[modify] https://crrev.com/bb0267315083d4fbc1fbd9f308370e5573e79c44/ash/mus/shell_delegate_mus.h
[modify] https://crrev.com/bb0267315083d4fbc1fbd9f308370e5573e79c44/ash/shelf/shelf_view_unittest.cc
[modify] https://crrev.com/bb0267315083d4fbc1fbd9f308370e5573e79c44/ash/shell.cc
[modify] https://crrev.com/bb0267315083d4fbc1fbd9f308370e5573e79c44/ash/shell.h
[modify] https://crrev.com/bb0267315083d4fbc1fbd9f308370e5573e79c44/ash/shell/shell_delegate_impl.cc
[modify] https://crrev.com/bb0267315083d4fbc1fbd9f308370e5573e79c44/ash/shell/shell_delegate_impl.h
[modify] https://crrev.com/bb0267315083d4fbc1fbd9f308370e5573e79c44/ash/shell_delegate.h
[modify] https://crrev.com/bb0267315083d4fbc1fbd9f308370e5573e79c44/ash/system/ime/tray_ime_chromeos.cc
[modify] https://crrev.com/bb0267315083d4fbc1fbd9f308370e5573e79c44/ash/system/ime/tray_ime_chromeos.h
[modify] https://crrev.com/bb0267315083d4fbc1fbd9f308370e5573e79c44/ash/system/ime/tray_ime_chromeos_unittest.cc
[modify] https://crrev.com/bb0267315083d4fbc1fbd9f308370e5573e79c44/ash/system/ime_menu/ime_list_view.cc
[modify] https://crrev.com/bb0267315083d4fbc1fbd9f308370e5573e79c44/ash/system/ime_menu/ime_menu_tray.cc
[modify] https://crrev.com/bb0267315083d4fbc1fbd9f308370e5573e79c44/ash/system/ime_menu/ime_menu_tray.h
[modify] https://crrev.com/bb0267315083d4fbc1fbd9f308370e5573e79c44/ash/system/ime_menu/ime_menu_tray_unittest.cc
[modify] https://crrev.com/bb0267315083d4fbc1fbd9f308370e5573e79c44/ash/system/screen_layout_observer_unittest.cc
[modify] https://crrev.com/bb0267315083d4fbc1fbd9f308370e5573e79c44/ash/system/supervised/tray_supervised_user_unittest.cc
[modify] https://crrev.com/bb0267315083d4fbc1fbd9f308370e5573e79c44/ash/system/tray/system_tray_delegate.cc
[modify] https://crrev.com/bb0267315083d4fbc1fbd9f308370e5573e79c44/ash/system/tray/system_tray_delegate.h
[modify] https://crrev.com/bb0267315083d4fbc1fbd9f308370e5573e79c44/ash/system/web_notification/web_notification_tray_unittest.cc
[modify] https://crrev.com/bb0267315083d4fbc1fbd9f308370e5573e79c44/ash/test/BUILD.gn
[modify] https://crrev.com/bb0267315083d4fbc1fbd9f308370e5573e79c44/ash/test/ash_test_base.cc
[modify] https://crrev.com/bb0267315083d4fbc1fbd9f308370e5573e79c44/ash/test/ash_test_base.h
[modify] https://crrev.com/bb0267315083d4fbc1fbd9f308370e5573e79c44/ash/test/ash_test_helper.cc
[modify] https://crrev.com/bb0267315083d4fbc1fbd9f308370e5573e79c44/ash/test/test_shell_delegate.cc
[modify] https://crrev.com/bb0267315083d4fbc1fbd9f308370e5573e79c44/ash/test/test_shell_delegate.h
[delete] https://crrev.com/6f879ea5e52d7d263def64d53109c6a5bda7a05e/ash/test/test_system_tray_delegate.cc
[delete] https://crrev.com/6f879ea5e52d7d263def64d53109c6a5bda7a05e/ash/test/test_system_tray_delegate.h
[modify] https://crrev.com/bb0267315083d4fbc1fbd9f308370e5573e79c44/ash/wm/maximize_mode/maximize_mode_controller_unittest.cc
[modify] https://crrev.com/bb0267315083d4fbc1fbd9f308370e5573e79c44/chrome/browser/chromeos/input_method/input_method_manager_impl.cc
[modify] https://crrev.com/bb0267315083d4fbc1fbd9f308370e5573e79c44/chrome/browser/chromeos/input_method/input_method_manager_impl.h
[modify] https://crrev.com/bb0267315083d4fbc1fbd9f308370e5573e79c44/chrome/browser/ui/ash/chrome_shell_delegate.cc
[modify] https://crrev.com/bb0267315083d4fbc1fbd9f308370e5573e79c44/chrome/browser/ui/ash/chrome_shell_delegate.h
[modify] https://crrev.com/bb0267315083d4fbc1fbd9f308370e5573e79c44/chrome/browser/ui/ash/system_tray_delegate_chromeos.cc
[modify] https://crrev.com/bb0267315083d4fbc1fbd9f308370e5573e79c44/chrome/browser/ui/ash/system_tray_delegate_chromeos.h

Project Member

Comment 8 by bugdroid1@chromium.org, Jun 12 2017

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

commit 8fc1bb617b0d3c20b279ca6744b3ee7fa4f22add
Author: James Cook <jamescook@chromium.org>
Date: Mon Jun 12 18:05:29 2017

chromeos: Convert ash::ImeInfo to mojo

This is part 2 of the conversion of ash system tray IME support to mojo.
The next CL will introduce a mojo interface that needs this type. The
struct is simple, so use it natively everywhere.

Rename IMEProperty to ImeMenuItem because it's a menu item, not a
generic property.

Bug:  724305 
Test: ash_unittests, unit_tests
Change-Id: I8a8bb21ce99158ce45d4a6d889254ab311eef877
Reviewed-on: https://chromium-review.googlesource.com/528345
Commit-Queue: James Cook <jamescook@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Zachary Kuznia <zork@chromium.org>
Reviewed-by: Hadi Moshayedi <moshayedi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#478680}
[modify] https://crrev.com/8fc1bb617b0d3c20b279ca6744b3ee7fa4f22add/ash/BUILD.gn
[modify] https://crrev.com/8fc1bb617b0d3c20b279ca6744b3ee7fa4f22add/ash/ime/ime_controller.cc
[modify] https://crrev.com/8fc1bb617b0d3c20b279ca6744b3ee7fa4f22add/ash/ime/ime_controller.h
[modify] https://crrev.com/8fc1bb617b0d3c20b279ca6744b3ee7fa4f22add/ash/public/interfaces/BUILD.gn
[add] https://crrev.com/8fc1bb617b0d3c20b279ca6744b3ee7fa4f22add/ash/public/interfaces/ime_info.mojom
[modify] https://crrev.com/8fc1bb617b0d3c20b279ca6744b3ee7fa4f22add/ash/system/ime/tray_ime_chromeos.cc
[modify] https://crrev.com/8fc1bb617b0d3c20b279ca6744b3ee7fa4f22add/ash/system/ime/tray_ime_chromeos.h
[modify] https://crrev.com/8fc1bb617b0d3c20b279ca6744b3ee7fa4f22add/ash/system/ime/tray_ime_chromeos_unittest.cc
[modify] https://crrev.com/8fc1bb617b0d3c20b279ca6744b3ee7fa4f22add/ash/system/ime_menu/ime_list_view.cc
[modify] https://crrev.com/8fc1bb617b0d3c20b279ca6744b3ee7fa4f22add/ash/system/ime_menu/ime_list_view.h
[modify] https://crrev.com/8fc1bb617b0d3c20b279ca6744b3ee7fa4f22add/ash/system/ime_menu/ime_menu_tray.cc
[modify] https://crrev.com/8fc1bb617b0d3c20b279ca6744b3ee7fa4f22add/ash/system/ime_menu/ime_menu_tray.h
[modify] https://crrev.com/8fc1bb617b0d3c20b279ca6744b3ee7fa4f22add/ash/system/ime_menu/ime_menu_tray_unittest.cc
[delete] https://crrev.com/7ad07b8cd01390ea3b8ec8e43c9827fb5f87891a/ash/system/tray/ime_info.cc
[delete] https://crrev.com/7ad07b8cd01390ea3b8ec8e43c9827fb5f87891a/ash/system/tray/ime_info.h
[modify] https://crrev.com/8fc1bb617b0d3c20b279ca6744b3ee7fa4f22add/chrome/browser/chromeos/input_method/input_method_manager_impl.cc
[modify] https://crrev.com/8fc1bb617b0d3c20b279ca6744b3ee7fa4f22add/chrome/browser/chromeos/input_method/input_method_manager_impl.h

Project Member

Comment 9 by bugdroid1@chromium.org, Jun 15 2017

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

commit f0c78fcfe80c8b182fc1bbcb6ce626676fefc6b1
Author: James Cook <jamescook@chromium.org>
Date: Thu Jun 15 16:59:48 2017

cros: Make chrome push information about IMEs into ash for system tray

Patch 3 of the mojo conversion of system tray IME. Having chrome push
the data into ash::ImeController means that ash UI code can
synchronously query the data. This makes the final conversion to mojo
easier.

Convert ImeController from an interface to a concrete object created
and owned by ash. Use it to store the IME data model. Stop caching
IME info in the UI view classes (TrayIME, ImeMenuTray).

Introduce an explicit SetImesManagedByPolicy() rather than inferring
that information from an "allowed list" being empty.

Bug:  724305 
Test: ash_unittests, chrome unit_tests
Change-Id: Id388e1ad39d2ade15c948f54dfd3118aaca43cea
Reviewed-on: https://chromium-review.googlesource.com/531444
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Terry Anderson <tdanderson@chromium.org>
Reviewed-by: Zachary Kuznia <zork@chromium.org>
Commit-Queue: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#479742}
[modify] https://crrev.com/f0c78fcfe80c8b182fc1bbcb6ce626676fefc6b1/ash/BUILD.gn
[modify] https://crrev.com/f0c78fcfe80c8b182fc1bbcb6ce626676fefc6b1/ash/ime/ime_controller.cc
[modify] https://crrev.com/f0c78fcfe80c8b182fc1bbcb6ce626676fefc6b1/ash/ime/ime_controller.h
[add] https://crrev.com/f0c78fcfe80c8b182fc1bbcb6ce626676fefc6b1/ash/ime/ime_controller_unittest.cc
[modify] https://crrev.com/f0c78fcfe80c8b182fc1bbcb6ce626676fefc6b1/ash/mus/shell_delegate_mus.cc
[modify] https://crrev.com/f0c78fcfe80c8b182fc1bbcb6ce626676fefc6b1/ash/mus/shell_delegate_mus.h
[modify] https://crrev.com/f0c78fcfe80c8b182fc1bbcb6ce626676fefc6b1/ash/shell.cc
[modify] https://crrev.com/f0c78fcfe80c8b182fc1bbcb6ce626676fefc6b1/ash/shell.h
[modify] https://crrev.com/f0c78fcfe80c8b182fc1bbcb6ce626676fefc6b1/ash/shell/shell_delegate_impl.cc
[modify] https://crrev.com/f0c78fcfe80c8b182fc1bbcb6ce626676fefc6b1/ash/shell/shell_delegate_impl.h
[modify] https://crrev.com/f0c78fcfe80c8b182fc1bbcb6ce626676fefc6b1/ash/shell_delegate.h
[modify] https://crrev.com/f0c78fcfe80c8b182fc1bbcb6ce626676fefc6b1/ash/system/ime/tray_ime_chromeos.cc
[modify] https://crrev.com/f0c78fcfe80c8b182fc1bbcb6ce626676fefc6b1/ash/system/ime/tray_ime_chromeos.h
[modify] https://crrev.com/f0c78fcfe80c8b182fc1bbcb6ce626676fefc6b1/ash/system/ime/tray_ime_chromeos_unittest.cc
[modify] https://crrev.com/f0c78fcfe80c8b182fc1bbcb6ce626676fefc6b1/ash/system/ime_menu/ime_list_view.cc
[modify] https://crrev.com/f0c78fcfe80c8b182fc1bbcb6ce626676fefc6b1/ash/system/ime_menu/ime_menu_tray.cc
[modify] https://crrev.com/f0c78fcfe80c8b182fc1bbcb6ce626676fefc6b1/ash/system/ime_menu/ime_menu_tray.h
[modify] https://crrev.com/f0c78fcfe80c8b182fc1bbcb6ce626676fefc6b1/ash/system/ime_menu/ime_menu_tray_unittest.cc
[modify] https://crrev.com/f0c78fcfe80c8b182fc1bbcb6ce626676fefc6b1/ash/test/test_shell_delegate.cc
[modify] https://crrev.com/f0c78fcfe80c8b182fc1bbcb6ce626676fefc6b1/ash/test/test_shell_delegate.h
[modify] https://crrev.com/f0c78fcfe80c8b182fc1bbcb6ce626676fefc6b1/chrome/browser/chromeos/input_method/input_method_manager_impl.cc
[modify] https://crrev.com/f0c78fcfe80c8b182fc1bbcb6ce626676fefc6b1/chrome/browser/chromeos/input_method/input_method_manager_impl.h
[modify] https://crrev.com/f0c78fcfe80c8b182fc1bbcb6ce626676fefc6b1/chrome/browser/chromeos/input_method/input_method_manager_impl_unittest.cc
[modify] https://crrev.com/f0c78fcfe80c8b182fc1bbcb6ce626676fefc6b1/chrome/browser/chromeos/input_method/mock_input_method_manager_impl.cc
[modify] https://crrev.com/f0c78fcfe80c8b182fc1bbcb6ce626676fefc6b1/chrome/browser/chromeos/input_method/mock_input_method_manager_impl.h
[modify] https://crrev.com/f0c78fcfe80c8b182fc1bbcb6ce626676fefc6b1/chrome/browser/chromeos/login/lock_screen_utils.cc
[modify] https://crrev.com/f0c78fcfe80c8b182fc1bbcb6ce626676fefc6b1/chrome/browser/chromeos/login/lock_screen_utils.h
[modify] https://crrev.com/f0c78fcfe80c8b182fc1bbcb6ce626676fefc6b1/chrome/browser/ui/BUILD.gn
[modify] https://crrev.com/f0c78fcfe80c8b182fc1bbcb6ce626676fefc6b1/chrome/browser/ui/ash/chrome_shell_delegate.cc
[modify] https://crrev.com/f0c78fcfe80c8b182fc1bbcb6ce626676fefc6b1/chrome/browser/ui/ash/chrome_shell_delegate.h
[add] https://crrev.com/f0c78fcfe80c8b182fc1bbcb6ce626676fefc6b1/chrome/browser/ui/ash/ime_controller_client.cc
[add] https://crrev.com/f0c78fcfe80c8b182fc1bbcb6ce626676fefc6b1/chrome/browser/ui/ash/ime_controller_client.h
[add] https://crrev.com/f0c78fcfe80c8b182fc1bbcb6ce626676fefc6b1/chrome/browser/ui/ash/ime_controller_client_unittest.cc
[modify] https://crrev.com/f0c78fcfe80c8b182fc1bbcb6ce626676fefc6b1/chrome/browser/ui/ash/system_tray_delegate_chromeos.cc
[modify] https://crrev.com/f0c78fcfe80c8b182fc1bbcb6ce626676fefc6b1/chrome/browser/ui/ash/system_tray_delegate_chromeos.h
[modify] https://crrev.com/f0c78fcfe80c8b182fc1bbcb6ce626676fefc6b1/chrome/browser/ui/views/ash/chrome_browser_main_extra_parts_ash.cc
[modify] https://crrev.com/f0c78fcfe80c8b182fc1bbcb6ce626676fefc6b1/chrome/browser/ui/views/ash/chrome_browser_main_extra_parts_ash.h
[modify] https://crrev.com/f0c78fcfe80c8b182fc1bbcb6ce626676fefc6b1/chrome/browser/ui/webui/chromeos/login/signin_screen_handler.cc
[modify] https://crrev.com/f0c78fcfe80c8b182fc1bbcb6ce626676fefc6b1/chrome/test/BUILD.gn

Project Member

Comment 10 by bugdroid1@chromium.org, Jun 21 2017

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

commit 23f56b7b5e6ce93edce36088fd1beb8d373abd7d
Author: James Cook <jamescook@chromium.org>
Date: Wed Jun 21 01:42:30 2017

chromeos: Send current IME to ash by ID rather than struct

The current IME is always in the list of available IMEs, so just send
its ID.

Also get rid of ash::ImeInfo.selected since that can be inferred by
comparing the ID of an IME to the current ID.

Also cleanup some test init code in ImeControllerClientTest.

Bug:  724305 
Test: ash_unittests and chrome unit_tests
Change-Id: Id830048980728e30c6ae39acd833e26beb83ff64
Reviewed-on: https://chromium-review.googlesource.com/540665
Commit-Queue: James Cook <jamescook@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#481069}
[modify] https://crrev.com/23f56b7b5e6ce93edce36088fd1beb8d373abd7d/ash/ime/ime_controller.cc
[modify] https://crrev.com/23f56b7b5e6ce93edce36088fd1beb8d373abd7d/ash/ime/ime_controller.h
[modify] https://crrev.com/23f56b7b5e6ce93edce36088fd1beb8d373abd7d/ash/ime/ime_controller_unittest.cc
[modify] https://crrev.com/23f56b7b5e6ce93edce36088fd1beb8d373abd7d/ash/public/interfaces/ime_controller.mojom
[modify] https://crrev.com/23f56b7b5e6ce93edce36088fd1beb8d373abd7d/ash/public/interfaces/ime_info.mojom
[modify] https://crrev.com/23f56b7b5e6ce93edce36088fd1beb8d373abd7d/ash/system/ime/tray_ime_chromeos.cc
[modify] https://crrev.com/23f56b7b5e6ce93edce36088fd1beb8d373abd7d/ash/system/ime/tray_ime_chromeos_unittest.cc
[modify] https://crrev.com/23f56b7b5e6ce93edce36088fd1beb8d373abd7d/ash/system/ime_menu/ime_list_view.cc
[modify] https://crrev.com/23f56b7b5e6ce93edce36088fd1beb8d373abd7d/ash/system/ime_menu/ime_list_view.h
[modify] https://crrev.com/23f56b7b5e6ce93edce36088fd1beb8d373abd7d/ash/system/ime_menu/ime_menu_tray.cc
[modify] https://crrev.com/23f56b7b5e6ce93edce36088fd1beb8d373abd7d/ash/system/ime_menu/ime_menu_tray_unittest.cc
[modify] https://crrev.com/23f56b7b5e6ce93edce36088fd1beb8d373abd7d/chrome/browser/chromeos/input_method/input_method_manager_impl_unittest.cc
[modify] https://crrev.com/23f56b7b5e6ce93edce36088fd1beb8d373abd7d/chrome/browser/ui/ash/ime_controller_client.cc
[modify] https://crrev.com/23f56b7b5e6ce93edce36088fd1beb8d373abd7d/chrome/browser/ui/ash/ime_controller_client_unittest.cc

Cc: azurewei@chromium.org
+azurewei as FYI, this bug is tracking some of my mustash / mojo IME work

Project Member

Comment 13 by bugdroid1@chromium.org, Jun 28 2017

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

commit bf68beb619663b0208826075bd2e8f00e3c65a7d
Author: James Cook <jamescook@chromium.org>
Date: Wed Jun 28 19:40:53 2017

chromeos: Make IME system tray menu work under mash

Don't call into InputMethodManager::Get(), which crashes under mash
because the InputMethodManagerImpl is in the browser process.

Instead call through ImeController and its mojo interface back to
the browser. This makes IME switching and IME property activation
work in the IME menu lists.

Bug:  724305 
Test: added to ash_unittests, chrome unit_tests
Change-Id: If5258d0fd9344b9eb9ac6642146072200b1070dc
Reviewed-on: https://chromium-review.googlesource.com/549072
Reviewed-by: James Cook <jamescook@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Commit-Queue: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#483102}
[modify] https://crrev.com/bf68beb619663b0208826075bd2e8f00e3c65a7d/ash/ime/ime_controller.cc
[modify] https://crrev.com/bf68beb619663b0208826075bd2e8f00e3c65a7d/ash/ime/ime_controller.h
[modify] https://crrev.com/bf68beb619663b0208826075bd2e8f00e3c65a7d/ash/ime/ime_controller_unittest.cc
[modify] https://crrev.com/bf68beb619663b0208826075bd2e8f00e3c65a7d/ash/public/interfaces/ime_controller.mojom
[modify] https://crrev.com/bf68beb619663b0208826075bd2e8f00e3c65a7d/ash/system/ime_menu/ime_list_view.cc
[modify] https://crrev.com/bf68beb619663b0208826075bd2e8f00e3c65a7d/chrome/browser/ui/ash/ime_controller_client.cc
[modify] https://crrev.com/bf68beb619663b0208826075bd2e8f00e3c65a7d/chrome/browser/ui/ash/ime_controller_client.h
[modify] https://crrev.com/bf68beb619663b0208826075bd2e8f00e3c65a7d/chrome/browser/ui/ash/ime_controller_client_unittest.cc

Status: Fixed (was: Started)
Calling this done. The methods on the delegate interface are gone. There are still some IME features that don't work yet (mode indicator bubble) but I'll file separate bugs for those.

Comment 15 by dchan@chromium.org, Jan 22 2018

Status: Archived (was: Fixed)

Sign in to add a comment