Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(241)

Issue 2891263002: chromeos: Remove some IME methods from ash::SystemTrayDelegate (Closed)

Created:
3 years, 7 months ago by James Cook
Modified:
3 years, 6 months ago
Reviewers:
Shu Chen, Azure Wei
CC:
chromium-reviews, sadrul, yusukes+watch_chromium.org, shuchen+watch_chromium.org, nona+watch_chromium.org, oshima+watch_chromium.org, kalyank, James Su, davemoore+watch_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

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} Committed: https://chromium.googlesource.com/chromium/src/+/1290776535cfda1fa1cd7867d5a2ae2ded7e92ec

Patch Set 1 #

Patch Set 2 : rebase #

Patch Set 3 : rebase #

Patch Set 4 : cleanup #

Total comments: 1

Patch Set 5 : rebase #

Unified diffs Side-by-side diffs Delta from patch set Stats (+169 lines, -358 lines) Patch
M ash/BUILD.gn View 1 2 3 4 1 chunk +1 line, -1 line 0 comments Download
M ash/accelerators/accelerator_controller.h View 3 chunks +0 lines, -6 lines 0 comments Download
M ash/accelerators/accelerator_controller.cc View 1 2 3 12 chunks +45 lines, -31 lines 0 comments Download
M ash/accelerators/accelerator_controller_unittest.cc View 1 2 3 9 chunks +83 lines, -86 lines 0 comments Download
A ash/ime/ime_switch_type.h View 1 chunk +21 lines, -0 lines 0 comments Download
D ash/ime_control_delegate.h View 1 chunk +0 lines, -40 lines 0 comments Download
M ash/system/ime_menu/ime_list_view.cc View 5 chunks +11 lines, -3 lines 0 comments Download
M ash/system/tray/system_tray_delegate.h View 1 1 chunk +0 lines, -6 lines 0 comments Download
M ash/system/tray/system_tray_delegate.cc View 1 1 chunk +0 lines, -4 lines 0 comments Download
M chrome/browser/chromeos/BUILD.gn View 1 2 3 4 1 chunk +0 lines, -2 lines 0 comments Download
M chrome/browser/chromeos/input_method/input_method_manager_impl.cc View 4 chunks +1 line, -6 lines 0 comments Download
D chrome/browser/chromeos/input_method/input_method_switch_recorder.h View 1 chunk +0 lines, -25 lines 0 comments Download
D chrome/browser/chromeos/input_method/input_method_switch_recorder.cc View 1 chunk +0 lines, -48 lines 0 comments Download
M chrome/browser/ui/BUILD.gn View 1 2 3 4 1 chunk +0 lines, -2 lines 0 comments Download
M chrome/browser/ui/ash/ash_init.cc View 2 chunks +0 lines, -3 lines 0 comments Download
D chrome/browser/ui/ash/ime_controller_chromeos.h View 1 chunk +0 lines, -30 lines 0 comments Download
D chrome/browser/ui/ash/ime_controller_chromeos.cc View 1 chunk +0 lines, -50 lines 0 comments Download
M chrome/browser/ui/ash/system_tray_delegate_chromeos.h View 1 1 chunk +0 lines, -2 lines 0 comments Download
M chrome/browser/ui/ash/system_tray_delegate_chromeos.cc View 1 2 chunks +0 lines, -13 lines 0 comments Download
M ui/base/ime/chromeos/mock_input_method_manager.h View 1 chunk +1 line, -0 lines 0 comments Download
M ui/base/ime/chromeos/mock_input_method_manager.cc View 1 chunk +6 lines, -0 lines 0 comments Download

Messages

Total messages: 36 (22 generated)
James Cook
azurewei, please take a look. https://codereview.chromium.org/2891263002/diff/60001/ui/base/ime/chromeos/mock_input_method_manager.h File ui/base/ime/chromeos/mock_input_method_manager.h (right): https://codereview.chromium.org/2891263002/diff/60001/ui/base/ime/chromeos/mock_input_method_manager.h#newcode24 ui/base/ime/chromeos/mock_input_method_manager.h:24: scoped_refptr<InputMethodManager::State> Clone() const override; ...
3 years, 7 months ago (2017-05-23 00:01:44 UTC) #12
Azure Wei
lgtm
3 years, 7 months ago (2017-05-23 07:36:59 UTC) #15
James Cook
shuchen, can I get OWNERS for chrome/browser/chromeos/input_method and ui/base/ime/chromeos?
3 years, 7 months ago (2017-05-23 15:34:24 UTC) #17
Shu Chen
On 2017/05/23 15:34:24, James Cook wrote: > shuchen, can I get OWNERS for chrome/browser/chromeos/input_method and ...
3 years, 7 months ago (2017-05-24 04:29:19 UTC) #18
Shu Chen
lgtm
3 years, 7 months ago (2017-05-24 04:29:26 UTC) #19
James Cook
Thanks for the review. I don't want to be in the OWNERS file, I just ...
3 years, 7 months ago (2017-05-24 08:07:51 UTC) #20
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2891263002/80001
3 years, 7 months ago (2017-05-24 08:08:33 UTC) #23
commit-bot: I haz the power
Try jobs failed on following builders: linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_android_rel_ng/builds/301682)
3 years, 7 months ago (2017-05-24 09:14:13 UTC) #25
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2891263002/80001
3 years, 6 months ago (2017-05-24 15:21:02 UTC) #27
commit-bot: I haz the power
Try jobs failed on following builders: linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_android_rel_ng/builds/301939)
3 years, 6 months ago (2017-05-24 17:36:07 UTC) #29
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2891263002/80001
3 years, 6 months ago (2017-05-24 17:40:53 UTC) #31
commit-bot: I haz the power
Committed patchset #5 (id:80001) as https://chromium.googlesource.com/chromium/src/+/1290776535cfda1fa1cd7867d5a2ae2ded7e92ec
3 years, 6 months ago (2017-05-24 19:07:25 UTC) #34
James Cook
A revert of this CL (patchset #5 id:80001) has been created in https://codereview.chromium.org/2925683002/ by jamescook@chromium.org. ...
3 years, 6 months ago (2017-06-05 20:03:38 UTC) #35
James Cook
3 years, 6 months ago (2017-06-05 20:06:21 UTC) #36
Message was sent while issue was closed.
On 2017/06/05 20:03:38, James Cook wrote:
> A revert of this CL (patchset #5 id:80001) has been created in
> https://codereview.chromium.org/2925683002/ by mailto:jamescook@chromium.org.
> 
> The reason for reverting is: 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.
> .

I didn't use the revert CL. I'll revert this by hand because I want to keep
parts of the CL.

Powered by Google App Engine
This is Rietveld 408576698