Re-land "Handle custom/bitmap cursors in ws2."

This re-lands commit a6dc11ea630378ea89ca4f587
which was reverted for leaking (caught by lsan bot).
Fix: unref custom cursors created in CursorData::ToNativeCursor

Go through CursorFactoryOzone to load the PlatformCursor (which
confusingly could be CursorData, but for now is X11CursorOzone). Also
set fields on the cross-platform Cursor type so that code which relies
on it such as CursorWindowController::UpdateCursorImage will work.
That code path can be exercised with --ash-enable-cursor-motion-blur
although there is currently no use of custom cursors with ws2.

TBR=sky@chromium.org, tsepez@chromium.org

Change-Id: I1d6324a6049d83850185171a42c164f7679f5a5e
Reviewed-on: https://chromium-review.googlesource.com/1120860
Commit-Queue: Evan Stade <estade@chromium.org>
Reviewed-by: Evan Stade <estade@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Cr-Commit-Position: refs/heads/master@{#575720}
diff --git a/ash/cursor_unittest.cc b/ash/cursor_unittest.cc
index c251828..27356b1 100644
--- a/ash/cursor_unittest.cc
+++ b/ash/cursor_unittest.cc
@@ -13,6 +13,7 @@
 #include "ui/events/event.h"
 #include "ui/events/event_constants.h"
 #include "ui/events/test/event_generator.h"
+#include "ui/gfx/image/image_unittest_util.h"
 
 namespace ash {
 
@@ -85,4 +86,28 @@
             ash::Shell::Get()->cursor_manager()->GetCursor().native_type());
 }
 
+TEST_F(CursorTest, Custom) {
+  // Create and hover a window.
+  std::unique_ptr<aura::Window> window =
+      CreateTestWindow(gfx::Rect(0, 0, 100, 100));
+  EXPECT_EQ(0U, GetTestWindowTreeClient()->input_events().size());
+  ui::test::EventGenerator generator(window.get());
+  generator.MoveMouseToInHost(50, 50);
+
+  // Set a custom cursor.
+  SkBitmap bitmap = gfx::test::CreateBitmap(10, 10);
+  const ui::CursorData image_cursor(gfx::Point(1, 4), {bitmap}, 1.f,
+                                    base::TimeDelta());
+  GetWindowTreeTestHelper()->SetCursor(window.get(), image_cursor);
+
+  // Make sure it worked.
+  EXPECT_EQ(ui::CursorType::kCustom,
+            window->delegate()->GetCursor({}).native_type());
+  aura::client::CursorClient* cursor_client =
+      aura::client::GetCursorClient(window->GetRootWindow());
+  EXPECT_EQ(ui::CursorType::kCustom, cursor_client->GetCursor().native_type());
+  EXPECT_EQ(bitmap.getGenerationID(),
+            cursor_client->GetCursor().GetBitmap().getGenerationID());
+}
+
 }  // namespace ash
