[ozone/wayland] Add frame callback support.

Instead of just assuming buffer is committed, use wayland provided
frame callbacks.

Once the GbmSurfaclessWayland (resides on the GPU process side)
requests to swap buffers from Wayland (the connection is established
on the browser process side), it sends a ScheduleBufferSwap requests
and provides a completion callback, which is then triggered once
Wayland sends a frame callback (setup for each new buffer swap).

This callback is then used to identify if the swap was successful
or not.

Bug: 859012
Change-Id: I3264db6bb4456ef610d11945199ecddffbc23024
Reviewed-on: https://chromium-review.googlesource.com/1193882
Commit-Queue: Maksim Sisov <msisov@igalia.com>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Robert Kroeger <rjkroege@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#589457}

ui/gfx/mojo/BUILD.gn

@@ -15,6 +15,7 @@ mojom("mojo") {
     "overlay_transform.mojom",
     "presentation_feedback.mojom",
     "selection_bound.mojom",
+    "swap_result.mojom",
     "transform.mojom",
   ]
 

ui/gfx/mojo/swap_result.mojom

+// Copyright 2018 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.
+
+module gfx.mojom;
+
+// SwapResult information which is used to indicate whether buffer swap
+// succeeded or not. These values correspond to gfx::SwapResult values in
+// ui/gfx/swap_result.h. Currently, it is used by the Ozone/Wayland to identify
+// whether a buffer swap requested by the GPU process has been successful on
+// the browser process side or not.
+enum SwapResult {
+  ACK,
+  FAILED,
+  NAK_RECREATE_BUFFERS,
+};

ui/gfx/mojo/swap_result.typemap

+# Copyright 2018 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.
+
+mojom = "//ui/gfx/mojo/swap_result.mojom"
+public_headers = [ "//ui/gfx/swap_result.h" ]
+traits_headers = [ "//ui/gfx/mojo/swap_result_enum_traits.h" ]
+type_mappings = [ "gfx.mojom.SwapResult=gfx::SwapResult" ]

ui/gfx/mojo/swap_result_enum_traits.h

+// Copyright 2018 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_GFX_MOJO_SWAP_RESULT_ENUM_TRAITS_H_
+#define UI_GFX_MOJO_SWAP_RESULT_ENUM_TRAITS_H_
+
+#include "mojo/public/cpp/bindings/enum_traits.h"
+#include "ui/gfx/mojo/swap_result.mojom-shared.h"
+#include "ui/gfx/swap_result.h"
+
+namespace mojo {
+
+template <>
+struct EnumTraits<gfx::mojom::SwapResult, gfx::SwapResult> {
+  static gfx::mojom::SwapResult ToMojom(gfx::SwapResult input) {
+    switch (input) {
+      case gfx::SwapResult::SWAP_ACK:
+        return gfx::mojom::SwapResult::ACK;
+      case gfx::SwapResult::SWAP_FAILED:
+        return gfx::mojom::SwapResult::FAILED;
+      case gfx::SwapResult::SWAP_NAK_RECREATE_BUFFERS:
+        return gfx::mojom::SwapResult::NAK_RECREATE_BUFFERS;
+    }
+    NOTREACHED();
+    return gfx::mojom::SwapResult::FAILED;
+  }
+
+  static bool FromMojom(gfx::mojom::SwapResult input, gfx::SwapResult* out) {
+    switch (input) {
+      case gfx::mojom::SwapResult::ACK:
+        *out = gfx::SwapResult::SWAP_ACK;
+        return true;
+      case gfx::mojom::SwapResult::FAILED:
+        *out = gfx::SwapResult::SWAP_FAILED;
+        return true;
+      case gfx::mojom::SwapResult::NAK_RECREATE_BUFFERS:
+        *out = gfx::SwapResult::SWAP_NAK_RECREATE_BUFFERS;
+        return true;
+    }
+    NOTREACHED();
+    return false;
+  }
+};
+
+}  // namespace mojo
+
+#endif  // UI_GFX_MOJO_SWAP_RESULT_ENUM_TRAITS_H_

ui/gfx/typemaps.gni

