[views-mus] Support custom cursors.

[views-mus] Support custom cursors.

This changes the mus interface to pass a CursorData instead of a
CursorType. The previous patch had mus clients create CursorDatas on
ozone platforms, and this patch passes CursorDatas across the mojo pipe
and passes it to the native ozone platform on the window server.

BUG=705037
Review-Url: https://codereview.chromium.org/2830703003
Cr-Commit-Position: refs/heads/master@{#467832}
Committed: https://chromium.googlesource.com/chromium/src/+/a3cfe1428faf4f5bd556e5d9aa4924a7d0fb02f4

[Elliot Glaysher, 2017-04-19 20:27:53]

The CQ bit was checked by erg@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-19 20:28:36]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-19 20:46:10]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-19 20:46:11]

Dry run: Try jobs failed on following builders:
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Elliot Glaysher, 2017-04-19 20:59:25]

The CQ bit was checked by erg@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-19 20:59:59]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-19 21:17:39]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-19 21:17:43]

Dry run: Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Elliot Glaysher, 2017-04-19 21:18:02]

The CQ bit was checked by erg@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-19 21:18:59]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-19 21:36:02]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-19 21:36:03]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Elliot Glaysher, 2017-04-19 21:37:20]

The CQ bit was checked by erg@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-19 21:37:51]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-19 22:44:21]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-19 22:44:22]

Dry run: This issue passed the CQ dry run.

[Elliot Glaysher, 2017-04-20 18:13:40]

The CQ bit was checked by erg@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-20 18:14:11]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-20 20:29:59]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-20 20:30:00]

Dry run: This issue passed the CQ dry run.

[Elliot Glaysher, 2017-04-25 19:37:45]

The CQ bit was checked by erg@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-25 19:38:14]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-25 19:52:52]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-25 19:52:58]

Dry run: Try jobs failed on following builders:
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Elliot Glaysher, 2017-04-25 19:58:48]

The CQ bit was checked by erg@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-25 19:59:32]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Elliot Glaysher, 2017-04-25 20:15:55]

The CQ bit was checked by erg@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-25 20:16:36]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-25 20:52:36]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-25 20:52:37]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Elliot Glaysher, 2017-04-25 20:56:27]

The CQ bit was checked by erg@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-25 20:56:52]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-25 21:16:35]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-25 21:16:37]

Dry run: Try jobs failed on following builders:
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)

[Elliot Glaysher, 2017-04-25 22:03:34]

The CQ bit was checked by erg@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-25 22:04:11]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Elliot Glaysher, 2017-04-25 23:10:34]

The CQ bit was checked by erg@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-25 23:11:23]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-26 01:25:43]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-26 01:25:44]

Dry run: This issue passed the CQ dry run.

[Elliot Glaysher, 2017-04-26 21:50:00]

The CQ bit was checked by erg@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-26 21:51:23]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Elliot Glaysher, 2017-04-26 21:51:50]

erg@chromium.org changed reviewers:
+ sadrul@chromium.org, sky@chromium.org, tsepez@chromium.org

[Elliot Glaysher, 2017-04-26 21:51:51]

tsepez: mojom changes to services/ui/public/interfaces/

sadrul: content/

sky: the rest

[sky, 2017-04-26 22:16:16]

https://codereview.chromium.org/2830703003/diff/220001/services/ui/public/int...
File services/ui/public/interfaces/cursor/cursor_struct_traits_unittest.cc
(right):

https://codereview.chromium.org/2830703003/diff/220001/services/ui/public/int...
services/ui/public/interfaces/cursor/cursor_struct_traits_unittest.cc:113:
ASSERT_TRUE(EchoCursorData(input, &output));
optional: you don't need to route through mojom to test serialization. See
https://groups.google.com/a/chromium.org/forum/#!searchin/chromium-mojo/trait...
.

https://codereview.chromium.org/2830703003/diff/220001/services/ui/ws/drag_co...
File services/ui/ws/drag_controller.h (right):

https://codereview.chromium.org/2830703003/diff/220001/services/ui/ws/drag_co...
services/ui/ws/drag_controller.h:56: ui::CursorData current_cursor() const {
return current_cursor_; }
const ui::CursorData& ?

https://codereview.chromium.org/2830703003/diff/220001/services/ui/ws/drag_co...
File services/ui/ws/drag_controller_unittest.cc (right):

https://codereview.chromium.org/2830703003/diff/220001/services/ui/ws/drag_co...
services/ui/ws/drag_controller_unittest.cc:208: ui::CursorType cursor_type() {
return cursor_; }
() const