diff --git a/services/ui/public/interfaces/cursor/cursor.mojom b/services/ui/public/interfaces/cursor/cursor.mojom
index ae4e453..0a267bc 100644
--- a/services/ui/public/interfaces/cursor/cursor.mojom
+++ b/services/ui/public/interfaces/cursor/cursor.mojom
@@ -69,7 +69,7 @@
 
 // A description of a cursor.
 struct CursorData {
-  // The type of cursor. If CUSTOM, the rest of the fields are relevant.
+  // The type of cursor. If kCustom, the rest of the fields are relevant.
   CursorType cursor_type;
 
   // The delay between cursor frames.
@@ -78,7 +78,7 @@
   // The hotspot in pixels in the source cursor frames.
   gfx.mojom.Point hotspot_in_pixels;
 
-  // The frames of the cursor.
+  // The frames of the cursor. Must be non-empty if |cursor_type| is kCustom.
   array<skia.mojom.Bitmap?> cursor_frames;
 
   // This is the image scale of this cursor.
diff --git a/services/ui/public/interfaces/cursor/cursor_struct_traits.cc b/services/ui/public/interfaces/cursor/cursor_struct_traits.cc
index f3a3888..17f0aff5 100644
--- a/services/ui/public/interfaces/cursor/cursor_struct_traits.cc
+++ b/services/ui/public/interfaces/cursor/cursor_struct_traits.cc
@@ -334,15 +334,21 @@
 
   gfx::Point hotspot_in_pixels;
   std::vector<SkBitmap> cursor_frames;
-  float scale_factor = data.scale_factor();
   base::TimeDelta frame_delay;
 
   if (!data.ReadHotspotInPixels(&hotspot_in_pixels) ||
-      !data.ReadCursorFrames(&cursor_frames) ||
+      !data.ReadCursorFrames(&cursor_frames) || cursor_frames.empty() ||
       !data.ReadFrameDelay(&frame_delay)) {
     return false;
   }
 
+  // Clamp the scale factor to a reasonable value. TODO(estade): do we even need
+  // this field? It doesn't appear to be used anywhere and is a property of the
+  // display, not the cursor.
+  float scale_factor = data.scale_factor();
+  if (scale_factor < 1.f || scale_factor > 3.f)
+    return false;
+
   *out = ui::CursorData(hotspot_in_pixels, cursor_frames, scale_factor,
                         frame_delay);
   return true;
diff --git a/services/ui/public/interfaces/cursor/cursor_struct_traits_unittest.cc b/services/ui/public/interfaces/cursor/cursor_struct_traits_unittest.cc
index 0b26d67..f844eed 100644
--- a/services/ui/public/interfaces/cursor/cursor_struct_traits_unittest.cc
+++ b/services/ui/public/interfaces/cursor/cursor_struct_traits_unittest.cc
@@ -114,4 +114,16 @@
   EXPECT_TRUE(output.cursor_frames().front().empty());
 }
 
+// For custom cursors, the frame image vector must be non-empty.
+TEST_F(CursorStructTraitsTest, TestMissingFrames) {
+  const base::TimeDelta kFrameDelay = base::TimeDelta::FromMilliseconds(15);
+  const gfx::Point kHotspot = gfx::Point(5, 2);
+  const float kScale = 2.0f;
+
+  ui::CursorData input(kHotspot, {}, kScale, kFrameDelay);
+
+  ui::CursorData output;
+  EXPECT_FALSE(EchoCursorData(input, &output));
+}
+
 }  // namespace ui