@@ -13,6 +13,7 @@ typemaps = [
   "//ui/gfx/mojo/gpu_fence_handle.typemap",
   "//ui/gfx/mojo/overlay_transform.typemap",
   "//ui/gfx/mojo/presentation_feedback.typemap",
+  "//ui/gfx/mojo/swap_result.typemap",
   "//ui/gfx/mojo/selection_bound.typemap",
   "//ui/gfx/mojo/transform.typemap",
   "//ui/gfx/range/mojo/range.typemap",

ui/ozone/platform/wayland/gpu/gbm_surfaceless_wayland.cc

@@ -198,16 +198,12 @@ void GbmSurfacelessWayland::SubmitFrame() {
       return;
     }
 
+    auto callback =
+        base::BindOnce(&GbmSurfacelessWayland::OnScheduleBufferSwapDone,
+                       weak_factory_.GetWeakPtr());
     uint32_t buffer_id = planes_.back().pixmap->GetUniqueId();
-    surface_factory_->ScheduleBufferSwap(widget_, buffer_id);
-
-    // Check comment in ::SupportsPresentationCallback.
-    OnSubmission(gfx::SwapResult::SWAP_ACK, nullptr);
-    OnPresentation(
-        gfx::PresentationFeedback(base::TimeTicks::Now(), base::TimeDelta(),
-                                  gfx::PresentationFeedback::kZeroCopy));
-
-    planes_.clear();
+    surface_factory_->ScheduleBufferSwap(widget_, buffer_id,
+                                         std::move(callback));
   }
 }
 
@@ -224,6 +220,15 @@ void GbmSurfacelessWayland::FenceRetired(PendingFrame* frame) {
   SubmitFrame();
 }
 
