mash: Remove use of InputMethodManager in Wayland server.

We're currently using InputMethodManager in the Wayland server to
observe changes in the keyboard layout. However, under mash, exo will be
in the ash process and InputMethodManager will be in the chrome process,
so we cannot depend on InputMethodManager from the Wayland server.

We remove the use of InputMethodManager in the Wayland server for
observing keyboard changes by using ImeController instead, which uses
Mojo for communication.

We also add a way of retrieving the current keyboard layout from
ImeController. This is a similar design to how ImeController handles
caps lock, but it's simpler because we never need to set the layout
from ash side. Changes to layout on the chrome side just propagate
to the ash side from ImeControllerClient to ImeController.

Bug: 772382
Change-Id: I84df1ee1b7e0f54716125fec9d4cd49f3442ed79
Reviewed-on: https://chromium-review.googlesource.com/961505
Commit-Queue: Darren Shen <shend@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Oliver Chang <ochang@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#543958}

ash/ime/ime_controller.cc

@@ -136,6 +136,14 @@ void ImeController::UpdateCapsLockState(bool caps_enabled) {
     observer.OnCapsLockChanged(caps_enabled);
 }
 
+void ImeController::OnKeyboardLayoutNameChanged(
+    const std::string& layout_name) {
+  keyboard_layout_name_ = layout_name;
+
+  for (ImeController::Observer& observer : observers_)
+    observer.OnKeyboardLayoutNameChanged(layout_name);
+}
+
 void ImeController::SetExtraInputOptionsEnabledState(
     bool is_extra_input_options_enabled,
     bool is_emoji_enabled,

ash/ime/ime_controller.h

@@ -28,6 +28,10 @@ class ASH_EXPORT ImeController : public mojom::ImeController {
    public:
     // Called when the caps lock state has changed.
     virtual void OnCapsLockChanged(bool enabled) = 0;
+
+    // Called when the keyboard layout name has changed.
+    virtual void OnKeyboardLayoutNameChanged(
+        const std::string& layout_name) = 0;
   };
 
   ImeController();
@@ -86,6 +90,7 @@ class ASH_EXPORT ImeController : public mojom::ImeController {
   void SetImesManagedByPolicy(bool managed) override;
   void ShowImeMenuOnShelf(bool show) override;
   void UpdateCapsLockState(bool caps_enabled) override;
+  void OnKeyboardLayoutNameChanged(const std::string& layout_name) override;
 
   void SetExtraInputOptionsEnabledState(bool is_extra_input_options_enabled,
                                         bool is_emoji_enabled,
@@ -95,6 +100,11 @@ class ASH_EXPORT ImeController : public mojom::ImeController {
   // Synchronously returns the cached caps lock state.
   bool IsCapsLockEnabled() const;
 
+  // Synchronously returns the cached keyboard layout name
+  const std::string& keyboard_layout_name() const {
+    return keyboard_layout_name_;
+  }
+
   void FlushMojoForTesting();
 
  private:
@@ -128,6 +138,11 @@ class ASH_EXPORT ImeController : public mojom::ImeController {
   // another process. This is required for synchronous method calls in ash.
   bool is_caps_lock_enabled_ = false;
 
+  // A slightly delayed state value that is updated by asynchronously reported
+  // changes from the ImeControllerClient client (source of truth) which is in
+  // another process. This is required for synchronous method calls in ash.
+  std::string keyboard_layout_name_;
+
   // True if the extended inputs should be available in general (emoji,
   // handwriting, voice).
   bool is_extra_input_options_enabled_ = false;

ash/ime/ime_controller_unittest.cc

@@ -61,6 +61,27 @@ class TestImeObserver : public IMEObserver {
   bool ime_menu_active_ = false;
 };
 
+class TestImeControllerObserver : public ImeController::Observer {
+ public:
+  TestImeControllerObserver() = default;
+
+  // IMEController::Observer:
+  void OnCapsLockChanged(bool enabled) override { ++caps_lock_count_; }
+  void OnKeyboardLayoutNameChanged(const std::string& layout_name) override {
+    last_keyboard_layout_name_ = layout_name;
+  }
+
+  const std::string& last_keyboard_layout_name() const {
+    return last_keyboard_layout_name_;
+  }
+
+ private:
+  int caps_lock_count_ = 0;
+  std::string last_keyboard_layout_name_;
+
+  DISALLOW_COPY_AND_ASSIGN(TestImeControllerObserver);
+};
+
 using ImeControllerTest = AshTestBase;
 
 TEST_F(ImeControllerTest, RefreshIme) {
@@ -264,5 +285,16 @@ TEST_F(ImeControllerTest, SetCapsLock) {
   EXPECT_FALSE(controller->IsCapsLockEnabled());
 }
 
+TEST_F(ImeControllerTest, OnKeyboardLayoutNameChanged) {
+  ImeController* controller = Shell::Get()->ime_controller();
+  EXPECT_TRUE(controller->keyboard_layout_name().empty());
+
+  TestImeControllerObserver observer;
+  controller->AddObserver(&observer);
+  controller->OnKeyboardLayoutNameChanged("us(dvorak)");
+  EXPECT_EQ("us(dvorak)", controller->keyboard_layout_name());
+  EXPECT_EQ("us(dvorak)", observer.last_keyboard_layout_name());
+}
+
 }  // namespace
 }  // namespace ash

ash/ime/test_ime_controller.cc

@@ -44,6 +44,11 @@ void TestImeController::UpdateCapsLockState(bool enabled) {
   is_caps_lock_enabled_ = enabled;
 }
 
+void TestImeController::OnKeyboardLayoutNameChanged(
+    const std::string& layout_name) {
+  keyboard_layout_name_ = layout_name;
+}
+
 void TestImeController::SetExtraInputOptionsEnabledState(
     bool is_extra_input_options_enabled,
     bool is_emoji_enabled,

ash/ime/test_ime_controller.h

@@ -31,6 +31,7 @@ class ASH_EXPORT TestImeController : mojom::ImeController {
   void SetImesManagedByPolicy(bool managed) override;
   void ShowImeMenuOnShelf(bool show) override;
   void UpdateCapsLockState(bool enabled) override;
+  void OnKeyboardLayoutNameChanged(const std::string& layout_name) override;
   void SetExtraInputOptionsEnabledState(bool is_extra_input_options_enabled,
                                         bool is_emoji_enabled,
                                         bool is_handwriting_enabled,
@@ -43,6 +44,7 @@ class ASH_EXPORT TestImeController : mojom::ImeController {
   bool managed_by_policy_ = false;
   bool show_ime_menu_on_shelf_ = false;
   bool is_caps_lock_enabled_ = false;
+  std::string keyboard_layout_name_;
   bool is_extra_input_options_enabled_ = false;
   bool is_emoji_enabled_ = false;
   bool is_handwriting_enabled_ = false;

ash/login/ui/login_password_view.h

@@ -118,6 +118,7 @@ class ASH_EXPORT LoginPasswordView : public views::View,
 
   // ImeController::Observer:
   void OnCapsLockChanged(bool enabled) override;
+  void OnKeyboardLayoutNameChanged(const std::string&) override {}
 
  private:
   class EasyUnlockIcon;

ash/public/interfaces/ime_controller.mojom

@@ -31,6 +31,12 @@ interface ImeController {
   // to the tray.
   UpdateCapsLockState(bool enabled);
 
+  // Report keyboard layout changes from chrome (which is the source of truth)
+  // This is also called when a connection is first established between
+  // ImeController and ImeControllerClient.
+  // The layout name is a XKB keyboard layout name (e.g. "us").
+  OnKeyboardLayoutNameChanged(string layout_name);
+
   // Report the enabled state of the various extra input options (currently
   // emoji, handwriting, and voice input). |is_extra_input_options_enabled| set
   // to false will disable all extra input option UI regardless of the enabled

ash/system/tray_caps_lock.h

@@ -29,6 +29,7 @@ class TrayCapsLock : public TrayImageItem, public ImeController::Observer {
 
   // Overridden from ImeController::Observer:
   void OnCapsLockChanged(bool enabled) override;
+  void OnKeyboardLayoutNameChanged(const std::string&) override {}
 
   // Overridden from TrayImageItem.
   bool GetInitialVisibility() override;

chrome/browser/ui/ash/ime_controller_client.cc

@@ -166,7 +166,9 @@ void ImeControllerClient::OnCapsLockChanged(bool enabled) {
   ime_controller_ptr_->UpdateCapsLockState(enabled);
 }
 
-void ImeControllerClient::OnLayoutChanging(const std::string& layout_name) {}
+void ImeControllerClient::OnLayoutChanging(const std::string& layout_name) {
+  ime_controller_ptr_->OnKeyboardLayoutNameChanged(layout_name);
+}
 
 void ImeControllerClient::FlushMojoForTesting() {
   ime_controller_ptr_.FlushForTesting();
@@ -180,6 +182,11 @@ void ImeControllerClient::BindAndSetClient() {
   // Now that the bridge is established, flush state from observed objects to
   // the ImeController, now that it will hear it.
   input_method_manager_->NotifyObserversImeExtraInputStateChange();
+  if (const chromeos::input_method::ImeKeyboard* keyboard =
+          input_method_manager_->GetImeKeyboard()) {
+    ime_controller_ptr_->OnKeyboardLayoutNameChanged(
+        keyboard->GetCurrentKeyboardLayoutName());
+  }
 }
 
 ash::mojom::ImeInfoPtr ImeControllerClient::GetAshImeInfo(

chrome/browser/ui/ash/ime_controller_client_unittest.cc

@@ -197,6 +197,19 @@ TEST_F(ImeControllerClientTest, CapsLock) {
   EXPECT_FALSE(ime_controller_.is_caps_lock_enabled_);
 }
 
+TEST_F(ImeControllerClientTest, LayoutName) {
+  ImeControllerClient client(&input_method_manager_);
+  client.InitForTesting(ime_controller_.CreateInterfacePtr());
+
+  client.OnLayoutChanging("us(dvorak)");
+  client.FlushMojoForTesting();
+  EXPECT_EQ("us(dvorak)", ime_controller_.keyboard_layout_name_);
+
+  client.OnLayoutChanging("us");
+  client.FlushMojoForTesting();
+  EXPECT_EQ("us", ime_controller_.keyboard_layout_name_);
+}
+
 TEST_F(ImeControllerClientTest, ExtraInputEnabledStateChange) {
   ImeControllerClient client(&input_method_manager_);
   client.InitForTesting(ime_controller_.CreateInterfacePtr());

components/exo/wayland/server.cc

@@ -36,6 +36,7 @@
 #include <utility>
 #include <vector>
 
+#include "ash/ime/ime_controller.h"
 #include "ash/public/cpp/shell_window_ids.h"
 #include "ash/public/cpp/window_properties.h"
 #include "ash/public/interfaces/window_pin_type.mojom.h"
@@ -109,8 +110,6 @@
 #include <drm_fourcc.h>
 #include <linux-dmabuf-unstable-v1-server-protocol.h>
 #if defined(OS_CHROMEOS)
-#include "ui/base/ime/chromeos/ime_keyboard.h"
-#include "ui/base/ime/chromeos/input_method_manager.h"
 #include "ui/events/ozone/layout/xkb/xkb_keyboard_layout_engine.h"
 #endif
 #endif
@@ -3374,13 +3373,12 @@ const struct wl_pointer_interface pointer_implementation = {pointer_set_cursor,
 
 // Keyboard delegate class that accepts events for surfaces owned by the same
 // client as a keyboard resource.
-class WaylandKeyboardDelegate
-    : public WaylandInputDelegate,
-      public KeyboardDelegate,
-      public KeyboardObserver
+class WaylandKeyboardDelegate : public WaylandInputDelegate,
+                                public KeyboardDelegate,
+                                public KeyboardObserver
 #if defined(OS_CHROMEOS)
     ,
-      public chromeos::input_method::ImeKeyboard::Observer
+                                public ash::ImeController::Observer
 #endif
 {
 #if BUILDFLAG(USE_XKBCOMMON)
@@ -3389,22 +3387,16 @@ class WaylandKeyboardDelegate
       : keyboard_resource_(keyboard_resource),
         xkb_context_(xkb_context_new(XKB_CONTEXT_NO_FLAGS)) {
 #if defined(OS_CHROMEOS)
-    chromeos::input_method::ImeKeyboard* keyboard =
-        chromeos::input_method::InputMethodManager::Get()->GetImeKeyboard();
-    if (keyboard) {
-      keyboard->AddObserver(this);
-      SendNamedLayout(keyboard->GetCurrentKeyboardLayoutName());
-    }
+    ash::ImeController* ime_controller = ash::Shell::Get()->ime_controller();
+    ime_controller->AddObserver(this);
+    SendNamedLayout(ime_controller->keyboard_layout_name());
 #else
     SendLayout(nullptr);
 #endif
   }
 #if defined(OS_CHROMEOS)
   ~WaylandKeyboardDelegate() override {
-    chromeos::input_method::ImeKeyboard* keyboard =
-        chromeos::input_method::InputMethodManager::Get()->GetImeKeyboard();
-    if (keyboard)
-      keyboard->RemoveObserver(this);
+    ash::Shell::Get()->ime_controller()->RemoveObserver(this);
   }
 #endif
 
@@ -3468,9 +3460,9 @@ class WaylandKeyboardDelegate
   }
 
 #if defined(OS_CHROMEOS)
-  // Overridden from input_method::ImeKeyboard::Observer:
+  // Overridden from ImeController::Observer:
   void OnCapsLockChanged(bool enabled) override {}
-  void OnLayoutChanging(const std::string& layout_name) override {
+  void OnKeyboardLayoutNameChanged(const std::string& layout_name) override {
     SendNamedLayout(layout_name);
   }
 #endif