Refactor EnableInputMethods() and remove SortInputMethodIdsByNames().

We no longer need to sort input method IDs by names in EnableInputMethods()
as the input method IDs returned from GetInputMethodIdsFromLanguageCode()
are sorted per the whitelist.txt.

BUG=chromium-os:12252
TEST=keyboard switching on the login screen works as before

Review URL: http://codereview.chromium.org/6532016

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@75239 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 36 insertions, 116 deletions

diff --git a/chrome/browser/chromeos/input_method/input_method_util.cc b/chrome/browser/chromeos/input_method/input_method_util.cc
index e31f2da..ed74ca2 100644
--- a/chrome/browser/chromeos/input_method/input_method_util.cc
+++ b/chrome/browser/chromeos/input_method/input_method_util.cc
@@ -271,37 +271,6 @@ struct CompareLanguageCodesByLanguageName
   icu::Collator* collator_;
 };
 
-// The comparator is used for sorting input method ids by their
-// corresponding language names, using the ICU collator.
-struct CompareInputMethodIdsByLanguageName
-    : std::binary_function<const std::string&, const std::string&, bool> {
-  CompareInputMethodIdsByLanguageName(
-      icu::Collator* collator,
-      const std::map<std::string, std::string>& id_to_language_code_map)
-      : comparator_(collator),
-        id_to_language_code_map_(id_to_language_code_map) {
-  }
-
-  bool operator()(const std::string& s1, const std::string& s2) const {
-    std::string language_code_1;
-    std::map<std::string, std::string>::const_iterator iter =
-        id_to_language_code_map_.find(s1);
-    if (iter != id_to_language_code_map_.end()) {
-      language_code_1 = iter->second;
-    }
-    std::string language_code_2;
-    iter = id_to_language_code_map_.find(s2);
-    if (iter != id_to_language_code_map_.end()) {
-      language_code_2 = iter->second;
-    }
-    return comparator_(language_code_1, language_code_2);
-  }
-
- private:
-  const CompareLanguageCodesByLanguageName comparator_;
-  const std::map<std::string, std::string>& id_to_language_code_map_;
-};
-
 bool GetLocalizedString(
     const std::string& english_string, string16 *out_string) {
   DCHECK(out_string);
@@ -515,29 +484,6 @@ void SortLanguageCodesByNames(std::vector<std::string>* language_codes) {
             CompareLanguageCodesByLanguageName(collator.get()));
 }
 
