Fix UI-thread blocking issue in SetImeConfig

chrome/browser/chromeos/cros/input_method_library.cc

 // Copyright (c) 2010 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/chromeos/cros/input_method_library.h"
 
 #include <glib.h>
 #include <signal.h>
 
 #include "unicode/uloc.h"
@@ skipping 45 matching lines @@
       : input_method_status_connection_(NULL),
         previous_input_method_("", "", "", ""),
         current_input_method_("", "", "", ""),
         should_launch_ime_(false),
         ime_connected_(false),
         defer_ime_startup_(false),
         enable_auto_ime_shutdown_(true),
         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);
     if (CrosLibrary::Get()->EnsureLoaded()) {
       current_input_method_id_ = chromeos::GetHardwareKeyboardLayoutName();
     }
     // Observe APP_EXITING to stop input method processes gracefully.
     // Note that even if we fail to stop input method processes from
     // Chrome in case of a sudden crash, we have a way to do it from an
     // upstart script. See crosbug.com/6515 and crosbug.com/6995 for
@@ skipping 65 matching lines @@
     scoped_ptr<InputMethodDescriptors> active_input_method_descriptors(
         GetActiveInputMethods());
     for (size_t i = 0; i < active_input_method_descriptors->size(); ++i) {
       if (active_input_method_descriptors->at(i).id == input_method_id) {
         return true;
       }
     }
     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;
     if (ime_connected_) {
       pending_config_requests_[key] = value;
       FlushImeConfig();
     }
+
+    // Stop input method process if necessary.
+    MaybeStopInputMethodProcesses(section, config_name, value);
     return pending_config_requests_.empty();
   }
 
   virtual const InputMethodDescriptor& previous_input_method() const {
     return previous_input_method_;
   }
   virtual const InputMethodDescriptor& current_input_method() const {
     return current_input_method_;
   }
 
   virtual const ImePropertyList& current_ime_properties() const {
     return current_ime_properties_;
   }
 
  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.
-        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_) {
+        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] == 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);
+      }
     }
   }
 
   // Changes the current input method to |input_method_id|. If the id is not in
   // the preload_engine list, this function changes the current method to the
   // first preloaded engine. Returns true if the current engine is switched to
   // |input_method_id| or the first one.
   bool ChangeInputMethodInternal(const std::string& input_method_id) {
     DCHECK(EnsureLoadedAndStarted());
     std::string input_method_id_to_switch = input_method_id;
@@ skipping 80 matching lines @@
                                                   current_input_method_,
                                                   num_active_input_methods));
     }
   }
 
   static void InputMethodChangedHandler(
       void* object,
       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";
       return;
     }
 
     InputMethodLibraryImpl* input_method_library =
         static_cast<InputMethodLibraryImpl*>(object);
     input_method_library->ChangeCurrentInputMethod(current_input_method);
   }
@@ skipping 261 matching lines @@
   // This is used to register this object to APP_EXITING notification.
   NotificationRegistrar notification_registrar_;
 
   // True if we should launch the input method processes.
   bool should_launch_ime_;
   // True if the connection to the IBus daemon is alive.
   bool ime_connected_;
   // 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_;
   // True if we should change the input method once the queue of the
   // pending config requests becomes empty.
   bool should_change_input_method_;
 
   // The process id of the IBus daemon. 0 if it's not running. The process
   // ID 0 is not used in Linux, hence it's safe to use 0 for this purpose.
   int ibus_daemon_process_id_;
@@ skipping 31 matching lines @@
     return CreateRealisticInputMethodDescriptors();
   }
 
   void ChangeInputMethod(const std::string& input_method_id) {}
   void SetImePropertyActivated(const std::string& key, bool activated) {}
 
   bool InputMethodIsActivated(const std::string& input_method_id) {
     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;
   }
 
   virtual const InputMethodDescriptor& previous_input_method() const {
     return previous_input_method_;
   }
 
   virtual const InputMethodDescriptor& current_input_method() const {
     return current_input_method_;
@@ skipping 150 matching lines @@
     return new InputMethodLibraryStubImpl();
   else
     return new InputMethodLibraryImpl();
 }
 
 }  // namespace chromeos
 
 // Allows InvokeLater without adding refcounting. This class is a Singleton and
 // won't be deleted until it's last InvokeLater is run.
 DISABLE_RUNNABLE_METHOD_REFCOUNT(chromeos::InputMethodLibraryImpl);