ozone/platform/wayland: Use more realistic event processing and request flushing in tests

ui/ozone/platform/wayland/wayland_display.cc

Index: ui/ozone/platform/wayland/wayland_display.cc
diff --git a/ui/ozone/platform/wayland/wayland_display.cc b/ui/ozone/platform/wayland/wayland_display.cc
index 1adc88e0b1b85aa6c2bc1c8f861544e410ad5f99..cbef9619b19cf421b2946b571b57af3c0907e0b3 100644
--- a/ui/ozone/platform/wayland/wayland_display.cc
+++ b/ui/ozone/platform/wayland/wayland_display.cc
@@ -6,6 +6,7 @@
 
 #include <xdg-shell-unstable-v5-client-protocol.h>
 
+#include "base/bind.h"
 #include "base/logging.h"
 #include "base/message_loop/message_loop.h"
 #include "ui/ozone/platform/wayland/wayland_object.h"
@@ -60,9 +61,29 @@ bool WaylandDisplay::Initialize() {
   return true;
 }
 
-void WaylandDisplay::Flush() {
+bool WaylandDisplay::StartProcessingEvents() {
+  if (watching_)
+    return true;
+
   DCHECK(display_);
   wl_display_flush(display_.get());
+
+  DCHECK(base::MessageLoopForUI::IsCurrent());
+  if (!base::MessageLoopForUI::current()->WatchFileDescriptor(
+          wl_display_get_fd(display_.get()), true,
+          base::MessagePumpLibevent::WATCH_READ, &controller_, this))
+    return false;
+
+  watching_ = true;
+  return true;
+}
+
+void WaylandDisplay::ScheduleFlush() {
+  if (scheduled_flush_ || !base::MessageLoopForUI::IsCurrent())

[spang, 2016/03/01 21:40:49]

I strongly disagree with making use of base::MessageLoopForUI::IsCurrent() for
flow control. Functions that care about the message loop should have it as a
precondition and write DCHECK() instead.

Why do we need to do this?

[Michael Forney, 2016/03/03 09:47:16]

The reason is that we process events and sometimes make requests in response to
these events (for example, the wl_registry.global event which triggers
wl_registry.bind requests) while WaylandDisplay::Initialize is running, which
happens before the MessageLoop is created. We also may process the same types of
events after it is created (for which we should schedule the flush in the
message loop).

If we cannot use base::MessageLoopForUI::IsCurrent() to determine whether or not
to post a flush task, an alternative would be to keep a bool variable to track
when Initialize has finished (though, I think this doesn't guarantee that the
message loop is created, which is out of control of the platform).

+    return;
+  base::MessageLoopForUI::current()->task_runner()->PostTask(
+      FROM_HERE, base::Bind(&WaylandDisplay::Flush, base::Unretained(this)));
+  scheduled_flush_ = true;
 }
 
 WaylandWindow* WaylandDisplay::GetWindow(gfx::AcceleratedWidget widget) {
@@ -80,15 +101,12 @@ void WaylandDisplay::RemoveWindow(gfx::AcceleratedWidget widget) {
 }
 
 void WaylandDisplay::OnDispatcherListChanged() {
-  if (watching_)
-    return;
+  StartProcessingEvents();
+}
 
-  DCHECK(display_);
-  DCHECK(base::MessageLoopForUI::IsCurrent());
-  base::MessageLoopForUI::current()->WatchFileDescriptor(
-      wl_display_get_fd(display_.get()), true,
-      base::MessagePumpLibevent::WATCH_READ, &controller_, this);
-  watching_ = true;
+void WaylandDisplay::Flush() {
+  wl_display_flush(display_.get());
+  scheduled_flush_ = false;
 }
 
 void WaylandDisplay::OnFileCanReadWithoutBlocking(int fd) {
@@ -131,6 +149,8 @@ void WaylandDisplay::Global(void* data,
     xdg_shell_use_unstable_version(display->shell_.get(),
                                    XDG_SHELL_VERSION_CURRENT);
   }
+
+  display->ScheduleFlush();
 }
 
 // static
@@ -142,7 +162,9 @@ void WaylandDisplay::GlobalRemove(void* data,
 
 // static
 void WaylandDisplay::Ping(void* data, xdg_shell* shell, uint32_t serial) {
+  WaylandDisplay* display = static_cast<WaylandDisplay*>(data);
   xdg_shell_pong(shell, serial);
+  display->ScheduleFlush();
 }
 
 }  // namespace ui