Implement ArcInputMethodManagerService::EnableIme().

This CL adds a new method, EnableIme(). ArcInputMethodManagerService
observes changes of the list of the active input methods and
enables/disables IMEs in container side if any ARC IME is
enabled/disabled.
This CL also introduces new category of input method id, ArcImeId.
It starts with '_arc_ime_' and used for distinguish Arc IMEs from other
extension-based IMEs.

Bug: 845079
Test: Unit tests are added.
Change-Id: I94c673c96e4157d8c9f18896606144f91438b7dc
Reviewed-on: https://chromium-review.googlesource.com/1098869
Commit-Queue: Yuichiro Hanada <yhanada@chromium.org>
Reviewed-by: Yusuke Sato <yusukes@chromium.org>
Reviewed-by: Shu Chen <shuchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#567563}

chrome/browser/chromeos/arc/input_method_manager/arc_input_method_manager_bridge_impl.cc

@@ -5,6 +5,7 @@
 #include "chrome/browser/chromeos/arc/input_method_manager/arc_input_method_manager_bridge_impl.h"
 
 #include "components/arc/arc_bridge_service.h"
+#include "components/arc/arc_features.h"
 
 namespace arc {
 
@@ -28,6 +29,9 @@ void ArcInputMethodManagerBridgeImpl::SendEnableIme(
   if (!imm_instance)
     return;
 
+  if (!base::FeatureList::IsEnabled(kEnableInputMethodFeature))
+    return;
+
   imm_instance->EnableIme(ime_id, enable, std::move(callback));
 }
 
@@ -39,6 +43,9 @@ void ArcInputMethodManagerBridgeImpl::SendSwitchImeTo(
   if (!imm_instance)
     return;
 
+  if (!base::FeatureList::IsEnabled(kEnableInputMethodFeature))
+    return;
+
   imm_instance->SwitchImeTo(ime_id, std::move(callback));
 }
 

chrome/browser/chromeos/arc/input_method_manager/arc_input_method_manager_service.cc

@@ -8,6 +8,7 @@
 #include "base/memory/singleton.h"
 #include "chrome/browser/chromeos/arc/input_method_manager/arc_input_method_manager_bridge_impl.h"
 #include "components/arc/arc_browser_context_keyed_service_factory_base.h"
+#include "ui/base/ime/chromeos/extension_ime_util.h"
 
 namespace arc {
 
@@ -75,4 +76,50 @@ void ArcInputMethodManagerService::OnImeInfoChanged(
   NOTIMPLEMENTED();
 }
 
+void ArcInputMethodManagerService::ImeMenuListChanged() {
+  auto* manager = chromeos::input_method::InputMethodManager::Get();
+  auto new_active_ime_ids =
+      manager->GetActiveIMEState()->GetActiveInputMethodIds();
+
+  // Filter out non ARC IME ids.
+  std::set<std::string> new_arc_active_ime_ids;
+  std::copy_if(
+      new_active_ime_ids.begin(), new_active_ime_ids.end(),
+      std::inserter(new_arc_active_ime_ids, new_arc_active_ime_ids.end()),
+      [](const auto& id) {
+        return chromeos::extension_ime_util::IsArcIME(id);
+      });
+
+  for (const auto& id : new_arc_active_ime_ids) {
+    // Enable the IME which is not currently enabled.
+    if (!active_arc_ime_ids_.count(id))
+      EnableIme(id, true /* enable */);
+  }
+
+  for (const auto& id : active_arc_ime_ids_) {
+    // Disable the IME which is currently enabled.
+    if (!new_arc_active_ime_ids.count(id))
+      EnableIme(id, false /* enable */);
+  }
+  active_arc_ime_ids_.swap(new_arc_active_ime_ids);
+}
+
+void ArcInputMethodManagerService::EnableIme(const std::string& ime_id,
+                                             bool enable) {
+  auto component_id =
+      chromeos::extension_ime_util::GetComponentIDByInputMethodID(ime_id);
+
+  // TODO(yhanada): Disable the IME in Chrome OS side if it fails.
+  imm_bridge_->SendEnableIme(
+      component_id, enable,
+      base::BindOnce(
+          [](const std::string& ime_id, bool enable, bool success) {
+            if (!success) {
+              LOG(ERROR) << (enable ? "Enabling" : "Disabling") << " \""
+                         << ime_id << "\" failed";
+            }
+          },
+          ime_id, enable));
+}
+
 }  // namespace arc

chrome/browser/chromeos/arc/input_method_manager/arc_input_method_manager_service.h

@@ -12,6 +12,7 @@
 #include "chrome/browser/chromeos/arc/input_method_manager/arc_input_method_manager_bridge.h"
 #include "components/arc/common/input_method_manager.mojom.h"
 #include "components/keyed_service/core/keyed_service.h"
+#include "ui/base/ime/chromeos/input_method_manager.h"
 
 namespace content {
 class BrowserContext;
@@ -23,7 +24,8 @@ class ArcBridgeService;
 
 class ArcInputMethodManagerService
     : public KeyedService,
-      public ArcInputMethodManagerBridge::Delegate {
+      public ArcInputMethodManagerBridge::Delegate,
+      public chromeos::input_method::InputMethodManager::ImeMenuObserver {
  public:
   // Returns the instance for the given BrowserContext, or nullptr if the
   // browser |context| is not allowed to use ARC.
@@ -45,8 +47,19 @@ class ArcInputMethodManagerService
   void OnActiveImeChanged(const std::string& ime_id) override;
   void OnImeInfoChanged(std::vector<mojom::ImeInfoPtr> ime_info_array) override;
 
+  // chromeos::input_method::InputMethodManager::ImeMenuObserver overrides:
+  void ImeMenuListChanged() override;
+  void ImeMenuActivationChanged(bool is_active) override {}
+  void ImeMenuItemsChanged(
+      const std::string& engine_id,
+      const std::vector<chromeos::input_method::InputMethodManager::MenuItem>&
+          items) override {}
+
  private:
+  void EnableIme(const std::string& ime_id, bool enable);
+
   std::unique_ptr<ArcInputMethodManagerBridge> imm_bridge_;
+  std::set<std::string> active_arc_ime_ids_;
 
   DISALLOW_COPY_AND_ASSIGN(ArcInputMethodManagerService);
 };

chrome/browser/chromeos/arc/input_method_manager/arc_input_method_manager_service_unittest.cc

@@ -5,16 +5,98 @@
 #include "chrome/browser/chromeos/arc/input_method_manager/arc_input_method_manager_service.h"
 
 #include <memory>
+#include <tuple>
+#include <vector>
 
 #include "base/macros.h"
+#include "base/memory/scoped_refptr.h"
+#include "base/test/scoped_feature_list.h"
+#include "components/arc/arc_features.h"
 #include "components/arc/arc_service_manager.h"
 #include "components/arc/test/test_browser_context.h"
+#include "components/crx_file/id_util.h"
 #include "content/public/test/test_browser_thread_bundle.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "ui/base/ime/chromeos/extension_ime_util.h"
+#include "ui/base/ime/chromeos/mock_input_method_manager.h"
 
 namespace arc {
 namespace {
 
+namespace im = chromeos::input_method;
+
+// The fake im::InputMethodManager for testing.
+class TestInputMethodManager : public im::MockInputMethodManager {
+ public:
+  // The fake im::InputMethodManager::State implementation for testing.
+  class TestState : public im::MockInputMethodManager::State {
+   public:
+    TestState() : active_input_method_ids_() {}
+
+    const std::vector<std::string>& GetActiveInputMethodIds() const override {
+      return active_input_method_ids_;
+    }
+
+    void AddActiveInputMethodId(const std::string& ime_id) {
+      if (!std::count(active_input_method_ids_.begin(),
+                      active_input_method_ids_.end(), ime_id)) {
+        active_input_method_ids_.push_back(ime_id);
+      }
+    }
+
+    void RemoveActiveInputMethodId(const std::string& ime_id) {
+      active_input_method_ids_.erase(std::remove_if(
+          active_input_method_ids_.begin(), active_input_method_ids_.end(),
+          [&ime_id](const std::string& id) { return id == ime_id; }));
+    }
+
+   protected:
+    friend base::RefCounted<InputMethodManager::State>;
+    ~TestState() override = default;
+
+   private:
+    std::vector<std::string> active_input_method_ids_;
+  };
+
+  TestInputMethodManager() {
+    state_ = scoped_refptr<TestState>(new TestState());
+  }
+  ~TestInputMethodManager() override = default;
+
+  scoped_refptr<InputMethodManager::State> GetActiveIMEState() override {
+    return state_;
+  }
+
+  TestState* state() { return state_.get(); }
+
+ private:
+  scoped_refptr<TestState> state_;
+
+  DISALLOW_COPY_AND_ASSIGN(TestInputMethodManager);
+};
+
+class TestInputMethodManagerBridge : public ArcInputMethodManagerBridge {
+ public:
+  TestInputMethodManagerBridge() = default;
+  ~TestInputMethodManagerBridge() override = default;
+
+  void SendEnableIme(const std::string& ime_id,
+                     bool enable,
+                     EnableImeCallback callback) override {
+    enable_ime_calls_.push_back(std::make_tuple(ime_id, enable));
+    std::move(callback).Run(true);
+  }
+  void SendSwitchImeTo(const std::string& ime_id,
+                       SwitchImeToCallback callback) override {
+    std::move(callback).Run(true);
+  }
+
+  std::vector<std::tuple<std::string, bool>> enable_ime_calls_;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(TestInputMethodManagerBridge);
+};
+
 class ArcInputMethodManagerServiceTest : public testing::Test {
  protected:
   ArcInputMethodManagerServiceTest()
@@ -26,12 +108,32 @@ class ArcInputMethodManagerServiceTest : public testing::Test {
 
   ArcInputMethodManagerService* service() { return service_; }
 
+  TestInputMethodManagerBridge* bridge() { return test_bridge_; }
+
+  TestInputMethodManager* imm() { return input_method_manager_; }
+
+  void SetUp() override {
+    input_method_manager_ = new TestInputMethodManager();
+    chromeos::input_method::InputMethodManager::Initialize(
+        input_method_manager_);
+    test_bridge_ = new TestInputMethodManagerBridge();
+    service_->SetInputMethodManagerBridgeForTesting(
+        base::WrapUnique(test_bridge_));
+  }
+
+  void TearDown() override {
+    test_bridge_ = nullptr;
+    chromeos::input_method::InputMethodManager::Shutdown();
+  }
+
  private:
   content::TestBrowserThreadBundle thread_bundle_;
   std::unique_ptr<ArcServiceManager> arc_service_manager_;
   std::unique_ptr<TestBrowserContext> context_;
+  TestInputMethodManager* input_method_manager_ = nullptr;
+  TestInputMethodManagerBridge* test_bridge_ = nullptr;  // Owned by |service_|
 
-  ArcInputMethodManagerService* const service_;
+  ArcInputMethodManagerService* const service_ = nullptr;
 
   DISALLOW_COPY_AND_ASSIGN(ArcInputMethodManagerServiceTest);
 };
@@ -46,4 +148,51 @@ TEST_F(ArcInputMethodManagerServiceTest, ConstructAndDestruct) {
   SUCCEED();
 }
 
+TEST_F(ArcInputMethodManagerServiceTest, EnableIme) {
+  using namespace chromeos::extension_ime_util;
+  using crx_file::id_util::GenerateId;
+
+  base::test::ScopedFeatureList feature;
+  feature.InitAndEnableFeature(kEnableInputMethodFeature);
+
+  ASSERT_EQ(0u, bridge()->enable_ime_calls_.size());
+
+  const std::string extension_ime_id =
+      GetInputMethodID(GenerateId("test.extension.ime"), "us");
+  const std::string component_extension_ime_id = GetComponentInputMethodID(
+      GenerateId("test.component.extension.ime"), "us");
+  const std::string arc_ime_id =
+      GetArcInputMethodID(GenerateId("test.arc.ime"), "us");
+
+  // EnableIme is called only when ARC IME is enable or disabled.
+  imm()->state()->AddActiveInputMethodId(extension_ime_id);
+  service()->ImeMenuListChanged();
+  EXPECT_EQ(0u, bridge()->enable_ime_calls_.size());
+
+  imm()->state()->AddActiveInputMethodId(component_extension_ime_id);
+  service()->ImeMenuListChanged();
+  EXPECT_EQ(0u, bridge()->enable_ime_calls_.size());
+
+  // Enable the ARC IME and verify that EnableIme is called.
+  imm()->state()->AddActiveInputMethodId(arc_ime_id);
+  service()->ImeMenuListChanged();
+  ASSERT_EQ(1u, bridge()->enable_ime_calls_.size());
+  EXPECT_EQ(GetComponentIDByInputMethodID(arc_ime_id),
+            std::get<std::string>(bridge()->enable_ime_calls_[0]));
+  EXPECT_TRUE(std::get<bool>(bridge()->enable_ime_calls_[0]));
+
+  // Disable the ARC IME and verify that EnableIme is called with false.
+  imm()->state()->RemoveActiveInputMethodId(arc_ime_id);
+  service()->ImeMenuListChanged();
+  ASSERT_EQ(2u, bridge()->enable_ime_calls_.size());
+  EXPECT_EQ(GetComponentIDByInputMethodID(arc_ime_id),
+            std::get<std::string>(bridge()->enable_ime_calls_[1]));
+  EXPECT_FALSE(std::get<bool>(bridge()->enable_ime_calls_[1]));
+
+  // EnableIme is not called when non ARC IME is disable.
+  imm()->state()->RemoveActiveInputMethodId(extension_ime_id);
+  service()->ImeMenuListChanged();
+  EXPECT_EQ(2u, bridge()->enable_ime_calls_.size());
+}
+
 }  // namespace arc

ui/base/ime/chromeos/extension_ime_util.cc

@@ -17,6 +17,9 @@ const char kComponentExtensionIMEPrefix[] = "_comp_ime_";
 const int kComponentExtensionIMEPrefixLength =
     sizeof(kComponentExtensionIMEPrefix) /
         sizeof(kComponentExtensionIMEPrefix[0]) - 1;
+const char kArcIMEPrefix[] = "_arc_ime_";
+const int kArcIMEPrefixLength =
+    sizeof(kArcIMEPrefix) / sizeof(kArcIMEPrefix[0]) - 1;
 const int kExtensionIdLength = 32;
 
 }  // namespace
@@ -42,6 +45,13 @@ std::string GetComponentInputMethodID(const std::string& extension_id,
   return kComponentExtensionIMEPrefix + extension_id + engine_id;
 }
 
+std::string GetArcInputMethodID(const std::string& extension_id,
+                                const std::string& engine_id) {
+  DCHECK(!extension_id.empty());
+  DCHECK(!engine_id.empty());
+  return kArcIMEPrefix + extension_id + engine_id;
+}
+
 std::string GetExtensionIDFromInputMethodID(
     const std::string& input_method_id) {
   if (IsExtensionIME(input_method_id)) {
@@ -52,6 +62,8 @@ std::string GetExtensionIDFromInputMethodID(
     return input_method_id.substr(kComponentExtensionIMEPrefixLength,
                                   kExtensionIdLength);
   }
+  if (IsArcIME(input_method_id))
+    return input_method_id.substr(kArcIMEPrefixLength, kExtensionIdLength);
   return "";
 }
 
@@ -62,6 +74,8 @@ std::string GetComponentIDByInputMethodID(const std::string& input_method_id) {
   if (IsExtensionIME(input_method_id))
     return input_method_id.substr(kExtensionIMEPrefixLength +
                                   kExtensionIdLength);
+  if (IsArcIME(input_method_id))
+    return input_method_id.substr(kArcIMEPrefixLength + kExtensionIdLength);
   return input_method_id;
 }
 
@@ -69,6 +83,8 @@ std::string GetInputMethodIDByEngineID(const std::string& engine_id) {
   if (base::StartsWith(engine_id, kComponentExtensionIMEPrefix,
                        base::CompareCase::SENSITIVE) ||
       base::StartsWith(engine_id, kExtensionIMEPrefix,
+                       base::CompareCase::SENSITIVE) ||
+      base::StartsWith(engine_id, kArcIMEPrefix,
                        base::CompareCase::SENSITIVE)) {
     return engine_id;
   }
@@ -114,6 +130,12 @@ bool IsComponentExtensionIME(const std::string& input_method_id) {
              kComponentExtensionIMEPrefixLength + kExtensionIdLength;
 }
 
+bool IsArcIME(const std::string& input_method_id) {
+  return base::StartsWith(input_method_id, kArcIMEPrefix,
+                          base::CompareCase::SENSITIVE) &&
+         input_method_id.size() > kArcIMEPrefixLength + kExtensionIdLength;
+}
+
 bool IsMemberOfExtension(const std::string& input_method_id,
                          const std::string& extension_id) {
   return base::StartsWith(input_method_id,

ui/base/ime/chromeos/extension_ime_util.h

@@ -54,6 +54,13 @@ std::string UI_BASE_IME_EXPORT
 GetComponentInputMethodID(const std::string& extension_id,
                           const std::string& engine_id);
 
+// Returns InputMethodID for |engine_id| in |extension_id| of ARC IME.
+// This function does not check |extension_id| is one for ARC IME nor
+// |engine_id| is really an installed ARC IME.
+std::string UI_BASE_IME_EXPORT
+GetArcInputMethodID(const std::string& extension_id,
+                    const std::string& engine_id);
+
 // Returns extension ID if |input_method_id| is extension IME ID or component
 // extension IME ID. Otherwise returns an empty string ("").
 std::string UI_BASE_IME_EXPORT
@@ -77,6 +84,10 @@ bool UI_BASE_IME_EXPORT IsExtensionIME(const std::string& input_method_id);
 bool UI_BASE_IME_EXPORT
 IsComponentExtensionIME(const std::string& input_method_id);
 
+// Returns true if |input_method_id| is a Arc IME ID. This function does not
+// check |input_method_id| is really a installed Arc IME.
+bool UI_BASE_IME_EXPORT IsArcIME(const std::string& input_method_id);
+
 // Returns true if the |input_method| is a member of |extension_id| of extension
 // IME, otherwise returns false.
 bool UI_BASE_IME_EXPORT IsMemberOfExtension(const std::string& input_method_id,

ui/base/ime/chromeos/extension_ime_util_unittest.cc

@@ -20,6 +20,11 @@ TEST(ExtensionIMEUtilTest, GetComponentInputMethodID) {
             extension_ime_util::GetComponentInputMethodID("ABCDE", "12345"));
 }
 
+TEST(ExtensionIMEUtilTest, GetArcInputMethodIDTest) {
+  EXPECT_EQ("_arc_ime_ABCDE12345",
+            extension_ime_util::GetArcInputMethodID("ABCDE", "12345"));
+}
+
 TEST(ExtensionIMEUtilTest, GetExtensionIDFromInputMethodIDTest) {
   EXPECT_EQ("",
             extension_ime_util::GetExtensionIDFromInputMethodID("mozc"));
@@ -42,6 +47,9 @@ TEST(ExtensionIMEUtilTest, IsExtensionIMETest) {
   EXPECT_FALSE(extension_ime_util::IsExtensionIME(
       extension_ime_util::GetComponentInputMethodID(
           "abcde_xxxxxxxxxxxxxxxxxxxxxxxxxx", "12345")));
+  EXPECT_FALSE(extension_ime_util::IsExtensionIME(
+      extension_ime_util::GetArcInputMethodID(
+          "abcde_xxxxxxxxxxxxxxxxxxxxxxxxxx", "12345")));
   EXPECT_FALSE(extension_ime_util::IsExtensionIME(""));
   EXPECT_FALSE(extension_ime_util::IsExtensionIME("mozc"));
 }
@@ -53,10 +61,27 @@ TEST(ExtensionIMEUtilTest, IsComponentExtensionIMETest) {
   EXPECT_FALSE(extension_ime_util::IsComponentExtensionIME(
       extension_ime_util::GetInputMethodID(
           "abcde_xxxxxxxxxxxxxxxxxxxxxxxxxx", "12345")));
+  EXPECT_FALSE(extension_ime_util::IsComponentExtensionIME(
+      extension_ime_util::GetArcInputMethodID(
+          "abcde_xxxxxxxxxxxxxxxxxxxxxxxxxx", "12345")));
   EXPECT_FALSE(extension_ime_util::IsComponentExtensionIME(""));
   EXPECT_FALSE(extension_ime_util::IsComponentExtensionIME("mozc"));
 }
 
+TEST(ExtensionIMEUtilTest, IsArcIMETest) {
+  EXPECT_TRUE(
+      extension_ime_util::IsArcIME(extension_ime_util::GetArcInputMethodID(
+          "abcde_xxxxxxxxxxxxxxxxxxxxxxxxxx", "12345")));
+  EXPECT_FALSE(
+      extension_ime_util::IsArcIME(extension_ime_util::GetInputMethodID(
+          "abcde_xxxxxxxxxxxxxxxxxxxxxxxxxx", "12345")));
+  EXPECT_FALSE(extension_ime_util::IsArcIME(
+      extension_ime_util::GetComponentInputMethodID(
+          "abcde_xxxxxxxxxxxxxxxxxxxxxxxxxx", "12345")));
+  EXPECT_FALSE(extension_ime_util::IsArcIME(""));
+  EXPECT_FALSE(extension_ime_util::IsArcIME("mozc"));
+}
+
 TEST(ExtensionIMEUtilTest, IsMemberOfExtension) {
   const char* extention1 = "abcdefg";
   const char* extention2 = "hijklmn";

ui/base/ime/chromeos/input_method_util.cc

@@ -455,7 +455,8 @@ bool InputMethodUtil::IsValidInputMethodId(
   // We can't check the component extension is whilelisted or not here because
   // it might not be initialized.
   return GetInputMethodDescriptorFromId(input_method_id) != NULL ||
-      extension_ime_util::IsComponentExtensionIME(input_method_id);
+         extension_ime_util::IsComponentExtensionIME(input_method_id) ||
+         extension_ime_util::IsArcIME(input_method_id);
 }
 
 // static