Implement sync of keyboard layout between Ozone and Wayland clients

components/exo/wayland/server.cc

 // Copyright 2015 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 "components/exo/wayland/server.h"
 
 #include <alpha-compositing-unstable-v1-server-protocol.h>
 #include <gaming-input-unstable-v1-server-protocol.h>
 #include <gaming-input-unstable-v2-server-protocol.h>
 #include <grp.h>
@@ skipping 67 matching lines @@
 #include "ui/gfx/buffer_format_util.h"
 #include "ui/gfx/buffer_types.h"
 #include "ui/views/widget/widget.h"
 #include "ui/views/widget/widget_observer.h"
 #include "ui/wm/core/coordinate_conversion.h"
 
 #if defined(USE_OZONE)
 #include <drm_fourcc.h>
 #include <linux-dmabuf-unstable-v1-server-protocol.h>
 #include <wayland-drm-server-protocol.h>
+#if defined(OS_CHROMEOS)

[sadrul, 2017/06/14 03:16:53]

Do you need this? Are there other platforms where exo is built?

[jclinton, 2017/06/14 15:49:04]

This was already reviewed and it is necessary. See review thread.

[sadrul, 2017/06/15 06:45:21]

Link please?

+#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
 
 #if BUILDFLAG(USE_XKBCOMMON)
 #include <xkbcommon/xkbcommon.h>
 #include "ui/events/keycodes/scoped_xkb.h"  // nogncheck
 #endif
 
 DECLARE_UI_CLASS_PROPERTY_TYPE(wl_resource*);
 
 namespace exo {
@@ skipping 2632 matching lines @@
 const struct wl_pointer_interface pointer_implementation = {pointer_set_cursor,
                                                             pointer_release};
 
 #if BUILDFLAG(USE_XKBCOMMON)
 
 ////////////////////////////////////////////////////////////////////////////////
 // wl_keyboard_interface:
 
 // Keyboard delegate class that accepts events for surfaces owned by the same
 // client as a keyboard resource.
-class WaylandKeyboardDelegate : public KeyboardDelegate {
+class WaylandKeyboardDelegate
+    : public KeyboardDelegate
+#if defined(USE_OZONE) && defined(OS_CHROMEOS)

[sadrul, 2017/06/14 03:16:53]

Do you need either of the define checks?

[jclinton, 2017/06/14 15:49:04]

This was already reviewed and it is necessary. See review thread.

[sadrul, 2017/06/15 06:57:27]

I am requesting more information here to understand why we need these checks,
because if we do need these, then it would be better to organize this code
differently. For example, have a separate a separate independent
ImeKeyboard::Observer object, owned by WaylandKeyboardDelegate, instead of
having to litter the ifdefs everywhere in WaylandKeyboardDelegate, which makes
this code difficult to read. Or an ozone+cros specific subclass of
WaylandKeyboardDelegate.

Also, USE_OZONE is turned on by default for chromeos [1], so you shouldn't need
both checks. And exo/wayland has a dependency on ash [2, 3], and ash is
supported only on chromeos [4]. That's why I want to understand better when we
will be building exo on other platforms.

[1] https://cs.chromium.org/chromium/src/build/config/ui.gni?l=30
[2]
https://cs.chromium.org/chromium/src/components/exo/BUILD.gn?sq=package:chrom...
[3]
https://cs.chromium.org/chromium/src/components/exo/wayland/BUILD.gn?sq=packa...
[4]
https://cs.chromium.org/chromium/src/ash/BUILD.gn?sq=package:chromium&dr&l=12

[reveman, 2017/06/15 11:44:15]

We're not making the assumption that exo will only be built on chromeos and with
ozone today. When exo was created we decided to not limit it to chromeos builds
but also support ash on linux but now that this is no longer a supported build
config we might want to change that design. Ozone was not used by default for
workstation cros builds when we created exo so ifdefs for that was necessary.
Organizing the code differently is fine with me if it improves readability but
I'm also OK with current version (prefer it if we intend to remove some of these
ifdefs in the near future).
Are non-ozone chromeos builds not supported at all anymore?

I don't mind changing these build assumptions but if we do that then we should
do it throughout all of exo and not just for this code. The current patch is
consistent with rest of exo today.

[sadrul, 2017/06/15 13:18:11]

The linux_chromium_chromeos_rel_ng still runs non-ozone config. But the support
is in the process of being removed
(https://bugs.chromium.org/p/chromium/issues/detail?id=671355).
Sounds reasonable. Thanks.

+      , public chromeos::input_method::ImeKeyboard::Observer
+#endif
+    {
  public:
   explicit WaylandKeyboardDelegate(wl_resource* keyboard_resource)
       : keyboard_resource_(keyboard_resource),
-        xkb_context_(xkb_context_new(XKB_CONTEXT_NO_FLAGS)),
-        // TODO(reveman): Keep keymap synchronized with the keymap used by
-        // chromium and the host OS.
-        xkb_keymap_(xkb_keymap_new_from_names(xkb_context_.get(),
-                                              nullptr,
-                                              XKB_KEYMAP_COMPILE_NO_FLAGS)),
-        xkb_state_(xkb_state_new(xkb_keymap_.get())) {
-    std::unique_ptr<char, base::FreeDeleter> keymap_string(
-        xkb_keymap_get_as_string(xkb_keymap_.get(), XKB_KEYMAP_FORMAT_TEXT_V1));
-    DCHECK(keymap_string.get());
-    size_t keymap_size = strlen(keymap_string.get()) + 1;
-    base::SharedMemory shared_keymap;
-    bool rv = shared_keymap.CreateAndMapAnonymous(keymap_size);
-    DCHECK(rv);
-    memcpy(shared_keymap.memory(), keymap_string.get(), keymap_size);
-    wl_keyboard_send_keymap(keyboard_resource_,
-                            WL_KEYBOARD_KEYMAP_FORMAT_XKB_V1,
-                            shared_keymap.handle().GetHandle(), keymap_size);
+        xkb_context_(xkb_context_new(XKB_CONTEXT_NO_FLAGS)) {
+#if defined(USE_OZONE) && defined(OS_CHROMEOS)
+    chromeos::input_method::ImeKeyboard* keyboard =
+        chromeos::input_method::InputMethodManager::Get()->GetImeKeyboard();
+    if (keyboard)
+      keyboard->AddObserver(this);
+    SendNamedLayout(keyboard->GetCurrentKeyboardLayoutName());

[sadrul, 2017/06/14 03:16:53]

Looks like you don't actually need the null-check in line 2758?

[jclinton, 2017/06/14 15:49:04]

This was already reviewed and it is necessary; null is returned from the fake.
See review thread.

[sadrul, 2017/06/15 13:18:11]

If |keyboard| can be null, won't this line cause a crash?

[jclinton, 2017/06/15 15:46:46]

Done.

+#else
+    SendLayout(nullptr);
+#endif
   }
+#if defined(USE_OZONE) && defined(OS_CHROMEOS)
+  ~WaylandKeyboardDelegate() override {
+    chromeos::input_method::ImeKeyboard* keyboard =
+        chromeos::input_method::InputMethodManager::Get()->GetImeKeyboard();
+    if (keyboard)
+      keyboard->RemoveObserver(this);
+  }
+#endif
 
   // Overridden from KeyboardDelegate:
   void OnKeyboardDestroying(Keyboard* keyboard) override { delete this; }
   bool CanAcceptKeyboardEventsForSurface(Surface* surface) const override {
     wl_resource* surface_resource = GetSurfaceResource(surface);
     // We can accept events for this surface if the client is the same as the
     // keyboard.
     return surface_resource &&
            wl_resource_get_client(surface_resource) == client();
   }
@@ skipping 36 matching lines @@
     wl_keyboard_send_modifiers(
         keyboard_resource_, next_serial(),
         xkb_state_serialize_mods(xkb_state_.get(), XKB_STATE_MODS_DEPRESSED),
         xkb_state_serialize_mods(xkb_state_.get(), XKB_STATE_MODS_LOCKED),
         xkb_state_serialize_mods(xkb_state_.get(), XKB_STATE_MODS_LATCHED),
         xkb_state_serialize_layout(xkb_state_.get(),
                                    XKB_STATE_LAYOUT_EFFECTIVE));
     wl_client_flush(client());
   }
 
+#if defined(USE_OZONE) && defined(OS_CHROMEOS)
+  // Overridden from input_method::ImeKeyboard::Observer:
+  void OnCapsLockChanged(bool enabled) override {}
+  void OnLayoutChanging(const std::string& layout_name) override {
+    SendNamedLayout(layout_name);
+  }
+#endif
+
  private:
   // Returns the corresponding key given a dom code.
   uint32_t DomCodeToKey(ui::DomCode code) const {
     // This assumes KeycodeConverter has been built with evdev/xkb codes.
     xkb_keycode_t xkb_keycode = static_cast<xkb_keycode_t>(
         ui::KeycodeConverter::DomCodeToNativeKeycode(code));
 
     // Keycodes are offset by 8 in Xkb.
     DCHECK_GE(xkb_keycode, 8u);
     return xkb_keycode - 8;
@@ skipping 17 matching lines @@
     uint32_t xkb_modifiers = 0;
     for (auto modifier : modifiers) {
       if (modifier_flags & modifier.flag) {
         xkb_modifiers |=
             1 << xkb_keymap_mod_get_index(xkb_keymap_.get(), modifier.xkb_name);
       }
     }
     return xkb_modifiers;
   }
 
+
+#if defined(USE_OZONE) && defined(OS_CHROMEOS)
+  // Send the named keyboard layout to the client.
+  void SendNamedLayout(const std::string& layout_name) {
+    std::string layout_id, layout_variant;
+    ui::XkbKeyboardLayoutEngine::ParseLayoutName(layout_name, &layout_id,
+                                                 &layout_variant);
+    xkb_rule_names names = {.rules = nullptr,
+                            .model = "pc101",
+                            .layout = layout_id.c_str(),
+                            .variant = layout_variant.c_str(),
+                            .options = ""};
+    SendLayout(&names);
+  }
+#endif
+
+  // Send the keyboard layout named by XKB rules to the client.
+  void SendLayout(const xkb_rule_names* names) {
+    xkb_keymap_.reset(xkb_keymap_new_from_names(xkb_context_.get(), names,
+                                                XKB_KEYMAP_COMPILE_NO_FLAGS));
+    xkb_state_.reset(xkb_state_new(xkb_keymap_.get()));
+    std::unique_ptr<char, base::FreeDeleter> keymap_string(
+        xkb_keymap_get_as_string(xkb_keymap_.get(), XKB_KEYMAP_FORMAT_TEXT_V1));
+    DCHECK(keymap_string.get());
+    size_t keymap_size = strlen(keymap_string.get()) + 1;
+    base::SharedMemory shared_keymap;
+    bool rv = shared_keymap.CreateAndMapAnonymous(keymap_size);
+    DCHECK(rv);
+    memcpy(shared_keymap.memory(), keymap_string.get(), keymap_size);
+    wl_keyboard_send_keymap(keyboard_resource_,
+                            WL_KEYBOARD_KEYMAP_FORMAT_XKB_V1,
+                            shared_keymap.handle().GetHandle(), keymap_size);
+  }
+
   // The client who own this keyboard instance.
   wl_client* client() const {
     return wl_resource_get_client(keyboard_resource_);
   }
 
   // Returns the next serial to use for keyboard events.
   uint32_t next_serial() const {
     return wl_display_next_serial(wl_client_get_display(client()));
   }
 
@@ skipping 1182 matching lines @@
   DCHECK(event_loop);
   wl_event_loop_dispatch(event_loop, timeout.InMilliseconds());
 }
 
 void Server::Flush() {
   wl_display_flush_clients(wl_display_.get());
 }
 
 }  // namespace wayland
 }  // namespace exo