+void GbmSurfacelessWayland::OnScheduleBufferSwapDone(gfx::SwapResult result) {
+  OnSubmission(result, nullptr);
+  // TODO: presentation.
+  OnPresentation(
+      gfx::PresentationFeedback(base::TimeTicks::Now(), base::TimeDelta(),
+                                gfx::PresentationFeedback::kZeroCopy));
+  planes_.clear();
+}
+
 void GbmSurfacelessWayland::OnSubmission(
     gfx::SwapResult result,
     std::unique_ptr<gfx::GpuFence> out_fence) {

ui/ozone/platform/wayland/gpu/gbm_surfaceless_wayland.h

@@ -78,6 +78,7 @@ class GbmSurfacelessWayland : public gl::SurfacelessEGL {
   EGLSyncKHR InsertFence(bool implicit);
   void FenceRetired(PendingFrame* frame);
 
+  void OnScheduleBufferSwapDone(gfx::SwapResult result);
   void OnSubmission(gfx::SwapResult result,
                     std::unique_ptr<gfx::GpuFence> out_fence);
   void OnPresentation(const gfx::PresentationFeedback& feedback);

ui/ozone/platform/wayland/gpu/wayland_connection_proxy.cc

@@ -86,11 +86,13 @@ void WaylandConnectionProxy::DestroyZwpLinuxDmabufInternal(uint32_t buffer_id) {
   wc_ptr_->DestroyZwpLinuxDmabuf(buffer_id);
 }
 
-void WaylandConnectionProxy::ScheduleBufferSwap(gfx::AcceleratedWidget widget,
-                                                uint32_t buffer_id) {
+void WaylandConnectionProxy::ScheduleBufferSwap(
+    gfx::AcceleratedWidget widget,
+    uint32_t buffer_id,
+    wl::BufferSwapCallback callback) {
   DCHECK(gpu_thread_runner_->BelongsToCurrentThread());
   DCHECK(wc_ptr_);
-  wc_ptr_->ScheduleBufferSwap(widget, buffer_id);
+  wc_ptr_->ScheduleBufferSwap(widget, buffer_id, std::move(callback));
 }
 
 WaylandWindow* WaylandConnectionProxy::GetWindow(

ui/ozone/platform/wayland/gpu/wayland_connection_proxy.h

@@ -11,6 +11,7 @@
 #include "mojo/public/cpp/bindings/binding_set.h"
 #include "ui/gfx/native_widget_types.h"
 #include "ui/ozone/platform/wayland/wayland_connection.h"
+#include "ui/ozone/platform/wayland/wayland_util.h"
 #include "ui/ozone/public/interfaces/wayland/wayland_connection.mojom.h"
 
 #if defined(WAYLAND_GBM)
@@ -19,6 +20,10 @@
 
 struct wl_shm;
 
+namespace gfx {
+enum class SwapResult;
+}
+
 namespace ui {
 
 class WaylandConnection;
@@ -58,7 +63,11 @@ class WaylandConnectionProxy : public ozone::mojom::WaylandConnectionClient {
 
   // Asks Wayland to find a wl_buffer with the |buffer_id| and schedule a
   // buffer swap for a WaylandWindow, which backs the following |widget|.
-  void ScheduleBufferSwap(gfx::AcceleratedWidget widget, uint32_t buffer_id);
+  // The |callback| is called once a frame callback from the Wayland server
+  // is received.
+  void ScheduleBufferSwap(gfx::AcceleratedWidget widget,
+                          uint32_t buffer_id,
+                          wl::BufferSwapCallback callback);
 
 #if defined(WAYLAND_GBM)
   // Returns a gbm_device based on a DRM render node.

ui/ozone/platform/wayland/wayland_buffer_manager.cc

@@ -89,9 +89,10 @@ bool WaylandBufferManager::CreateBuffer(base::File file,
 
 // TODO(msisov): handle buffer swap failure or success.
 bool WaylandBufferManager::ScheduleBufferSwap(gfx::AcceleratedWidget widget,
-                                              uint32_t buffer_id) {
-  TRACE_EVENT1("Wayland", "WaylandBufferManager::SwapBuffer", "Buffer id",
-               buffer_id);
+                                              uint32_t buffer_id,
+                                              wl::BufferSwapCallback callback) {
+  TRACE_EVENT1("Wayland", "WaylandBufferManager::ScheduleSwapBuffer",
+               "Buffer id", buffer_id);
 
   if (!ValidateDataFromGpu(widget, buffer_id))
     return false;
@@ -109,6 +110,7 @@ bool WaylandBufferManager::ScheduleBufferSwap(gfx::AcceleratedWidget widget,
   // Assign a widget to this buffer, which is used to find a corresponding
   // WaylandWindow.
   buffer->widget = widget;
+  buffer->buffer_swap_callback = std::move(callback);
 
   if (buffer->wl_buffer) {
     // A wl_buffer might not exist by this time. Silently return.
@@ -127,6 +129,11 @@ bool WaylandBufferManager::DestroyBuffer(uint32_t buffer_id) {
     error_message_ = "Trying to destroy non-existing buffer";
     return false;
   }
+  // It can happen that a buffer is destroyed before a frame callback comes.
+  // Thus, just mark this as a successful swap, which is ok to do.
+  Buffer* buffer = it->second.get();
+  if (!buffer->buffer_swap_callback.is_null())
+    std::move(buffer->buffer_swap_callback).Run(gfx::SwapResult::SWAP_ACK);
   buffers_.erase(it);
 
   connection_->ScheduleFlush();
@@ -139,6 +146,9 @@ void WaylandBufferManager::ClearState() {
 
 // TODO(msisov): handle buffer swap failure or success.
 bool WaylandBufferManager::SwapBuffer(Buffer* buffer) {
+  TRACE_EVENT1("Wayland", "WaylandBufferManager::SwapBuffer", "Buffer id",
+               buffer->buffer_id);
+
   WaylandWindow* window = connection_->GetWindow(buffer->widget);
   if (!window) {
     error_message_ = "A WaylandWindow with current widget does not exist";
@@ -147,12 +157,17 @@ bool WaylandBufferManager::SwapBuffer(Buffer* buffer) {
 
   // TODO(msisov): it would be beneficial to use real damage regions to improve
   // performance.
-  //
-  // TODO(msisov): also start using wl_surface_frame callbacks for better
-  // performance.
   wl_surface_damage(window->surface(), 0, 0, window->GetBounds().width(),
                     window->GetBounds().height());
   wl_surface_attach(window->surface(), buffer->wl_buffer.get(), 0, 0);
+
+  static const wl_callback_listener frame_listener = {
+      WaylandBufferManager::FrameCallbackDone};
+  DCHECK(!buffer->wl_frame_callback);
+  buffer->wl_frame_callback.reset(wl_surface_frame(window->surface()));
+  wl_callback_add_listener(buffer->wl_frame_callback.get(), &frame_listener,
+                           this);
+
   wl_surface_commit(window->surface());
 
   connection_->ScheduleFlush();
@@ -250,7 +265,7 @@ void WaylandBufferManager::CreateSucceededInternal(
   zwp_linux_buffer_params_v1_destroy(params);
 
   if (buffer->widget != gfx::kNullAcceleratedWidget)
-    ScheduleBufferSwap(buffer->widget, buffer->buffer_id);
+    SwapBuffer(buffer);
 }
 
 // static
@@ -294,4 +309,22 @@ void WaylandBufferManager::CreateFailed(
   LOG(FATAL) << "zwp_linux_buffer_params.create failed";
 }
 
+// static
+void WaylandBufferManager::FrameCallbackDone(void* data,
+                                             wl_callback* callback,
+                                             uint32_t time) {
+  WaylandBufferManager* self = static_cast<WaylandBufferManager*>(data);
+  DCHECK(self);
+  for (auto& item : self->buffers_) {
+    Buffer* buffer = item.second.get();
+    if (buffer->wl_frame_callback.get() == callback) {
+      std::move(buffer->buffer_swap_callback).Run(gfx::SwapResult::SWAP_ACK);
+      buffer->wl_frame_callback.reset();
+      return;
+    }
+  }
+
+  NOTREACHED();
+}
+
 }  // namespace ui

ui/ozone/platform/wayland/wayland_buffer_manager.h

@@ -13,6 +13,7 @@
 #include "base/macros.h"
 #include "ui/gfx/native_widget_types.h"
 #include "ui/ozone/platform/wayland/wayland_object.h"
+#include "ui/ozone/platform/wayland/wayland_util.h"
 
 struct zwp_linux_dmabuf_v1;
 struct zwp_linux_buffer_params_v1;
@@ -53,7 +54,9 @@ class WaylandBufferManager {
 
   // Assigns a wl_buffer with |buffer_id| to a window with the same |widget|. On
   // error, false is returned and |error_message_| is set.
-  bool ScheduleBufferSwap(gfx::AcceleratedWidget widget, uint32_t buffer_id);
+  bool ScheduleBufferSwap(gfx::AcceleratedWidget widget,
+                          uint32_t buffer_id,
+                          wl::BufferSwapCallback callback);
 
   // Destroys a buffer with |buffer_id| in |buffers_|. On error, false is
   // returned and |error_message_| is set.
@@ -87,6 +90,15 @@ class WaylandBufferManager {
     // A wl_buffer backed by a dmabuf created on the GPU side.
     wl::Object<wl_buffer> wl_buffer;
 
+    // A callback, which is called once the |wl_frame_callback| from the server
+    // is received.
+    wl::BufferSwapCallback buffer_swap_callback;
+
+    // A Wayland callback, which is triggered once wl_buffer has been committed
+    // and it is right time to notify the GPU that it can start a new drawing
+    // operation.
+    wl::Object<wl_callback> wl_frame_callback;
+
     DISALLOW_COPY_AND_ASSIGN(Buffer);
   };
 
@@ -126,6 +138,11 @@ class WaylandBufferManager {
   static void CreateFailed(void* data,
                            struct zwp_linux_buffer_params_v1* params);
 
+  // wl_callback_listener
+  static void FrameCallbackDone(void* data,
+                                wl_callback* callback,
+                                uint32_t time);
+
   // Stores announced buffer formats supported by the compositor.
   std::vector<gfx::BufferFormat> supported_buffer_formats_;
 

ui/ozone/platform/wayland/wayland_connection.cc

@@ -14,6 +14,7 @@
 #include "base/message_loop/message_loop_current.h"
 #include "base/strings/string_util.h"
 #include "base/threading/thread_task_runner_handle.h"
+#include "ui/gfx/swap_result.h"
 #include "ui/ozone/platform/wayland/wayland_buffer_manager.h"
 #include "ui/ozone/platform/wayland/wayland_object.h"
 #include "ui/ozone/platform/wayland/wayland_window.h"
@@ -182,10 +183,13 @@ void WaylandConnection::DestroyZwpLinuxDmabuf(uint32_t buffer_id) {
   }
 }
 
-void WaylandConnection::ScheduleBufferSwap(gfx::AcceleratedWidget widget,
-                                           uint32_t buffer_id) {
+void WaylandConnection::ScheduleBufferSwap(
+    gfx::AcceleratedWidget widget,
+    uint32_t buffer_id,
+    ScheduleBufferSwapCallback callback) {
   DCHECK(base::MessageLoopForUI::IsCurrent());
-  if (!buffer_manager_->ScheduleBufferSwap(widget, buffer_id)) {
+  if (!buffer_manager_->ScheduleBufferSwap(widget, buffer_id,
+                                           std::move(callback))) {
     TerminateGpuProcess(buffer_manager_->error_message());
   }
 }

ui/ozone/platform/wayland/wayland_connection.h

@@ -60,7 +60,8 @@ class WaylandConnection : public PlatformEventSource,
   // Called by the GPU and asks to attach a wl_buffer with a |buffer_id| to a
   // WaylandWindow with the specified |widget|.
   void ScheduleBufferSwap(gfx::AcceleratedWidget widget,
-                          uint32_t buffer_id) override;
+                          uint32_t buffer_id,
+                          ScheduleBufferSwapCallback callback) override;
 
   // Schedules a flush of the Wayland connection.
   void ScheduleFlush();

ui/ozone/platform/wayland/wayland_surface_factory.cc

@@ -231,9 +231,11 @@ GbmSurfacelessWayland* WaylandSurfaceFactory::GetSurface(
   return it->second;
 }
 
-void WaylandSurfaceFactory::ScheduleBufferSwap(gfx::AcceleratedWidget widget,
-                                               uint32_t buffer_id) {
-  connection_->ScheduleBufferSwap(widget, buffer_id);
+void WaylandSurfaceFactory::ScheduleBufferSwap(
+    gfx::AcceleratedWidget widget,
+    uint32_t buffer_id,
+    wl::BufferSwapCallback callback) {
+  connection_->ScheduleBufferSwap(widget, buffer_id, std::move(callback));
 }
 
 std::unique_ptr<SurfaceOzoneCanvas>

ui/ozone/platform/wayland/wayland_surface_factory.h

@@ -13,11 +13,12 @@
 #include "base/posix/eintr_wrapper.h"
 #include "base/single_thread_task_runner.h"
 #include "base/threading/sequenced_task_runner_handle.h"
+#include "ui/ozone/platform/wayland/wayland_util.h"
 
 namespace ui {
 
-class WaylandConnectionProxy;
 class GbmSurfacelessWayland;
+class WaylandConnectionProxy;
 
 class WaylandSurfaceFactory : public SurfaceFactoryOzone {
  public:
@@ -25,7 +26,9 @@ class WaylandSurfaceFactory : public SurfaceFactoryOzone {
   ~WaylandSurfaceFactory() override;
 
   // These methods are used, when a dmabuf based approach is used.
-  void ScheduleBufferSwap(gfx::AcceleratedWidget widget, uint32_t buffer_id);
+  void ScheduleBufferSwap(gfx::AcceleratedWidget widget,
+                          uint32_t buffer_id,
+                          wl::BufferSwapCallback callback);
   void RegisterSurface(gfx::AcceleratedWidget widget,
                        GbmSurfacelessWayland* surface);
   void UnregisterSurface(gfx::AcceleratedWidget widget);

ui/ozone/platform/wayland/wayland_util.h

@@ -7,6 +7,7 @@
 
 #include <wayland-client.h>
 
+#include "base/callback.h"
 #include "base/macros.h"
 #include "ui/ozone/platform/wayland/wayland_object.h"
 
@@ -18,10 +19,14 @@ class SharedMemory;
 
 namespace gfx {
 class Size;
+enum class SwapResult;
 }
 
 namespace wl {
 
+// Corresponds to mojom::WaylandConnection::ScheduleBufferSwapCallback.
+using BufferSwapCallback = base::OnceCallback<void(gfx::SwapResult)>;
+
 wl_buffer* CreateSHMBuffer(const gfx::Size& size,
                            base::SharedMemory* shared_memory,
                            wl_shm* shm);

ui/ozone/public/interfaces/wayland/wayland_connection.mojom

@@ -7,6 +7,7 @@ module ui.ozone.mojom;
 import "mojo/public/mojom/base/file.mojom";
 import "mojo/public/mojom/base/file_path.mojom";
 import "ui/gfx/mojo/accelerated_widget.mojom";
+import "ui/gfx/mojo/swap_result.mojom";
 
 // Used by the GPU for communication with a WaylandConnection on the browser
 // process.
@@ -26,7 +27,8 @@ interface WaylandConnection {
   DestroyZwpLinuxDmabuf(uint32 buffer_id);
 
   // Swaps wl_buffers for a WaylandWindow with the following |widget|.
-  ScheduleBufferSwap(gfx.mojom.AcceleratedWidget widget, uint32 buffer_id);
+  ScheduleBufferSwap(gfx.mojom.AcceleratedWidget widget, uint32 buffer_id)
+        => (gfx.mojom.SwapResult swap_result);
 };
 
 // Used by the browser process to provide the GPU process with a mojo ptr to a