vm_tools: Rename zcr_text_input_x11 to zcr_text_input_crostini & tidy

We use the zcr_text_input_x11 protocol between sommelier and cros_im to support IMEs on X11 applications. This CL renames the protocol to zcr_text_input_crostini, as we want to add additional Crostini-only communications between sommelier and cros_im for IME support that are not specific to X11. In addition to renaming, we also make the protocol a bit more standard: - Instead of having functions on a singleton which take a text_input object, use a factory zcr_text_input_crostini_manager to create zcr_text_input_crostini objects which are already bound to a text_input object. This will also make it simpler to add events (calls from sommelier to cros_im) later on. - Add a destructor to zcr_text_input_crostini, we may end up creating and destroying multiple of these and this prevents leaks in the compositor while an app is running. As users may end up with outdated cros_im versions, sommelier will need to continue supporting zcr_text_input_x11 for some time (at least a few milestones). cros_im still falls back to zcr_text_input_x11, but we can remove that as soon as M118 branches off. BUG=b:264949241 TEST=Manually test VS Code and gedit with new cros_im only/new sommelier only/new cros_im & sommelier. Change-Id: Iaa1e8fa8881f2d2af7ef1e7a19f403526e34ccba Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform2/+/4764984 Reviewed-by: Sophia Lin <[email protected]> Commit-Queue: Timothy Loh <[email protected]> Tested-by: kokoro <[email protected]> Reviewed-by: Nic Hollingum <[email protected]>
WhatAmISupposedToPutHere · Aug 14, 2023 · 7fd5a5e · 7fd5a5e
1 parent 965c914
commit 7fd5a5e
Show file tree

Hide file tree

