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

Issue 14200032: Decouple l10n_util from InputMethodUtil. (Closed)

Created:
7 years, 8 months ago by Seigo Nonaka
Modified:
7 years, 8 months ago
Reviewers:
Nikita (slow), satorux1
CC:
chromium-reviews, dbeam+watch-options_chromium.org, nkostylev+watch_chromium.org, oshima+watch_chromium.org, stevenjb+watch_chromium.org, davemoore+watch_chromium.org, yusukes+watch_chromium.org
Visibility:
Public.

Description

Decouple l10n_util from InputMethodUtil. This CL changes: - Moving localization related function into delegate class. - Removing GetLanguageDisplayNameFromCode and GetLanguageNativeDisplayNameFromCode from InputMethodUtil because they are just thin wrapper function of l10n_util and only used from CrosLanguageOptionHandler. This CL does not change any test expectations except TestGetLanguageNativeDisplayNameFromCode for checking regressions. And I'm going to refactor strings related stuff to more appropriate location. So let me keep tests as is. BUG=164375 TEST=ran unit_tests and also checked there is no regression with this CL on actual devices. Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=194927

Patch Set 1 : #

Total comments: 2

Patch Set 2 : /MockInputMethodDelegate/FakeInputMethodDelegate/ #

Unified diffs Side-by-side diffs Delta from patch set Stats (+149 lines, -157 lines) Patch
M chrome/browser/chromeos/input_method/input_method_delegate_impl.h View 1 chunk +3 lines, -1 line 0 comments Download
M chrome/browser/chromeos/input_method/input_method_delegate_impl.cc View 2 chunks +11 lines, -5 lines 0 comments Download
M chrome/browser/chromeos/input_method/input_method_manager_impl_unittest.cc View 1 3 chunks +3 lines, -3 lines 0 comments Download
M chrome/browser/chromeos/input_method/input_method_util.h View 1 chunk +0 lines, -15 lines 0 comments Download
M chrome/browser/chromeos/input_method/input_method_util.cc View 7 chunks +6 lines, -44 lines 0 comments Download
M chrome/browser/chromeos/input_method/input_method_util_unittest.cc View 1 4 chunks +11 lines, -7 lines 0 comments Download
M chrome/browser/chromeos/input_method/mock_input_method_manager.h View 1 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/ui/webui/options/chromeos/cros_language_options_handler.cc View 1 chunk +3 lines, -5 lines 0 comments Download
M chromeos/chromeos.gyp View 1 1 chunk +2 lines, -2 lines 0 comments Download
A chromeos/ime/fake_input_method_delegate.h View 1 1 chunk +62 lines, -0 lines 0 comments Download
A chromeos/ime/fake_input_method_delegate.cc View 1 1 chunk +35 lines, -0 lines 0 comments Download
M chromeos/ime/input_method_delegate.h View 2 chunks +11 lines, -2 lines 0 comments Download
M chromeos/ime/mock_input_method_delegate.h View 1 1 chunk +0 lines, -42 lines 0 comments Download
M chromeos/ime/mock_input_method_delegate.cc View 1 1 chunk +0 lines, -26 lines 0 comments Download
M chromeos/ime/xkeyboard_unittest.cc View 1 2 chunks +0 lines, -3 lines 0 comments Download

Messages

Total messages: 6 (0 generated)
Seigo Nonaka
Hi Satoru, could you take a look? This is a part of refactoring that moving ...
7 years, 8 months ago (2013-04-17 13:43:03 UTC) #1
satorux1
LGTM with nits https://codereview.chromium.org/14200032/diff/5001/chromeos/ime/mock_input_method_delegate.h File chromeos/ime/mock_input_method_delegate.h (right): https://codereview.chromium.org/14200032/diff/5001/chromeos/ime/mock_input_method_delegate.h#newcode17 chromeos/ime/mock_input_method_delegate.h:17: class CHROMEOS_EXPORT MockInputMethodDelegate : public InputMethodDelegate ...
7 years, 8 months ago (2013-04-18 06:09:26 UTC) #2
Seigo Nonaka
Satoru, thank you for your review! Adding Nikita as the owner of chrome/browser/ui/webui/options/chromeos/. Hi Nikita, ...
7 years, 8 months ago (2013-04-18 11:50:59 UTC) #3
Nikita (slow)
lgtm
7 years, 8 months ago (2013-04-18 12:10:57 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/nona@chromium.org/14200032/9001
7 years, 8 months ago (2013-04-18 12:35:46 UTC) #5
commit-bot: I haz the power
7 years, 8 months ago (2013-04-18 15:20:18 UTC) #6
Message was sent while issue was closed.
Change committed as 194927

Powered by Google App Engine
This is Rietveld 408576698