path: root/0002-ozone-wayland-Sway-avoid-sending-presentation-early.patch

From c644b4aba05233a8a78d7708a98d9a583ff78a27 Mon Sep 17 00:00:00 2001
From: Maksim Sisov <msisov@igalia.com>
Date: Tue, 30 Jul 2019 05:53:48 +0000
Subject: [PATCH 2/5] [ozone/wayland] Sway: avoid sending presentation early.

In Sway, presentation callbacks may come much earlier than we send
submission callbacks. That results in unexpected crashes in
GbmSurfacelessWayland, because of early presentation callbacks.
Briefly, a submitted frame may not receive the submission
callback and not be moved to the list waiting for presentation
frames. That means GbmSurfacelessWayland::OnPresentation accesses
an invalid pointer to a frame when the presentation feedback comes.

Bug: 974456
Change-Id: Iae7ab57a00d06f0dde6057ed05df885239e099bd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1719013
Reviewed-by: Robert Kroeger <rjkroege@chromium.org>
Commit-Queue: Maksim Sisov <msisov@igalia.com>
Cr-Commit-Position: refs/heads/master@{#682157}
---
 ui/ozone/platform/wayland/BUILD.gn            |  3 ++
 .../wayland/gpu/wayland_surface_gpu.h         |  2 +
 .../host/wayland_buffer_manager_host.cc       | 25 +++++++--
 .../platform/wayland/test/mock_surface.cc     | 44 ++++++++++++++-
 ui/ozone/platform/wayland/test/mock_surface.h | 14 +++++
 .../wayland/test/mock_wp_presentation.cc      | 53 +++++++++++++++++++
 .../wayland/test/mock_wp_presentation.h       | 47 ++++++++++++++++
 .../test/test_wayland_server_thread.cc        |  6 +++
 .../wayland/test/test_wayland_server_thread.h |  5 ++
 9 files changed, 194 insertions(+), 5 deletions(-)
 create mode 100644 ui/ozone/platform/wayland/test/mock_wp_presentation.cc
 create mode 100644 ui/ozone/platform/wayland/test/mock_wp_presentation.h

diff --git a/ui/ozone/platform/wayland/BUILD.gn b/ui/ozone/platform/wayland/BUILD.gn
index 303173b4ba6c..686138bb614a 100644
--- a/ui/ozone/platform/wayland/BUILD.gn
+++ b/ui/ozone/platform/wayland/BUILD.gn
@@ -173,6 +173,8 @@ source_set("test_support") {
     "test/mock_pointer.h",
     "test/mock_surface.cc",
     "test/mock_surface.h",
+    "test/mock_wp_presentation.cc",
+    "test/mock_wp_presentation.h",
     "test/mock_xdg_popup.cc",
     "test/mock_xdg_popup.h",
     "test/mock_xdg_shell.cc",
@@ -225,6 +227,7 @@ source_set("test_support") {
     "//testing/gmock",
     "//third_party/wayland:wayland_server",
     "//third_party/wayland-protocols:linux_dmabuf_protocol",
+    "//third_party/wayland-protocols:presentation_time_protocol",
     "//third_party/wayland-protocols:text_input_protocol",
     "//third_party/wayland-protocols:xdg_shell_protocol",
     "//ui/gfx/geometry:geometry",
diff --git a/ui/ozone/platform/wayland/gpu/wayland_surface_gpu.h b/ui/ozone/platform/wayland/gpu/wayland_surface_gpu.h
index f3593766eed8..ace5279e838e 100644
--- a/ui/ozone/platform/wayland/gpu/wayland_surface_gpu.h
+++ b/ui/ozone/platform/wayland/gpu/wayland_surface_gpu.h
@@ -22,6 +22,8 @@ namespace ui {
 // the buffer.
 class WaylandSurfaceGpu {
  public:
+  virtual ~WaylandSurfaceGpu() {}
+
   // Tells the surface the result of the last swap of buffer with the
   // |buffer_id|.
   virtual void OnSubmission(uint32_t buffer_id,
diff --git a/ui/ozone/platform/wayland/host/wayland_buffer_manager_host.cc b/ui/ozone/platform/wayland/host/wayland_buffer_manager_host.cc
index bbcfa84016f9..5f7efebac400 100644
--- a/ui/ozone/platform/wayland/host/wayland_buffer_manager_host.cc
+++ b/ui/ozone/platform/wayland/host/wayland_buffer_manager_host.cc
@@ -207,6 +207,12 @@ class WaylandBufferManagerHost::Surface {
     // surface can tell the gpu about successful swap.
     bool released = true;
 
+    // In some cases, a presentation feedback can come earlier than we fire a
+    // submission callback. Thus, instead of sending it immediately to the GPU
+    // process, we store it and fire as soon as the submission callback is
+    // fired.
+    bool needs_send_feedback = false;
+
     gfx::PresentationFeedback feedback;
 
     DISALLOW_COPY_AND_ASSIGN(WaylandBuffer);
@@ -334,6 +340,11 @@ class WaylandBufferManagerHost::Surface {
   void CompleteSubmission() {
     DCHECK(submitted_buffer_);
     auto id = submitted_buffer_->buffer_id;
+
+    auto feedback = std::move(submitted_buffer_->feedback);
+    bool needs_send_feedback = submitted_buffer_->needs_send_feedback;
+    submitted_buffer_->needs_send_feedback = false;
+
     prev_submitted_buffer_ = submitted_buffer_;
     submitted_buffer_ = nullptr;
     // We can now complete the latest submission. We had to wait for this
@@ -349,14 +360,22 @@ class WaylandBufferManagerHost::Surface {
       OnPresentation(id, gfx::PresentationFeedback(
                              base::TimeTicks::Now(), base::TimeDelta(),
                              GetPresentationKindFlags(0)));
+    } else if (needs_send_feedback) {
+      OnPresentation(id, std::move(feedback));
     }
   }
 
   void OnPresentation(uint32_t buffer_id,
                       const gfx::PresentationFeedback& feedback) {
-    // The order of submission and presentation callbacks is checked on the GPU
-    // side, but it must never happen, because the Submission is called
-    // immediately after the buffer is swapped.
+    // The order of submission and presentation callbacks cannot be controlled.
+    // Some Wayland compositors may fire presentation callbacks earlier than we
+    // are able to send submission callbacks and this is bad. Thus, handle it here.
+    if (submitted_buffer_ && submitted_buffer_->buffer_id == buffer_id) {
+      submitted_buffer_->needs_send_feedback = true;
+      submitted_buffer_->feedback = feedback;
+      return;
+    }
+
     buffer_manager_->OnPresentation(window_->GetWidget(), buffer_id, feedback);
   }
 
diff --git a/ui/ozone/platform/wayland/test/mock_surface.cc b/ui/ozone/platform/wayland/test/mock_surface.cc
index 9d2333683a41..6ee1c0a9c543 100644
--- a/ui/ozone/platform/wayland/test/mock_surface.cc
+++ b/ui/ozone/platform/wayland/test/mock_surface.cc
@@ -13,7 +13,8 @@ void Attach(wl_client* client,
             wl_resource* buffer_resource,
             int32_t x,
             int32_t y) {
-  GetUserDataAs<MockSurface>(resource)->Attach(buffer_resource, x, y);
+  auto* surface = GetUserDataAs<MockSurface>(resource);
+  surface->AttachNewBuffer(buffer_resource, x, y);
 }
 
 void SetOpaqueRegion(wl_client* client,
@@ -40,7 +41,13 @@ void Damage(wl_client* client,
 void Frame(struct wl_client* client,
            struct wl_resource* resource,
            uint32_t callback) {
-  GetUserDataAs<MockSurface>(resource)->Frame(callback);
+  auto* surface = GetUserDataAs<MockSurface>(resource);
+
+  wl_resource* callback_resource =
+      wl_resource_create(client, &wl_callback_interface, 1, callback);
+  surface->set_frame_callback(callback_resource);
+
+  surface->Frame(callback);
 }
 
 void Commit(wl_client* client, wl_resource* resource) {
@@ -85,4 +92,37 @@ MockSurface* MockSurface::FromResource(wl_resource* resource) {
   return GetUserDataAs<MockSurface>(resource);
 }
 
+void MockSurface::AttachNewBuffer(wl_resource* buffer_resource,
+                                  int32_t x,
+                                  int32_t y) {
+  if (attached_buffer_) {
+    DCHECK(!prev_attached_buffer_);
+    prev_attached_buffer_ = attached_buffer_;
+  }
+  attached_buffer_ = buffer_resource;
+
+  Attach(buffer_resource, x, y);
+}
+
+void MockSurface::ReleasePrevAttachedBuffer() {
+  if (!prev_attached_buffer_)
+    return;
+
+  wl_buffer_send_release(prev_attached_buffer_);
+  wl_client_flush(wl_resource_get_client(prev_attached_buffer_));
+  prev_attached_buffer_ = nullptr;
+}
+
+void MockSurface::SendFrameCallback() {
+  if (!frame_callback_)
+    return;
+
+  wl_callback_send_done(
+      frame_callback_,
+      0 /* trequest-specific data for the callback. not used */);
+  wl_client_flush(wl_resource_get_client(frame_callback_));
+  wl_resource_destroy(frame_callback_);
+  frame_callback_ = nullptr;
+}
+
 }  // namespace wl
diff --git a/ui/ozone/platform/wayland/test/mock_surface.h b/ui/ozone/platform/wayland/test/mock_surface.h
index 1ea9c52dea27..0b44ba090187 100644
--- a/ui/ozone/platform/wayland/test/mock_surface.h
+++ b/ui/ozone/platform/wayland/test/mock_surface.h
@@ -50,10 +50,24 @@ class MockSurface : public ServerObject {
   }
   MockXdgPopup* xdg_popup() const { return xdg_popup_.get(); }
 
+  void set_frame_callback(wl_resource* callback_resource) {
+    DCHECK(!frame_callback_);
+    frame_callback_ = callback_resource;
+  }
+
+  void AttachNewBuffer(wl_resource* buffer_resource, int32_t x, int32_t y);
+  void ReleasePrevAttachedBuffer();
+  void SendFrameCallback();
+
  private:
   std::unique_ptr<MockXdgSurface> xdg_surface_;
   std::unique_ptr<MockXdgPopup> xdg_popup_;
 
+  wl_resource* frame_callback_ = nullptr;
+
+  wl_resource* attached_buffer_ = nullptr;
+  wl_resource* prev_attached_buffer_ = nullptr;
+
   DISALLOW_COPY_AND_ASSIGN(MockSurface);
 };
 
diff --git a/ui/ozone/platform/wayland/test/mock_wp_presentation.cc b/ui/ozone/platform/wayland/test/mock_wp_presentation.cc
new file mode 100644
index 000000000000..d24c13658aa4
--- /dev/null
+++ b/ui/ozone/platform/wayland/test/mock_wp_presentation.cc
@@ -0,0 +1,53 @@
+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "ui/ozone/platform/wayland/test/mock_wp_presentation.h"
+
+#include <wayland-server-core.h>
+
+#include "ui/ozone/platform/wayland/test/server_object.h"
+
+namespace wl {
+
+namespace {
+
+void Feedback(struct wl_client* client,
+              struct wl_resource* resource,
+              struct wl_resource* surface,
+              uint32_t callback) {
+  auto* wp_presentation = GetUserDataAs<MockWpPresentation>(resource);
+  wl_resource* presentation_feedback_resource =
+      wl_resource_create(client, &wp_presentation_feedback_interface,
+                         wl_resource_get_version(resource), callback);
+  wp_presentation->set_presentation_callback(presentation_feedback_resource);
+  wp_presentation->Feedback(client, resource, surface, callback);
+}
+
+}  // namespace
+
+const struct wp_presentation_interface kMockWpPresentationImpl = {
+    &DestroyResource,  // destroy
+    &Feedback,         // feedback
+};
+
+MockWpPresentation::MockWpPresentation()
+    : GlobalObject(&wp_presentation_interface, &kMockWpPresentationImpl, 1) {}
+
+MockWpPresentation::~MockWpPresentation() {}
+
+void MockWpPresentation::SendPresentationCallback() {
+  if (!presentation_callback_)
+    return;
+
+  // TODO(msisov): add support for test provided presentation feedback values.
+  wp_presentation_feedback_send_presented(
+      presentation_callback_, 0 /* tv_sec_hi */, 0 /* tv_sec_lo */,
+      0 /* tv_nsec */, 0 /* refresh */, 0 /* seq_hi */, 0 /* seq_lo */,
+      0 /* flags */);
+  wl_client_flush(wl_resource_get_client(presentation_callback_));
+  wl_resource_destroy(presentation_callback_);
+  presentation_callback_ = nullptr;
+}
+
+}  // namespace wl
diff --git a/ui/ozone/platform/wayland/test/mock_wp_presentation.h b/ui/ozone/platform/wayland/test/mock_wp_presentation.h
new file mode 100644
index 000000000000..a1bb344fd5f1
--- /dev/null
+++ b/ui/ozone/platform/wayland/test/mock_wp_presentation.h
@@ -0,0 +1,47 @@
+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef UI_OZONE_PLATFORM_WAYLAND_TEST_MOCK_WP_PRESENTATION_H_
+#define UI_OZONE_PLATFORM_WAYLAND_TEST_MOCK_WP_PRESENTATION_H_
+
+#include <presentation-time-server-protocol.h>
+
+#include "base/logging.h"
+#include "base/macros.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "ui/ozone/platform/wayland/test/global_object.h"
+
+namespace wl {
+
+extern const struct wp_presentation_interface kMockWpPresentationImpl;
+
+class MockWpPresentation : public GlobalObject {
+ public:
+  MockWpPresentation();
+  ~MockWpPresentation() override;
+
+  MOCK_METHOD2(Destroy,
+               void(struct wl_client* client, struct wl_resource* resource));
+  MOCK_METHOD4(Feedback,
+               void(struct wl_client* client,
+                    struct wl_resource* resource,
+                    struct wl_resource* surface,
+                    uint32_t callback));
+
+  void set_presentation_callback(wl_resource* callback_resource) {
+    DCHECK(!presentation_callback_);
+    presentation_callback_ = callback_resource;
+  }
+
+  void SendPresentationCallback();
+
+ private:
+  wl_resource* presentation_callback_ = nullptr;
+
+  DISALLOW_COPY_AND_ASSIGN(MockWpPresentation);
+};
+
+}  // namespace wl
+
+#endif  // UI_OZONE_PLATFORM_WAYLAND_TEST_MOCK_WP_PRESENTATION_H_
diff --git a/ui/ozone/platform/wayland/test/test_wayland_server_thread.cc b/ui/ozone/platform/wayland/test/test_wayland_server_thread.cc
index 22b1776ebc4e..1101cccb6922 100644
--- a/ui/ozone/platform/wayland/test/test_wayland_server_thread.cc
+++ b/ui/ozone/platform/wayland/test/test_wayland_server_thread.cc
@@ -107,6 +107,12 @@ void TestWaylandServerThread::Resume() {
   resume_event_.Signal();
 }
 
+MockWpPresentation* TestWaylandServerThread::EnsureWpPresentation() {
+  if (wp_presentation_.Initialize(display_.get()))
+    return &wp_presentation_;
+  return nullptr;
+}
+
 void TestWaylandServerThread::DoPause() {
   base::RunLoop().RunUntilIdle();
   pause_event_.Signal();
diff --git a/ui/ozone/platform/wayland/test/test_wayland_server_thread.h b/ui/ozone/platform/wayland/test/test_wayland_server_thread.h
index fddd426db690..110e3ffbc1c2 100644
--- a/ui/ozone/platform/wayland/test/test_wayland_server_thread.h
+++ b/ui/ozone/platform/wayland/test/test_wayland_server_thread.h
@@ -13,6 +13,7 @@
 #include "base/synchronization/waitable_event.h"
 #include "base/threading/thread.h"
 #include "ui/ozone/platform/wayland/test/global_object.h"
+#include "ui/ozone/platform/wayland/test/mock_wp_presentation.h"
 #include "ui/ozone/platform/wayland/test/mock_xdg_shell.h"
 #include "ui/ozone/platform/wayland/test/mock_zwp_linux_dmabuf.h"
 #include "ui/ozone/platform/wayland/test/test_compositor.h"
@@ -53,6 +54,9 @@ class TestWaylandServerThread : public base::Thread,
   // Resumes the server thread after flushing client connections.
   void Resume();
 
+  // Initializes and returns WpPresentation.
+  MockWpPresentation* EnsureWpPresentation();
+
   template <typename T>
   T* GetObject(uint32_t id) {
     wl_resource* resource = wl_client_get_object(client_, id);
@@ -104,6 +108,7 @@ class TestWaylandServerThread : public base::Thread,
   MockZxdgShellV6 zxdg_shell_v6_;
   TestZwpTextInputManagerV1 zwp_text_input_manager_v1_;
   MockZwpLinuxDmabufV1 zwp_linux_dmabuf_v1_;
+  MockWpPresentation wp_presentation_;
 
   std::vector<std::unique_ptr<GlobalObject>> globals_;
 
-- 
2.22.0