Fix deadlock caused by XFlush() reading events in X11SoftwareBitmapPresenter

Explanation copied from comment added by this CL:

XFlush() flushes all requests and *also* reads events. It's unclear why it does
this, but there's no alternative Xlib function that flushes the requests and
*doesn't* read any events, so this workaround is necessary. In
|event_task_runner_|'s message loop, poll() is called on the underlying
XDisplay's fd to dispatch toplevel events. When the fd is readable, poll() exits
and we (via Xlib) check for new events by read()ing from the fd. But if the
event dispatcher is currently dispatching an event, then our call to XFlush()
may read events into the event queue which will make the fd blocking since
there's no more data to read, so poll() won't wake up until a new event comes,
which may take a long time. Forcing the event loop to wake up with a dummy event
fixes the race condition.

BUG=1036742
R=msisov

Change-Id: I4ce5dd1bbfd9c31a5e1adc992241937251fc3822
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1999436Reviewed-by: Maksim Sisov <msisov@igalia.com>
Commit-Queue: Thomas Anderson <thomasanderson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#731615}

ui/base/x/x11_software_bitmap_presenter.cc

@@ -118,7 +118,6 @@ bool X11SoftwareBitmapPresenter::CompositeBitmap(XDisplay* display,
 
   XPutImage(display, widget, gc, bg.get(), x, y, x, y, width, height);
 
-  XFlush(display);
   return true;
 }
 
@@ -129,7 +128,8 @@ X11SoftwareBitmapPresenter::X11SoftwareBitmapPresenter(
     : widget_(widget),
       display_(gfx::GetXDisplay()),
       gc_(nullptr),
-      host_task_runner_(host_task_runner) {
+      host_task_runner_(host_task_runner),
+      event_task_runner_(event_task_runner) {
   DCHECK_NE(widget_, gfx::kNullAcceleratedWidget);
   gc_ = XCreateGC(display_, widget_, 0, nullptr);
   memset(&attributes_, 0, sizeof(attributes_));
@@ -166,6 +166,28 @@ bool X11SoftwareBitmapPresenter::ShmPoolReady() const {
   return shm_pool_ && shm_pool_->Ready();
 }
 
+void X11SoftwareBitmapPresenter::FlushAfterPutImage() {
+  // Ensure the new window content appears immediately. On a TYPE_UI thread we
+  // can rely on the message loop to flush for us so XFlush() isn't necessary.
+  // However, this code can run on a different thread and would have to wait for
+  // the TYPE_UI thread to no longer be idle before a flush happens.
+  XFlush(display_);
+
+  // Work around a race condition caused by XFlush above.  Explanation: XFlush()
+  // flushes all requests and *also* reads events.  It's unclear why it does
+  // this, but there's no alternative Xlib function that flushes the requests
+  // and *doesn't* read any events, so this workaround is necessary. In
+  // |event_task_runner_|'s message loop, poll() is called on the underlying
+  // XDisplay's fd to dispatch toplevel events.  When the fd is readable, poll()
+  // exits and we (via Xlib) check for new events by read()ing from the fd.  But
+  // if the event dispatcher is currently dispatching an event, then our call to
+  // XFlush() may read events into the event queue which will make the fd
+  // blocking since there's no more data to read, so poll() won't wake up until
+  // a new event comes, which may take a long time.  Forcing the event loop to
+  // wake up with a dummy event fixes the race condition.
+  event_task_runner_->PostTask(FROM_HERE, base::BindOnce([] {}));
+}
+
 void X11SoftwareBitmapPresenter::Resize(const gfx::Size& pixel_size) {
   if (pixel_size == viewport_pixel_size_)
     return;
@@ -208,7 +230,7 @@ void X11SoftwareBitmapPresenter::EndPaint(const gfx::Rect& damage_rect) {
                      rect.x(), rect.y(), rect.x(), rect.y(), rect.width(),
                      rect.height(), x11::True)) {
       needs_swap_ = true;
-      XFlush(display_);
+      FlushAfterPutImage();
       return;
     }
     skia_pixmap = shm_pool_->CurrentBitmap().pixmap();
@@ -220,6 +242,7 @@ void X11SoftwareBitmapPresenter::EndPaint(const gfx::Rect& damage_rect) {
       CompositeBitmap(display_, widget_, rect.x(), rect.y(), rect.width(),
                       rect.height(), attributes_.depth, gc_,
                       skia_pixmap.addr())) {
+    FlushAfterPutImage();
     return;
   }
 
@@ -244,11 +267,7 @@ void X11SoftwareBitmapPresenter::EndPaint(const gfx::Rect& damage_rect) {
   XPutImage(display_, widget_, gc_, &image, rect.x(), rect.y(), rect.x(),
             rect.y(), rect.width(), rect.height());
 
-  // Ensure the new window content appears immediately. On a TYPE_UI thread we
-  // can rely on the message loop to flush for us so XFlush() isn't necessary.
-  // However, this code can run on a different thread and would have to wait for
-  // the TYPE_UI thread to no longer be idle before a flush happens.
-  XFlush(display_);
+  FlushAfterPutImage();
 }
 
 void X11SoftwareBitmapPresenter::OnSwapBuffers(

ui/base/x/x11_software_bitmap_presenter.h

@@ -56,6 +56,8 @@ class COMPONENT_EXPORT(UI_BASE_X) X11SoftwareBitmapPresenter {
 
   bool ShmPoolReady() const;
 
+  void FlushAfterPutImage();
+
   gfx::AcceleratedWidget widget_;
   XDisplay* display_;
   GC gc_;
@@ -69,6 +71,7 @@ class COMPONENT_EXPORT(UI_BASE_X) X11SoftwareBitmapPresenter {
   bool needs_swap_ = false;
 
   scoped_refptr<base::SequencedTaskRunner> host_task_runner_;
+  scoped_refptr<base::SequencedTaskRunner> event_task_runner_;
   sk_sp<SkSurface> surface_;
 
   gfx::Size viewport_pixel_size_;