https://codereview.chromium.org/2830703003/diff/220001/services/ui/ws/server_...
File services/ui/ws/server_window.h (right):

https://codereview.chromium.org/2830703003/diff/220001/services/ui/ws/server_...
services/ui/ws/server_window.h:152: void SetCursor(const ui::CursorData&
cursor);
Should these two functions take a ui::CursorData that is std::move()d? If you
don't do that, aren't you potentially doing unnecessary copying of the bitmaps?

https://codereview.chromium.org/2830703003/diff/220001/services/ui/ws/server_...
File services/ui/ws/server_window_observer.h (right):

https://codereview.chromium.org/2830703003/diff/220001/services/ui/ws/server_...
services/ui/ws/server_window_observer.h:70: const ui::CursorData& cursor_id) {}
cursor_id -> cursor_data ? (same comment for next function).

https://codereview.chromium.org/2830703003/diff/220001/services/ui/ws/window_...
File services/ui/ws/window_tree.cc (right):

https://codereview.chromium.org/2830703003/diff/220001/services/ui/ws/window_...
services/ui/ws/window_tree.cc:1779: ServerWindow* window =
Should this fail if cursor type is custom and cursor_frames is empty?

[Elliot Glaysher, 2017-04-26 22:56:18]

The CQ bit was checked by erg@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-26 22:56:50]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Elliot Glaysher, 2017-04-26 23:09:05]

https://codereview.chromium.org/2830703003/diff/220001/services/ui/public/int...
File services/ui/public/interfaces/cursor/cursor_struct_traits_unittest.cc
(right):

https://codereview.chromium.org/2830703003/diff/220001/services/ui/public/int...
services/ui/public/interfaces/cursor/cursor_struct_traits_unittest.cc:113:
ASSERT_TRUE(EchoCursorData(input, &output));
On 2017/04/26 22:16:15, sky wrote:
> optional: you don't need to route through mojom to test serialization. See
>
https://groups.google.com/a/chromium.org/forum/#!searchin/chromium-mojo/trait...
> .

I'm confused. Isn't that what I'm doing here? This calls the
mojom::Deserialize(mojom::Serialize()) pattern wrapped in EchoCursorData().

https://codereview.chromium.org/2830703003/diff/220001/services/ui/ws/drag_co...
File services/ui/ws/drag_controller.h (right):

https://codereview.chromium.org/2830703003/diff/220001/services/ui/ws/drag_co...
services/ui/ws/drag_controller.h:56: ui::CursorData current_cursor() const {
return current_cursor_; }
On 2017/04/26 22:16:15, sky wrote:
> const ui::CursorData& ?

Done.

https://codereview.chromium.org/2830703003/diff/220001/services/ui/ws/server_...
File services/ui/ws/server_window.h (right):

https://codereview.chromium.org/2830703003/diff/220001/services/ui/ws/server_...
services/ui/ws/server_window.h:152: void SetCursor(const ui::CursorData&
cursor);
On 2017/04/26 22:16:16, sky wrote:
> Should these two functions take a ui::CursorData that is std::move()d? If you
> don't do that, aren't you potentially doing unnecessary copying of the
bitmaps?

Done.

(A previous version of this patch actually did this, but I undid it because I
was confused about the observer interfaces.)

https://codereview.chromium.org/2830703003/diff/220001/services/ui/ws/server_...
File services/ui/ws/server_window_observer.h (right):

https://codereview.chromium.org/2830703003/diff/220001/services/ui/ws/server_...
services/ui/ws/server_window_observer.h:70: const ui::CursorData& cursor_id) {}
On 2017/04/26 22:16:16, sky wrote:
> cursor_id -> cursor_data ? (same comment for next function).

fixed here and elsewhere

https://codereview.chromium.org/2830703003/diff/220001/services/ui/ws/window_...
File services/ui/ws/window_tree.cc (right):

https://codereview.chromium.org/2830703003/diff/220001/services/ui/ws/window_...
services/ui/ws/window_tree.cc:1779: ServerWindow* window =
On 2017/04/26 22:16:16, sky wrote:
> Should this fail if cursor type is custom and cursor_frames is empty?

Changed this method to do lots of validation with more of the DVLOGs you love.

[sky, 2017-04-26 23:35:01]

LGTM

https://codereview.chromium.org/2830703003/diff/220001/services/ui/public/int...
File services/ui/public/interfaces/cursor/cursor_struct_traits_unittest.cc
(right):

