[ozone/wayland] Gardening: move buffer management into own class.

This is a preliminary work before introducing frame and presentation
callbacks.

Summary: It's too much for WaylandConnection to hold that much
functionality. Instead, move the buffer management mechanism to its own
WaylandBufferManager class.

It's mostly moving code around with no functionality changes except
that CreateBuffer, SwapBuffer and DestroyBuffer return the result of
their job. On false, an error message is set and WaylandConnection
uses it when calling TerminateGpuProcess.

In followup CLs, I will make the buffer management easier.
That will allow us to use frame callbacks in quite a nice way.

Bug: 820047
Change-Id: Ib64d4d5d3e4c64bd7e95154e8d280509bbdac8b6
Reviewed-on: https://chromium-review.googlesource.com/1179835
Commit-Queue: Maksim Sisov <msisov@igalia.com>
Reviewed-by: Robert Kroeger <rjkroege@chromium.org>
Reviewed-by: Antonio Gomes <tonikitoo@igalia.com>
Cr-Commit-Position: refs/heads/master@{#586201}

ui/ozone/platform/wayland/BUILD.gn

@@ -21,6 +21,8 @@ source_set("wayland") {
     "gpu/wayland_connection_proxy.h",
     "ozone_platform_wayland.cc",
     "ozone_platform_wayland.h",
+    "wayland_buffer_manager.cc",
+    "wayland_buffer_manager.h",
     "wayland_connection.cc",
     "wayland_connection.h",
     "wayland_connection_connector.cc",

ui/ozone/platform/wayland/wayland_buffer_manager.cc

+// 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.
+
+#include "ui/ozone/platform/wayland/wayland_buffer_manager.h"
+
+#include <drm_fourcc.h>
+
+#include <linux-dmabuf-unstable-v1-client-protocol.h>
+
+#include "base/trace_event/trace_event.h"
+#include "ui/ozone/common/linux/drm_util_linux.h"
+#include "ui/ozone/platform/wayland/wayland_connection.h"
+#include "ui/ozone/platform/wayland/wayland_window.h"
+
+namespace ui {
+
+WaylandBufferManager::WaylandBufferManager(
+    zwp_linux_dmabuf_v1* zwp_linux_dmabuf,
+    WaylandConnection* connection)
+    : zwp_linux_dmabuf_(zwp_linux_dmabuf), connection_(connection) {
+  static const zwp_linux_dmabuf_v1_listener dmabuf_listener = {
+      &WaylandBufferManager::Format, &WaylandBufferManager::Modifiers,
+  };
+  zwp_linux_dmabuf_v1_add_listener(zwp_linux_dmabuf_.get(), &dmabuf_listener,
+                                   this);
+
+  // A roundtrip after binding guarantees that the client has received all
+  // supported formats.
+  wl_display_roundtrip(connection->display());
+}
+
+WaylandBufferManager::~WaylandBufferManager() {
+  DCHECK(pending_buffer_map_.empty() && params_to_id_map_.empty() &&
+         buffers_.empty());
+}
+
+bool WaylandBufferManager::CreateBuffer(base::File file,
+                                        uint32_t width,
+                                        uint32_t height,
+                                        const std::vector<uint32_t>& strides,
+                                        const std::vector<uint32_t>& offsets,
+                                        uint32_t format,
+                                        const std::vector<uint64_t>& modifiers,
+                                        uint32_t planes_count,
+                                        uint32_t buffer_id) {
+  TRACE_EVENT2("Wayland", "WaylandBufferManager::CreateZwpLinuxDmabuf",
+               "Format", format, "Buffer id", buffer_id);
+
+  static const struct zwp_linux_buffer_params_v1_listener params_listener = {
+      WaylandBufferManager::CreateSucceeded,
+      WaylandBufferManager::CreateFailed};
+  if (!ValidateDataFromGpu(file, width, height, strides, offsets, format,
+                           modifiers, planes_count, buffer_id)) {
+    // base::File::Close() has an assertion that checks if blocking operations
+    // are allowed. Thus, manually close the fd here.
+    base::ScopedFD fd(file.TakePlatformFile());
+    fd.reset();
+    return false;
+  }
+
+  // Store |params| connected to |buffer_id| to track buffer creation and
+  // identify, which buffer a client wants to use.
+  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));
+  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],
+                                   strides[i], 0 /* modifier hi */,
+                                   0 /* modifier lo */);
+  }
+  zwp_linux_buffer_params_v1_add_listener(params, &params_listener, this);
+  zwp_linux_buffer_params_v1_create(params, width, height, format, 0);
+
+  connection_->ScheduleFlush();
+  return true;
+}
+
+// TODO(msisov): handle buffer swap failure or success.
+bool WaylandBufferManager::SwapBuffer(gfx::AcceleratedWidget widget,
+                                      uint32_t buffer_id) {
+  TRACE_EVENT1("Wayland", "WaylandBufferManager::SwapBuffer", "Buffer id",
+               buffer_id);
+
+  if (!ValidateDataFromGpu(widget, buffer_id))
+    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";
+    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());
+
+  connection_->ScheduleFlush();
+  return true;
+}
+
+bool WaylandBufferManager::DestroyBuffer(uint32_t buffer_id) {
+  TRACE_EVENT1("Wayland", "WaylandBufferManager::DestroyZwpLinuxDmabuf",
+               "Buffer id", buffer_id);
+
+  auto it = buffers_.find(buffer_id);
+  if (it == buffers_.end()) {
+    error_message_ = "Trying to destroy non-existing buffer";
+    return false;
+  }
+  buffers_.erase(it);
+
+  connection_->ScheduleFlush();
+  return true;
+}
+
+void WaylandBufferManager::ClearState() {
+  buffers_.clear();
+  params_to_id_map_.clear();
+  pending_buffer_map_.clear();
+}
+
+bool WaylandBufferManager::ValidateDataFromGpu(
+    const base::File& file,
+    uint32_t width,
+    uint32_t height,
+    const std::vector<uint32_t>& strides,
+    const std::vector<uint32_t>& offsets,
+    uint32_t format,
+    const std::vector<uint64_t>& modifiers,
+    uint32_t planes_count,
+    uint32_t buffer_id) {
+  std::string reason;
+  if (!file.IsValid())
+    reason = "Buffer fd is invalid";
+
+  if (width == 0 || height == 0)
+    reason = "Buffer size is invalid";
+
+  if (planes_count < 1)
+    reason = "Planes count cannot be less than 1";
+
+  if (planes_count != strides.size() || planes_count != offsets.size() ||
+      planes_count != modifiers.size()) {
+    reason = "Number of strides(" + std::to_string(strides.size()) +
+             ")/offsets(" + std::to_string(offsets.size()) + ")/modifiers(" +
+             std::to_string(modifiers.size()) +
+             ") does not correspond to the number of planes(" +
+             std::to_string(planes_count) + ")";
+  }
+
+  for (auto stride : strides) {
+    if (stride == 0)
+      reason = "Strides are invalid";
+  }
+
+  if (!IsValidBufferFormat(format))
+    reason = "Buffer format is invalid";
+
+  if (buffer_id < 1)
+    reason = "Invalid buffer id: " + std::to_string(buffer_id);
+
+  if (!reason.empty()) {
+    error_message_ = std::move(reason);
+    return false;
+  }
+  return true;
+}
+
+bool WaylandBufferManager::ValidateDataFromGpu(
+    const gfx::AcceleratedWidget& widget,
+    uint32_t buffer_id) {
+  std::string reason;
+
+  if (widget == gfx::kNullAcceleratedWidget)
+    reason = "Invalid accelerated widget";
+
+  if (buffer_id < 1)
+    reason = "Invalid buffer id: " + std::to_string(buffer_id);
+
+  if (!reason.empty()) {
+    error_message_ = std::move(reason);
+    return false;
+  }
+
+  return true;
+}
+
+void WaylandBufferManager::CreateSucceededInternal(
+    struct zwp_linux_buffer_params_v1* params,
+    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)));
+
+  TRACE_EVENT1("Wayland", "WaylandBufferManager::CreateSucceeded", "Buffer id",
+               buffer_id);
+
+  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);
+  }
+}
+
+// static
+void WaylandBufferManager::Modifiers(
+    void* data,
+    struct zwp_linux_dmabuf_v1* zwp_linux_dmabuf,
+    uint32_t format,
+    uint32_t modifier_hi,
+    uint32_t modifier_lo) {
+  NOTIMPLEMENTED();
+}
+
+// static
+void WaylandBufferManager::Format(void* data,
+                                  struct zwp_linux_dmabuf_v1* zwp_linux_dmabuf,
+                                  uint32_t format) {
+  WaylandBufferManager* self = static_cast<WaylandBufferManager*>(data);
+  // Return on not the supported ARGB format.
+  if (format == DRM_FORMAT_ARGB2101010)
+    return;
+  self->supported_buffer_formats_.push_back(
+      GetBufferFormatFromFourCCFormat(format));
+}
+
+// static
+void WaylandBufferManager::CreateSucceeded(
+    void* data,
+    struct zwp_linux_buffer_params_v1* params,
+    struct wl_buffer* new_buffer) {
+  WaylandBufferManager* self = static_cast<WaylandBufferManager*>(data);
+
+  DCHECK(self);
+  self->CreateSucceededInternal(params, new_buffer);
+}
+
+// static
+void WaylandBufferManager::CreateFailed(
+    void* data,
+    struct zwp_linux_buffer_params_v1* params) {
+  zwp_linux_buffer_params_v1_destroy(params);
+  LOG(FATAL) << "zwp_linux_buffer_params.create failed";
+}
+
+}  // namespace ui

