Merge util GetInputMethodShortName() into OO GetIndicator().

Despite being called "util", InputMethodUtil is a stateful class (!)
Despite being member of stateful class InputMethodUtil, actually
GetInputMethodShortName() is stateless; it's a static util method (!)

Except for minor difference in the return type, it operates only on
a non-null InputMethodDescriptor instance and is functionally identical
to properly object-oriented InputMethodDescriptor::GetIndicator(). The
latter is never used anywhere else other than by the former (!)

In an attempt to reduce unnecessarily random bags of static utils
and promote object-oriented-ness where it deserves for readability,
let's fully merge InputMethodUtil::GetInputMethodShortName() into
InputMethodDescriptor::GetIndicator().

Bug: 1162216
Change-Id: I085d89ec97bdab03872df696efdb1bcee6865112
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2601013Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: My Nguyen <myy@chromium.org>
Reviewed-by: Darren Shen <shend@chromium.org>
Reviewed-by: Keith Lee <keithlee@chromium.org>
Commit-Queue: Bao-Duy Tran <tranbaoduy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#840486}

chrome/browser/chromeos/extensions/input_method_api.cc

@@ -184,7 +184,7 @@ InputMethodPrivateGetInputMethodsFunction::Run() {
     auto val = std::make_unique<base::DictionaryValue>();
     val->SetString("id", input_method.id());
     val->SetString("name", util->GetInputMethodLongName(input_method));
-    val->SetString("indicator", util->GetInputMethodShortName(input_method));
+    val->SetString("indicator", input_method.GetIndicator());
     output->Append(std::move(val));
   }
   return RespondNow(

chrome/browser/ui/ash/ime_controller_client.cc

@@ -129,9 +129,7 @@ void ImeControllerClient::ShowModeIndicator() {
   // Get the short name of the changed input method (e.g. US, JA, etc.)
   const InputMethodDescriptor descriptor =
       input_method_manager_->GetActiveIMEState()->GetCurrentInputMethod();
-  const base::string16 short_name =
-      input_method_manager_->GetInputMethodUtil()->GetInputMethodShortName(
-          descriptor);
+  const base::string16 short_name = descriptor.GetIndicator();
 
   chromeos::IMECandidateWindowHandlerInterface* cw_handler =
       ui::IMEBridge::Get()->GetCandidateWindowHandler();
@@ -209,7 +207,7 @@ ash::ImeInfo ImeControllerClient::GetAshImeInfo(
   ash::ImeInfo info;
   info.id = ime.id();
   info.name = util->GetInputMethodLongName(ime);
-  info.short_name = util->GetInputMethodShortName(ime);
+  info.short_name = ime.GetIndicator();
   info.third_party = chromeos::extension_ime_util::IsExtensionIME(ime.id());
   return info;
 }

ui/base/BUILD.gn

@@ -1033,6 +1033,7 @@ test("ui_base_unittests") {
         "ime/chromeos/extension_ime_util_unittest.cc",
         "ime/chromeos/ime_keyboard_unittest.cc",
         "ime/chromeos/input_method_chromeos_unittest.cc",
+        "ime/chromeos/input_method_descriptor_unittest.cc",
         "ime/chromeos/input_method_util_unittest.cc",
       ]
       deps += [ "//ui/base/ime/chromeos" ]

ui/base/ime/chromeos/input_method_descriptor.cc

@@ -9,6 +9,7 @@
 #include "base/check.h"
 #include "base/strings/string_split.h"
 #include "base/strings/string_util.h"
+#include "base/strings/utf_string_conversions.h"
 #include "ui/base/ime/chromeos/extension_ime_util.h"
 #include "url/gurl.h"
 
@@ -36,21 +37,23 @@ InputMethodDescriptor::InputMethodDescriptor(
 InputMethodDescriptor::InputMethodDescriptor(
     const InputMethodDescriptor& other) = default;
 
-std::string InputMethodDescriptor::GetIndicator() const {
+base::string16 InputMethodDescriptor::GetIndicator() const {
   // Return the empty string for ARC IMEs.
   if (extension_ime_util::IsArcIME(id_))
-    return std::string();
+    return base::string16();
 
   // If indicator is empty, use the first two character in its keyboard layout
   // or language code.
   if (indicator_.empty()) {
     if (extension_ime_util::IsKeyboardLayoutExtension(id_)) {
-      return base::ToUpperASCII(keyboard_layout_.substr(0, 2));
+      return base::UTF8ToUTF16(
+          base::ToUpperASCII(keyboard_layout_.substr(0, 2)));
     }
     DCHECK(language_codes_.size() > 0);
-    return base::ToUpperASCII(language_codes_[0].substr(0, 2));
+    return base::UTF8ToUTF16(
+        base::ToUpperASCII(language_codes_[0].substr(0, 2)));
   }
-  return indicator_;
+  return base::UTF8ToUTF16(indicator_);
 }
 
 InputMethodDescriptor::InputMethodDescriptor() = default;

ui/base/ime/chromeos/input_method_descriptor.h

@@ -9,6 +9,7 @@
 #include <vector>
 
 #include "base/component_export.h"
+#include "base/strings/string16.h"
 #include "url/gurl.h"
 
 namespace chromeos {
@@ -39,11 +40,9 @@ class COMPONENT_EXPORT(UI_BASE_IME_CHROMEOS) InputMethodDescriptor {
   const GURL& options_page_url() const { return options_page_url_; }
   const GURL& input_view_url() const { return input_view_url_; }
   const std::string& keyboard_layout() const { return keyboard_layout_; }
-
   bool is_login_keyboard() const { return is_login_keyboard_; }
 
-  // Returns the indicator text of this input method.
-  std::string GetIndicator() const;
+  base::string16 GetIndicator() const;
 
  private:
   // An ID that identifies an input method engine (e.g., "t:latn-post",

ui/base/ime/chromeos/input_method_descriptor_unittest.cc

+// Copyright (c) 2020 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 "ui/base/ime/chromeos/input_method_descriptor.h"
+
+#include <stddef.h>
+#include <string>
+
+#include "base/strings/utf_string_conversions.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "ui/base/ime/chromeos/extension_ime_util.h"
+
+using base::ASCIIToUTF16;
+
+namespace chromeos {
+namespace input_method {
+namespace {
+
+InputMethodDescriptor CreateDesc(const std::string& id,
+                                 const std::string& layout,
+                                 const std::vector<std::string>& language_codes,
+                                 const std::string& indicator,
+                                 bool is_login_keyboard) {
+  return InputMethodDescriptor(
+      extension_ime_util::GetInputMethodIDByEngineID(id), /* name= */ "",
+      indicator, layout, language_codes, /* is_login_keyboard= */ true,
+      /* options_page_url= */ GURL(), /* input_view_url= */ GURL());
+}
+
+TEST(InputMethodDescriptorTest, GetIndicatorTest) {
+  // Test invalid cases. Two-letter language code should be returned.
+  {
+    InputMethodDescriptor desc =
+        CreateDesc("invalid-id", "us", {"xx"}, "", true);
+    // Upper-case string of the unknown language code, "xx", should be returned.
+    EXPECT_EQ(ASCIIToUTF16("XX"), desc.GetIndicator());
+  }
+
+  // Test special cases.
+  {
+    InputMethodDescriptor desc =
+        CreateDesc("xkb:us:dvorak:eng", "us", {"en-US"}, "DV", true);
+    EXPECT_EQ(ASCIIToUTF16("DV"), desc.GetIndicator());
+  }
+  {
+    InputMethodDescriptor desc =
+        CreateDesc("xkb:us:colemak:eng", "us", {"en-US"}, "CO", true);
+    EXPECT_EQ(ASCIIToUTF16("CO"), desc.GetIndicator());
+  }
+  {
+    InputMethodDescriptor desc =
+        CreateDesc("xkb:us:altgr-intl:eng", "us", {"en-US"}, "EXTD", true);
+    EXPECT_EQ(ASCIIToUTF16("EXTD"), desc.GetIndicator());
+  }
+  {
+    InputMethodDescriptor desc =
+        CreateDesc("xkb:us:intl:eng", "us", {"en-US"}, "INTL", true);
+    EXPECT_EQ(ASCIIToUTF16("INTL"), desc.GetIndicator());
+  }
+  {
+    InputMethodDescriptor desc =
+        CreateDesc("xkb:de:neo:ger", "de(neo)", {"de"}, "NEO", true);
+    EXPECT_EQ(ASCIIToUTF16("NEO"), desc.GetIndicator());
+  }
+  {
+    InputMethodDescriptor desc =
+        CreateDesc("xkb:es:cat:cat", "es(cat)", {"ca"}, "CAT", true);
+    EXPECT_EQ(ASCIIToUTF16("CAT"), desc.GetIndicator());
+  }
+  {
+    InputMethodDescriptor desc =
+        CreateDesc("zh-t-i0-pinyin", "us", {"zh-CN"}, "拼", true);
+    EXPECT_EQ(base::UTF8ToUTF16("拼"), desc.GetIndicator());
+  }
+  {
+    InputMethodDescriptor desc =
+        CreateDesc("zh-hant-t-i0-und", "us", {"zh-TW"}, "注", true);
+    EXPECT_EQ(base::UTF8ToUTF16("注"), desc.GetIndicator());
+  }
+}
+
+}  // namespace
+}  // namespace input_method
+}  // namespace chromeos

ui/base/ime/chromeos/input_method_util.cc

@@ -482,13 +482,6 @@ bool InputMethodUtil::IsKeyboardLayout(const std::string& input_method_id) {
          extension_ime_util::IsKeyboardLayoutExtension(input_method_id);
 }
 
-base::string16 InputMethodUtil::GetInputMethodShortName(
-    const InputMethodDescriptor& input_method) const {
-  // TODO(shuchen): remove this method, as the client can directly use
-  // input_method.GetIndicator().
-  return base::UTF8ToUTF16(input_method.GetIndicator());
-}
-
 base::string16 InputMethodUtil::GetInputMethodMediumName(
     const InputMethodDescriptor& input_method) const {
   // For the "Your input method has changed to..." bubble. In most cases
@@ -500,7 +493,7 @@ base::string16 InputMethodUtil::GetInputMethodMediumName(
       return delegate_->GetLocalizedString(i.resource_id);
     }
   }
-  return GetInputMethodShortName(input_method);
+  return input_method.GetIndicator();
 }
 
 base::string16 InputMethodUtil::GetInputMethodLongNameInternal(

ui/base/ime/chromeos/input_method_util.h

@@ -40,8 +40,6 @@ class COMPONENT_EXPORT(UI_BASE_IME_CHROMEOS) InputMethodUtil {
   std::string GetInputMethodDisplayNameFromId(
       const std::string& input_method_id) const;
 
-  base::string16 GetInputMethodShortName(
-      const InputMethodDescriptor& input_method) const;
   base::string16 GetInputMethodMediumName(
       const InputMethodDescriptor& input_method) const;
   base::string16 GetInputMethodLongNameStripped(

ui/base/ime/chromeos/input_method_util_unittest.cc

@@ -98,60 +98,6 @@ class InputMethodUtilTest : public testing::Test {
   InputMethodDescriptors non_xkb_input_method_descriptors_;
 };
 
-TEST_F(InputMethodUtilTest, GetInputMethodShortNameTest) {
-  // Test invalid cases. Two-letter language code should be returned.
-  {
-    InputMethodDescriptor desc =
-        GetDesc("invalid-id", "", "us", {"xx"}, "", true);
-    // Upper-case string of the unknown language code, "xx", should be returned.
-    EXPECT_EQ(ASCIIToUTF16("XX"), util_.GetInputMethodShortName(desc));
-  }
-
-  // Test special cases.
-  {
-    InputMethodDescriptor desc =
-        GetDesc("xkb:us:dvorak:eng", "", "us", {"en-US"}, "DV", true);
-    EXPECT_EQ(ASCIIToUTF16("DV"), util_.GetInputMethodShortName(desc));
-  }
-  {
-    InputMethodDescriptor desc =
-        GetDesc("xkb:us:colemak:eng", "", "us", {"en-US"}, "CO", true);
-    EXPECT_EQ(ASCIIToUTF16("CO"), util_.GetInputMethodShortName(desc));
-  }
-  {
-    InputMethodDescriptor desc =
-        GetDesc("xkb:us:altgr-intl:eng", "", "us", {"en-US"}, "EXTD", true);
-    EXPECT_EQ(ASCIIToUTF16("EXTD"), util_.GetInputMethodShortName(desc));
-  }
-  {
-    InputMethodDescriptor desc =
-        GetDesc("xkb:us:intl:eng", "", "us", {"en-US"}, "INTL", true);
-    EXPECT_EQ(ASCIIToUTF16("INTL"), util_.GetInputMethodShortName(desc));
-  }
-  {
-    InputMethodDescriptor desc =
-        GetDesc("xkb:de:neo:ger", "", "de(neo)", {"de"}, "NEO", true);
-    EXPECT_EQ(ASCIIToUTF16("NEO"), util_.GetInputMethodShortName(desc));
-  }
-  {
-    InputMethodDescriptor desc =
-        GetDesc("xkb:es:cat:cat", "", "es(cat)", {"ca"}, "CAT", true);
-    EXPECT_EQ(ASCIIToUTF16("CAT"), util_.GetInputMethodShortName(desc));
-  }
-  {
-    InputMethodDescriptor desc =
-        GetDesc(pinyin_ime_id, "", "us", {"zh-CN"}, "\xe6\x8b\xbc", true);
-    EXPECT_EQ(base::UTF8ToUTF16("\xe6\x8b\xbc"),
-              util_.GetInputMethodShortName(desc));
-  }
-  {
-    InputMethodDescriptor desc =
-        GetDesc(zhuyin_ime_id, "", "us", {"zh-TW"}, "\xE6\xB3\xA8", true);
-    EXPECT_EQ(base::UTF8ToUTF16("\xE6\xB3\xA8"),
-              util_.GetInputMethodShortName(desc));
-  }
-}
-
 TEST_F(InputMethodUtilTest, GetInputMethodMediumNameTest) {
   {
     // input methods with medium name equal to short name
@@ -163,8 +109,7 @@ TEST_F(InputMethodUtilTest, GetInputMethodMediumNameTest) {
     for (const char* id : input_method_ids) {
       InputMethodDescriptor desc = GetDesc(id, "", "", {""}, "", true);
       base::string16 medium_name = util_.GetInputMethodMediumName(desc);
-      base::string16 short_name = util_.GetInputMethodShortName(desc);
-      EXPECT_EQ(medium_name, short_name);
+      EXPECT_EQ(medium_name, desc.GetIndicator());
     }
   }
   {
@@ -176,8 +121,7 @@ TEST_F(InputMethodUtilTest, GetInputMethodMediumNameTest) {
     for (const char* id : input_method_ids) {
       InputMethodDescriptor desc = GetDesc(id, "", "", {""}, "", true);
       base::string16 medium_name = util_.GetInputMethodMediumName(desc);
-      base::string16 short_name = util_.GetInputMethodShortName(desc);
-      EXPECT_NE(medium_name, short_name);
+      EXPECT_NE(medium_name, desc.GetIndicator());
     }
   }
 }