Fix UI-thread blocking issue in SetImeConfig.
Do not kill ibus-daemon before FlushImeConfig() since it can lead the ibus_config_set_value() call to block for 25 seconds (i.e. GDBus default timeout) when ibus-1.4 and GDBus libraries are in use.
BUG=chromium-os:9685
TEST=manually with ibus-1.3 and 1.4.
Review URL: http://codereview.chromium.org/6032005
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@70598 0039d316-1c4b-4281-b951-d872f2087c98
diff --git a/chrome/browser/chromeos/cros/input_method_library.cc b/chrome/browser/chromeos/cros/input_method_library.cc
index 9b5316f..369f797 100644
--- a/chrome/browser/chromeos/cros/input_method_library.cc
+++ b/chrome/browser/chromeos/cros/input_method_library.cc
@@ -63,6 +63,9 @@
should_change_input_method_(false),
ibus_daemon_process_id_(0),
candidate_window_process_id_(0) {
+ // TODO(yusukes): Using both CreateFallbackInputMethodDescriptors and
+ // chromeos::GetHardwareKeyboardLayoutName doesn't look clean. Probably
+ // we should unify these APIs.
scoped_ptr<InputMethodDescriptors> input_method_descriptors(
CreateFallbackInputMethodDescriptors());
current_input_method_ = input_method_descriptors->at(0);
@@ -148,19 +151,22 @@
return false;
}
- bool GetImeConfig(const char* section, const char* config_name,
+ bool GetImeConfig(const std::string& section, const std::string& config_name,
ImeConfigValue* out_value) {
bool success = false;
if (EnsureLoadedAndStarted()) {
success = chromeos::GetImeConfig(input_method_status_connection_,
- section, config_name, out_value);
+ section.c_str(),
+ config_name.c_str(),
+ out_value);
}
return success;
}
- bool SetImeConfig(const char* section, const char* config_name,
+ bool SetImeConfig(const std::string& section, const std::string& config_name,
const ImeConfigValue& value) {
- MaybeStartOrStopInputMethodProcesses(section, config_name, value);
+ // Before calling FlushImeConfig(), start input method process if necessary.
+ MaybeStartInputMethodProcesses(section, config_name, value);
const ConfigKeyType key = std::make_pair(section, config_name);
current_config_values_[key] = value;
@@ -168,6 +174,9 @@
pending_config_requests_[key] = value;
FlushImeConfig();
}
+
+ // Stop input method process if necessary.
+ MaybeStopInputMethodProcesses(section, config_name, value);
return pending_config_requests_.empty();
}
@@ -183,24 +192,52 @@
}
private:
- // Starts or stops the input method processes based on the current state.
- void MaybeStartOrStopInputMethodProcesses(
- const char* section,
- const char* config_name,
- const ImeConfigValue& value) {
- if (!strcmp(language_prefs::kGeneralSectionName, section) &&
- !strcmp(language_prefs::kPreloadEnginesConfigName, config_name)) {
+ // Starts input method processes based on the |defer_ime_startup_| flag and
+ // input method configuration being updated. |section| is a section name of
+ // the input method configuration (e.g. "general", "general/hotkey").
+ // |config_name| is a name of the configuration (e.g. "preload_engines",
+ // "previous_engine"). |value| is the configuration value to be set.
+ void MaybeStartInputMethodProcesses(const std::string& section,
+ const std::string& config_name,
+ const ImeConfigValue& value) {
+ if (section == language_prefs::kGeneralSectionName &&
+ config_name == language_prefs::kPreloadEnginesConfigName) {
if (EnsureLoadedAndStarted()) {
- // If there are no input methods other than one for the hardware
- // keyboard, we'll stop the input method processes.
+ const std::string hardware_layout_name =
+ chromeos::GetHardwareKeyboardLayoutName(); // e.g. "xkb:us::eng"
+ if (!(value.type == ImeConfigValue::kValueTypeStringList &&
+ value.string_list_value.size() == 1 &&
+ value.string_list_value[0] == hardware_layout_name) &&
+ !defer_ime_startup_) {
+ // If there are no input methods other than one for the hardware
+ // keyboard, we don't start the input method processes.
+ // When |defer_ime_startup_| is true, we don't start it either.
+ StartInputMethodProcesses();
+ }
+ chromeos::SetActiveInputMethods(input_method_status_connection_, value);
+ }
+ }
+ }
+
+ // Stops input method processes based on the |enable_auto_ime_shutdown_| flag
+ // and input method configuration being updated.
+ // See also: MaybeStartInputMethodProcesses().
+ void MaybeStopInputMethodProcesses(const std::string& section,
+ const std::string& config_name,
+ const ImeConfigValue& value) {
+ if (section == language_prefs::kGeneralSectionName &&
+ config_name == language_prefs::kPreloadEnginesConfigName) {
+ if (EnsureLoadedAndStarted()) {
+ const std::string hardware_layout_name =
+ chromeos::GetHardwareKeyboardLayoutName(); // e.g. "xkb:us::eng"
if (value.type == ImeConfigValue::kValueTypeStringList &&
value.string_list_value.size() == 1 &&
- value.string_list_value[0] ==
- chromeos::GetHardwareKeyboardLayoutName()) {
- if (enable_auto_ime_shutdown_)
- StopInputMethodProcesses();
- } else if (!defer_ime_startup_) {
- StartInputMethodProcesses();
+ value.string_list_value[0] == hardware_layout_name &&
+ enable_auto_ime_shutdown_) {
+ // If there are no input methods other than one for the hardware
+ // keyboard, and |enable_auto_ime_shutdown_| is true, we'll stop the
+ // input method processes.
+ StopInputMethodProcesses();
}
chromeos::SetActiveInputMethods(input_method_status_connection_, value);
}
@@ -304,7 +341,7 @@
const chromeos::InputMethodDescriptor& current_input_method) {
// The handler is called when the input method method change is
// notified via a DBus connection. Since the DBus notificatiosn are
- // handled in the UI thread, we can assume that this functionalways
+ // handled in the UI thread, we can assume that this function always
// runs on the UI thread, but just in case.
if (!BrowserThread::CurrentlyOn(BrowserThread::UI)) {
LOG(ERROR) << "Not on UI thread";
@@ -586,6 +623,8 @@
// If true, we'll defer the startup until a non-default method is
// activated.
bool defer_ime_startup_;
+ // True if we should stop input method processes when there are no input
+ // methods other than one for the hardware keyboard.
bool enable_auto_ime_shutdown_;
// The ID of the current input method (ex. "mozc").
std::string current_input_method_id_;
@@ -637,14 +676,14 @@
return true;
}
- bool GetImeConfig(const char* section,
- const char* config_name,
+ bool GetImeConfig(const std::string& section,
+ const std::string& config_name,
ImeConfigValue* out_value) {
return false;
}
- bool SetImeConfig(const char* section,
- const char* config_name,
+ bool SetImeConfig(const std::string& section,
+ const std::string& config_name,
const ImeConfigValue& value) {
return false;
}
diff --git a/chrome/browser/chromeos/cros/input_method_library.h b/chrome/browser/chromeos/cros/input_method_library.h
index c30db2fb..0c62ead74 100644
--- a/chrome/browser/chromeos/cros/input_method_library.h
+++ b/chrome/browser/chromeos/cros/input_method_library.h
@@ -84,9 +84,9 @@
// |out_value|. Returns true if |out_value| is successfully updated.
// When you would like to retrieve 'panel/custom_font', |section| should
// be "panel", and |config_name| should be "custom_font".
- virtual bool GetImeConfig(
- const char* section, const char* config_name,
- ImeConfigValue* out_value) = 0;
+ virtual bool GetImeConfig(const std::string& section,
+ const std::string& config_name,
+ ImeConfigValue* out_value) = 0;
// Updates a configuration of ibus-daemon or IBus engines with |value|.
// Returns true if the configuration (and all pending configurations, if any)
@@ -97,8 +97,8 @@
// Notice: This function might call the Observer::ActiveInputMethodsChanged()
// callback function immediately, before returning from the SetImeConfig
// function. See also http://crosbug.com/5217.
- virtual bool SetImeConfig(const char* section,
- const char* config_name,
+ virtual bool SetImeConfig(const std::string& section,
+ const std::string& config_name,
const ImeConfigValue& value) = 0;
// Sets the IME state to enabled, and launches its processes if needed.
diff --git a/chrome/browser/chromeos/cros/mock_input_method_library.h b/chrome/browser/chromeos/cros/mock_input_method_library.h
index c4284e8..dc4f3ac 100644
--- a/chrome/browser/chromeos/cros/mock_input_method_library.h
+++ b/chrome/browser/chromeos/cros/mock_input_method_library.h
@@ -27,8 +27,9 @@
MOCK_METHOD1(ChangeInputMethod, void(const std::string&));
MOCK_METHOD2(SetImePropertyActivated, void(const std::string&, bool));
MOCK_METHOD1(InputMethodIsActivated, bool(const std::string&));
- MOCK_METHOD3(GetImeConfig, bool(const char*, const char*, ImeConfigValue*));
- MOCK_METHOD3(SetImeConfig, bool(const char*, const char*,
+ MOCK_METHOD3(GetImeConfig, bool(const std::string&, const std::string&,
+ ImeConfigValue*));
+ MOCK_METHOD3(SetImeConfig, bool(const std::string&, const std::string&,
const ImeConfigValue&));
MOCK_CONST_METHOD0(previous_input_method, const InputMethodDescriptor&(void));
MOCK_CONST_METHOD0(current_input_method, const InputMethodDescriptor&(void));
