[ozone/wayland] Introduce WaylandBufferManager::Buffer instance

The concept is pretty simple here - instead of using many map
structures, just use a single Buffer structure, which will keep all the
neccessary data we need to create/swap/destroy buffers/wl_buffers.

In other words, the Buffer is both representation of dmabuf on the GPU
side and the wl_buffer on the browser side. And it has a direct
association between the |widget| and the dmabuf.

Note that different Buffers can have the same widget they are attached/
being attached/were attached to.

tl:dr;

The Buffer structure holds all the necessary data to create native
wl_buffers now. That is, it holds zwp linux params variable, which is
used for creation of wl_buffers and asynchronous mapping between
the Buffer and the wl_buffer on CreateSucceeded calls.

Also, the Buffer holds associated widget and a buffer_id, which is
to find and attach a wl_buffer to a wl_surface requested by the GPU
process.

In the future, the Buffer is going to keep a gpu callback, which
will hold a swap result and a presentation feedback of the swap request.

There is no functionality change, but I'm just rather improving the
code.

Bug: 859012, 820047
Change-Id: I7812d10e0a9e7fb472298231a191dfa09701bd2a
Reviewed-on: https://chromium-review.googlesource.com/1193447
Commit-Queue: Maksim Sisov <msisov@igalia.com>
Reviewed-by: Robert Kroeger <rjkroege@chromium.org>
Cr-Commit-Position: refs/heads/master@{#588371}

ui/ozone/platform/wayland/wayland_buffer_manager.cc

@@ -15,6 +15,12 @@
 
 namespace ui {
 
+WaylandBufferManager::Buffer::Buffer() = default;
+WaylandBufferManager::Buffer::Buffer(uint32_t id,
+                                     zwp_linux_buffer_params_v1* zwp_params)
+    : buffer_id(id), params(zwp_params) {}
+WaylandBufferManager::Buffer::~Buffer() = default;
+
 WaylandBufferManager::WaylandBufferManager(
     zwp_linux_dmabuf_v1* zwp_linux_dmabuf,
     WaylandConnection* connection)
@@ -31,8 +37,7 @@ WaylandBufferManager::WaylandBufferManager(
 }
 
 WaylandBufferManager::~WaylandBufferManager() {
-  DCHECK(pending_buffer_map_.empty() && params_to_id_map_.empty() &&
-         buffers_.empty());
+  DCHECK(buffers_.empty());
 }
 
 bool WaylandBufferManager::CreateBuffer(base::File file,
@@ -64,9 +69,11 @@ bool WaylandBufferManager::CreateBuffer(base::File file,
   DCHECK(zwp_linux_dmabuf_);
   struct zwp_linux_buffer_params_v1* params =
       zwp_linux_dmabuf_v1_create_params(zwp_linux_dmabuf_.get());
-  params_to_id_map_.insert(
-      std::pair<struct zwp_linux_buffer_params_v1*, uint32_t>(params,
-                                                              buffer_id));
+
+  std::unique_ptr<Buffer> buffer = std::make_unique<Buffer>(buffer_id, params);
+  buffers_.insert(std::pair<uint32_t, std::unique_ptr<Buffer>>(
+      buffer_id, std::move(buffer)));
+
   uint32_t fd = file.TakePlatformFile();
   for (size_t i = 0; i < planes_count; i++) {
     zwp_linux_buffer_params_v1_add(params, fd, i /* plane id */, offsets[i],
@@ -81,8 +88,8 @@ bool WaylandBufferManager::CreateBuffer(base::File file,
 }
 
 // TODO(msisov): handle buffer swap failure or success.
-bool WaylandBufferManager::SwapBuffer(gfx::AcceleratedWidget widget,
-                                      uint32_t buffer_id) {
+bool WaylandBufferManager::ScheduleBufferSwap(gfx::AcceleratedWidget widget,
+                                              uint32_t buffer_id) {
   TRACE_EVENT1("Wayland", "WaylandBufferManager::SwapBuffer", "Buffer id",
                buffer_id);
 
@@ -90,39 +97,24 @@ bool WaylandBufferManager::SwapBuffer(gfx::AcceleratedWidget widget,
     return false;
 
   auto it = buffers_.find(buffer_id);
-  // A buffer might not exist by this time. So, store the request and process
-  // it once it is created.
   if (it == buffers_.end()) {
-    auto pending_buffers_it = pending_buffer_map_.find(buffer_id);
-    if (pending_buffers_it != pending_buffer_map_.end()) {
-      // If a buffer didn't exist and second call for a swap comes, buffer must
-      // be associated with the same widget.
-      DCHECK_EQ(pending_buffers_it->second, widget);
-    } else {
-      pending_buffer_map_.insert(
-          std::pair<uint32_t, gfx::AcceleratedWidget>(buffer_id, widget));
-    }
-    return true;
-  }
-  struct wl_buffer* buffer = it->second.get();
-
-  WaylandWindow* window = connection_->GetWindow(widget);
-  if (!window) {
-    error_message_ = "A WaylandWindow with current widget does not exist";
+    error_message_ =
+        "Buffer with " + std::to_string(buffer_id) + " id does not exist";
     return false;
   }
 
-  // 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, 0, 0);
-  wl_surface_commit(window->surface());
+  Buffer* buffer = it->second.get();
+  DCHECK(buffer);
 
-  connection_->ScheduleFlush();
+  // Assign a widget to this buffer, which is used to find a corresponding
+  // WaylandWindow.
+  buffer->widget = widget;
+
+  if (buffer->wl_buffer) {
+    // A wl_buffer might not exist by this time. Silently return.
+    // TODO: check this.
+    return SwapBuffer(buffer);
+  }
   return true;
 }
 
@@ -143,8 +135,28 @@ bool WaylandBufferManager::DestroyBuffer(uint32_t buffer_id) {
 
 void WaylandBufferManager::ClearState() {
   buffers_.clear();
-  params_to_id_map_.clear();
-  pending_buffer_map_.clear();
+}
+
+// TODO(msisov): handle buffer swap failure or success.
+bool WaylandBufferManager::SwapBuffer(Buffer* buffer) {
+  WaylandWindow* window = connection_->GetWindow(buffer->widget);
+  if (!window) {
+    error_message_ = "A WaylandWindow with current widget does not exist";
+    return false;
+  }
+
+  // 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);
+  wl_surface_commit(window->surface());
+
+  connection_->ScheduleFlush();
+  return true;
 }
 
 bool WaylandBufferManager::ValidateDataFromGpu(
@@ -187,6 +199,12 @@ bool WaylandBufferManager::ValidateDataFromGpu(
   if (buffer_id < 1)
     reason = "Invalid buffer id: " + std::to_string(buffer_id);
 
+  auto it = buffers_.find(buffer_id);
+  if (it != buffers_.end()) {
+    reason = "A buffer with " + std::to_string(buffer_id) +
+             " id has already existed";
+  }
+
   if (!reason.empty()) {
     error_message_ = std::move(reason);
     return false;
@@ -218,24 +236,21 @@ void WaylandBufferManager::CreateSucceededInternal(
     struct wl_buffer* new_buffer) {
   // Find which buffer id |params| belong to and store wl_buffer
   // with that id.
-  auto it = params_to_id_map_.find(params);
-  CHECK(it != params_to_id_map_.end());
-  uint32_t buffer_id = it->second;
-  params_to_id_map_.erase(params);
-  zwp_linux_buffer_params_v1_destroy(params);
-
-  buffers_.insert(std::pair<uint32_t, wl::Object<wl_buffer>>(
-      buffer_id, wl::Object<wl_buffer>(new_buffer)));
+  Buffer* buffer = nullptr;
+  for (auto& item : buffers_) {
+    if (item.second.get()->params == params) {
+      buffer = item.second.get();
+      break;
+    }
+  }
 
-  TRACE_EVENT1("Wayland", "WaylandBufferManager::CreateSucceeded", "Buffer id",
-               buffer_id);
+  DCHECK(buffer);
+  buffer->wl_buffer.reset(new_buffer);
+  buffer->params = nullptr;
+  zwp_linux_buffer_params_v1_destroy(params);
 
-  auto pending_buffers_it = pending_buffer_map_.find(buffer_id);
-  if (pending_buffers_it != pending_buffer_map_.end()) {
-    gfx::AcceleratedWidget widget = pending_buffers_it->second;
-    pending_buffer_map_.erase(pending_buffers_it);
-    SwapBuffer(widget, buffer_id);
-  }
+  if (buffer->widget != gfx::kNullAcceleratedWidget)
+    ScheduleBufferSwap(buffer->widget, buffer->buffer_id);
 }
 
 // static

ui/ozone/platform/wayland/wayland_buffer_manager.h

@@ -53,7 +53,7 @@ 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 SwapBuffer(gfx::AcceleratedWidget widget, uint32_t buffer_id);
+  bool ScheduleBufferSwap(gfx::AcceleratedWidget widget, uint32_t buffer_id);
 
   // Destroys a buffer with |buffer_id| in |buffers_|. On error, false is
   // returned and |error_message_| is set.
@@ -63,6 +63,35 @@ class WaylandBufferManager {
   void ClearState();
 
  private:
+  // 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 simplier buffer management and other members specific
+  // to this Buffer object on run-time.
+  struct Buffer {
+    Buffer();
+    Buffer(uint32_t id, zwp_linux_buffer_params_v1* zwp_params);
+    ~Buffer();
+
+    // GPU GbmPixmapWayland corresponding buffer id.
+    uint32_t buffer_id = 0;
+
+    // Widget to attached/being attach WaylandWindow.
+    gfx::AcceleratedWidget widget = gfx::kNullAcceleratedWidget;
+
+    // Params that are used to create a wl_buffer.
+    zwp_linux_buffer_params_v1* params = nullptr;
+
+    // A wl_buffer backed by a dmabuf created on the GPU side.
+    wl::Object<wl_buffer> wl_buffer;
+
+    DISALLOW_COPY_AND_ASSIGN(Buffer);
+  };
+
+  bool SwapBuffer(Buffer* buffer);
+
   // Validates data sent from GPU. If invalid, returns false and sets an error
   // message to |error_message_|.
   bool ValidateDataFromGpu(const base::File& file,
@@ -97,25 +126,12 @@ class WaylandBufferManager {
   static void CreateFailed(void* data,
                            struct zwp_linux_buffer_params_v1* params);
 
-  // Stores a wl_buffer and it's id provided by the GbmBuffer object on the
-  // GPU process side.
-  base::flat_map<uint32_t, wl::Object<wl_buffer>> buffers_;
-
-  // A temporary params-to-buffer id map, which is used to identify which
-  // id wl_buffer should be assigned when storing it in the |buffers_| map
-  // during CreateSucceeded call.
-  base::flat_map<struct zwp_linux_buffer_params_v1*, uint32_t>
-      params_to_id_map_;
-
-  // It might happen that GPU asks to swap buffers, when a wl_buffer hasn't
-  // been created yet. Thus, store the request in a pending map. Once a buffer
-  // is created, it will be attached to the requested WaylandWindow based on the
-  // gfx::AcceleratedWidget.
-  base::flat_map<uint32_t, gfx::AcceleratedWidget> pending_buffer_map_;
-
   // Stores announced buffer formats supported by the compositor.
   std::vector<gfx::BufferFormat> supported_buffer_formats_;
 
+  // A container of created buffers.
+  base::flat_map<uint32_t, std::unique_ptr<Buffer>> buffers_;
+
   // Set when invalid data is received from the GPU process.
   std::string error_message_;
 

ui/ozone/platform/wayland/wayland_connection.cc

@@ -177,7 +177,7 @@ void WaylandConnection::DestroyZwpLinuxDmabuf(uint32_t buffer_id) {
 void WaylandConnection::ScheduleBufferSwap(gfx::AcceleratedWidget widget,
                                            uint32_t buffer_id) {
   DCHECK(base::MessageLoopForUI::IsCurrent());
-  if (!buffer_manager_->SwapBuffer(widget, buffer_id)) {
+  if (!buffer_manager_->ScheduleBufferSwap(widget, buffer_id)) {
     TerminateGpuProcess(buffer_manager_->error_message());
   }
 }