ui/ozone/platform/wayland/wayland_buffer_manager.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_OZONE_PLATFORM_WAYLAND_WAYLAND_BUFFER_MANAGER_H_
+#define UI_OZONE_PLATFORM_WAYLAND_WAYLAND_BUFFER_MANAGER_H_
+
+#include <map>
+#include <vector>
+
+#include "base/containers/flat_map.h"
+#include "base/files/file.h"
+#include "base/macros.h"
+#include "ui/gfx/native_widget_types.h"
+#include "ui/ozone/platform/wayland/wayland_object.h"
+
+struct zwp_linux_dmabuf_v1;
+struct zwp_linux_buffer_params_v1;
+
+namespace gfx {
+enum class BufferFormat;
+}  // namespace gfx
+
+namespace ui {
+
+class WaylandConnection;
+
+// The manager uses zwp_linux_dmabuf protocol to create wl_buffers from added
+// dmabuf buffers. Only used when GPU runs in own process.
+class WaylandBufferManager {
+ public:
+  WaylandBufferManager(zwp_linux_dmabuf_v1* zwp_linux_dmabuf,
+                       WaylandConnection* connection);
+  ~WaylandBufferManager();
+
+  std::string error_message() { return std::move(error_message_); }
+
+  const std::vector<gfx::BufferFormat>& supported_buffer_formats() {
+    return supported_buffer_formats_;
+  }
+
+  // Creates a wl_buffer based on the dmabuf |file| descriptor. On error, false
+  // is returned and |error_message_| is set.
+  bool CreateBuffer(base::File file,
+                    uint32_t width,
+                    uint32_t height,
+                    const std::vector<uint32_t>& strides,
+                    const std::vector<uint32_t>& offsets,
+                    uint32_t format,
+                    const std::vector<uint64_t>& modifiers,
+                    uint32_t planes_count,
+                    uint32_t buffer_id);
+
+  // 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);
+
+  // Destroys a buffer with |buffer_id| in |buffers_|. On error, false is
+  // returned and |error_message_| is set.
+  bool DestroyBuffer(uint32_t buffer_id);
+
+  // Destroys all the data and buffers stored in own containers.
+  void ClearState();
+
+ private:
+  // Validates data sent from GPU. If invalid, returns false and sets an error
+  // message to |error_message_|.
+  bool ValidateDataFromGpu(const base::File& file,
+                           uint32_t width,
+                           uint32_t height,
+                           const std::vector<uint32_t>& strides,
+                           const std::vector<uint32_t>& offsets,
+                           uint32_t format,
+                           const std::vector<uint64_t>& modifiers,
+                           uint32_t planes_count,
+                           uint32_t buffer_id);
+  bool ValidateDataFromGpu(const gfx::AcceleratedWidget& widget,
+                           uint32_t buffer_id);
+
+  void CreateSucceededInternal(struct zwp_linux_buffer_params_v1* params,
+                               struct wl_buffer* new_buffer);
+
+  // zwp_linux_dmabuf_v1_listener
+  static void Modifiers(void* data,
+                        struct zwp_linux_dmabuf_v1* zwp_linux_dmabuf,
+                        uint32_t format,
+                        uint32_t modifier_hi,
+                        uint32_t modifier_lo);
+  static void Format(void* data,
+                     struct zwp_linux_dmabuf_v1* zwp_linux_dmabuf,
+                     uint32_t format);
+
+  // zwp_linux_buffer_params_v1_listener
+  static void CreateSucceeded(void* data,
+                              struct zwp_linux_buffer_params_v1* params,
+                              struct wl_buffer* new_buffer);
+  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_;
+
+  // Set when invalid data is received from the GPU process.
+  std::string error_message_;
+
+  wl::Object<zwp_linux_dmabuf_v1> zwp_linux_dmabuf_;
+
+  // Non-owned pointer to the main connection.
+  WaylandConnection* connection_ = nullptr;
+};
+
+}  // namespace ui
+
+#endif  // UI_OZONE_PLATFORM_WAYLAND_WAYLAND_BUFFER_MANAGER_H_

