[ozone/wayland]: Allow to create wl_buffers for null widgets.

The SkiaRenderer uses an image factory that creates anonymous images
and native pixmaps. That means a GbmPixmapWayland does not
receive an AcceleratedWidget that it will be attached to until a
buffer commit request comes.

With the implementation we have at the moment, that results in
GbmPixmapWayland being a staging buffer without its counterpart
created on the browser process side.

However, when the SkiaRenderer feature is enabled, we want to create a
GbmPixmapWayland with a null widget passed and create a dmabuf based wl_buffer
for it. Let's call that buffer an "Anonymous buffer".

But that slightly contradicts with the implementation of the GbmPixmapWayland
and the WaylandBufferManagerHost classes. The problem is that the
GbmPixmapWayland may not send requests to create dmabuf based wl_buffers if a
widget is null. The manager simply fires a gpu termination callback if
GbmPixmapWayland sends a request to create a wl_buffer for a null widget.

To fix the problem, I had to change the implementation a little bit
and allow it to create wl_buffers for null widgets.

Now, the GbmPixmapWayland always asks the host manager to create wl_buffers
regardless of the AcceleratedWidget's id. And to be able to process
these requests, a new logic to store these wl_buffers has been added.

Previously, the manager host always delegated creation of WaylandBuffers
(which is an internal buffer representation) to surfaces. And whenever
wl_buffers were created, AttachWlBuffer was called.

Now, the manager may create WaylandBuffers by itself if there is no
widget passed and it is impossible to know the surface that must own
that buffer. When a CommitBuffer request comes, a surface searches
for the buffer in its own container and if it is not found, it asks
the WaylandBufferManagerHost to search for an anonymous buffer and
pass the ownership of that buffer to that surface.

Then, the normal flow continues.