diff --git a/services/ui/ws/threaded_image_cursors.cc b/services/ui/ws/threaded_image_cursors.cc
index aaab5ca..f5178d7 100644
--- a/services/ui/ws/threaded_image_cursors.cc
+++ b/services/ui/ws/threaded_image_cursors.cc
@@ -91,10 +91,8 @@
     scoped_refptr<base::SingleThreadTaskRunner> ui_service_task_runner_,
     base::WeakPtr<ThreadedImageCursors> threaded_image_cursors_weak_ptr) {
   if (image_cursors_weak_ptr) {
-    ui::PlatformCursor platform_cursor = cursor_factory->CreateAnimatedCursor(
-        cursor_data->cursor_frames(), cursor_data->hotspot_in_pixels(),
-        cursor_data->frame_delay().InMilliseconds(),
-        cursor_data->scale_factor());
+    ui::PlatformCursor platform_cursor =
+        cursor_data->ToNativeCursor().platform();
     // |platform_window| is owned by the UI Service thread, so setting the
     // cursor on it also needs to happen on that thread.
     ui_service_task_runner_->PostTask(
diff --git a/services/ui/ws2/window_tree.cc b/services/ui/ws2/window_tree.cc
index 63de3e2..b3aa310 100644
--- a/services/ui/ws2/window_tree.cc
+++ b/services/ui/ws2/window_tree.cc
@@ -1067,18 +1067,13 @@
     return false;
   }
   if (!IsClientCreatedWindow(window) && !IsClientRootWindow(window)) {
-    DVLOG(1) << "SerCursor failed (access denied)";
+    DVLOG(1) << "SetCursor failed (access denied)";
     return false;
   }
 
   auto* server_window = ServerWindow::GetMayBeNull(window);
 
-  // Convert from CursorData to Cursor. TODO(estade): remove this conversion.
-  // See class level comment on CursorData. Also support kCustom, i.e. image
-  // cursors.
-  ui::Cursor old_cursor_type(cursor.cursor_type());
-  if (cursor.cursor_type() == ui::CursorType::kCustom)
-    NOTIMPLEMENTED_LOG_ONCE();
+  ui::Cursor old_cursor_type = cursor.ToNativeCursor();
 
   // Ask our delegate to set the cursor. This will save the cursor for toplevels
   // and also update the active cursor if appropriate (i.e. if |window| is the
diff --git a/ui/base/cursor/cursor_data.cc b/ui/base/cursor/cursor_data.cc
index 6c39f5a..2ba8500 100644
--- a/ui/base/cursor/cursor_data.cc
+++ b/ui/base/cursor/cursor_data.cc
@@ -5,7 +5,15 @@
 #include "ui/base/cursor/cursor_data.h"
 
 #include "third_party/skia/include/core/SkBitmap.h"
+#include "ui/base/cursor/cursor_type.h"
+
+#if defined(USE_AURA)
 #include "ui/base/cursor/cursor.h"
+#endif
+
+#if defined(USE_OZONE)
+#include "ui/ozone/public/cursor_factory_ozone.h"
+#endif
 
 namespace ui {
 
@@ -48,4 +56,38 @@
          generator_ids_ == rhs.generator_ids_;
 }
 
+#if defined(USE_AURA)
+gfx::NativeCursor CursorData::ToNativeCursor() const {
+  Cursor cursor(cursor_type_);
+
+#if defined(USE_OZONE)
+  auto* factory = CursorFactoryOzone::GetInstance();
+  if (cursor_type_ != CursorType::kCustom) {
+    cursor.SetPlatformCursor(factory->GetDefaultCursor(cursor_type_));
+  } else if (cursor_frames_.size() > 1U) {
+    cursor.SetPlatformCursor(factory->CreateAnimatedCursor(
+        cursor_frames_, hotspot_, frame_delay_.InMilliseconds(),
+        scale_factor_));
+    // CreateAnimatedCursor() and CreateImageCursor() below both return a cursor
+    // with a ref count of 1, which we need to account for after storing it in
+    // |cursor|.
+    cursor.UnrefCustomCursor();
+  } else {
+    DCHECK_EQ(1U, cursor_frames_.size());
+    cursor.SetPlatformCursor(
+        factory->CreateImageCursor(cursor_frames_[0], hotspot_, scale_factor_));
+    cursor.UnrefCustomCursor();
+  }
+#else
+  NOTIMPLEMENTED();
+#endif
+
+  if (!cursor_frames_.empty()) {
+    cursor.set_custom_bitmap(cursor_frames_[0]);
+    cursor.set_custom_hotspot(hotspot_);
+  }
+  return cursor;
+}
+#endif
+
 }  // namespace ui
diff --git a/ui/base/cursor/cursor_data.h b/ui/base/cursor/cursor_data.h
index 6b3f5d2..ada9dd6 100644
--- a/ui/base/cursor/cursor_data.h
+++ b/ui/base/cursor/cursor_data.h
@@ -11,6 +11,7 @@
 #include "build/build_config.h"
 #include "ui/base/ui_base_export.h"
 #include "ui/gfx/geometry/point.h"
+#include "ui/gfx/native_widget_types.h"
 
 class SkBitmap;
 
@@ -72,6 +73,11 @@
   // data, but different generation ids.
   bool IsSameAs(const CursorData& rhs) const;
 
+  // Until CursorData replaces Cursor, it's necessary to convert back to Cursor.
+  // Some code such as CursorWindowController still uses Cursor (and not its
+  // PlatformCursor).
+  gfx::NativeCursor ToNativeCursor() const;
+
  private:
   // A native type constant from cursor.h.
   CursorType cursor_type_;