ui/ozone/platform/wayland/wayland_connection.h

@@ -7,12 +7,11 @@
 
 #include <map>
 
-#include "ui/gfx/buffer_types.h"
-
 #include "base/files/file.h"
 #include "base/message_loop/message_pump_libevent.h"
 #include "mojo/public/cpp/bindings/binding.h"
 #include "ui/events/platform/platform_event_source.h"
+#include "ui/gfx/buffer_types.h"
 #include "ui/gfx/native_widget_types.h"
 #include "ui/ozone/platform/wayland/wayland_data_device.h"
 #include "ui/ozone/platform/wayland/wayland_data_device_manager.h"
@@ -23,15 +22,12 @@
 #include "ui/ozone/platform/wayland/wayland_pointer.h"
 #include "ui/ozone/platform/wayland/wayland_touch.h"
 #include "ui/ozone/public/clipboard_delegate.h"
-#include "ui/ozone/public/gpu_platform_support_host.h"
 #include "ui/ozone/public/interfaces/wayland/wayland_connection.mojom.h"
 
-struct zwp_linux_dmabuf_v1;
-struct zwp_linux_buffer_params_v1;
-
 namespace ui {
 
 class WaylandWindow;
+class WaylandBufferManager;
 
 class WaylandConnection : public PlatformEventSource,
                           public ClipboardDelegate,
@@ -46,7 +42,7 @@ class WaylandConnection : public PlatformEventSource,
 
   // ozone::mojom::WaylandConnection overrides:
   //
-  // The overridden methods below are invoked by GPU.
+  // These overridden methods below are invoked by the GPU.
   //
   // Called by the GPU and asks to import a wl_buffer based on a gbm file
   // descriptor.
@@ -77,7 +73,6 @@ class WaylandConnection : public PlatformEventSource,
   zxdg_shell_v6* shell_v6() { return shell_v6_.get(); }
   wl_seat* seat() { return seat_.get(); }
   wl_data_device* data_device() { return data_device_->data_device(); }
-  zwp_linux_dmabuf_v1* zwp_linux_dmabuf() { return zwp_linux_dmabuf_.get(); }
 
   WaylandWindow* GetWindow(gfx::AcceleratedWidget widget);
   WaylandWindow* GetCurrentFocusedWindow();
@@ -120,7 +115,7 @@ class WaylandConnection : public PlatformEventSource,
   // Returns bound pointer to own mojo interface.
   ozone::mojom::WaylandConnectionPtr BindInterface();
 
-  std::vector<gfx::BufferFormat> GetSupportedBufferFormats();
+  const std::vector<gfx::BufferFormat>& GetSupportedBufferFormats();
 
   void SetTerminateGpuCallback(
       base::OnceCallback<void(std::string)> terminate_gpu_cb);
@@ -136,19 +131,6 @@ class WaylandConnection : public PlatformEventSource,
   void OnFileCanReadWithoutBlocking(int fd) override;
   void OnFileCanWriteWithoutBlocking(int fd) override;
 
-  // Validates data sent by the GPU. If anything, terminates the gpu process.
-  bool ValidateDataFromGpu(const base::File& file,
-                           uint32_t width,
-                           uint32_t height,
-                           const std::vector<uint32_t>& strides,
-                           const std::vector<uint32_t>& offsets,
-                           uint32_t format,
-                           const std::vector<uint64_t>& modifiers,
-                           uint32_t planes_count,
-                           uint32_t buffer_id);
-  bool ValidateDataFromGpu(const gfx::AcceleratedWidget& widget,
-                           uint32_t buffer_id);
-
   // Terminates the GPU process on invalid data received
   void TerminateGpuProcess(std::string reason);
 
@@ -170,22 +152,6 @@ class WaylandConnection : public PlatformEventSource,
   // xdg_shell_listener
   static void Ping(void* data, xdg_shell* shell, uint32_t serial);
 
-  // zwp_linux_dmabuf_v1_listener
-  static void Modifiers(void* data,
-                        struct zwp_linux_dmabuf_v1* zwp_linux_dmabuf,
-                        uint32_t format,
-                        uint32_t modifier_hi,
-                        uint32_t modifier_lo);
-  static void Format(void* data,
-                     struct zwp_linux_dmabuf_v1* zwp_linux_dmabuf,
-                     uint32_t format);
-
-  static void CreateSucceeded(void* data,
-                              struct zwp_linux_buffer_params_v1* params,
-                              struct wl_buffer* new_buffer);
-  static void CreateFailed(void* data,
-                           struct zwp_linux_buffer_params_v1* params);
-
   std::map<gfx::AcceleratedWidget, WaylandWindow*> window_map_;
 
   wl::Object<wl_display> display_;
@@ -195,23 +161,8 @@ class WaylandConnection : public PlatformEventSource,
   wl::Object<wl_seat> seat_;
   wl::Object<wl_shm> shm_;
   wl::Object<xdg_shell> shell_;
-  wl::Object<zwp_linux_dmabuf_v1> zwp_linux_dmabuf_;
   wl::Object<zxdg_shell_v6> shell_v6_;
 
-  // 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 buffer
-  // is created, it will be attached to requested WaylandWindow based on the
-  // gfx::AcceleratedWidget.
-  base::flat_map<uint32_t, gfx::AcceleratedWidget> pending_buffer_map_;
-
   std::unique_ptr<WaylandDataDeviceManager> data_device_manager_;
   std::unique_ptr<WaylandDataDevice> data_device_;
   std::unique_ptr<WaylandDataSource> data_source_;
@@ -219,6 +170,9 @@ class WaylandConnection : public PlatformEventSource,
   std::unique_ptr<WaylandKeyboard> keyboard_;
   std::unique_ptr<WaylandTouch> touch_;
 
+  // Objects that are using when GPU runs in own process.
+  std::unique_ptr<WaylandBufferManager> buffer_manager_;
+
   bool scheduled_flush_ = false;
   bool watching_ = false;
   base::MessagePumpLibevent::FdWatchController controller_;
@@ -237,8 +191,6 @@ class WaylandConnection : public PlatformEventSource,
 
   mojo::Binding<ozone::mojom::WaylandConnection> binding_;
 
-  std::vector<gfx::BufferFormat> buffer_formats_;
-
   // A callback, which is used to terminate a GPU process in case of invalid
   // data sent by the GPU to the browser process.
   base::OnceCallback<void(std::string)> terminate_gpu_cb_;