metabug: Replace InputMethodManager access in ash with mojo calls to browser |
||||||||||||||
Issue descriptionFor mustash, code in //ash will run in a separate process. See go/mustash. Ash contains code that uses the chromeos::input_method::InputMethodManager interface, which is declared in //ui/base/ime/chromeos. However, the only concrete implementation is InputMethodManagerImpl, which lives in //chrome/browser/chromeos/input_method and exists only in the browser process. This means that code in ash that calls InputMethodManager::Get() will get a null pointer and IME-related code will not work. The code needs to be refactored to use mojo interfaces to talk to the browser. For high level things like system tray: Design: go/mustash-tray-ime //ash/ime/ime_controller.h //ash/public/interfaces/ime_controller.mojom And for lower-level things (like individual input event handling): https://docs.google.com/document/d/1LNxH7fRpQcuNzKxzMs4kKl4xnO-2Wa_ocZkNsr9vfdo/edit# https://docs.google.com/document/d/1oRLyNIrJmA-15w-vy9gTguVrY-kSAwJRMWd3I6PgUMw/edit# //services/ui/public/interfaces/ime/ime.mojom To Satoru for reassignment.
,
Jun 29 2017
,
Jun 29 2017
,
Jul 7 2017
,
Dec 26 2017
Covered bugs are all fixed.
,
Jan 3 2018
I still see access to InputMethodManager in ash/accelerators/accelerator_controller.cc, ash/login/ui and ash/system/ime_menu/ime_menu_tray.cc I think there's more work to do here. Sorry there wasn't a full list of blocking bugs filed. Eventually it would be nice to change ash/DEPS to have a -ui/base/ime/chromeos entry.
,
Jan 3 2018
Feel free to reassign if you don't have cycles for this. I'd like to keep it open to track that there's still more work to do.
,
Feb 26 2018
,
Feb 27 2018
,
Mar 6 2018
,
Mar 6 2018
,
Mar 6 2018
,
Mar 13 2018
Issue 772382 has been merged into this issue.
,
Mar 13 2018
There are also a couple calls in //components/exo, which is in the ash process under mash.
,
Mar 19 2018
Hi James, are there any more references to InputMethodManager that we need to remove? Otherwise I'll close this bug.
,
Mar 19 2018
I see references in tests in ash. Those should not be necessary anymore (since the code under test doesn't use it, right?). So they should be removed.
jamescook@jamescook:/w/chrome/src/ash (tapdrag)$ git grep InputMethodManager
accelerators/accelerator_controller_unittest.cc:using chromeos::input_method::InputMethodManager;
accelerators/accelerator_controller_unittest.cc:class TestInputMethodManager
accelerators/accelerator_controller_unittest.cc: : public chromeos::input_method::MockInputMethodManager {
accelerators/accelerator_controller_unittest.cc: TestInputMethodManager() = default;
accelerators/accelerator_controller_unittest.cc: ~TestInputMethodManager() override = default;
accelerators/accelerator_controller_unittest.cc: // MockInputMethodManager:
accelerators/accelerator_controller_unittest.cc: DISALLOW_COPY_AND_ASSIGN(TestInputMethodManager);
accelerators/accelerator_controller_unittest.cc: test_input_method_manager_ = new TestInputMethodManager;
accelerators/accelerator_controller_unittest.cc: InputMethodManager::Initialize(test_input_method_manager_);
accelerators/accelerator_controller_unittest.cc: InputMethodManager::Shutdown();
accelerators/accelerator_controller_unittest.cc: // Owned by InputMethodManager.
accelerators/accelerator_controller_unittest.cc: TestInputMethodManager* test_input_method_manager_ = nullptr;
system/ime_menu/ime_menu_tray_unittest.cc:using chromeos::input_method::InputMethodManager;
system/ime_menu/ime_menu_tray_unittest.cc:using chromeos::input_method::MockInputMethodManager;
system/ime_menu/ime_menu_tray_unittest.cc: // MockInputMethodManager enables emoji, handwriting and voice input by
system/ime_menu/ime_menu_tray_unittest.cc: InputMethodManager::Initialize(new MockInputMethodManager);
system/ime_menu/ime_menu_tray_unittest.cc: InputMethodManager::Shutdown();
This reference is probably OK, I think erg@ worked on remoting and input injection:
remoting/host/input_injector_chromeos.cc: chromeos::input_method::InputMethodManager::Get();
So my best guess is that after the tests are scrubbed this can be closed.
,
Mar 20 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/136153c85da7c1ad5eb9f3ce8abd365c2a84f62e commit 136153c85da7c1ad5eb9f3ce8abd365c2a84f62e Author: Darren Shen <shend@chromium.org> Date: Tue Mar 20 22:02:30 2018 [mash] Remove references to InputMethodManager from ash/ unittests. This patch removes unused references to InputMethodManager from unit tests, as InputMethodManager won't be available to the ash process under mash. Bug: 738210 Change-Id: I4495a46f3c9cad0b12a8cd539b8a765bfdc24a26 Reviewed-on: https://chromium-review.googlesource.com/969661 Commit-Queue: Darren Shen <shend@chromium.org> Reviewed-by: James Cook <jamescook@chromium.org> Cr-Commit-Position: refs/heads/master@{#544547} [modify] https://crrev.com/136153c85da7c1ad5eb9f3ce8abd365c2a84f62e/ash/DEPS [modify] https://crrev.com/136153c85da7c1ad5eb9f3ce8abd365c2a84f62e/ash/accelerators/accelerator_controller_unittest.cc [modify] https://crrev.com/136153c85da7c1ad5eb9f3ce8abd365c2a84f62e/ash/system/ime_menu/ime_menu_tray_unittest.cc
,
Mar 20 2018
|
||||||||||||||
►
Sign in to add a comment |
||||||||||||||
Comment 1 by jamescook@chromium.org
, Jun 29 2017