When we change the keyboard keyset (e.g. 'emoji'), we call InputMethodManager::OverrideKeyboardUrlRef with the keyset name directly from ash/system/ime_menu/ImeMenuTray (e.g [1]) However, under mash, InputMethodManager will not be available from ImeMenuTray, so we'll need to override the keyboard URL via mojo. [1] https://cs.chromium.org/chromium/src/ash/system/ime_menu/ime_menu_tray.cc?q=ash/system/ime_menu/ime_menu_tray.cc&sq=package:chromium&l=369
Hi folks, this is my first time doing this :) Is the idea to replace calls of InputMethodManager::OverrideKeyboardUrlRef with Mojo calls? I'll have to add a new method to the IME controller mojo interface right?
I commented in your CL (https://chromium-review.googlesource.com/c/chromium/src/+/950523). Thanks for looking into this. This stuff is tricky, and unfortunately I'm not super familiar with the history of either IME or virtual keyboard.
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a4bf749ba6642c9cf84d13f56240c22f0525dda4 commit a4bf749ba6642c9cf84d13f56240c22f0525dda4 Author: Darren Shen <shend@chromium.org> Date: Fri Mar 16 04:25:52 2018 mash: Override keyboard URL ref via Mojo ImeController. When we change the keyboard keyset (e.g. 'emoji') in ash/system/ime_menu/ImeMenuTray, we directly call InputMethodManager:: OverrideKeyboardUrlRef. Under mash, InputMethodManager will not be available from ImeMenuTray, so we'll need to call this via a Mojo in ImeControllerClient. For security reasons, we pass in an enum representing the keyset, instead of an arbitrary string. Bug: 819018 Change-Id: I7b79af660c421da86d248a6472ce568e84770909 Reviewed-on: https://chromium-review.googlesource.com/950523 Reviewed-by: Daniel Cheng <dcheng@chromium.org> Reviewed-by: Yuichiro Hanada <yhanada@chromium.org> Reviewed-by: Ilya Sherman <isherman@chromium.org> Reviewed-by: James Cook <jamescook@chromium.org> Commit-Queue: Darren Shen <shend@chromium.org> Cr-Commit-Position: refs/heads/master@{#543624} [modify] https://crrev.com/a4bf749ba6642c9cf84d13f56240c22f0525dda4/ash/ime/ime_controller.cc [modify] https://crrev.com/a4bf749ba6642c9cf84d13f56240c22f0525dda4/ash/ime/ime_controller.h [modify] https://crrev.com/a4bf749ba6642c9cf84d13f56240c22f0525dda4/ash/ime/test_ime_controller_client.cc [modify] https://crrev.com/a4bf749ba6642c9cf84d13f56240c22f0525dda4/ash/ime/test_ime_controller_client.h [modify] https://crrev.com/a4bf749ba6642c9cf84d13f56240c22f0525dda4/ash/public/interfaces/ime_controller.mojom [modify] https://crrev.com/a4bf749ba6642c9cf84d13f56240c22f0525dda4/ash/system/ime_menu/ime_menu_tray.cc [modify] https://crrev.com/a4bf749ba6642c9cf84d13f56240c22f0525dda4/ash/system/ime_menu/ime_menu_tray.h [modify] https://crrev.com/a4bf749ba6642c9cf84d13f56240c22f0525dda4/ash/system/ime_menu/ime_menu_tray_unittest.cc [modify] https://crrev.com/a4bf749ba6642c9cf84d13f56240c22f0525dda4/chrome/browser/ui/ash/ime_controller_client.cc [modify] https://crrev.com/a4bf749ba6642c9cf84d13f56240c22f0525dda4/chrome/browser/ui/ash/ime_controller_client.h [modify] https://crrev.com/a4bf749ba6642c9cf84d13f56240c22f0525dda4/chrome/browser/ui/ash/ime_controller_client_unittest.cc [modify] https://crrev.com/a4bf749ba6642c9cf84d13f56240c22f0525dda4/tools/metrics/histograms/enums.xml
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8d50a44281146bf1481673f5871513fc2df1d61b commit 8d50a44281146bf1481673f5871513fc2df1d61b Author: Yuichiro Hanada <yhanada@chromium.org> Date: Fri Mar 16 23:18:21 2018 Remove {Set, Get}OverrideContentUrl in keyboard_content_util. Before this change, URL of custom input view of IMEs, which is loaded by ChromeKeyboardUI when an user invokes Virtual Keyboard, is stored in global variables in keyboard_content_util and everyone can override it accidentally. This change moves that state into InputMethodManagerImpl and adds some methods to interact with that state. Bug: 819018 Test: Related unit tests pass and manual testing with ime menu tray. Change-Id: Ic6f8e0e108c3706ad1ff2ce90aeeb1388704ff36 Reviewed-on: https://chromium-review.googlesource.com/953023 Commit-Queue: Yuichiro Hanada <yhanada@chromium.org> Reviewed-by: Shu Chen <shuchen@chromium.org> Cr-Commit-Position: refs/heads/master@{#543864} [modify] https://crrev.com/8d50a44281146bf1481673f5871513fc2df1d61b/chrome/browser/chromeos/input_method/input_method_engine.cc [modify] https://crrev.com/8d50a44281146bf1481673f5871513fc2df1d61b/chrome/browser/chromeos/input_method/input_method_manager_impl.cc [modify] https://crrev.com/8d50a44281146bf1481673f5871513fc2df1d61b/chrome/browser/chromeos/input_method/input_method_manager_impl.h [modify] https://crrev.com/8d50a44281146bf1481673f5871513fc2df1d61b/chrome/browser/chromeos/input_method/input_method_manager_impl_unittest.cc [modify] https://crrev.com/8d50a44281146bf1481673f5871513fc2df1d61b/chrome/browser/extensions/api/input_ime/input_ime_api_chromeos.cc [modify] https://crrev.com/8d50a44281146bf1481673f5871513fc2df1d61b/chrome/browser/ui/ash/chrome_keyboard_ui.cc [modify] https://crrev.com/8d50a44281146bf1481673f5871513fc2df1d61b/chrome/browser/ui/ash/chrome_keyboard_ui.h [modify] https://crrev.com/8d50a44281146bf1481673f5871513fc2df1d61b/chrome/browser/ui/ash/keyboard_controller_browsertest.cc [modify] https://crrev.com/8d50a44281146bf1481673f5871513fc2df1d61b/ui/base/ime/chromeos/input_method_manager.h [modify] https://crrev.com/8d50a44281146bf1481673f5871513fc2df1d61b/ui/base/ime/chromeos/mock_input_method_manager.cc [modify] https://crrev.com/8d50a44281146bf1481673f5871513fc2df1d61b/ui/base/ime/chromeos/mock_input_method_manager.h [modify] https://crrev.com/8d50a44281146bf1481673f5871513fc2df1d61b/ui/keyboard/content/keyboard_content_util.cc [modify] https://crrev.com/8d50a44281146bf1481673f5871513fc2df1d61b/ui/keyboard/content/keyboard_content_util.h
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2b66e9c46f7739a46093c4468e9bdc98b16ddc69 commit 2b66e9c46f7739a46093c4468e9bdc98b16ddc69 Author: Yuichiro Hanada <yhanada@chromium.org> Date: Tue Mar 27 03:08:02 2018 Move ash::mojom::ImeKeyset into //ui/base/ime/chromeos/public/interfaces. and use it everywhere to express keysets. This CL aims to reduce the risk to pass unexpected values to InputMethodManager::OverrideKeyboardUrlRef. - Rename OverrideKeyboardUrlRef to OverrideKeyboardKeyset This CL addresses the comment in crrev.com/c/950523. Bug: 819018 Test: No behavior change. Unit tests still pass. Change-Id: I96139abaa867c91de039113bc9006b72f210be26 Reviewed-on: https://chromium-review.googlesource.com/974668 Reviewed-by: Daniel Cheng <dcheng@chromium.org> Reviewed-by: James Cook <jamescook@chromium.org> Commit-Queue: Yuichiro Hanada <yhanada@chromium.org> Cr-Commit-Position: refs/heads/master@{#545931} [modify] https://crrev.com/2b66e9c46f7739a46093c4468e9bdc98b16ddc69/ash/ime/ime_controller.cc [modify] https://crrev.com/2b66e9c46f7739a46093c4468e9bdc98b16ddc69/ash/ime/ime_controller.h [modify] https://crrev.com/2b66e9c46f7739a46093c4468e9bdc98b16ddc69/ash/ime/test_ime_controller_client.cc [modify] https://crrev.com/2b66e9c46f7739a46093c4468e9bdc98b16ddc69/ash/ime/test_ime_controller_client.h [modify] https://crrev.com/2b66e9c46f7739a46093c4468e9bdc98b16ddc69/ash/public/interfaces/BUILD.gn [modify] https://crrev.com/2b66e9c46f7739a46093c4468e9bdc98b16ddc69/ash/public/interfaces/ime_controller.mojom [modify] https://crrev.com/2b66e9c46f7739a46093c4468e9bdc98b16ddc69/ash/system/ime_menu/ime_menu_tray.cc [modify] https://crrev.com/2b66e9c46f7739a46093c4468e9bdc98b16ddc69/ash/system/ime_menu/ime_menu_tray.h [modify] https://crrev.com/2b66e9c46f7739a46093c4468e9bdc98b16ddc69/ash/system/ime_menu/ime_menu_tray_unittest.cc [modify] https://crrev.com/2b66e9c46f7739a46093c4468e9bdc98b16ddc69/chrome/browser/chromeos/input_method/input_method_manager_impl.cc [modify] https://crrev.com/2b66e9c46f7739a46093c4468e9bdc98b16ddc69/chrome/browser/chromeos/input_method/input_method_manager_impl.h [modify] https://crrev.com/2b66e9c46f7739a46093c4468e9bdc98b16ddc69/chrome/browser/chromeos/input_method/input_method_manager_impl_unittest.cc [modify] https://crrev.com/2b66e9c46f7739a46093c4468e9bdc98b16ddc69/chrome/browser/ui/ash/ime_controller_client.cc [modify] https://crrev.com/2b66e9c46f7739a46093c4468e9bdc98b16ddc69/chrome/browser/ui/ash/ime_controller_client.h [modify] https://crrev.com/2b66e9c46f7739a46093c4468e9bdc98b16ddc69/chrome/browser/ui/ash/ime_controller_client_unittest.cc [modify] https://crrev.com/2b66e9c46f7739a46093c4468e9bdc98b16ddc69/ui/base/ime/BUILD.gn [modify] https://crrev.com/2b66e9c46f7739a46093c4468e9bdc98b16ddc69/ui/base/ime/chromeos/input_method_manager.h [modify] https://crrev.com/2b66e9c46f7739a46093c4468e9bdc98b16ddc69/ui/base/ime/chromeos/mock_input_method_manager.cc [modify] https://crrev.com/2b66e9c46f7739a46093c4468e9bdc98b16ddc69/ui/base/ime/chromeos/mock_input_method_manager.h [add] https://crrev.com/2b66e9c46f7739a46093c4468e9bdc98b16ddc69/ui/base/ime/chromeos/public/interfaces/BUILD.gn [add] https://crrev.com/2b66e9c46f7739a46093c4468e9bdc98b16ddc69/ui/base/ime/chromeos/public/interfaces/OWNERS [add] https://crrev.com/2b66e9c46f7739a46093c4468e9bdc98b16ddc69/ui/base/ime/chromeos/public/interfaces/ime_keyset.mojom
Comment 1 by shend@chromium.org
, Mar 6 2018