diff options
Diffstat (limited to '0011-ozone-wayland-Do-not-use-possibly-blocking-dispatch-.patch')
| -rw-r--r-- | 0011-ozone-wayland-Do-not-use-possibly-blocking-dispatch-.patch | 315 |
1 files changed, 315 insertions, 0 deletions
diff --git a/0011-ozone-wayland-Do-not-use-possibly-blocking-dispatch-.patch b/0011-ozone-wayland-Do-not-use-possibly-blocking-dispatch-.patch new file mode 100644 index 000000000000..def2841ee476 --- /dev/null +++ b/0011-ozone-wayland-Do-not-use-possibly-blocking-dispatch-.patch @@ -0,0 +1,315 @@ +From 4f4c199c6654ef75319eabf3977e916b2a744d3f Mon Sep 17 00:00:00 2001 +From: Maksim Sisov <msisov@igalia.com> +Date: Thu, 8 Aug 2019 11:47:41 +0000 +Subject: [PATCH 11/11] [ozone/wayland] Do not use possibly blocking dispatch + API + +Using wl_display_dispatch is a wrong thing to do as it is a blocking +method (see [1]). It can cause freezes and deadlocks. + +Instead, we need to do something like this - + +while (wl_display_prepare_read_queue(display, queue) != 0) + wl_display_dispatch_queue_pending(display, queue); +wl_display_flush(display); + +ret = poll(fds, nfds, -1); +if (has_error(ret)) + wl_display_cancel_read(display); +else + wl_display_read_events(display); + +wl_display_dispatch_queue_pending(display, queue); + +However, there is no need to do a second poll as long +as we are notified by libevent that the display fd is +non-blocking. However, to ensure we can read, we have +to prepare the queue, which tells other clients we +are going to read from the queue. If prepare fails, +it means there is nothing to read. Thus, we push +the events into the queue and return early instead. + +We have also to add an "automatic" flush as part +of the polling mechanism. This poll must ensure +that all the data has been written to the queue. +If not, -1 is returned and EAGAIN is set. In this +case, we have to stop watching the fd for read +access and start watching it for the write access. + +As soon as all the data is written, we can get +back to reading. + +[1] https://people.freedesktop.org/~whot/wayland-doxygen/wayland/Client/classwl__display.html#a30a9c4f020f3e77581c7a81ecdb4913d + +Bug: 987821 +Change-Id: I536513034c18a47da079a2b56a75d32f20f72ad6 +Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1730159 +Reviewed-by: Robert Kroeger <rjkroege@chromium.org> +Commit-Queue: Maksim Sisov <msisov@igalia.com> +Cr-Commit-Position: refs/heads/master@{#685148} +--- + .../wayland/host/wayland_connection.cc | 74 +++++++++++++++---- + .../wayland/host/wayland_connection.h | 4 + + .../wayland/host/wayland_data_device.cc | 2 +- + .../wayland/host/wayland_data_source.cc | 2 +- + .../platform/wayland/host/wayland_keyboard.cc | 2 +- + .../platform/wayland/host/wayland_window.cc | 3 +- + .../wayland/host/wayland_window_unittest.cc | 17 +++-- + .../wayland/host/xdg_surface_wrapper_v5.cc | 1 + + .../wayland/host/xdg_surface_wrapper_v6.cc | 3 +- + 9 files changed, 84 insertions(+), 24 deletions(-) + +diff --git a/ui/ozone/platform/wayland/host/wayland_connection.cc b/ui/ozone/platform/wayland/host/wayland_connection.cc +index bf6d12717861..203db09c7f15 100644 +--- a/ui/ozone/platform/wayland/host/wayland_connection.cc ++++ b/ui/ozone/platform/wayland/host/wayland_connection.cc +@@ -101,20 +101,29 @@ bool WaylandConnection::StartProcessingEvents() { + return true; + + DCHECK(display_); ++ ++ MaybePrepareReadQueue(); ++ ++ // Displatch event from display to server. + wl_display_flush(display_.get()); + +- DCHECK(base::MessageLoopCurrentForUI::IsSet()); +- if (!base::MessageLoopCurrentForUI::Get()->WatchFileDescriptor( +- wl_display_get_fd(display_.get()), true, +- base::MessagePumpLibevent::WATCH_READ, &controller_, this)) +- return false; ++ return BeginWatchingFd(base::MessagePumpLibevent::WATCH_READ); ++} + +- watching_ = true; +- return true; ++void WaylandConnection::MaybePrepareReadQueue() { ++ if (prepared_) ++ return; ++ ++ if (wl_display_prepare_read(display()) != -1) { ++ prepared_ = true; ++ return; ++ } ++ // Nothing to read, send events to the queue. ++ wl_display_dispatch_pending(display()); + } + + void WaylandConnection::ScheduleFlush() { +- if (scheduled_flush_ || !watching_) ++ if (scheduled_flush_) + return; + DCHECK(base::MessageLoopCurrentForUI::IsSet()); + base::ThreadTaskRunnerHandle::Get()->PostTask( +@@ -207,13 +216,38 @@ void WaylandConnection::DispatchUiEvent(Event* event) { + } + + void WaylandConnection::OnFileCanReadWithoutBlocking(int fd) { +- wl_display_dispatch(display_.get()); +- std::vector<WaylandWindow*> windows = wayland_window_manager_.GetAllWindows(); +- for (auto* window : windows) +- window->ApplyPendingBounds(); ++ if (prepared_) { ++ prepared_ = false; ++ if (wl_display_read_events(display()) == -1) ++ return; ++ wl_display_dispatch_pending(display()); ++ } ++ ++ MaybePrepareReadQueue(); ++ ++ if (!prepared_) ++ return; ++ ++ // Automatic Flush. ++ int ret = wl_display_flush(display_.get()); ++ if (ret != -1 || errno != EAGAIN) ++ return; ++ ++ // if all data could not be written, errno will be set to EAGAIN and -1 ++ // returned. In that case, use poll on the display file descriptor to wait for ++ // it to become writable again. ++ BeginWatchingFd(base::MessagePumpLibevent::WATCH_WRITE); + } + +-void WaylandConnection::OnFileCanWriteWithoutBlocking(int fd) {} ++void WaylandConnection::OnFileCanWriteWithoutBlocking(int fd) { ++ int ret = wl_display_flush(display_.get()); ++ if (ret != -1 || errno != EAGAIN) ++ BeginWatchingFd(base::MessagePumpLibevent::WATCH_READ); ++ else if (ret < 0 && errno != EPIPE && prepared_) ++ wl_display_cancel_read(display()); ++ ++ // Otherwise just continue watching in the same mode. ++} + + void WaylandConnection::EnsureDataDevice() { + if (!data_device_manager_ || !seat_) +@@ -225,6 +259,20 @@ void WaylandConnection::EnsureDataDevice() { + data_device_.get()); + } + ++bool WaylandConnection::BeginWatchingFd( ++ base::WatchableIOMessagePumpPosix::Mode mode) { ++ if (watching_) { ++ // Stop watching first. ++ watching_ = !controller_.StopWatchingFileDescriptor(); ++ DCHECK(!watching_); ++ } ++ ++ DCHECK(base::MessageLoopCurrentForUI::IsSet()); ++ watching_ = base::MessageLoopCurrentForUI::Get()->WatchFileDescriptor( ++ wl_display_get_fd(display_.get()), true, mode, &controller_, this); ++ return watching_; ++} ++ + // static + void WaylandConnection::Global(void* data, + wl_registry* registry, +diff --git a/ui/ozone/platform/wayland/host/wayland_connection.h b/ui/ozone/platform/wayland/host/wayland_connection.h +index 8a56ebc6b921..ff587ae9ee6f 100644 +--- a/ui/ozone/platform/wayland/host/wayland_connection.h ++++ b/ui/ozone/platform/wayland/host/wayland_connection.h +@@ -146,6 +146,9 @@ class WaylandConnection : public PlatformEventSource, + // Make sure data device is properly initialized + void EnsureDataDevice(); + ++ bool BeginWatchingFd(base::WatchableIOMessagePumpPosix::Mode mode); ++ void MaybePrepareReadQueue(); ++ + // wl_registry_listener + static void Global(void* data, + wl_registry* registry, +@@ -192,6 +195,7 @@ class WaylandConnection : public PlatformEventSource, + + bool scheduled_flush_ = false; + bool watching_ = false; ++ bool prepared_ = false; + base::MessagePumpLibevent::FdWatchController controller_; + + uint32_t serial_ = 0; +diff --git a/ui/ozone/platform/wayland/host/wayland_data_device.cc b/ui/ozone/platform/wayland/host/wayland_data_device.cc +index a36cd3b7a888..56e568fbd164 100644 +--- a/ui/ozone/platform/wayland/host/wayland_data_device.cc ++++ b/ui/ozone/platform/wayland/host/wayland_data_device.cc +@@ -355,7 +355,7 @@ void WaylandDataDevice::RegisterDeferredReadCallback() { + deferred_read_callback_.reset(wl_display_sync(connection_->display())); + wl_callback_add_listener(deferred_read_callback_.get(), + &kDeferredReadListener, this); +- wl_display_flush(connection_->display()); ++ connection_->ScheduleFlush(); + } + + void WaylandDataDevice::DeferredReadCallback(void* data, +diff --git a/ui/ozone/platform/wayland/host/wayland_data_source.cc b/ui/ozone/platform/wayland/host/wayland_data_source.cc +index dcd6b75aa299..1f41fd6786b9 100644 +--- a/ui/ozone/platform/wayland/host/wayland_data_source.cc ++++ b/ui/ozone/platform/wayland/host/wayland_data_source.cc +@@ -36,7 +36,7 @@ void WaylandDataSource::WriteToClipboard( + wl_data_device_set_selection(connection_->data_device(), data_source_.get(), + connection_->serial()); + +- wl_display_flush(connection_->display()); ++ connection_->ScheduleFlush(); + } + + void WaylandDataSource::UpdateDataMap( +diff --git a/ui/ozone/platform/wayland/host/wayland_keyboard.cc b/ui/ozone/platform/wayland/host/wayland_keyboard.cc +index 181e6e56b4b6..69cd9686bdf8 100644 +--- a/ui/ozone/platform/wayland/host/wayland_keyboard.cc ++++ b/ui/ozone/platform/wayland/host/wayland_keyboard.cc +@@ -165,7 +165,7 @@ void WaylandKeyboard::FlushInput(base::OnceClosure closure) { + // get spurious repeats. + sync_callback_.reset(wl_display_sync(connection_->display())); + wl_callback_add_listener(sync_callback_.get(), &callback_listener_, this); +- wl_display_flush(connection_->display()); ++ connection_->ScheduleFlush(); + } + + void WaylandKeyboard::DispatchKey(uint32_t key, +diff --git a/ui/ozone/platform/wayland/host/wayland_window.cc b/ui/ozone/platform/wayland/host/wayland_window.cc +index 12b3661985a6..cf0f3c336414 100644 +--- a/ui/ozone/platform/wayland/host/wayland_window.cc ++++ b/ui/ozone/platform/wayland/host/wayland_window.cc +@@ -285,7 +285,6 @@ void WaylandWindow::ApplyPendingBounds() { + + SetBoundsDip(pending_bounds_dip_); + xdg_surface_->SetWindowGeometry(pending_bounds_dip_); +- xdg_surface_->AckConfigure(); + pending_bounds_dip_ = gfx::Rect(); + connection_->ScheduleFlush(); + +@@ -659,6 +658,8 @@ void WaylandWindow::HandleSurfaceConfigure(int32_t width, + delegate_->OnWindowStateChanged(state_); + } + ++ ApplyPendingBounds(); ++ + if (did_active_change) + delegate_->OnActivationChanged(is_active_); + +diff --git a/ui/ozone/platform/wayland/host/wayland_window_unittest.cc b/ui/ozone/platform/wayland/host/wayland_window_unittest.cc +index 8083f7a84dcd..5fdf4a6184ba 100644 +--- a/ui/ozone/platform/wayland/host/wayland_window_unittest.cc ++++ b/ui/ozone/platform/wayland/host/wayland_window_unittest.cc +@@ -677,19 +677,26 @@ TEST_P(WaylandWindowTest, HasCaptureUpdatedOnPointerEvents) { + + TEST_P(WaylandWindowTest, ConfigureEvent) { + ScopedWlArray states; +- SendConfigureEvent(1000, 1000, 12, states.get()); +- SendConfigureEvent(1500, 1000, 13, states.get()); + +- // Make sure that the implementation does not call OnBoundsChanged for each +- // configure event if it receives multiple in a row. +- EXPECT_CALL(delegate_, OnBoundsChanged(Eq(gfx::Rect(0, 0, 1500, 1000)))); ++ // The surface must react on each configure event and send bounds to its ++ // delegate. ++ ++ EXPECT_CALL(delegate_, OnBoundsChanged(Eq(gfx::Rect(0, 0, 1000, 1000)))); + // Responding to a configure event, the window geometry in here must respect + // the sizing negotiations specified by the configure event. + // |xdg_surface_| must receive the following calls in both xdg_shell_v5 and + // xdg_shell_v6. Other calls like SetTitle or SetMaximized are recieved by + // xdg_toplevel in xdg_shell_v6 and by xdg_surface_ in xdg_shell_v5. ++ EXPECT_CALL(*xdg_surface_, SetWindowGeometry(0, 0, 1000, 1000)).Times(1); ++ EXPECT_CALL(*xdg_surface_, AckConfigure(12)); ++ SendConfigureEvent(1000, 1000, 12, states.get()); ++ ++ Sync(); ++ ++ EXPECT_CALL(delegate_, OnBoundsChanged(Eq(gfx::Rect(0, 0, 1500, 1000)))); + EXPECT_CALL(*xdg_surface_, SetWindowGeometry(0, 0, 1500, 1000)).Times(1); + EXPECT_CALL(*xdg_surface_, AckConfigure(13)); ++ SendConfigureEvent(1500, 1000, 13, states.get()); + } + + TEST_P(WaylandWindowTest, ConfigureEventWithNulledSize) { +diff --git a/ui/ozone/platform/wayland/host/xdg_surface_wrapper_v5.cc b/ui/ozone/platform/wayland/host/xdg_surface_wrapper_v5.cc +index 7d4172d2f0cb..c89fa87bff9b 100644 +--- a/ui/ozone/platform/wayland/host/xdg_surface_wrapper_v5.cc ++++ b/ui/ozone/platform/wayland/host/xdg_surface_wrapper_v5.cc +@@ -99,6 +99,7 @@ void XDGSurfaceWrapperV5::Configure(void* data, + surface->pending_configure_serial_ = serial; + surface->wayland_window_->HandleSurfaceConfigure(width, height, is_maximized, + is_fullscreen, is_activated); ++ surface->AckConfigure(); + } + + // static +diff --git a/ui/ozone/platform/wayland/host/xdg_surface_wrapper_v6.cc b/ui/ozone/platform/wayland/host/xdg_surface_wrapper_v6.cc +index 9cac792bf630..c7c9fc079590 100644 +--- a/ui/ozone/platform/wayland/host/xdg_surface_wrapper_v6.cc ++++ b/ui/ozone/platform/wayland/host/xdg_surface_wrapper_v6.cc +@@ -123,8 +123,7 @@ void XDGSurfaceWrapperV6::Configure(void* data, + XDGSurfaceWrapperV6* surface = static_cast<XDGSurfaceWrapperV6*>(data); + surface->pending_configure_serial_ = serial; + +- if (surface->surface_for_popup_) +- surface->AckConfigure(); ++ surface->AckConfigure(); + } + + // static +-- +2.22.0 + |