Change ui cursor identifiers to an enum class.

Change ui cursor identifiers to an enum class.

ui::Cursor used to use ints across the codebase to identify
cursors. This patch changes that to an enum class, changes all usages of
these constants in chrome, and then creates an EnumTraits mapping of
this new enum to the mojo transport enum.

This means cursor enums are now validated when they previously were just
assumed to be correct.

BUG=705037
R=sky,tsepez
TBR=jam

Review-Url: https://codereview.chromium.org/2833163002
Cr-Commit-Position: refs/heads/master@{#467047}
Committed: https://chromium.googlesource.com/chromium/src/+/eeba7c6297fb07e155f3fef779a6ea448be621bd

[Elliot Glaysher, 2017-04-21 18:11:34]

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

[commit-bot: I haz the power, 2017-04-21 18:11:57]

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-21 18:34:58]

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

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

Dry run: Try jobs failed on following builders:
  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-21 19:10:11]

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

[commit-bot: I haz the power, 2017-04-21 19:10: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-21 19:30:31]

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

[commit-bot: I haz the power, 2017-04-21 19:30:32]

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_...)

[Elliot Glaysher, 2017-04-21 19:34:33]

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

[commit-bot: I haz the power, 2017-04-21 19:35:01]

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-21 19:56:44]

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

[commit-bot: I haz the power, 2017-04-21 19:56:45]

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-21 20:06:25]

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

[commit-bot: I haz the power, 2017-04-21 20:06: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-21 20:24:01]

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

[commit-bot: I haz the power, 2017-04-21 20:24:02]

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_...)
  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-21 20:28:20]

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

[commit-bot: I haz the power, 2017-04-21 20:28:48]

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-21 20:46:03]

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

[commit-bot: I haz the power, 2017-04-21 20:46:33]

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-21 21:06:06]

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

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

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-21 21:26:03]

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

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

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-21 22:05:20]

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

[commit-bot: I haz the power, 2017-04-21 22:05:57]

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-21 22:50:17]

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

[commit-bot: I haz the power, 2017-04-21 22:50:18]

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-21 22:52:02]

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

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

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-21 23:16:34]

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

[commit-bot: I haz the power, 2017-04-21 23:16:46]

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-22 00:03:17]

Description was changed from

==========
Change ui cursor identifiers to an enum class.

ui::Cursor used to use ints across the codebase to identify
cursors. This patch changes that to an enum class, changes all usages of
these constants in chrome, and then creates an EnumTraits mapping of
this new enum to the mojo transport enum.

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

to

==========
Change ui cursor identifiers to an enum class.

ui::Cursor used to use ints across the codebase to identify
cursors. This patch changes that to an enum class, changes all usages of
these constants in chrome, and then creates an EnumTraits mapping of
this new enum to the mojo transport enum.

This means cursor enums are now validated when they previously were just
assumed to be correct.

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

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

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

[commit-bot: I haz the power, 2017-04-22 00:53:45]

Dry run: This issue passed the CQ dry run.

[Elliot Glaysher, 2017-04-24 17:15:25]

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

[Elliot Glaysher, 2017-04-24 18:19:56]

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

[commit-bot: I haz the power, 2017-04-24 18:20:40]

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-24 19:39:10]

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

[commit-bot: I haz the power, 2017-04-24 19:39:11]

Dry run: This issue passed the CQ dry run.

[Elliot Glaysher, 2017-04-24 19:53:43]

tsepez: cursor_struct_traits.cc. we're now doing validation on the incoming
cursor id while we weren't previously.

jam: content, components; patch is just renaming mostly.

sky: rest. this is mostly just renaming and changing int->CursorType, but real
changes are in cursor.h.

[Tom Sepez, 2017-04-24 20:06:45]

lgtm

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

https://codereview.chromium.org/2833163002/diff/200001/services/ui/public/int...
services/ui/public/interfaces/cursor/cursor_struct_traits_unittest.cc:50: for
(int i = 0; i < 43; ++i) {
nit: is there a symbolic constant for 43 we can use?

[sky, 2017-04-24 20:12:44]

Sorry to bring up the style thing here given how much code you're touching:(

https://codereview.chromium.org/2833163002/diff/200001/ui/base/cursor/cursor.h
File ui/base/cursor/cursor.h (right):

https://codereview.chromium.org/2833163002/diff/200001/ui/base/cursor/cursor....
ui/base/cursor/cursor.h:29: CURSOR_NULL = 0,
Sorry to mention this here, but the new style is kFoo, e.g. kNull here. The
style guide allows both, but says, "new code should prefer constant-style naming
if possible.".

https://codereview.chromium.org/2833163002/diff/200001/ui/base/cursor/cursor....
ui/base/cursor/cursor.h:96: Cursor(CursorType type);
explicit (I'm not sure why it wasn't explicit before).

[Elliot Glaysher, 2017-04-24 22:31:13]

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

[commit-bot: I haz the power, 2017-04-24 22:32:29]

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-24 22:48:52]

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

[commit-bot: I haz the power, 2017-04-24 22:48:53]

Dry run: 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-24 22:54:43]

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

[commit-bot: I haz the power, 2017-04-24 22:55: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-25 00:16:14]

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

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

Dry run: This issue passed the CQ dry run.

[Elliot Glaysher, 2017-04-25 17:16:46]

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

