exo: Move XkbTracker from WaylandKeyboardDelegate to Seat.

This is preparation to share XkbTracker with wayland IME server.

BUG=1123705
TEST=Built locally. Run exo_unittests.

Change-Id: I7859c2193578cc5a18fa39a96c3fe86d2f39f4dc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2428505
Commit-Queue: Hidehiko Abe <hidehiko@chromium.org>
Reviewed-by: Jun Mukai <mukai@chromium.org>
Cr-Commit-Position: refs/heads/master@{#810863}

ash/public/cpp/external_arc/message_center/arc_notification_content_view_unittest.cc

@@ -29,6 +29,7 @@
 #include "components/exo/buffer.h"
 #include "components/exo/keyboard.h"
 #include "components/exo/keyboard_delegate.h"
+#include "components/exo/keyboard_modifiers.h"
 #include "components/exo/notification_surface.h"
 #include "components/exo/seat.h"
 #include "components/exo/surface.h"
@@ -75,15 +76,15 @@ class MockKeyboardDelegate : public exo::KeyboardDelegate {
               OnKeyboardKey,
               (base::TimeTicks, ui::DomCode, bool),
               (override));
-  MOCK_METHOD(void, OnKeyboardModifiers, (int), (override));
   MOCK_METHOD(void,
-              OnKeyRepeatSettingsChanged,
-              (bool, base::TimeDelta, base::TimeDelta),
+              OnKeyboardModifiers,
+              (const exo::KeyboardModifiers&),
               (override));
   MOCK_METHOD(void,
-              OnKeyboardLayoutUpdated,
-              (const std::string& layout_name),
+              OnKeyRepeatSettingsChanged,
+              (bool, base::TimeDelta, base::TimeDelta),
               (override));
+  MOCK_METHOD(void, OnKeyboardLayoutUpdated, (base::StringPiece), (override));
 };
 
 class FakeNotificationSurface : public exo::NotificationSurface {

components/exo/BUILD.gn

@@ -2,9 +2,17 @@
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
+import("//build/config/linux/pkg_config.gni")
 import("//build/config/ui.gni")
 import("//chrome/common/features.gni")
 import("//testing/test.gni")
+import("//ui/base/ui_features.gni")
+
+if (use_xkbcommon) {
+  pkg_config("xkbcommon") {
+    packages = [ "xkbcommon" ]
+  }
+}
 
 static_library("exo") {
   sources = [
@@ -156,6 +164,7 @@ static_library("exo") {
       "xkb_tracker.cc",
       "xkb_tracker.h",
     ]
+    configs += [ ":xkbcommon" ]
   }
 
   if (is_chromecast) {

components/exo/keyboard.cc

@@ -14,11 +14,13 @@
 #include "components/exo/input_trace.h"
 #include "components/exo/keyboard_delegate.h"
 #include "components/exo/keyboard_device_configuration_delegate.h"
+#include "components/exo/keyboard_modifiers.h"
 #include "components/exo/seat.h"
 #include "components/exo/shell_surface.h"
 #include "components/exo/shell_surface_util.h"
 #include "components/exo/surface.h"
 #include "components/exo/wm_helper.h"
+#include "components/exo/xkb_tracker.h"
 #include "ui/aura/client/aura_constants.h"
 #include "ui/aura/client/focus_client.h"
 #include "ui/aura/window.h"
@@ -169,7 +171,7 @@ Keyboard::Keyboard(std::unique_ptr<KeyboardDelegate> delegate, Seat* seat)
   ash::ImeControllerImpl* ime_controller = ash::Shell::Get()->ime_controller();
   ime_controller->AddObserver(this);
 
-  delegate_->OnKeyboardLayoutUpdated(ime_controller->keyboard_layout_name());
+  delegate_->OnKeyboardLayoutUpdated(seat_->xkb_tracker()->GetKeymap().get());
   OnSurfaceFocused(seat_->GetFocusedSurface());
   OnKeyRepeatSettingsChanged(
       ash::KeyboardController::Get()->GetKeyRepeatSettings());
@@ -278,7 +280,9 @@ void Keyboard::OnKeyEvent(ui::KeyEvent* event) {
       ConsumedByIme(focus_, event);
 
   // Always update modifiers.
-  delegate_->OnKeyboardModifiers(event->flags());
+  // XkbTracker must be updated in the Seat, before calling this method.
+  // Ensured by the observer registration order.
+  delegate_->OnKeyboardModifiers(seat_->xkb_tracker()->GetModifiers());
 
   // TODO(yhanada): This is a quick fix for https://crbug.com/859071. Remove
   // ARC-specific code path once we can find a way to manage press/release
@@ -401,7 +405,9 @@ void Keyboard::OnKeyRepeatSettingsChanged(
 void Keyboard::OnCapsLockChanged(bool enabled) {}
 
 void Keyboard::OnKeyboardLayoutNameChanged(const std::string& layout_name) {
-  delegate_->OnKeyboardLayoutUpdated(layout_name);
+  // XkbTracker must be updated in the Seat, before calling this method.
+  // Ensured by the observer registration order.
+  delegate_->OnKeyboardLayoutUpdated(seat_->xkb_tracker()->GetKeymap().get());
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -416,7 +422,7 @@ void Keyboard::SetFocus(Surface* surface) {
   }
   if (surface) {
     pressed_keys_ = seat_->pressed_keys();
-    delegate_->OnKeyboardModifiers(seat_->modifier_flags());
+    delegate_->OnKeyboardModifiers(seat_->xkb_tracker()->GetModifiers());
     delegate_->OnKeyboardEnter(surface, pressed_keys_);
     focus_ = surface;
     focus_->AddSurfaceObserver(this);

components/exo/keyboard_delegate.h

@@ -7,6 +7,7 @@
 
 #include "base/containers/flat_map.h"
 #include "base/containers/flat_set.h"
+#include "base/strings/string_piece.h"
 #include "base/time/time.h"
 
 namespace ui {
@@ -14,6 +15,7 @@ enum class DomCode;
 }
 
 namespace exo {
+struct KeyboardModifiers;
 class Surface;
 
 // Handles events on keyboards in context-specific ways.
@@ -42,7 +44,7 @@ class KeyboardDelegate {
                                  bool pressed) = 0;
 
   // Called when keyboard modifier state changed.
-  virtual void OnKeyboardModifiers(int modifier_flags) = 0;
+  virtual void OnKeyboardModifiers(const KeyboardModifiers& modifiers) = 0;
 
   // Called when key repeat settings are changed.
   virtual void OnKeyRepeatSettingsChanged(bool enabled,
@@ -50,9 +52,7 @@ class KeyboardDelegate {
                                           base::TimeDelta interval) = 0;
 
   // Called when keyboard layout is updated.
-  // TODO(hidehiko): Update the argument to pass the keymap
-  // when XkbTracker is moved out from WaylandKeyboardDelegate.
-  virtual void OnKeyboardLayoutUpdated(const std::string& layout_name) = 0;
+  virtual void OnKeyboardLayoutUpdated(base::StringPiece keymap) = 0;
 };
 
 }  // namespace exo

components/exo/seat.cc

@@ -25,6 +25,7 @@
 #include "components/exo/shell_surface_util.h"
 #include "components/exo/surface.h"
 #include "components/exo/wm_helper.h"
+#include "components/exo/xkb_tracker.h"
 #include "services/data_decoder/public/cpp/decode_image.h"
 #include "ui/aura/client/focus_client.h"
 #include "ui/base/clipboard/clipboard_data_endpoint.h"
@@ -62,6 +63,14 @@ Seat::Seat() : changing_clipboard_data_to_selection_source_(false) {
     ui::PlatformEventSource::GetInstance()->AddPlatformEventObserver(this);
 #if defined(OS_CHROMEOS)
   ui_lock_controller_ = std::make_unique<UILockController>(this);
+
+  // Seat needs to be registered as observers before any Keyboard,
+  // because Keyboard expects that the XkbTracker is up-to-date when its
+  // observer method is called.
+  xkb_tracker_ = std::make_unique<XkbTracker>();
+  ash::ImeControllerImpl* ime_controller = ash::Shell::Get()->ime_controller();
+  xkb_tracker_->UpdateKeyboardLayout(ime_controller->keyboard_layout_name());
+  ime_controller->AddObserver(this);
 #endif
 }
 
@@ -74,6 +83,9 @@ void Seat::Shutdown() {
     return;
   shutdown_ = true;
   DCHECK(!selection_source_) << "DataSource must be released before Seat";
+#if defined(OS_CHROMEOS)
+  ash::Shell::Get()->ime_controller()->RemoveObserver(this);
+#endif
   WMHelper::GetInstance()->RemoveFocusObserver(this);
   WMHelper::GetInstance()->RemovePreTargetHandler(this);
   ui::ClipboardMonitor::GetInstance()->RemoveObserver(this);
@@ -291,7 +303,9 @@ void Seat::OnKeyEvent(ui::KeyEvent* event) {
         break;
     }
   }
-  modifier_flags_ = event->flags();
+#if defined(OS_CHROMEOS)
+  xkb_tracker_->UpdateKeyboardModifiers(event->flags());
+#endif
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -304,6 +318,17 @@ void Seat::OnClipboardDataChanged() {
   selection_source_.reset();
 }
 
+#if defined(OS_CHROMEOS)
+////////////////////////////////////////////////////////////////////////////////
+// ash::ImeControllerImpl::Observer overrides:
+
+void Seat::OnCapsLockChanged(bool enabled) {}
+
+void Seat::OnKeyboardLayoutNameChanged(const std::string& layout_name) {
+  xkb_tracker_->UpdateKeyboardLayout(layout_name);
+}
+#endif
+
 ////////////////////////////////////////////////////////////////////////////////
 // DataSourceObserver overrides:
 

components/exo/seat.h

@@ -19,6 +19,7 @@
 #include "ui/events/platform/platform_event_observer.h"
 
 #if defined(OS_CHROMEOS)
+#include "ash/ime/ime_controller_impl.h"
 #include "components/exo/ui_lock_controller.h"
 #endif
 
@@ -32,6 +33,7 @@ class DragDropOperation;
 class ScopedDataSource;
 class SeatObserver;
 class Surface;
+class XkbTracker;
 
 // The maximum number of different data types that we will write to the
 // clipboard (plain text, RTF, HTML, image)
@@ -43,6 +45,9 @@ class Seat : public aura::client::FocusChangeObserver,
              public ui::PlatformEventObserver,
              public ui::EventHandler,
              public ui::ClipboardObserver,
+#if defined(OS_CHROMEOS)
+             public ash::ImeControllerImpl::Observer,
+#endif
              public DataSourceObserver {
  public:
   Seat();
@@ -62,8 +67,9 @@ class Seat : public aura::client::FocusChangeObserver,
     return pressed_keys_;
   }
 
-  // Returns current set of modifier flags.
-  int modifier_flags() const { return modifier_flags_; }
+#if defined(OS_CHROMEOS)
+  const XkbTracker* xkb_tracker() const { return xkb_tracker_.get(); }
+#endif
 
   // Returns physical code for the currently processing event.
   ui::DomCode physical_code_for_currently_processing_event() const {
@@ -102,6 +108,12 @@ class Seat : public aura::client::FocusChangeObserver,
   // Overridden from DataSourceObserver:
   void OnDataSourceDestroying(DataSource* source) override;
 
+#if defined(OS_CHROMEOS)
+  // Overridden from ash::ImeControllerImpl::Observer:
+  void OnCapsLockChanged(bool enabled) override;
+  void OnKeyboardLayoutNameChanged(const std::string& layout_name) override;
+#endif
+
   void set_physical_code_for_currently_processing_event_for_testing(
       ui::DomCode physical_code_for_currently_processing_event) {
     physical_code_for_currently_processing_event_ =
@@ -149,7 +161,6 @@ class Seat : public aura::client::FocusChangeObserver,
   // physical key press generated.
   base::flat_map<ui::DomCode, ui::DomCode> pressed_keys_;
   ui::DomCode physical_code_for_currently_processing_event_ = ui::DomCode::NONE;
-  int modifier_flags_ = 0;
 
   // Data source being used as a clipboard content.
   std::unique_ptr<ScopedDataSource> selection_source_;
@@ -165,6 +176,7 @@ class Seat : public aura::client::FocusChangeObserver,
 
 #if defined(OS_CHROMEOS)
   std::unique_ptr<UILockController> ui_lock_controller_;
+  std::unique_ptr<XkbTracker> xkb_tracker_;
 #endif  // defined(OS_CHROMEOS)
 
   base::WeakPtrFactory<Seat> weak_ptr_factory_{this};

components/exo/wayland/BUILD.gn

@@ -2,19 +2,12 @@
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
-import("//build/config/linux/pkg_config.gni")
 import("//build/config/ui.gni")
 import("//gpu/vulkan/features.gni")
 import("//testing/test.gni")
 import("//ui/base/ui_features.gni")
 import("//ui/ozone/ozone.gni")
 
-if (use_xkbcommon) {
-  pkg_config("xkbcommon") {
-    packages = [ "xkbcommon" ]
-  }
-}
-
 source_set("wayland") {
   sources = [
     "scoped_wl.cc",
@@ -133,7 +126,6 @@ source_set("wayland") {
   }
 
   if (use_xkbcommon) {
-    configs += [ ":xkbcommon" ]
     deps += [ "//ui/events/keycodes:xkb" ]
   }
 

components/exo/wayland/fuzzer/harness_unittest.cc

@@ -5,7 +5,6 @@
 #include "components/exo/wayland/fuzzer/harness.h"
 
 #include "base/files/scoped_temp_dir.h"
-#include "base/macros.h"
 #include "base/threading/thread.h"
 #include "base/time/time.h"
 #include "components/exo/display.h"
@@ -13,20 +12,36 @@
 #include "components/exo/wayland/fuzzer/actions.pb.h"
 #include "components/exo/wayland/server.h"
 
+#if defined(OS_CHROMEOS)
+#include "components/exo/test/exo_test_base.h"
+#endif
+
 namespace exo {
 namespace wayland_fuzzer {
 namespace {
 
-class WaylandFuzzerTest : public test::ExoTestBaseViews {
+// Use ExoTestBase on Chrome OS because Server starts to depends on ash::Shell,
+// which is unavailable on other platforms so then ExoTestBaseViews instead.
+using TestBase =
+#if defined(OS_CHROMEOS)
+    test::ExoTestBase
+#else
+    test::ExoTestBaseViews
+#endif
+    ;
+
+class WaylandFuzzerTest : public TestBase {
  protected:
   WaylandFuzzerTest() = default;
+  WaylandFuzzerTest(const WaylandFuzzerTest&) = delete;
+  WaylandFuzzerTest& operator=(const WaylandFuzzerTest&) = delete;
   ~WaylandFuzzerTest() override = default;
 
   void SetUp() override {
     ASSERT_TRUE(xdg_temp_dir_.CreateUniqueTempDir());
     setenv("XDG_RUNTIME_DIR", xdg_temp_dir_.GetPath().MaybeAsASCII().c_str(),
            1 /* overwrite */);
-    test::ExoTestBaseViews::SetUp();
+    TestBase::SetUp();
     display_ = std::make_unique<exo::Display>();
     server_ = wayland::Server::Create(display_.get());
   }
@@ -34,15 +49,12 @@ class WaylandFuzzerTest : public test::ExoTestBaseViews {
   void TearDown() override {
     server_.reset();
     display_.reset();
-    test::ExoTestBaseViews::TearDown();
+    TestBase::TearDown();
   }
 
   base::ScopedTempDir xdg_temp_dir_;
   std::unique_ptr<exo::Display> display_;
   std::unique_ptr<wayland::Server> server_;
-
- private:
-  DISALLOW_COPY_AND_ASSIGN(WaylandFuzzerTest);
 };
 
 void RunHarness(Harness* harness, base::WaitableEvent* event) {

components/exo/wayland/server_unittest.cc

@@ -20,6 +20,10 @@
 #include "components/exo/test/exo_test_base_views.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
+#if defined(OS_CHROMEOS)
+#include "components/exo/test/exo_test_base.h"
+#endif
+
 namespace exo {
 namespace wayland {
 namespace {
@@ -31,22 +35,32 @@ std::string GetUniqueSocketName() {
                             g_next_socket_id.GetNext());
 }
 
-class ServerTest : public test::ExoTestBaseViews {
+// Use ExoTestBase on Chrome OS because Server starts to depends on ash::Shell,
+// which is unavailable on other platforms so then ExoTestBaseViews instead.
+using TestBase =
+#if defined(OS_CHROMEOS)
+    test::ExoTestBase
+#else
+    test::ExoTestBaseViews
+#endif
+    ;
+
+class ServerTest : public TestBase {
  public:
-  ServerTest() {}
-  ~ServerTest() override {}
+  ServerTest() = default;
+  ServerTest(const ServerTest&) = delete;
+  ServerTest& operator=(const ServerTest&) = delete;
+  ~ServerTest() override = default;
 
   void SetUp() override {
     ASSERT_TRUE(xdg_temp_dir_.CreateUniqueTempDir());
     setenv("XDG_RUNTIME_DIR", xdg_temp_dir_.GetPath().MaybeAsASCII().c_str(),
            1 /* overwrite */);
-    test::ExoTestBaseViews::SetUp();
+    TestBase::SetUp();
   }
 
  private:
   base::ScopedTempDir xdg_temp_dir_;
-
-  DISALLOW_COPY_AND_ASSIGN(ServerTest);
 };
 
 TEST_F(ServerTest, AddSocket) {

components/exo/wayland/wayland_keyboard_delegate.cc

@@ -10,11 +10,13 @@
 #include <wayland-server-protocol-core.h>
 
 #include "base/containers/flat_map.h"
-#include "components/exo/keyboard_modifiers.h"
 #include "components/exo/wayland/serial_tracker.h"
-#include "components/exo/xkb_tracker.h"
 #include "ui/events/keycodes/dom/dom_code.h"
 
+#if BUILDFLAG(USE_XKBCOMMON)
+#include <xkbcommon/xkbcommon.h>
+#endif
+
 namespace exo {
 namespace wayland {
 
@@ -22,9 +24,7 @@ namespace wayland {
 
 WaylandKeyboardDelegate::WaylandKeyboardDelegate(wl_resource* keyboard_resource,
                                                  SerialTracker* serial_tracker)
-    : keyboard_resource_(keyboard_resource),
-      serial_tracker_(serial_tracker),
-      xkb_tracker_(std::make_unique<XkbTracker>()) {}
+    : keyboard_resource_(keyboard_resource), serial_tracker_(serial_tracker) {}
 
 WaylandKeyboardDelegate::~WaylandKeyboardDelegate() = default;
 
@@ -88,11 +88,9 @@ uint32_t WaylandKeyboardDelegate::OnKeyboardKey(base::TimeTicks time_stamp,
   return serial;
 }
 
-void WaylandKeyboardDelegate::OnKeyboardModifiers(int modifier_flags) {
-  xkb_tracker_->UpdateKeyboardModifiers(modifier_flags);
-
+void WaylandKeyboardDelegate::OnKeyboardModifiers(
+    const KeyboardModifiers& modifiers) {
   // Send the update only when they're different.
-  const KeyboardModifiers modifiers = xkb_tracker_->GetModifiers();
   if (current_modifiers_ == modifiers)
     return;
   current_modifiers_ = modifiers;
@@ -100,9 +98,23 @@ void WaylandKeyboardDelegate::OnKeyboardModifiers(int modifier_flags) {
 }
 
 void WaylandKeyboardDelegate::OnKeyboardLayoutUpdated(
-    const std::string& layout_name) {
-  xkb_tracker_->UpdateKeyboardLayout(layout_name);
-  SendLayout();
+    base::StringPiece keymap) {
+  // Sent the content of |keymap| with trailing '\0' termination via shared
+  // memory.
+  base::UnsafeSharedMemoryRegion shared_keymap_region =
+      base::UnsafeSharedMemoryRegion::Create(keymap.size() + 1);
+  base::WritableSharedMemoryMapping shared_keymap = shared_keymap_region.Map();
+  base::subtle::PlatformSharedMemoryRegion platform_shared_keymap =
+      base::UnsafeSharedMemoryRegion::TakeHandleForSerialization(
+          std::move(shared_keymap_region));
+  DCHECK(shared_keymap.IsValid());
+
+  std::memcpy(shared_keymap.memory(), keymap.data(), keymap.size());
+  static_cast<uint8_t*>(shared_keymap.memory())[keymap.size()] = '\0';
+  wl_keyboard_send_keymap(keyboard_resource_, WL_KEYBOARD_KEYMAP_FORMAT_XKB_V1,
+                          platform_shared_keymap.GetPlatformHandle().fd,
+                          keymap.size() + 1);
+  wl_client_flush(client());
 }
 
 uint32_t WaylandKeyboardDelegate::DomCodeToKey(ui::DomCode code) const {
@@ -124,25 +136,6 @@ void WaylandKeyboardDelegate::SendKeyboardModifiers() {
   wl_client_flush(client());
 }
 
-void WaylandKeyboardDelegate::SendLayout() {
-  auto keymap = xkb_tracker_->GetKeymap();
-  size_t keymap_size = strlen(keymap.get()) + 1;
-
-  base::UnsafeSharedMemoryRegion shared_keymap_region =
-      base::UnsafeSharedMemoryRegion::Create(keymap_size);
-  base::WritableSharedMemoryMapping shared_keymap = shared_keymap_region.Map();
-  base::subtle::PlatformSharedMemoryRegion platform_shared_keymap =
-      base::UnsafeSharedMemoryRegion::TakeHandleForSerialization(
-          std::move(shared_keymap_region));
-  DCHECK(shared_keymap.IsValid());
-
-  std::memcpy(shared_keymap.memory(), keymap.get(), keymap_size);
-  wl_keyboard_send_keymap(keyboard_resource_, WL_KEYBOARD_KEYMAP_FORMAT_XKB_V1,
-                          platform_shared_keymap.GetPlatformHandle().fd,
-                          keymap_size);
-  wl_client_flush(client());
-}
-
 // Convert from ChromeOS's key repeat interval to Wayland's key repeat rate.
 // For example, an interval of 500ms is a rate of 1000/500 = 2 Hz.
 //

components/exo/wayland/wayland_keyboard_delegate.h

@@ -16,18 +16,10 @@
 #include "ui/base/buildflags.h"
 #include "ui/events/keycodes/dom/keycode_converter.h"
 
-#if BUILDFLAG(USE_XKBCOMMON)
-#include <xkbcommon/xkbcommon.h>
-#include "ui/events/keycodes/scoped_xkb.h"  // nogncheck
-#endif
-
 struct wl_client;
 struct wl_resource;
 
 namespace exo {
-
-class XkbTracker;
-
 namespace wayland {
 class SerialTracker;
 
@@ -50,11 +42,11 @@ class WaylandKeyboardDelegate : public WaylandInputDelegate,
   uint32_t OnKeyboardKey(base::TimeTicks time_stamp,
                          ui::DomCode key,
                          bool pressed) override;
-  void OnKeyboardModifiers(int modifier_flags) override;
+  void OnKeyboardModifiers(const KeyboardModifiers& modifiers) override;
   void OnKeyRepeatSettingsChanged(bool enabled,
                                   base::TimeDelta delay,
                                   base::TimeDelta interval) override;
-  void OnKeyboardLayoutUpdated(const std::string& layout_name) override;
+  void OnKeyboardLayoutUpdated(base::StringPiece keymap) override;
 
  private:
   // Returns the corresponding key given a dom code.
@@ -63,9 +55,6 @@ class WaylandKeyboardDelegate : public WaylandInputDelegate,
   // Sends the current modifiers to the client.
   void SendKeyboardModifiers();
 
-  // Send the current keyboard layout to the client.
-  void SendLayout();
-
   // The client who own this keyboard instance.
   wl_client* client() const;
 
@@ -75,10 +64,6 @@ class WaylandKeyboardDelegate : public WaylandInputDelegate,
   // Owned by Server, which always outlives this delegate.
   SerialTracker* const serial_tracker_;
 
-  // TODO(hidehiko): Move this to the server in order to share it with
-  // zwp_text_input.
-  std::unique_ptr<XkbTracker> xkb_tracker_;
-
   // Tracks the latest modifiers.
   KeyboardModifiers current_modifiers_{};