[ozone/wayland] Fixed buffer handling in the WaylandCursor class.

The visual shape of a pointer cursor is rendered at a surface attached
to the wl_pointer object and backed by a wl_buffer.  The WaylandCursor
class tried to reuse the buffer regardless of whether the compositor
released it, which resulted in that the cursor shape didn't change
when a new bitmap had been submitted.

This CL simplifies the process: a new buffer is created and attached
each time the new bitmap is submitted, and the old ones are held until
the compositor releases them.

R=msisov@igalia.com

Bug: 928260
Change-Id: I54cf679568276eddbd5c8042d05fe05de25f61d8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1503374
Commit-Queue: Alexander Dunaev <adunaev@igalia.com>
Reviewed-by: Maksim Sisov <msisov@igalia.com>
Cr-Commit-Position: refs/heads/master@{#638521}

ui/ozone/platform/wayland/BUILD.gn

@@ -158,6 +158,8 @@ source_set("test_support") {
     "test/global_object.h",
     "test/mock_buffer.cc",
     "test/mock_buffer.h",
+    "test/mock_pointer.cc",
+    "test/mock_pointer.h",
     "test/mock_surface.cc",
     "test/mock_surface.h",
     "test/mock_xdg_popup.cc",
@@ -188,8 +190,6 @@ source_set("test_support") {
     "test/test_keyboard.h",
     "test/test_output.cc",
     "test/test_output.h",
-    "test/test_pointer.cc",
-    "test/test_pointer.h",
     "test/test_positioner.cc",
     "test/test_positioner.h",
     "test/test_region.cc",

ui/ozone/platform/wayland/test/mock_pointer.cc

+// 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_pointer.h"
+
+namespace wl {
+
+namespace {
+
+void SetCursor(wl_client* client,
+               wl_resource* pointer_resource,
+               uint32_t serial,
+               wl_resource* surface_resource,
+               int32_t hotspot_x,
+               int32_t hotspot_y) {
+  GetUserDataAs<MockPointer>(pointer_resource)
+      ->SetCursor(surface_resource, hotspot_x, hotspot_y);
+}
+
+}  // namespace
+
+const struct wl_pointer_interface kMockPointerImpl = {
+    &SetCursor,        // set_cursor
+    &DestroyResource,  // release
+};
+
+MockPointer::MockPointer(wl_resource* resource) : ServerObject(resource) {}
+
+MockPointer::~MockPointer() = default;
+
+}  // namespace wl

ui/ozone/platform/wayland/test/test_pointer.h → ui/ozone/platform/wayland/test/mock_pointer.h

@@ -2,27 +2,33 @@
 // 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_TEST_POINTER_H_
-#define UI_OZONE_PLATFORM_WAYLAND_TEST_TEST_POINTER_H_
+#ifndef UI_OZONE_PLATFORM_WAYLAND_TEST_MOCK_POINTER_H_
+#define UI_OZONE_PLATFORM_WAYLAND_TEST_MOCK_POINTER_H_
 
 #include <wayland-server-protocol.h>
 
 #include "base/macros.h"
+#include "testing/gmock/include/gmock/gmock.h"
 #include "ui/ozone/platform/wayland/test/server_object.h"
 
 struct wl_resource;
 
 namespace wl {
 
-extern const struct wl_pointer_interface kTestPointerImpl;
+extern const struct wl_pointer_interface kMockPointerImpl;
 
-class TestPointer : public ServerObject {
+class MockPointer : public ServerObject {
  public:
-  explicit TestPointer(wl_resource* resource);
-  ~TestPointer() override;
+  explicit MockPointer(wl_resource* resource);
+  ~MockPointer() override;
+
+  MOCK_METHOD3(SetCursor,
+               void(wl_resource* surface_resource,
+                    int32_t hotspot_x,
+                    int32_t hotspot_y));
 
  private:
-  DISALLOW_COPY_AND_ASSIGN(TestPointer);
+  DISALLOW_COPY_AND_ASSIGN(MockPointer);
 };
 
 }  // namespace wl

ui/ozone/platform/wayland/test/test_pointer.cc

-// 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/test_pointer.h"
-
-namespace wl {
-
-const struct wl_pointer_interface kTestPointerImpl = {
-    nullptr,           // set_cursor
-    &DestroyResource,  // release
-};
-
-TestPointer::TestPointer(wl_resource* resource) : ServerObject(resource) {}
-
-TestPointer::~TestPointer() = default;
-
-}  // namespace wl

ui/ozone/platform/wayland/test/test_seat.cc

@@ -4,8 +4,8 @@
 
 #include "ui/ozone/platform/wayland/test/test_seat.h"
 
+#include "ui/ozone/platform/wayland/test/mock_pointer.h"
 #include "ui/ozone/platform/wayland/test/test_keyboard.h"
-#include "ui/ozone/platform/wayland/test/test_pointer.h"
 #include "ui/ozone/platform/wayland/test/test_touch.h"
 
 namespace wl {
@@ -15,11 +15,11 @@ namespace {
 constexpr uint32_t kSeatVersion = 4;
 
 void GetPointer(wl_client* client, wl_resource* resource, uint32_t id) {
-  wl_resource* pointer_resource = CreateResourceWithImpl<TestPointer>(
+  wl_resource* pointer_resource = CreateResourceWithImpl<MockPointer>(
       client, &wl_pointer_interface, wl_resource_get_version(resource),
-      &kTestPointerImpl, id);
+      &kMockPointerImpl, id);
   GetUserDataAs<TestSeat>(resource)->set_pointer(
-      GetUserDataAs<TestPointer>(pointer_resource));
+      GetUserDataAs<MockPointer>(pointer_resource));
 }
 
 void GetKeyboard(wl_client* client, wl_resource* resource, uint32_t id) {

ui/ozone/platform/wayland/test/test_seat.h

@@ -14,8 +14,8 @@ namespace wl {
 
 extern const struct wl_seat_interface kTestSeatImpl;
 
+class MockPointer;
 class TestKeyboard;
-class TestPointer;
 class TestTouch;
 
 // Manages a global wl_seat object.
@@ -29,8 +29,8 @@ class TestSeat : public GlobalObject {
   TestSeat();
   ~TestSeat() override;
 
-  void set_pointer(TestPointer* pointer) { pointer_ = pointer; }
-  TestPointer* pointer() const { return pointer_; }
+  void set_pointer(MockPointer* pointer) { pointer_ = pointer; }
+  MockPointer* pointer() const { return pointer_; }
 
   void set_keyboard(TestKeyboard* keyboard) { keyboard_ = keyboard; }
   TestKeyboard* keyboard() const { return keyboard_; }
@@ -39,7 +39,7 @@ class TestSeat : public GlobalObject {
   TestTouch* touch() const { return touch_; }
 
  private:
-  TestPointer* pointer_;
+  MockPointer* pointer_;
   TestKeyboard* keyboard_;
   TestTouch* touch_;
 

ui/ozone/platform/wayland/wayland_cursor.cc

@@ -16,80 +16,71 @@
 
 namespace ui {
 
-WaylandCursor::WaylandCursor() : shared_memory_(new base::SharedMemory()) {}
+WaylandCursor::WaylandCursor() = default;
+
+WaylandCursor::~WaylandCursor() = default;
+
+// static
+void WaylandCursor::OnBufferRelease(void* data, wl_buffer* buffer) {
+  auto* cursor = static_cast<WaylandCursor*>(data);
+  DCHECK(cursor->buffers_.count(buffer) > 0);
+  cursor->buffers_.erase(buffer);
+}
 
 void WaylandCursor::Init(wl_pointer* pointer, WaylandConnection* connection) {
-  if (input_pointer_ == pointer)
-    return;
+  DCHECK(connection);
 
   input_pointer_ = pointer;
 
-  DCHECK(connection);
   shm_ = connection->shm();
   pointer_surface_.reset(
       wl_compositor_create_surface(connection->compositor()));
 }
 
-WaylandCursor::~WaylandCursor() {
-  pointer_surface_.reset();
-  buffer_.reset();
-
-  if (shared_memory_->handle().GetHandle()) {
-    shared_memory_->Unmap();
-    shared_memory_->Close();
-  }
-}
-
 void WaylandCursor::UpdateBitmap(const std::vector<SkBitmap>& cursor_image,
-                                 const gfx::Point& location,
+                                 const gfx::Point& hotspot,
                                  uint32_t serial) {
   if (!input_pointer_)
     return;
 
-  if (!cursor_image.size()) {
-    HideCursor(serial);
-    return;
-  }
+  if (!cursor_image.size())
+    return HideCursor(serial);
 
   const SkBitmap& image = cursor_image[0];
   SkISize size = image.dimensions();
-  if (size.isEmpty()) {
-    HideCursor(serial);
-    return;
-  }
+  if (size.isEmpty())
+    return HideCursor(serial);
 
   gfx::Size image_size = gfx::SkISizeToSize(size);
-  if (image_size != size_) {
-    wl_buffer* buffer =
-        wl::CreateSHMBuffer(image_size, shared_memory_.get(), shm_);
+  auto shared_memory = std::make_unique<base::SharedMemory>();
+  wl::Object<wl_buffer> buffer(
+      wl::CreateSHMBuffer(image_size, shared_memory.get(), shm_));
   if (!buffer) {
     LOG(ERROR) << "Failed to create SHM buffer for Cursor Bitmap.";
-      wl_pointer_set_cursor(input_pointer_, serial, nullptr, 0, 0);
-      return;
-    }
-    buffer_.reset(buffer);
-    size_ = image_size;
+    return HideCursor(serial);
   }
-  wl::DrawBitmapToSHMB(size_, *shared_memory_, image);
+
+  static const struct wl_buffer_listener wl_buffer_listener {
+    &WaylandCursor::OnBufferRelease
+  };
+  wl_buffer_add_listener(buffer.get(), &wl_buffer_listener, this);
+
+  wl::DrawBitmapToSHMB(image_size, *shared_memory, image);
 
   wl_pointer_set_cursor(input_pointer_, serial, pointer_surface_.get(),
-                        location.x(), location.y());
-  wl_surface_attach(pointer_surface_.get(), buffer_.get(), 0, 0);
-  wl_surface_damage(pointer_surface_.get(), 0, 0, size_.width(),
-                    size_.height());
+                        hotspot.x(), hotspot.y());
+  wl_surface_attach(pointer_surface_.get(), buffer.get(), 0, 0);
   wl_surface_commit(pointer_surface_.get());
+
+  buffers_.emplace(
+      buffer.get(),
+      std::pair<wl::Object<wl_buffer>, std::unique_ptr<base::SharedMemory>>(
+          std::move(buffer), std::move(shared_memory)));
 }
 
 void WaylandCursor::HideCursor(uint32_t serial) {
-  size_ = gfx::Size();
+  DCHECK(input_pointer_);
   wl_pointer_set_cursor(input_pointer_, serial, nullptr, 0, 0);
-
-  buffer_.reset();
-
-  if (shared_memory_->handle().GetHandle()) {
-    shared_memory_->Unmap();
-    shared_memory_->Close();
-  }
 }
 
 }  // namespace ui

ui/ozone/platform/wayland/wayland_cursor.h

@@ -6,12 +6,14 @@
 #define UI_OZONE_PLATFORM_WAYLAND_WAYLAND_CURSOR_H_
 
 #include <wayland-client.h>
+#include <memory>
+#include <utility>
 #include <vector>
 
+#include "base/containers/flat_map.h"
 #include "base/macros.h"
 #include "third_party/skia/include/core/SkBitmap.h"
 #include "third_party/skia/include/core/SkSurface.h"
-#include "ui/gfx/geometry/size.h"
 #include "ui/ozone/platform/wayland/wayland_object.h"
 
 namespace base {
@@ -26,10 +28,14 @@ namespace ui {
 
 class WaylandConnection;
 
-// The WaylandCursor class wraps the actual visual representation
-// (what users see drawn) of a wl_pointer.
+// Manages the actual visual representation (what users see drawn) of the
+// 'pointer' (which is the Wayland term for mouse/mice).
 //
-// 'pointer' is the Wayland terminology for mouse/mice.
+// An instance of this class is aggregated by an instance of WaylandPointer
+// and is exposed for updating the pointer bitmap with the single method call.
+//
+// Encapsulates the low-level job such as surface and buffer management and
+// Wayland protocol calls.
 class WaylandCursor {
  public:
   WaylandCursor();
@@ -38,25 +44,29 @@ class WaylandCursor {
   void Init(wl_pointer* pointer, WaylandConnection* connection);
 
   // Updates wl_pointer's visual representation with the given bitmap
-  // image set, at the hotspot specified by 'location'.
+  // image set and hotspot.
   void UpdateBitmap(const std::vector<SkBitmap>& bitmaps,
-                    const gfx::Point& location,
+                    const gfx::Point& hotspot,
                     uint32_t serial);
 
  private:
+  // wl_buffer_listener:
+  static void OnBufferRelease(void* data, wl_buffer* wl_buffer);
+
   void HideCursor(uint32_t serial);
 
   wl_shm* shm_ = nullptr;                // Owned by WaylandConnection.
   wl_pointer* input_pointer_ = nullptr;  // Owned by WaylandPointer.
 
-  wl::Object<wl_buffer> buffer_;
+  // Holds the buffers and their memory until the compositor releases them.
+  base::flat_map<
+      wl_buffer*,
+      std::pair<wl::Object<wl_buffer>, std::unique_ptr<base::SharedMemory>>>
+      buffers_;
   wl::Object<wl_surface> pointer_surface_;
 
-  std::unique_ptr<base::SharedMemory> shared_memory_;
   sk_sp<SkSurface> sk_surface_;
 
-  gfx::Size size_;
-
   DISALLOW_COPY_AND_ASSIGN(WaylandCursor);
 };
 

ui/ozone/platform/wayland/wayland_pointer.cc

@@ -6,6 +6,7 @@
 
 #include <linux/input.h>
 #include <wayland-client.h>
+#include <memory>
 
 #include "ui/events/base_event_utils.h"
 #include "ui/events/event.h"
@@ -41,7 +42,7 @@ WaylandPointer::WaylandPointer(wl_pointer* pointer,
 
   wl_pointer_add_listener(obj_.get(), &listener, this);
 
-  cursor_.reset(new WaylandCursor);
+  cursor_ = std::make_unique<WaylandCursor>();
 }
 
 WaylandPointer::~WaylandPointer() {

ui/ozone/platform/wayland/wayland_pointer.h

@@ -16,6 +16,10 @@ namespace ui {
 
 class WaylandWindow;
 
+// Wraps the wl_pointer object and transmits events to the dispatcher callback.
+//
+// Exposes an aggregated WaylandCursor that manages the visual shape of the
+// pointer.
 class WaylandPointer {
  public:
   WaylandPointer(wl_pointer* pointer, const EventDispatchCallback& callback);

ui/ozone/platform/wayland/wayland_pointer_unittest.cc

@@ -8,16 +8,19 @@
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/events/event.h"
+#include "ui/ozone/platform/wayland/test/mock_pointer.h"
 #include "ui/ozone/platform/wayland/test/mock_surface.h"
-#include "ui/ozone/platform/wayland/test/test_pointer.h"
 #include "ui/ozone/platform/wayland/test/test_wayland_server_thread.h"
+#include "ui/ozone/platform/wayland/wayland_cursor.h"
 #include "ui/ozone/platform/wayland/wayland_test.h"
 #include "ui/ozone/platform/wayland/wayland_window.h"
 #include "ui/ozone/test/mock_platform_window_delegate.h"
 #include "ui/platform_window/platform_window_init_properties.h"
 
-using ::testing::SaveArg;
 using ::testing::_;
+using ::testing::Mock;
+using ::testing::Ne;
+using ::testing::SaveArg;
 
 namespace ui {
 
@@ -38,7 +41,7 @@ class WaylandPointerTest : public WaylandTest {
   }
 
  protected:
-  wl::TestPointer* pointer_;
+  wl::MockPointer* pointer_;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(WaylandPointerTest);
@@ -312,6 +315,24 @@ TEST_P(WaylandPointerTest, AxisHorizontal) {
   EXPECT_EQ(gfx::PointF(50, 75), mouse_wheel_event->root_location_f());
 }
 
+TEST_P(WaylandPointerTest, SetBitmap) {
+  SkBitmap dummy_cursor;
+  dummy_cursor.setInfo(
+      SkImageInfo::Make(16, 16, kUnknown_SkColorType, kUnknown_SkAlphaType));
+
+  EXPECT_CALL(*pointer_, SetCursor(nullptr, 0, 0));
+  connection_->SetCursorBitmap({}, {});
+  connection_->ScheduleFlush();
+  Sync();
+  Mock::VerifyAndClearExpectations(pointer_);
+
+  EXPECT_CALL(*pointer_, SetCursor(Ne(nullptr), 5, 8));
+  connection_->SetCursorBitmap({dummy_cursor}, gfx::Point(5, 8));
+  connection_->ScheduleFlush();
+  Sync();
+  Mock::VerifyAndClearExpectations(pointer_);
+}
+
 INSTANTIATE_TEST_SUITE_P(XdgVersionV5Test,
                          WaylandPointerTest,
                          ::testing::Values(kXdgShellV5));

ui/ozone/platform/wayland/wayland_screen_unittest.cc

@@ -8,8 +8,8 @@
 
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/display/display_observer.h"
+#include "ui/ozone/platform/wayland/test/mock_pointer.h"
 #include "ui/ozone/platform/wayland/test/mock_surface.h"
-#include "ui/ozone/platform/wayland/test/test_pointer.h"
 #include "ui/ozone/platform/wayland/test/test_wayland_server_thread.h"
 #include "ui/ozone/platform/wayland/wayland_connection.h"
 #include "ui/ozone/platform/wayland/wayland_output_manager.h"
@@ -450,7 +450,7 @@ TEST_P(WaylandScreenTest, GetCursorScreenPoint) {
 
   Sync();
 
-  wl::TestPointer* pointer = server_.seat()->pointer();
+  wl::MockPointer* pointer = server_.seat()->pointer();
   ASSERT_TRUE(pointer);
 
   uint32_t serial = 0;

ui/ozone/platform/wayland/wayland_window_unittest.cc

@@ -16,8 +16,8 @@
 #include "ui/base/hit_test.h"
 #include "ui/events/base_event_utils.h"
 #include "ui/events/event.h"
+#include "ui/ozone/platform/wayland/test/mock_pointer.h"
 #include "ui/ozone/platform/wayland/test/mock_surface.h"
-#include "ui/ozone/platform/wayland/test/test_pointer.h"
 #include "ui/ozone/platform/wayland/test/test_wayland_server_thread.h"
 #include "ui/ozone/platform/wayland/wayland_test.h"
 #include "ui/ozone/platform/wayland/wayland_util.h"
@@ -597,7 +597,7 @@ TEST_P(WaylandWindowTest, HasCaptureUpdatedOnPointerEvents) {
 
   Sync();
 
-  wl::TestPointer* pointer = server_.seat()->pointer();
+  wl::MockPointer* pointer = server_.seat()->pointer();
   ASSERT_TRUE(pointer);
 
   wl_pointer_send_enter(pointer->resource(), 1, surface_->resource(), 0, 0);