https://codereview.chromium.org/2830703003/diff/220001/services/ui/public/int...
services/ui/public/interfaces/cursor/cursor_struct_traits_unittest.cc:113:
ASSERT_TRUE(EchoCursorData(input, &output));
On 2017/04/26 23:09:04, Elliot Glaysher wrote:
> On 2017/04/26 22:16:15, sky wrote:
> > optional: you don't need to route through mojom to test serialization. See
> >
>
https://groups.google.com/a/chromium.org/forum/#!searchin/chromium-mojo/trait...
> > .
> 
> I'm confused. Isn't that what I'm doing here? This calls the
> mojom::Deserialize(mojom::Serialize()) pattern wrapped in EchoCursorData().

Indeed you are. I was confused by the name. Sorry.

https://codereview.chromium.org/2830703003/diff/220001/services/ui/ws/window_...
File services/ui/ws/window_tree.cc (right):

https://codereview.chromium.org/2830703003/diff/220001/services/ui/ws/window_...
services/ui/ws/window_tree.cc:1779: ServerWindow* window =
On 2017/04/26 23:09:04, Elliot Glaysher wrote:
> On 2017/04/26 22:16:16, sky wrote:
> > Should this fail if cursor type is custom and cursor_frames is empty?
> 
> Changed this method to do lots of validation with more of the DVLOGs you love.

I think you meant *love* :)
Seriously though, I have found the DVLOGs some what helpful in tracking down
random failures. Hopefully I'm not the only one...

[commit-bot: I haz the power, 2017-04-27 00:42:39]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-27 00:42:40]

Dry run: This issue passed the CQ dry run.

[Elliot Glaysher, 2017-04-27 18:19:39]

ping tsepez and sadrul

[Tom Sepez, 2017-04-27 19:52:47]

lgtm

[sadrul, 2017-04-27 20:25:28]

lgtm (there's a nit)

https://codereview.chromium.org/2830703003/diff/240001/content/renderer/mus/r...
File content/renderer/mus/renderer_window_tree_client.h (right):

https://codereview.chromium.org/2830703003/diff/240001/content/renderer/mus/r...
content/renderer/mus/renderer_window_tree_client.h:137: void
OnWindowCursorChanged(ui::Id window_id, ui::CursorData cursor) override;
const ui::CursorData&?

[Elliot Glaysher, 2017-04-27 20:42:12]

https://codereview.chromium.org/2830703003/diff/240001/content/renderer/mus/r...
File content/renderer/mus/renderer_window_tree_client.h (right):

https://codereview.chromium.org/2830703003/diff/240001/content/renderer/mus/r...
content/renderer/mus/renderer_window_tree_client.h:137: void
OnWindowCursorChanged(ui::Id window_id, ui::CursorData cursor) override;
On 2017/04/27 20:25:28, sadrul wrote:
> const ui::CursorData&?

Can't. This is a copyable data type autogenerated in a mojom header.

[Elliot Glaysher, 2017-04-27 20:42:19]

The CQ bit was checked by erg@chromium.org

[Elliot Glaysher, 2017-04-27 20:42:20]

The patchset sent to the CQ was uploaded after l-g-t-m from tsepez@chromium.org,
sadrul@chromium.org, sky@chromium.org
Link to the patchset: https://codereview.chromium.org/2830703003/#ps260001
(title: "rebase to tot")

[commit-bot: I haz the power, 2017-04-27 20:42:47]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[sadrul, 2017-04-27 20:43:06]

https://codereview.chromium.org/2830703003/diff/240001/content/renderer/mus/r...
File content/renderer/mus/renderer_window_tree_client.h (right):

https://codereview.chromium.org/2830703003/diff/240001/content/renderer/mus/r...
content/renderer/mus/renderer_window_tree_client.h:137: void
OnWindowCursorChanged(ui::Id window_id, ui::CursorData cursor) override;
On 2017/04/27 20:42:12, Elliot Glaysher wrote:
> On 2017/04/27 20:25:28, sadrul wrote:
> > const ui::CursorData&?
> 
> Can't. This is a copyable data type autogenerated in a mojom header.

Can we make CursorData movable, then?

[Elliot Glaysher, 2017-04-27 20:49:48]

On 2017/04/27 20:43:06, sadrul wrote:
>
https://codereview.chromium.org/2830703003/diff/240001/content/renderer/mus/r...
> File content/renderer/mus/renderer_window_tree_client.h (right):
> 
>
https://codereview.chromium.org/2830703003/diff/240001/content/renderer/mus/r...
> content/renderer/mus/renderer_window_tree_client.h:137: void
> OnWindowCursorChanged(ui::Id window_id, ui::CursorData cursor) override;
> On 2017/04/27 20:42:12, Elliot Glaysher wrote:
> > On 2017/04/27 20:25:28, sadrul wrote:
> > > const ui::CursorData&?
> > 
> > Can't. This is a copyable data type autogenerated in a mojom header.
> 
> Can we make CursorData movable, then?

Isn't it already? I'm std::move()ing the structure around.

[Elliot Glaysher, 2017-04-27 20:52:27]

The CQ bit was unchecked by erg@chromium.org

[Elliot Glaysher, 2017-04-27 20:53:04]

On 2017/04/27 20:49:48, Elliot Glaysher wrote:
> On 2017/04/27 20:43:06, sadrul wrote:
> >
>
https://codereview.chromium.org/2830703003/diff/240001/content/renderer/mus/r...
> > File content/renderer/mus/renderer_window_tree_client.h (right):
> > 
> >
>
https://codereview.chromium.org/2830703003/diff/240001/content/renderer/mus/r...
> > content/renderer/mus/renderer_window_tree_client.h:137: void
> > OnWindowCursorChanged(ui::Id window_id, ui::CursorData cursor) override;
> > On 2017/04/27 20:42:12, Elliot Glaysher wrote:
> > > On 2017/04/27 20:25:28, sadrul wrote:
> > > > const ui::CursorData&?
> > > 
> > > Can't. This is a copyable data type autogenerated in a mojom header.
> > 
> > Can we make CursorData movable, then?
> 
> Isn't it already? I'm std::move()ing the structure around.

Apparently the answer is no and std::move() decays to a reference.

[Elliot Glaysher, 2017-04-27 20:58:09]

The CQ bit was checked by erg@chromium.org

[Elliot Glaysher, 2017-04-27 20:58:09]

The patchset sent to the CQ was uploaded after l-g-t-m from tsepez@chromium.org,
sadrul@chromium.org, sky@chromium.org
Link to the patchset: https://codereview.chromium.org/2830703003/#ps280001
(title: "Add default move ctor and assignment to CursorData.")

[commit-bot: I haz the power, 2017-04-27 20:58:48]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-27 21:19:12]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-27 21:19:14]