Showing 7 changed files with 177 additions and 7 deletions.
diff --git a/BUILD.gn b/BUILD.gn
@@ -49,6 +49,7 @@ wayland_protocols = [
   "protocol/relative-pointer-unstable-v1.xml",
   "protocol/stylus-unstable-v2.xml",
   "protocol/tablet-unstable-v2.xml",
+  "protocol/text-input-crostini-unstable-v1.xml",
   "protocol/text-input-extension-unstable-v1.xml",
   "protocol/text-input-unstable-v1.xml",
   "protocol/text-input-x11-unstable-v1.xml",

diff --git a/meson.build b/meson.build
@@ -48,6 +48,7 @@ wl_protocols = [
     'protocol/relative-pointer-unstable-v1.xml',
     'protocol/stylus-unstable-v2.xml',
     'protocol/tablet-unstable-v2.xml',
+    'protocol/text-input-crostini-unstable-v1.xml',
     'protocol/text-input-unstable-v1.xml',
     'protocol/text-input-extension-unstable-v1.xml',
     'protocol/text-input-x11-unstable-v1.xml',

diff --git a/protocol/text-input-crostini-unstable-v1.xml b/protocol/text-input-crostini-unstable-v1.xml
@@ -0,0 +1,50 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<protocol name="text_input_crostini_unstable_v1">
+
+  <copyright>
+      Copyright 2023 The ChromiumOS Authors
+      Use of this source code is governed by a BSD-style license that can be
+      found in the LICENSE file.
+  </copyright>
+
+  <interface name="zcr_text_input_crostini_manager_v1" version="1">
+    <description summary="factory for zcr_text_input_crostini_v1">
+    </description>
+
+    <enum name="error">
+      <entry name="text_input_crostini_exists" value="0"
+             summary="the text_input already has an text_input_crostini object associated"/>
+    </enum>
+
+    <request name="get_text_input_crostini">
+      <description summary="get zcr_text_input_crostini_v1 object">
+        Create a zcr_text_input_crostini_v1 object for a text_input.
+        If an object has already been created, the text_input_crostini_exists
+        protocol error is raised.
+      </description>
+      <arg name="id" type="new_id" interface="zcr_text_input_crostini_v1"/>
+      <arg name="text_input" type="object" interface="zwp_text_input_v1"/>
+    </request>
+
+  </interface>
+
+  <interface name="zcr_text_input_crostini_v1" version="1">
+    <description summary="Crostini extension support for text_input">
+      Adds extra text_input functionality specific to Crostini. This is only
+      used between sommelier and cros_im.
+    </description>
+
+    <request name="destroy" type="destructor">
+      <description summary="destroy zcr_text_input_crostini_v1 object"/>
+    </request>
+
+    <request name="activate_x11">
+      <description summary="request activation">
+        Calls text_input::activate with the X11 window id converted to the
+        matching wl_surface.
+      </description>
+      <arg name="seat" type="object" interface="wl_seat"/>
+      <arg name="x11_window_id" type="uint"/>
+    </request>
+  </interface>
+</protocol>
diff --git a/protocol/text-input-x11-unstable-v1.xml b/protocol/text-input-x11-unstable-v1.xml
@@ -11,6 +11,10 @@
     <description summary="Extension support for using text_input with X11 apps">
       Adds functionality for X11 apps running on Xwayland to connect to the
       Wayland compositor and receive text_input support.
+
+      TODO(b/295265862): Remove this protocol and its implementation some time
+      after M122. It has been replaced by zcr_text_input_crostini_v1 since M118,
+      but users can sometimes end up on older version of cros_im.
     </description>
 
     <request name="activate">

diff --git a/sommelier-text-input.cc b/sommelier-text-input.cc
@@ -11,6 +11,7 @@
 #include <stdlib.h>
 #include <string.h>
 
+#include "text-input-crostini-unstable-v1-server-protocol.h"  // NOLINT(build/include_directory)
 #include "text-input-extension-unstable-v1-client-protocol.h"  // NOLINT(build/include_directory)
 #include "text-input-extension-unstable-v1-server-protocol.h"  // NOLINT(build/include_directory)
 #include "text-input-unstable-v1-client-protocol.h"  // NOLINT(build/include_directory)
@@ -22,6 +23,7 @@ namespace {
 // Versions supported by sommelier.
 constexpr uint32_t kTextInputManagerVersion = 1;
 constexpr uint32_t kTextInputExtensionVersion = 11;
+constexpr uint32_t kTextInputCrostiniManagerVersion = 1;
 constexpr uint32_t kTextInputX11Version = 1;
 
 }  // namespace
@@ -37,6 +39,8 @@ struct sl_host_text_input {
   struct wl_resource* resource;
   struct zwp_text_input_v1* proxy;
 
+  struct wl_resource* text_input_crostini_resource;
+
   WeakResourcePtr<sl_host_surface> active_surface;
 };
 MAP_STRUCTS(zwp_text_input_v1, sl_host_text_input);
@@ -55,6 +59,9 @@ struct sl_host_extended_text_input {
 };
 MAP_STRUCTS(zcr_extended_text_input_v1, sl_host_extended_text_input);
 
+////////////////////////////////////////////////////////////////////////////////
+// zwp_text_input_v1 implementation
+
 static void sl_text_input_activate(wl_client* client,
                                    wl_resource* resource,
                                    wl_resource* seat,
@@ -127,7 +134,7 @@ static void sl_text_input_enter(void* data,
   // This is not currently used by cros_im. We can't simply forward the event
   // as for an x11-hosted cros_im instance the text_input and wl_surface
   // objects will be on different clients. We could add a corresponding event
-  // to text_input_x11 if needed.
+  // to zcr_text_input_crostini if needed.
 }
 
 static void sl_text_input_leave(void* data,
@@ -270,6 +277,9 @@ static void sl_destroy_host_text_input(struct wl_resource* resource) {
   delete host;
 }
 
+////////////////////////////////////////////////////////////////////////////////
+// zwp_text_input_manager_v1 implementation
+
 static void sl_text_input_manager_create_text_input(
     struct wl_client* client, struct wl_resource* resource, uint32_t id) {
   struct sl_host_text_input_manager* host =
@@ -333,6 +343,9 @@ struct sl_global* sl_text_input_manager_global_create(struct sl_context* ctx,
                           sl_bind_host_text_input_manager);
 }
 
+////////////////////////////////////////////////////////////////////////////////
+// zcr_extended_text_input_v1 implementation
+
 static void sl_extended_text_input_destroy(struct wl_client* client,
                                            struct wl_resource* resource) {
   wl_resource_destroy(resource);
@@ -454,6 +467,9 @@ static void sl_destroy_host_extended_text_input(struct wl_resource* resource) {
   delete host;
 }
 
+////////////////////////////////////////////////////////////////////////////////
+// zcr_text_input_extension_v1 implementation
+
 static void sl_text_input_extension_get_extended_text_input(
     struct wl_client* client,
     struct wl_resource* resource,
@@ -527,13 +543,23 @@ struct sl_global* sl_text_input_extension_global_create(struct sl_context* ctx,
                           ctx, sl_bind_host_text_input_extension);
 }
 
-static void sl_text_input_x11_activate(wl_client* client,
-                                       wl_resource* resource,
-                                       wl_resource* text_input,
-                                       wl_resource* seat,
-                                       uint32_t x11_window_id) {
+////////////////////////////////////////////////////////////////////////////////
+// zcr_text_input_crostini implementation
+
+static void sl_text_input_crostini_destroy(struct wl_client* client,
+                                           struct wl_resource* resource) {
+  wl_resource_destroy(resource);
+}
+
+// Used by both text_input_crostini and (deprecated) text_input_x11.
+// |resource| is a zcr_text_input_crostini in the former and a zwp_text_input
+// in the latter, in either case the user data is the sl_host_text_input.
+static void sl_text_input_crostini_activate_x11(wl_client* client,
+                                                wl_resource* resource,
+                                                wl_resource* seat,
+                                                uint32_t x11_window_id) {
   struct sl_host_text_input* host_text_input =
-      static_cast<sl_host_text_input*>(wl_resource_get_user_data(text_input));
+      static_cast<sl_host_text_input*>(wl_resource_get_user_data(resource));
   struct sl_host_seat* host_seat =
       static_cast<sl_host_seat*>(wl_resource_get_user_data(seat));
   assert(host_text_input);
@@ -560,6 +586,88 @@ static void sl_text_input_x11_activate(wl_client* client,
   }
 }
 
+static const struct zcr_text_input_crostini_v1_interface
+    sl_text_input_crostini_implementation = {
+        sl_text_input_crostini_destroy,
+        sl_text_input_crostini_activate_x11,
+};
+
+static void sl_destroy_host_text_input_crostini(struct wl_resource* resource) {
+  struct sl_host_text_input* host =
+      static_cast<sl_host_text_input*>(wl_resource_get_user_data(resource));
+
+  host->text_input_crostini_resource = nullptr;
+}
+
+////////////////////////////////////////////////////////////////////////////////
+// zcr_text_input_crostini_manager implementation
+
+static void sl_text_input_crostini_manager_get_text_input_crostini(
+    struct wl_client* client,
+    struct wl_resource* resource,
+    uint32_t id,
+    struct wl_resource* text_input) {
+  struct sl_host_text_input* host_text_input =
+      static_cast<sl_host_text_input*>(wl_resource_get_user_data(text_input));
+
+  if (host_text_input->text_input_crostini_resource) {
+    wl_resource_post_error(
+        resource,
+        ZCR_TEXT_INPUT_CROSTINI_MANAGER_V1_ERROR_TEXT_INPUT_CROSTINI_EXISTS,
+        "text_input has already been associated with a text_input_crostini "
+        "object");
+    return;
+  }
+
+  wl_resource* text_input_crostini_resource =
+      wl_resource_create(client, &zcr_text_input_crostini_v1_interface,
+                         wl_resource_get_version(resource), id);
+  host_text_input->text_input_crostini_resource = text_input_crostini_resource;
+
+  wl_resource_set_implementation(
+      text_input_crostini_resource, &sl_text_input_crostini_implementation,
+      host_text_input, sl_destroy_host_text_input_crostini);
+}  // NOLINT(whitespace/indent)
+
+static struct zcr_text_input_crostini_manager_v1_interface
+    sl_text_input_crostini_manager_implementation = {
+        sl_text_input_crostini_manager_get_text_input_crostini,
+};
+
+static void sl_bind_host_text_input_crostini_manager(struct wl_client* client,
+                                                     void* data,
+                                                     uint32_t app_version,
+                                                     uint32_t id) {
+  // This exists only between sommelier and its clients and there is no proxy
+  // to the host. We don't use a sl_host_text_input_crostini_manager type as it
+  // is not needed.
+  wl_resource* resource = wl_resource_create(
+      client, &zcr_text_input_crostini_manager_v1_interface, app_version, id);
+  wl_resource_set_implementation(resource,
+                                 &sl_text_input_crostini_manager_implementation,
+                                 nullptr, nullptr);
+}
+
+struct sl_global* sl_text_input_crostini_manager_global_create(
+    struct sl_context* ctx) {
+  return sl_global_create(ctx, &zcr_text_input_crostini_manager_v1_interface,
+                          kTextInputCrostiniManagerVersion, ctx,
+                          sl_bind_host_text_input_crostini_manager);
+}
+
+////////////////////////////////////////////////////////////////////////////////
+// zcr_text_input_x11_v1 implementation
+// TODO(b/295265862): Deprecated, remove some time after M122.
+
+static void sl_text_input_x11_activate(wl_client* client,
+                                       wl_resource* resource,
+                                       wl_resource* text_input,
+                                       wl_resource* seat,
+                                       uint32_t x11_window_id) {
+  return sl_text_input_crostini_activate_x11(client, text_input, seat,
+                                             x11_window_id);
+}
+
 static const struct zcr_text_input_x11_v1_interface
     sl_text_input_x11_implementation = {
         sl_text_input_x11_activate,

diff --git a/sommelier.cc b/sommelier.cc
@@ -492,6 +492,7 @@ bool sl_client_supports_interface(const sl_context* ctx,
   return strcmp(name, "wl_seat") == 0 ||
          strcmp(name, "zwp_text_input_manager_v1") == 0 ||
          strcmp(name, "zcr_text_input_extension_v1") == 0 ||
+         strcmp(name, "zcr_text_input_crostini_manager_v1") == 0 ||
          strcmp(name, "zcr_text_input_x11_v1") == 0;
 }
 
@@ -709,6 +710,8 @@ void sl_registry_handler(void* data,
     text_input_manager->id = id;
     text_input_manager->host_global =
         sl_text_input_manager_global_create(ctx, version);
+    text_input_manager->host_crostini_manager_global =
+        sl_text_input_crostini_manager_global_create(ctx);
     text_input_manager->host_x11_global = sl_text_input_x11_global_create(ctx);
     assert(!ctx->text_input_manager);
     ctx->text_input_manager = text_input_manager;

diff --git a/sommelier.h b/sommelier.h
@@ -334,6 +334,7 @@ struct sl_text_input_manager {
   struct sl_context* ctx;
   uint32_t id;
   struct sl_global* host_global;
+  struct sl_global* host_crostini_manager_global;
   struct sl_global* host_x11_global;
 };
 
@@ -512,6 +513,8 @@ struct sl_global* sl_text_input_manager_global_create(struct sl_context* ctx,
                                                       uint32_t exo_version);
 
 struct sl_global* sl_text_input_x11_global_create(struct sl_context* ctx);
+struct sl_global* sl_text_input_crostini_manager_global_create(
+    struct sl_context* ctx);
 
 struct sl_global* sl_pointer_constraints_global_create(struct sl_context* ctx);