-void SortInputMethodIdsByNames(std::vector<std::string>* input_method_ids) {
-  SortInputMethodIdsByNamesInternal(
-      *(IdMaps::GetInstance()->id_to_language_code), input_method_ids);
-}
-
-void SortInputMethodIdsByNamesInternal(
-    const std::map<std::string, std::string>& id_to_language_code_map,
-    std::vector<std::string>* input_method_ids) {
-  if (!g_browser_process) {
-    return;
-  }
-  UErrorCode error = U_ZERO_ERROR;
-  icu::Locale locale(g_browser_process->GetApplicationLocale().c_str());
-  scoped_ptr<icu::Collator> collator(
-      icu::Collator::createInstance(locale, error));
-  if (U_FAILURE(error)) {
-    collator.reset();
-  }
-  std::stable_sort(input_method_ids->begin(), input_method_ids->end(),
-                   CompareInputMethodIdsByLanguageName(
-                       collator.get(), id_to_language_code_map));
-}
-
 bool GetInputMethodIdsFromLanguageCode(
     const std::string& normalized_language_code,
     InputMethodType type,
@@ -575,26 +521,42 @@ bool GetInputMethodIdsFromLanguageCodeInternal(
 
 void EnableInputMethods(const std::string& language_code, InputMethodType type,
                         const std::string& initial_input_method_id) {
+  std::vector<std::string> candidates;
+  // Add input methods associated with the language.
+  GetInputMethodIdsFromLanguageCode(language_code, type, &candidates);
+  // Add the hardware keyboard as well. We should always add this so users
+  // can use the hardware keyboard on the login screen and the screen locker.
+  candidates.push_back(GetHardwareInputMethodId());
+
   std::vector<std::string> input_method_ids;
-  GetInputMethodIdsFromLanguageCode(language_code, type, &input_method_ids);
+  // First, add the initial input method ID, if it's requested, to
+  // input_method_ids, so it appears first on the list of active input
+  // methods at the input language status menu.
+  if (!initial_input_method_id.empty()) {
+    input_method_ids.push_back(initial_input_method_id);
+  }
 
-  // Add the hardware keyboard.
-  const std::string keyboard = GetHardwareInputMethodId();
-  if (std::count(input_method_ids.begin(), input_method_ids.end(),
-                 keyboard) == 0) {
-    input_method_ids.push_back(keyboard);
+  // Add candidates to input_method_ids, while skipping duplicates.
+  for (size_t i = 0; i < candidates.size(); ++i) {
+    const std::string& candidate = candidates[i];
+    // Not efficient, but should be fine, as the two vectors are very
+    // short (2-5 items).
+    if (std::count(input_method_ids.begin(), input_method_ids.end(),
+                   candidate) == 0) {
+      input_method_ids.push_back(candidate);
+    }
   }
-  // First, sort the vector by input method id, then by its display name.
-  std::sort(input_method_ids.begin(), input_method_ids.end());
-  SortInputMethodIdsByNames(&input_method_ids);
 
-  // Update ibus-daemon setting.
+  // Update ibus-daemon setting. Here, we don't save the input method list
+  // in the user's preferences.
   ImeConfigValue value;
   value.type = ImeConfigValue::kValueTypeStringList;
   value.string_list_value = input_method_ids;
   InputMethodLibrary* library = CrosLibrary::Get()->GetInputMethodLibrary();
   library->SetImeConfig(language_prefs::kGeneralSectionName,
                         language_prefs::kPreloadEnginesConfigName, value);
+
+  // Finaly, change to the initial input method, as needed.
   if (!initial_input_method_id.empty()) {
     library->ChangeInputMethod(initial_input_method_id);
   }
diff --git a/chrome/browser/chromeos/input_method/input_method_util.h b/chrome/browser/chromeos/input_method/input_method_util.h
index 184fa87..1e3f9cd 100644
--- a/chrome/browser/chromeos/input_method/input_method_util.h
+++ b/chrome/browser/chromeos/input_method/input_method_util.h
@@ -124,20 +124,19 @@ string16 GetLanguageNativeDisplayNameFromCode(const std::string& language_code);
 // using the unicode string comparator. Uses unstable sorting.
 void SortLanguageCodesByNames(std::vector<std::string>* language_codes);
 
-// Sorts the given input method ids by their corresponding language names,
-// using the unicode string comparator. Uses stable sorting.
-void SortInputMethodIdsByNames(std::vector<std::string>* input_method_ids);
-
+// Used for EnableInputMethods() etc.
 enum InputMethodType {
   kKeyboardLayoutsOnly,
   kAllInputMethods,
 };
 
-// Gets input method ids that belong to |language_code|.
+// Gets input method IDs that belong to |language_code|.
 // If |type| is |kKeyboardLayoutsOnly|, the function does not return input
 // methods that are not for keybord layout switching. Returns true on success.
-// Note that the function might return false if ibus-daemon is not running, or
-// |language_code| is unknown.
+// Note that the function might return false or |language_code| is unknown.
+//
+// The retured input method IDs are sorted per
+// chromeos/platform/assets/input_methods/whitelist.txt.
 bool GetInputMethodIdsFromLanguageCode(
     const std::string& language_code,
     InputMethodType type,
@@ -153,6 +152,10 @@ bool GetInputMethodIdsFromLanguageCode(
 // are enabled. If it's kKeyboardLayoutsOnly, only keyboard layouts are enabled.
 // For example, for Japanese, xkb:jp::jpn is enabled when kKeyboardLayoutsOnly,
 // and xkb:jp::jpn, mozc, mozc-jp, mozc-dv are enabled when kAllInputMethods.
+//
+// Note that this function does not save the input methods in the user's
+// preferences, as this function is designed for the login screen and the
+// screen locker, where we shouldn't change the user's preferences.
 void EnableInputMethods(const std::string& language_code, InputMethodType type,
                         const std::string& initial_input_method_id);
 
@@ -168,11 +171,6 @@ InputMethodDescriptor GetFallbackInputMethodDescriptor();
 // changed, so that the internal maps of this library is reloaded.
 void OnLocaleChanged();
 
-// DO NOT USE Functions below. These are only exported for unit tests.
-void SortInputMethodIdsByNamesInternal(
-    const std::map<std::string, std::string>& id_to_language_code_map,
-    std::vector<std::string>* input_method_ids);
-
 bool GetInputMethodIdsFromLanguageCodeInternal(
     const std::multimap<std::string, std::string>& language_code_to_ids,
     const std::string& normalized_language_code,
diff --git a/chrome/browser/chromeos/input_method/input_method_util_unittest.cc b/chrome/browser/chromeos/input_method/input_method_util_unittest.cc
index 3890438..329a7ec 100644
--- a/chrome/browser/chromeos/input_method/input_method_util_unittest.cc
+++ b/chrome/browser/chromeos/input_method/input_method_util_unittest.cc
@@ -165,46 +165,6 @@ TEST_F(InputMethodUtilTest, SortLanguageCodesByNames) {
   ASSERT_EQ("t",  language_codes[3]);  // Others
 }
 
-TEST_F(InputMethodUtilTest, SortInputMethodIdsByNamesInternal) {
-  std::map<std::string, std::string> id_to_language_code_map;
-  id_to_language_code_map.insert(std::make_pair("mozc", "ja"));
-  id_to_language_code_map.insert(std::make_pair("mozc-jp", "ja"));
-  id_to_language_code_map.insert(std::make_pair("xkb:jp::jpn", "ja"));
-  id_to_language_code_map.insert(std::make_pair("xkb:fr::fra", "fr"));
-
-  std::vector<std::string> input_method_ids;
-  // Check if this function can handle an empty list.
-  SortInputMethodIdsByNamesInternal(id_to_language_code_map,
-                                    &input_method_ids);
-
-  input_method_ids.push_back("mozc");           // Japanese
-  input_method_ids.push_back("xkb:fr::fra");    // French
-  SortInputMethodIdsByNamesInternal(id_to_language_code_map,
-                                    &input_method_ids);
-  ASSERT_EQ(2U, input_method_ids.size());
-  ASSERT_EQ("xkb:fr::fra", input_method_ids[0]);     // French
-  ASSERT_EQ("mozc", input_method_ids[1]);            // Japanese
-
-  // Add a duplicate entry and see if it works.
-  // Note that SortInputMethodIdsByNamesInternal uses std::stable_sort.
-  input_method_ids.push_back("xkb:jp::jpn");  // also Japanese
-  SortInputMethodIdsByNamesInternal(id_to_language_code_map,
-                                    &input_method_ids);
-  ASSERT_EQ(3U, input_method_ids.size());
-  ASSERT_EQ("xkb:fr::fra", input_method_ids[0]);     // French
-  ASSERT_EQ("mozc", input_method_ids[1]);            // Japanese
-  ASSERT_EQ("xkb:jp::jpn", input_method_ids[2]);     // Japanese
-
-  input_method_ids.push_back("mozc-jp");  // also Japanese
-  SortInputMethodIdsByNamesInternal(id_to_language_code_map,
-                                    &input_method_ids);
-  ASSERT_EQ(4U, input_method_ids.size());
-  ASSERT_EQ("xkb:fr::fra", input_method_ids[0]);     // French
-  ASSERT_EQ("mozc", input_method_ids[1]);            // Japanese
-  ASSERT_EQ("xkb:jp::jpn", input_method_ids[2]);     // Japanese
-  ASSERT_EQ("mozc-jp", input_method_ids[3]);         // Japanese
-}
-
 TEST_F(InputMethodUtilTest, GetInputMethodIdsForLanguageCode) {
   std::multimap<std::string, std::string> language_code_to_ids_map;
   language_code_to_ids_map.insert(std::make_pair("ja", "mozc"));