Try jobs failed on following builders:
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Elliot Glaysher, 2017-04-27 21:46:20]

The CQ bit was checked by erg@chromium.org

[Elliot Glaysher, 2017-04-27 21:46:21]

The patchset sent to the CQ was uploaded after l-g-t-m from tsepez@chromium.org,
sadrul@chromium.org, sky@chromium.org
Link to the patchset: https://codereview.chromium.org/2830703003/#ps300001
(title: "fix cast_shell_linux")

[commit-bot: I haz the power, 2017-04-27 21:46:56]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-28 02:06:38]

CQ is committing da patch.

Bot data: {"patchset_id": 300001, "attempt_start_ts": 1493329580939580,
"parent_rev": "8bea4c8f8976bc786f89853aa482b468cc62177d", "commit_rev":
"a3cfe1428faf4f5bd556e5d9aa4924a7d0fb02f4"}

[commit-bot: I haz the power, 2017-04-28 02:06:49]

Description was changed from

==========
[views-mus] Support custom cursors.

This changes the mus interface to pass a CursorData instead of a
CursorType. The previous patch had mus clients create CursorDatas on
ozone platforms, and this patch passes CursorDatas across the mojo pipe
and passes it to the native ozone platform on the window server.

BUG=705037
==========

to

==========
[views-mus] Support custom cursors.

This changes the mus interface to pass a CursorData instead of a
CursorType. The previous patch had mus clients create CursorDatas on
ozone platforms, and this patch passes CursorDatas across the mojo pipe
and passes it to the native ozone platform on the window server.

BUG=705037

Review-Url: https://codereview.chromium.org/2830703003
Cr-Commit-Position: refs/heads/master@{#467832}
Committed:
https://chromium.googlesource.com/chromium/src/+/a3cfe1428faf4f5bd556e5d9aa49...
==========

[commit-bot: I haz the power, 2017-04-28 02:06:51]

Committed patchset #16 (id:300001) as
https://chromium.googlesource.com/chromium/src/+/a3cfe1428faf4f5bd556e5d9aa49...