[CRD] Fix crash when connecting to a Linux host

base::FileDescriptorWatcher is not usable on the main thread because it
is initialized as a UI thread, when an IO thread is required.  Since
it's a UI thread, it's able to dispatch events already without special
FD watching.  All that's required is creating an X11EventSource.  This
CL creates the event source where necessary so that
KeyboardLayoutMonitorLinux can dispatch events.

R=lambroslambrou
CC=rkjnsn@chromium.org

Fixes: 1133481
Change-Id: I6e0a380c7c1168c4b1515be10ec34e67d0ae0d47
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2444530
Commit-Queue: Thomas Anderson <thomasanderson@chromium.org>
Auto-Submit: Thomas Anderson <thomasanderson@chromium.org>
Reviewed-by: Lambros Lambrou <lambroslambrou@chromium.org>
Cr-Commit-Position: refs/heads/master@{#813864}

remoting/host/BUILD.gn

@@ -323,6 +323,7 @@ static_library("common") {
     ]
     deps += [
       "//remoting/host/linux:x11",
+      "//ui/events/platform/x11",
       "//ui/gfx/x",
     ]
     if (is_linux) {

remoting/host/it2me/it2me_native_messaging_host_main.cc

@@ -33,6 +33,7 @@
 #include <gtk/gtk.h>
 
 #include "base/linux_util.h"
+#include "ui/events/platform/x11/x11_event_source.h"
 #include "ui/gfx/x/x11.h"
 #endif  // defined(OS_LINUX) || defined(OS_CHROMEOS)
 
@@ -116,6 +117,11 @@ int It2MeNativeMessagingHostMain(int argc, char** argv) {
   // Required in order for us to run multiple X11 threads.
   XInitThreads();
 
+  // Create an X11EventSource so the global X11 connection
+  // (x11::Connection::Get()) can dispatch X events.
+  auto event_source =
+      std::make_unique<ui::X11EventSource>(x11::Connection::Get());
+
   // Required for any calls into GTK functions, such as the Disconnect and
   // Continue windows. Calling with nullptr arguments because we don't have
   // any command line arguments for gtk to consume.

remoting/host/keyboard_layout_monitor_linux.cc

@@ -18,6 +18,7 @@
 #include "ui/base/glib/glib_signal.h"
 #include "ui/events/keycodes/dom/dom_code.h"
 #include "ui/events/keycodes/dom/keycode_converter.h"
+#include "ui/events/platform/x11/x11_event_source.h"
 #include "ui/gfx/x/x11.h"
 #include "ui/gfx/x/xkb.h"
 #include "ui/gfx/x/xproto.h"
@@ -42,7 +43,7 @@ class GtkThreadDeleter {
 
 // Can be constructed on any thread, but must be started and destroyed on the
 // main GTK+ thread (i.e., the GLib global default main context).
-class GdkLayoutMonitorOnGtkThread : public x11::Connection::Delegate {
+class GdkLayoutMonitorOnGtkThread : public ui::XEventDispatcher {
  public:
   GdkLayoutMonitorOnGtkThread(
       scoped_refptr<base::SequencedTaskRunner> task_runner,
@@ -53,19 +54,17 @@ class GdkLayoutMonitorOnGtkThread : public x11::Connection::Delegate {
   void Start();
 
  private:
-  // x11::Connection::Delegate:
-  bool ShouldContinueStream() const override;
-  void DispatchXEvent(x11::Event* event) override;
+  // ui::XEventDispatcher:
+  bool DispatchXEvent(x11::Event* event) override;
 
   void QueryLayout();
-  void OnConnectionData();
   CHROMEG_CALLBACK_0(GdkLayoutMonitorOnGtkThread,
                      void,
                      OnKeysChanged,
                      GdkKeymap*);
   scoped_refptr<base::SequencedTaskRunner> task_runner_;
   base::WeakPtr<KeyboardLayoutMonitorLinux> weak_ptr_;
-  std::unique_ptr<x11::Connection> connection_;
+  x11::Connection* connection_;
   std::unique_ptr<base::FileDescriptorWatcher::Controller> controller_;
   GdkDisplay* display_ = nullptr;
   GdkKeymap* keymap_ = nullptr;
@@ -120,8 +119,10 @@ GdkLayoutMonitorOnGtkThread::GdkLayoutMonitorOnGtkThread(
 
 GdkLayoutMonitorOnGtkThread::~GdkLayoutMonitorOnGtkThread() {
   DCHECK(g_main_context_is_owner(g_main_context_default()));
-  if (handler_id_)
+  if (handler_id_) {
     g_signal_handler_disconnect(keymap_, handler_id_);
+    ui::X11EventSource::GetInstance()->RemoveXEventDispatcher(this);
+  }
 }
 
 void GdkLayoutMonitorOnGtkThread::Start() {
@@ -143,14 +144,10 @@ void GdkLayoutMonitorOnGtkThread::Start() {
   // when switching between groups with different writing directions. As a
   // result, we have to use Xkb directly to get and monitor that information,
   // which is a pain.
-  connection_ = std::make_unique<x11::Connection>();
+  connection_ = x11::Connection::Get();
   auto& xkb = connection_->xkb();
   if (xkb.UseExtension({x11::Xkb::major_version, x11::Xkb::minor_version})
           .Sync()) {
-    auto req = xkb.GetState(
-        {static_cast<x11::Xkb::DeviceSpec>(x11::Xkb::Id::UseCoreKbd)});
-    if (auto reply = req.Sync()) {
-      current_group_ = static_cast<int>(reply->group);
     constexpr auto kXkbAllStateComponentsMask =
         static_cast<x11::Xkb::StatePart>(0x3fff);
     xkb.SelectEvents({
@@ -161,12 +158,8 @@ void GdkLayoutMonitorOnGtkThread::Start() {
         .stateDetails = x11::Xkb::StatePart::GroupState,
     });
     connection_->Flush();
-      controller_ = base::FileDescriptorWatcher::WatchReadable(
-          connection_->GetFd(),
-          base::BindRepeating(&GdkLayoutMonitorOnGtkThread::OnConnectionData,
-                              base::Unretained(this)));
-    }
   }
+  ui::X11EventSource::GetInstance()->AddXEventDispatcher(this);
 
   keymap_ = gdk_keymap_get_for_display(display_);
   handler_id_ = g_signal_connect(keymap_, "keys-changed",
@@ -174,19 +167,18 @@ void GdkLayoutMonitorOnGtkThread::Start() {
   QueryLayout();
 }
 
-bool GdkLayoutMonitorOnGtkThread::ShouldContinueStream() const {
-  return true;
-}
-
-void GdkLayoutMonitorOnGtkThread::DispatchXEvent(x11::Event* event) {
-  if (auto* notify = event->As<x11::Xkb::StateNotifyEvent>()) {
+bool GdkLayoutMonitorOnGtkThread::DispatchXEvent(x11::Event* event) {
+  if (event->As<x11::MappingNotifyEvent>() ||
+      event->As<x11::Xkb::NewKeyboardNotifyEvent>()) {
+    QueryLayout();
+  } else if (auto* notify = event->As<x11::Xkb::StateNotifyEvent>()) {
     int new_group = notify->baseGroup + notify->latchedGroup +
                     static_cast<int16_t>(notify->lockedGroup);
-    if (new_group != current_group_) {
-      current_group_ = new_group;
+    if (new_group != current_group_)
       QueryLayout();
+    return true;
   }
-  }
+  return false;
 }
 
 void GdkLayoutMonitorOnGtkThread::QueryLayout() {
@@ -198,6 +190,11 @@ void GdkLayoutMonitorOnGtkThread::QueryLayout() {
 
   bool have_altgr = false;
 
+  auto req = connection_->xkb().GetState(
+      {static_cast<x11::Xkb::DeviceSpec>(x11::Xkb::Id::UseCoreKbd)});
+  if (auto reply = req.Sync())
+    current_group_ = static_cast<int>(reply->group);
+
   for (ui::DomCode key : KeyboardLayoutMonitorLinux::kSupportedKeys) {
     // Skip single-layout IME keys for now, as they are always present in the
     // keyboard map but not present on most keyboards. Client-side IME is likely
@@ -298,10 +295,6 @@ void GdkLayoutMonitorOnGtkThread::QueryLayout() {
                                 weak_ptr_, std::move(layout_message)));
 }
 
-void GdkLayoutMonitorOnGtkThread::OnConnectionData() {
-  connection_->Dispatch(this);
-}
-
 void GdkLayoutMonitorOnGtkThread::OnKeysChanged(GdkKeymap* keymap) {
   QueryLayout();
 }

remoting/host/remoting_me2me_host.cc

@@ -120,9 +120,11 @@
 
 #if defined(OS_LINUX) || defined(OS_CHROMEOS)
 #include <gtk/gtk.h>
+
 #include "base/linux_util.h"
 #include "remoting/host/audio_capturer_linux.h"
 #include "remoting/host/linux/certificate_watcher.h"
+#include "ui/events/platform/x11/x11_event_source.h"
 #include "ui/gfx/x/x11.h"
 #endif  // defined(OS_LINUX) || defined(OS_CHROMEOS)
 
@@ -1686,11 +1688,16 @@ int HostProcessMain() {
   HOST_LOG << "Starting host process: version " << STRINGIZE(VERSION);
 
 #if defined(OS_LINUX) || defined(OS_CHROMEOS)
+  std::unique_ptr<ui::X11EventSource> event_source;
   if (!base::CommandLine::ForCurrentProcess()->HasSwitch(
           kReportOfflineReasonSwitchName)) {
     // Required in order for us to run multiple X11 threads.
     XInitThreads();
 
+    // Create an X11EventSource so the global X11 connection
+    // (x11::Connection::Get()) can dispatch X events.
+    event_source = std::make_unique<ui::X11EventSource>(x11::Connection::Get());
+
     // Required for any calls into GTK functions, such as the Disconnect and
     // Continue windows, though these should not be used for the Me2Me case
     // (crbug.com/104377).

remoting/test/DEPS

@@ -11,6 +11,7 @@ include_rules = [
   "+remoting/signaling",
   "+ui/gfx",
   "+ui/events/keycodes/dom",
+  "+ui/events/platform/x11",
   "+services/network",
   "+third_party/skia",
 ]

remoting/test/it2me_standalone_host_main.cc

@@ -13,6 +13,7 @@
 #include <gtk/gtk.h>
 
 #include "base/linux_util.h"
+#include "ui/events/platform/x11/x11_event_source.h"
 #include "ui/gfx/x/x11.h"
 #endif  // defined(OS_LINUX) || defined(OS_CHROMEOS)
 
@@ -25,6 +26,11 @@ int main(int argc, const char** argv) {
   // Required in order for us to run multiple X11 threads.
   XInitThreads();
 
+  // Create an X11EventSource so the global X11 connection
+  // (x11::Connection::Get()) can dispatch X events.
+  auto event_source =
+      std::make_unique<ui::X11EventSource>(x11::Connection::Get());
+
   // Required for any calls into GTK functions, such as the Disconnect and
   // Continue windows. Calling with nullptr arguments because we don't have
   // any command line arguments for gtk to consume.