https://codereview.chromium.org/2833163002/diff/200001/services/ui/public/int...
services/ui/public/interfaces/cursor/cursor_struct_traits_unittest.cc:50: for
(int i = 0; i < 43; ++i) {
On 2017/04/24 20:06:44, Tom Sepez wrote:
> nit: is there a symbolic constant for 43 we can use?

Done.

https://codereview.chromium.org/2833163002/diff/200001/ui/base/cursor/cursor.h
File ui/base/cursor/cursor.h (right):

https://codereview.chromium.org/2833163002/diff/200001/ui/base/cursor/cursor....
ui/base/cursor/cursor.h:29: CURSOR_NULL = 0,
On 2017/04/24 20:12:43, sky wrote:
> Sorry to mention this here, but the new style is kFoo, e.g. kNull here. The
> style guide allows both, but says, "new code should prefer constant-style
naming
> if possible.".

Done.

https://codereview.chromium.org/2833163002/diff/200001/ui/base/cursor/cursor....
ui/base/cursor/cursor.h:96: Cursor(CursorType type);
On 2017/04/24 20:12:43, sky wrote:
> explicit (I'm not sure why it wasn't explicit before).

This can't be explicit.

gfx::NativeCursor is the typedef used in cross platform code to pass cursors
around. It is only a ui::Cursor on USE_AURA. Everywhere else it is a pointer
type, and the integer (now enum) is coerced to the underlying pointer type
instead of implicit construction.

[sky, 2017-04-25 17:50:44]

LGTM

[Elliot Glaysher, 2017-04-25 17:54:57]

Description was changed from

==========
Change ui cursor identifiers to an enum class.

ui::Cursor used to use ints across the codebase to identify
cursors. This patch changes that to an enum class, changes all usages of
these constants in chrome, and then creates an EnumTraits mapping of
this new enum to the mojo transport enum.

This means cursor enums are now validated when they previously were just
assumed to be correct.

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

to

==========
Change ui cursor identifiers to an enum class.

ui::Cursor used to use ints across the codebase to identify
cursors. This patch changes that to an enum class, changes all usages of
these constants in chrome, and then creates an EnumTraits mapping of
this new enum to the mojo transport enum.

This means cursor enums are now validated when they previously were just
assumed to be correct.

BUG=705037
r=sky,tsepez
tbr=jam
==========

[Elliot Glaysher, 2017-04-25 17:55:12]

Description was changed from

==========
Change ui cursor identifiers to an enum class.

ui::Cursor used to use ints across the codebase to identify
cursors. This patch changes that to an enum class, changes all usages of
these constants in chrome, and then creates an EnumTraits mapping of
this new enum to the mojo transport enum.

This means cursor enums are now validated when they previously were just
assumed to be correct.

BUG=705037
r=sky,tsepez
tbr=jam
==========

to

==========
Change ui cursor identifiers to an enum class.

ui::Cursor used to use ints across the codebase to identify
cursors. This patch changes that to an enum class, changes all usages of
these constants in chrome, and then creates an EnumTraits mapping of
this new enum to the mojo transport enum.

This means cursor enums are now validated when they previously were just
assumed to be correct.

BUG=705037
R=sky,tsepez
TBR=jam
==========

[Elliot Glaysher, 2017-04-25 17:55:59]

tbring jam because all the changes in content/ and components/ are simple
renames of constants; no type changes.

[Elliot Glaysher, 2017-04-25 17:56:10]

The CQ bit was checked by erg@chromium.org

[Elliot Glaysher, 2017-04-25 17:56:11]

The patchset sent to the CQ was uploaded after l-g-t-m from tsepez@chromium.org
Link to the patchset: https://codereview.chromium.org/2833163002/#ps240001
(title: "OK, it can't be explicit for mac.")

[commit-bot: I haz the power, 2017-04-25 17:56:37]

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 18:06:20]

CQ is committing da patch.

Bot data: {"patchset_id": 240001, "attempt_start_ts": 1493142970840320,
"parent_rev": "d0e89425386049c68ae2b997d4fd874e4c65b2b6", "commit_rev":
"de8bcdd1089b7626903ab01be576769149e34cbd"}

[commit-bot: I haz the power, 2017-04-25 18:06:36]

CQ is committing da patch.

Bot data: {"patchset_id": 240001, "attempt_start_ts": 1493142970840320,
"parent_rev": "09058898202bcfc400cb59f3b3e2ab45b1fff117", "commit_rev":
"eeba7c6297fb07e155f3fef779a6ea448be621bd"}

[commit-bot: I haz the power, 2017-04-25 18:06:53]

Description was changed from

==========
Change ui cursor identifiers to an enum class.

ui::Cursor used to use ints across the codebase to identify
cursors. This patch changes that to an enum class, changes all usages of
these constants in chrome, and then creates an EnumTraits mapping of
this new enum to the mojo transport enum.

This means cursor enums are now validated when they previously were just
assumed to be correct.

BUG=705037
R=sky,tsepez
TBR=jam
==========

to

==========
Change ui cursor identifiers to an enum class.

ui::Cursor used to use ints across the codebase to identify
cursors. This patch changes that to an enum class, changes all usages of
these constants in chrome, and then creates an EnumTraits mapping of
this new enum to the mojo transport enum.

This means cursor enums are now validated when they previously were just
assumed to be correct.

BUG=705037
R=sky,tsepez
TBR=jam

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

[commit-bot: I haz the power, 2017-04-25 18:06:55]

Committed patchset #13 (id:240001) as
https://chromium.googlesource.com/chromium/src/+/eeba7c6297fb07e155f3fef779a6...