Bug: 1008364
Change-Id: I5c582381afc3e16b589d53ceb6ba774ac6da8f5d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1837614
Commit-Queue: Maksim Sisov <msisov@igalia.com>
Reviewed-by: Robert Kroeger <rjkroege@chromium.org>
Cr-Commit-Position: refs/heads/master@{#702743}

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

@@ -28,12 +28,11 @@
 
 namespace ui {
 
-GbmPixmapWayland::GbmPixmapWayland(WaylandBufferManagerGpu* buffer_manager,
-                                   gfx::AcceleratedWidget widget)
-    : buffer_manager_(buffer_manager), widget_(widget) {}
+GbmPixmapWayland::GbmPixmapWayland(WaylandBufferManagerGpu* buffer_manager)
+    : buffer_manager_(buffer_manager) {}
 
 GbmPixmapWayland::~GbmPixmapWayland() {
-  if (gbm_bo_ && widget_ != gfx::kNullAcceleratedWidget)
+  if (gbm_bo_)
     buffer_manager_->DestroyBuffer(widget_, GetUniqueId());
 }
 
@@ -81,13 +80,16 @@ bool GbmPixmapWayland::InitializeBuffer(gfx::Size size,
     return false;
   }
 
-  // The pixmap can be created as a staging buffer and not be mapped to any of
-  // the existing widgets.
-  if (widget_ != gfx::kNullAcceleratedWidget)
-    CreateDmabufBasedBuffer();
+  CreateDmabufBasedBuffer();
   return true;
 }
 
+void GbmPixmapWayland::SetAcceleratedWiget(gfx::AcceleratedWidget widget) {
+  DCHECK(widget != gfx::kNullAcceleratedWidget);
+  DCHECK(widget_ == gfx::kNullAcceleratedWidget);
+  widget_ = widget;
+}
+
 bool GbmPixmapWayland::AreDmaBufFdsValid() const {
   return gbm_bo_->AreFdsValid();
 }
@@ -136,6 +138,12 @@ bool GbmPixmapWayland::ScheduleOverlayPlane(
     const gfx::RectF& crop_rect,
     bool enable_blend,
     std::unique_ptr<gfx::GpuFence> gpu_fence) {
+  // If the widget this pixmap backs has not been assigned before, do it now.
+  if (widget_ == gfx::kNullAcceleratedWidget)
+    SetAcceleratedWiget(widget);
+
+  DCHECK_EQ(widget_, widget);
+
   auto* surface = buffer_manager_->GetSurface(widget);
   // This must never be hit.
   DCHECK(surface);

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

@@ -21,14 +21,17 @@ class WaylandBufferManagerGpu;
 
 class GbmPixmapWayland : public gfx::NativePixmap {
  public:
-  GbmPixmapWayland(WaylandBufferManagerGpu* buffer_manager,
-                   gfx::AcceleratedWidget widget);
+  explicit GbmPixmapWayland(WaylandBufferManagerGpu* buffer_manager);
 
   // Creates a buffer object and initializes the pixmap buffer.
   bool InitializeBuffer(gfx::Size size,
                         gfx::BufferFormat format,
                         gfx::BufferUsage usage);
 
+  // The widget that this pixmap backs can be assigned later. Can be assigned
+  // only once.
+  void SetAcceleratedWiget(gfx::AcceleratedWidget widget);
+
   // gfx::NativePixmap overrides:
   bool AreDmaBufFdsValid() const override;
   int GetDmaBufFd(size_t plane) const override;
@@ -62,7 +65,7 @@ class GbmPixmapWayland : public gfx::NativePixmap {
   WaylandBufferManagerGpu* const buffer_manager_;
 
   // Represents widget this pixmap backs.
-  const gfx::AcceleratedWidget widget_;
+  gfx::AcceleratedWidget widget_ = gfx::kNullAcceleratedWidget;
 
   DISALLOW_COPY_AND_ASSIGN(GbmPixmapWayland);
 };

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

@@ -162,7 +162,11 @@ scoped_refptr<gfx::NativePixmap> WaylandSurfaceFactory::CreateNativePixmap(
     gfx::BufferUsage usage) {
 #if defined(WAYLAND_GBM)
   scoped_refptr<GbmPixmapWayland> pixmap =
-      base::MakeRefCounted<GbmPixmapWayland>(buffer_manager_, widget);
+      base::MakeRefCounted<GbmPixmapWayland>(buffer_manager_);
+
+  if (widget != gfx::kNullAcceleratedWidget)
+    pixmap->SetAcceleratedWiget(widget);
+
   if (!pixmap->InitializeBuffer(size, format, usage))
     return nullptr;
   return pixmap;

ui/ozone/platform/wayland/host/wayland_buffer_manager_host.h

@@ -31,6 +31,51 @@ namespace ui {
 class WaylandConnection;
 class WaylandWindow;
 
+// This is an internal helper representation of a wayland buffer object, which
+// the GPU process creates when CreateBuffer is called. It's used for
+// asynchronous buffer creation and stores |params| parameter to find out,
+// which Buffer the wl_buffer corresponds to when CreateSucceeded is called.
+// What is more, the Buffer stores such information as a widget it is attached
+// to, its buffer id for simpler buffer management and other members specific
+// to this Buffer object on run-time.
+struct WaylandBuffer {
+  WaylandBuffer() = delete;
+  WaylandBuffer(const gfx::Size& size, uint32_t buffer_id);
+  ~WaylandBuffer();
+
+  // Actual buffer size.
+  const gfx::Size size;
+
+  // Damage region this buffer describes. Must be emptied once buffer is
+  // submitted.
+  gfx::Rect damage_region;
+
+  // The id of this buffer.
+  const uint32_t buffer_id;
+
+  // A wl_buffer backed by a dmabuf created on the GPU side.
+  wl::Object<struct wl_buffer> wl_buffer;
+
+  // Tells if the buffer has the wl_buffer attached. This can be used to
+  // identify potential problems, when the Wayland compositor fails to create
+  // wl_buffers.
+  bool attached = false;
+
+  // Tells if the buffer has already been released aka not busy, and the
+  // 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);
+};
+
 // This is the buffer manager which creates wl_buffers based on dmabuf (hw
 // accelerated compositing) or shared memory (software compositing) and uses
 // internal representation of surfaces, which are used to store buffers
@@ -101,6 +146,9 @@ class WaylandBufferManagerHost : public ozone::mojom::WaylandBufferManagerHost,
   // contents of a hidden surface on their screens.
   void ResetSurfaceContents(gfx::AcceleratedWidget widget);
 
+  // Returns the anonymously created WaylandBuffer.
+  std::unique_ptr<WaylandBuffer> PassAnonymousWlBuffer(uint32_t buffer_id);
+
  private:
   // This is an internal representation of a real surface, which holds a pointer
   // to WaylandWindow. Also, this object holds buffers, frame callbacks and
@@ -166,6 +214,11 @@ class WaylandBufferManagerHost : public ozone::mojom::WaylandBufferManagerHost,
   // data sent by the GPU to the browser process.
   base::OnceCallback<void(std::string)> terminate_gpu_cb_;
 
+  // Contains anonymous buffers aka buffers that are not attached to any of the
+  // existing surfaces and that will be mapped to surfaces later.  Typically
+  // created when CreateAnonymousImage is called on the gpu process side.
+  base::flat_map<uint32_t, std::unique_ptr<WaylandBuffer>> anonymous_buffers_;
+
   base::WeakPtrFactory<WaylandBufferManagerHost> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(WaylandBufferManagerHost);

ui/ozone/platform/wayland/wayland_buffer_manager_unittest.cc

@@ -532,6 +532,195 @@ TEST_P(WaylandBufferManagerTest, TestCommitBufferConditions) {
   Sync();
 }
 
+TEST_P(WaylandBufferManagerTest, HandleAnonymousBuffers) {
+  constexpr uint32_t kBufferId1 = 1;
+  constexpr uint32_t kBufferId2 = 2;
+
+  const gfx::AcceleratedWidget widget = window_->GetWidget();
+  constexpr gfx::AcceleratedWidget null_widget = gfx::kNullAcceleratedWidget;
+
+  // This section tests that it is impossible to create buffers with the same
+  // id regardless of the passed widget.
+  {
+    EXPECT_CALL(*server_.zwp_linux_dmabuf_v1(), CreateParams(_, _, _)).Times(2);
+    CreateDmabufBasedBufferAndSetTerminateExpecation(false /*fail*/,
+                                                     null_widget, kBufferId1);
+    CreateDmabufBasedBufferAndSetTerminateExpecation(false /*fail*/, widget,
+                                                     kBufferId2);
+
+    // Can't create buffer with existing id.
+    CreateDmabufBasedBufferAndSetTerminateExpecation(true /*fail*/, null_widget,
+                                                     kBufferId2);
+    // Can't destroy buffer with non-existing id (the manager cleared the
+    // state after the previous failure).
+    DestroyBufferAndSetTerminateExpectation(null_widget, kBufferId2,
+                                            true /*fail*/);
+  }
+
+  // Tests that can't destroy anonymous buffer with the same id as the
+  // previous non-anonymous buffer and other way round.
+  {
+    EXPECT_CALL(*server_.zwp_linux_dmabuf_v1(), CreateParams(_, _, _)).Times(2);
+    // Same id for anonymous and non-anonymous.
+    CreateDmabufBasedBufferAndSetTerminateExpecation(false /*fail*/, widget,
+                                                     kBufferId1);
+    DestroyBufferAndSetTerminateExpectation(null_widget, kBufferId1,
+                                            true /*fail*/);
+
+    // Same id for non-anonymous and anonymous.
+    CreateDmabufBasedBufferAndSetTerminateExpecation(false /*fail*/,
+                                                     null_widget, kBufferId1);
+    DestroyBufferAndSetTerminateExpectation(widget, kBufferId1, true
+                                            /*fail*/);
+  }
+
+  // This section tests that it is impossible to destroy buffers with
+  // non-existing ids (for example, if the have already been destroyed) for
+  // anonymous buffers.
+  {
+    EXPECT_CALL(*server_.zwp_linux_dmabuf_v1(), CreateParams(_, _, _)).Times(1);
+    CreateDmabufBasedBufferAndSetTerminateExpecation(false
+                                                     /*fail*/,
+                                                     null_widget, kBufferId1);
+    DestroyBufferAndSetTerminateExpectation(null_widget, kBufferId1, false
+                                            /*fail*/);
+    // Can't destroy the same buffer twice (non-existing id).
+    DestroyBufferAndSetTerminateExpectation(null_widget, kBufferId1, true
+                                            /*fail*/);
+  }
+
+  // Makes sure the anonymous buffer can be attached to a surface and
+  // destroyed.
+  {
+    EXPECT_CALL(*server_.zwp_linux_dmabuf_v1(), CreateParams(_, _, _)).Times(1);
+    CreateDmabufBasedBufferAndSetTerminateExpecation(false
+                                                     /*fail*/,
+                                                     null_widget, kBufferId1);
+
+    buffer_manager_gpu_->CommitBuffer(widget, kBufferId1, window_->GetBounds());
+
+    // Now, we must be able to destroy this buffer with widget provided. That
+    // is, if the buffer has been attached to a surface, it can be destroyed.
+    DestroyBufferAndSetTerminateExpectation(widget, kBufferId1, false
+                                            /*fail*/);
+
+    // And now test we can't destroy the same buffer providing a null widget.
+    DestroyBufferAndSetTerminateExpectation(null_widget, kBufferId1, true
+                                            /*fail*/);
+  }
+}
+
+// The buffer that is not originally attached to any of the surfaces,
+// must be attached when a commit request comes. Also, it must setup a buffer
+// release listener and OnSubmission must be called for that buffer if it is
+// released.
+TEST_P(WaylandBufferManagerTest, AnonymousBufferAttachedAndReleased) {
+  constexpr uint32_t kBufferId1 = 1;
+  constexpr uint32_t kBufferId2 = 2;
+  constexpr uint32_t kBufferId3 = 3;
+
+  const gfx::AcceleratedWidget widget = window_->GetWidget();
+  const gfx::Rect bounds = gfx::Rect({0, 0}, kDefaultSize);
+  window_->SetBounds(bounds);
+
+  MockSurfaceGpu mock_surface_gpu(buffer_manager_gpu_.get(), widget_);
+
+  auto* linux_dmabuf = server_.zwp_linux_dmabuf_v1();
+  EXPECT_CALL(*linux_dmabuf, CreateParams(_, _, _)).Times(1);
+  CreateDmabufBasedBufferAndSetTerminateExpecation(
+      false /*fail*/, gfx::kNullAcceleratedWidget, kBufferId1);
+
+  Sync();
+
+  ProcessCreatedBufferResourcesWithExpectation(1u /* expected size */,
+                                               false /* fail */);
+
+  auto* mock_surface = server_.GetObject<wl::MockSurface>(widget);
+
+  constexpr uint32_t kNumberOfCommits = 3;
+  EXPECT_CALL(*mock_surface, Attach(_, _, _)).Times(kNumberOfCommits);
+  EXPECT_CALL(*mock_surface, Frame(_)).Times(kNumberOfCommits);
+  EXPECT_CALL(*mock_surface, Commit()).Times(kNumberOfCommits);
+
+  // All the other expectations must come in order.
+  ::testing::InSequence sequence;
+  EXPECT_CALL(mock_surface_gpu,
+              OnSubmission(kBufferId1, gfx::SwapResult::SWAP_ACK))
+      .Times(1);
+  EXPECT_CALL(mock_surface_gpu, OnPresentation(kBufferId1, _)).Times(1);
+
+  // Commit second buffer now.
+  buffer_manager_gpu_->CommitBuffer(widget, kBufferId1, bounds);
+
+  Sync();
+
+  mock_surface->SendFrameCallback();
+
+  Sync();
+
+  // Now synchronously create a second buffer and commit it. The release
+  // callback must be setup and OnSubmission must be called.
+  EXPECT_CALL(*linux_dmabuf, CreateParams(_, _, _)).Times(1);
+  CreateDmabufBasedBufferAndSetTerminateExpecation(
+      false /*fail*/, gfx::kNullAcceleratedWidget, kBufferId2);
+
+  Sync();
+
+  ProcessCreatedBufferResourcesWithExpectation(1u /* expected size */,
+                                               false /* fail */);
+
+  EXPECT_CALL(mock_surface_gpu,
+              OnSubmission(kBufferId2, gfx::SwapResult::SWAP_ACK))
+      .Times(1);
+  EXPECT_CALL(mock_surface_gpu, OnPresentation(kBufferId2, _)).Times(1);
+
+  // Commit second buffer now.
+  buffer_manager_gpu_->CommitBuffer(widget, kBufferId2, bounds);
+
+  Sync();
+
+  mock_surface->ReleasePrevAttachedBuffer();
+
+  Sync();
+
+  mock_surface->SendFrameCallback();
+
+  // Now asynchronously create another buffer so that a commit request
+  // comes earlier than it is created by the Wayland compositor, but it can
+  // released once the buffer is committed and processed (that is, it must be
+  // able to setup a buffer release callback).
+  EXPECT_CALL(*linux_dmabuf, CreateParams(_, _, _)).Times(1);
+  CreateDmabufBasedBufferAndSetTerminateExpecation(
+      false /*fail*/, gfx::kNullAcceleratedWidget, kBufferId3);
+
+  Sync();
+
+  EXPECT_CALL(mock_surface_gpu,
+              OnSubmission(kBufferId3, gfx::SwapResult::SWAP_ACK))
+      .Times(0);
+  EXPECT_CALL(mock_surface_gpu, OnPresentation(kBufferId3, _)).Times(0);
+
+  buffer_manager_gpu_->CommitBuffer(widget, kBufferId3, bounds);
+
+  Sync();
+
+  EXPECT_CALL(mock_surface_gpu,
+              OnSubmission(kBufferId3, gfx::SwapResult::SWAP_ACK))
+      .Times(1);
+  EXPECT_CALL(mock_surface_gpu, OnPresentation(kBufferId3, _)).Times(1);
+
+  // Now, create the buffer from the Wayland compositor side and let the buffer
+  // manager complete the commit request.
+  ProcessCreatedBufferResourcesWithExpectation(1u /* expected size */,
+                                               false /* fail */);
+
+  Sync();
+
+  mock_surface->ReleasePrevAttachedBuffer();
+
+  Sync();
+}
+
 INSTANTIATE_TEST_SUITE_P(XdgVersionV5Test,
                          WaylandBufferManagerTest,
                          ::testing::Values(kXdgShellV5));