Chromium Code Reviews Chromium Code Reviews
Issue
9999018:
chrome/browser/chromeos/input_method/ refactoring [part 6 of 6] (Closed)
Descriptionchrome/browser/chromeos/input_method/ refactoring [part 6 of 6].
* Overview of the change
The main purpose of this CL is to make InputMethodManager fully unit-testable. To accomplish that, InputMethodManager is modified as follows:
- Add setters to set mock versions of IBusController, CandidateWindowController, and XKeyboard.
- Move ibus-daemon startup and shutdown code from the manager to IBusControllerImpl. They lived in the manager only for historical reasons.
- Remove code for monitoring the status of the connection between Chrome and ibus-daemon (InputMethodManager::OnConnectionChange(bool)). This kind of thing should also be done in IBusControllerImpl.
* Details
ash/shell.cc:
ash/system/ime/tray_ime.cc:
ash/system/tray/system_tray*.*:
chrome/browser/chromeos/system/ash_system_tray_delegate.cc:
Remove unused parameter |is_selection| from the SystemTrayDelegate interface following sadrul's suggestion in http://codereview.chromium.org/10008043/.
chrome/browser/chromeos/chrome_browser_main_chromeos.cc:
Initialize() and Shutdown() InputMethodManager.
chrome/browser/chromeos/extensions/input_method_event_router.h:
chrome/browser/chromeos/extensions/input_method_event_router.cc:
Follow the interface change of InputMethodManager::Observer. Implementation is not changed at all.
chrome/browser/chromeos/input_method/browser_state_monitor.h:
chrome/browser/chromeos/input_method/browser_state_monitor.cc:
Follow the interface change of InputMethodManager::Observer.
Make the class testable by adding some setters and getters.
Remove STATE_LOGGING_IN. Since preferences.cc is fixed in #5 of the series of patches, the intermediate state is no longer needed.
chrome/browser/chromeos/input_method/browser_state_monitor_unittest.cc:
Verify if the implementation updates the correct pref file (i.e. Local\ State or Preferences depending on the browser state).
chrome/browser/chromeos/input_method/ibus_controller.h:
Simplify the IBusController::Observer interface.
Add Start() which is called when a non-XKB input method is enabled.
Rename SetImePropertyActivated() to ActivateInputMethodProperty() and remove the second parameter |is_selection| which is no longer needed.
Remove CreateInputMethodDescriptor() as we can simply use a constructor in InputMethodDescriptor.
chrome/browser/chromeos/input_method/ibus_controller.cc:
Move HAVE_IBUS part to chrome/browser/chromeos/input_method/ibus_controller_impl.cc.
Move !HAVE_IBUS part to chrome/browser/chromeos/input_method/mock_ibus_controller.cc.
Move some functions which do not depend on libibus to chrome/browser/chromeos/input_method/ibus_controller_base.cc.
chrome/browser/chromeos/input_method/ibus_controller_base.h:
chrome/browser/chromeos/input_method/ibus_controller_base.cc:
chrome/browser/chromeos/input_method/ibus_controller_base_unittest.cc:
Implement some part of the IBusController interface. Since the files do not depend on libibus, they are unit-testable.
chrome/browser/chromeos/input_method/ibus_controller_impl.h:
chrome/browser/chromeos/input_method/ibus_controller_impl.cc:
Implement libibus dependent part of the IBusController interface.
Moved ibus-daemon startup and shutdown code from input_method_manager.cc. Most of other part are unchanged.
chrome/browser/chromeos/input_method/ibus_controller_impl_unittest.cc:
Tests FindAndUpdateProperty() function in ibus_controller_impl.cc since the function does not use libibus structues/functions. Other functions in ibus_controller_impl.cc will become unit testable soon once crosbug.com/26334 is fixed (by nona@c).
chrome/browser/chromeos/input_method/input_method_engine.cc:
Follow InputMethodManager interface changes.
chrome/browser/chromeos/input_method/input_method_manager.h:
Simplify InputMethodManager::Observer interface.
Add EnableInputMethods(). We used to set an ibus-daemon config called "preload_engines" to enable input methods, but it is difficult to understand. In addition to the readability problem, the "preload_engines" config has been removed from ibus-1.5. On the daemon side, what we have to do to switch the current input method is just to call ibus_bus_set_global_engine("engine-name") without calling ibus_config_set_value("preload_engiens", ...). So enabling engines by calling SetInputMethodConfig() does not make sense at all now.
Rename AddActiveIme() to AddInputMethodExtension(). Do the same for RemoveActiveIme().
Move StartInputMethodDaemon() and StopInputMethodDaemon() to IBusControllerImpl. This should be done automatically in the libibus layer.
Add Initialize() and Shutdown() to stop initializing the instance as a singleton.
chrome/browser/chromeos/input_method/input_method_manager.cc:
Move to input_method_manager_impl.cc.
chrome/browser/chromeos/input_method/input_method_manager_impl.h:
chrome/browser/chromeos/input_method/input_method_manager_impl.cc:
The manager implementation. Fully tested.
chrome/browser/chromeos/input_method/input_method_manager_browsertest.cc:
chrome/browser/chromeos/input_method/input_method_manager_impl_unittest.cc:
Rename input_method_manager_browsertest.cc to input_method_manager_impl_unittest.cc.
Added bunch of unit tests that tests all methods in the manager implementation.
chrome/browser/chromeos/login/login_utils_browsertest.cc:
chrome/browser/ui/webui/options2/language_options_handler2_unittest.cc:
Initialize() and Shutdown() the input method manager instance.
chrome/browser/chromeos/login/screen_locker.cc:
Move code for changing the set of enabled input methods on lock/unlock to input_method_manager_impl.cc.
chrome/browser/chromeos/preferences_unittest.cc:
Add a unit test which checks if the correct input method is set as a default one on signing in.
Note that the CL compiles and works correctly only with app-i18n/ibus >= 1.4.99.
BUG=chromium-os:19655
TEST=try
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=132751
Patch Set 1 : all-in-one-change only for git try #Patch Set 2 : rebase, add a test to util_unittest.cc #Patch Set 3 : review #Patch Set 4 : rebase #
Total comments: 43
Patch Set 5 : rebase, remove preferences.h and language_preference.* from this CL #
Total comments: 10
Patch Set 6 : review fix #Patch Set 7 : review fix #
Total comments: 5
Patch Set 8 : rebase, remove |should_hide_properties_| #
Total comments: 22
Patch Set 9 : review fix #
Total comments: 2
Patch Set 10 : review fix #
Total comments: 9
Patch Set 11 : review fix #Patch Set 12 : rebase #Messages
Total messages: 29 (0 generated)
|
