Change mojo enums to be scoped enums in the generated C++ bindings.

Change mojo enums to be scoped enums in the generated C++ bindings.

Some mojo enums were previously not proper enums: they were constants
for bitfields. These have been replaced by const int32s in the mojoms:
- mus.mojom.EventFlags
- mus.mojom.MouseEventFlags
- mus.mojom.ResizeBehavior
- mus.mojom.WindowTree.AccessPolicy

Some mojo enum values now conflict with macros (mostly on Windows) and
needed to change:
- mus.mojom.Cursor.NULL became CURSOR_NULL (again)
- mus.mojom.KeyboardCode.DELETE became DELETE_KEY
- mus.mojom.WindowManagerErrorCode.ERROR_ACCESS_DENIED became
  ACCESS_DENIED
- device.usb.TransferDirection.IN became INBOUND
- device.usb.TransferDirection.OUT became OUTBOUND
- device.usb.TransferStatus.ERROR became TRANSFER_ERROR
- device.NFCRecordType.OPAQUE became OPAQUE_RECORD
- media.interfaces.Decryptor.Status.ERROR became DECRYPTION_ERROR
- skia.AlphaType.OPAQUE became ALPHA_TYPE_OPAQUE

Committed: https://crrev.com/16fb38a8b87fe700645055e472d84de7aefe48a5
Cr-Commit-Position: refs/heads/master@{#370632}

[Sam McNally, 2016-01-04 07:57:29]

Description was changed from

==========
Change mojo enums to be scoped enums in the generated C++ bindings.
==========

to

==========
Change mojo enums to be scoped enums in the generated C++ bindings.

Some mojo enums were previously not proper enums: they were constants
for bitfields. These have been replaced by const int32s in the mojoms:
- mus.mojom.EventFlags
- mus.mojom.MouseEventFlags
- mus.mojom.ResizeBehavior
- mus.mojom.WindowTree.AccessPolicy

Some mojo enum values now conflict with macros (mostly on Windows) and
needed to change:
- mus.mojom.Cursor.NULL became CURSOR_NULL (again)
- mus.mojom.KeyboardCode.DELETE became DELETE_KEY
- mus.mojom.WindowManagerErrorCode.ERROR_ACCESS_DENIED became
  ACCESS_DENIED
- device.usb.TransferDirection.IN became INBOUND
- device.usb.TransferDirection.OUT became OUTBOUND
- device.usb.TransferStatus.ERROR became TRANSFER_ERROR
- device.NFCRecordType.OPAQUE became OPAQUE_RECORD
- media.interfaces.Decryptor.Status.ERROR became DECRYPTION_ERROR
- skia.AlphaType.OPAQUE became ALPHA_TYPE_OPAQUE
==========

[Sam McNally, 2016-01-05 01:40:16]

Patchset #1 (id:1) has been deleted

[Sam McNally, 2016-01-05 01:40:55]

Patchset #1 (id:20001) has been deleted

[Sam McNally, 2016-01-05 01:41:33]

Patchset #1 (id:40001) has been deleted

[Sam McNally, 2016-01-05 01:42:22]

Patchset #1 (id:60001) has been deleted

[Sam McNally, 2016-01-05 01:42:58]

Patchset #1 (id:80001) has been deleted

[Sam McNally, 2016-01-05 01:43:33]

Patchset #1 (id:100001) has been deleted

[Sam McNally, 2016-01-05 01:44:11]

Patchset #1 (id:120001) has been deleted

[Sam McNally, 2016-01-05 01:44:46]

Patchset #1 (id:140001) has been deleted

[Sam McNally, 2016-01-05 01:45:14]

Patchset #1 (id:160001) has been deleted

[Sam McNally, 2016-01-05 01:45:50]

Patchset #1 (id:180001) has been deleted

[Sam McNally, 2016-01-05 01:46:39]

Patchset #1 (id:200001) has been deleted

[Sam McNally, 2016-01-05 02:32:21]

sammc@chromium.org changed reviewers:
+ jam@chromium.org

[jam, 2016-01-05 21:24:34]

jam@chromium.org changed reviewers:
+ yzshen@chromium.org

[jam, 2016-01-05 21:24:35]

Yuzshu: can you review this? Thanks

[yzshen1, 2016-01-05 21:33:39]

On 2016/01/05 21:24:35, jam wrote:
> Yuzshu: can you review this? Thanks

Sure! I will review it.

[yzshen1, 2016-01-07 19:24:51]

Thanks for the change!

One general comment is that when changing SomeEnumType to int32 for a
variable/parameter/method return, please always consider whether comments need
to be added to clarify what that field means. (I added review comments for all
places where I have found, but maybe I have missed some.)

https://codereview.chromium.org/1527183003/diff/220001/components/mus/public/...
File components/mus/public/interfaces/input_event_constants.mojom (right):

https://codereview.chromium.org/1527183003/diff/220001/components/mus/public/...
components/mus/public/interfaces/input_event_constants.mojom:25: const int32
kEventFlagLeftMouseButton    =     16;
Ah, this is because we don't support shift operators...
Is it better to use hex numbers?

https://codereview.chromium.org/1527183003/diff/220001/components/mus/public/...
File components/mus/public/interfaces/input_event_matcher.mojom (right):

https://codereview.chromium.org/1527183003/diff/220001/components/mus/public/...
components/mus/public/interfaces/input_event_matcher.mojom:28: int32 flags;
Please add comments about what flags is. Previously the type EventFlags at least
give hints about where to look for more info.

https://codereview.chromium.org/1527183003/diff/220001/components/mus/public/...
File components/mus/public/interfaces/input_events.mojom (right):

https://codereview.chromium.org/1527183003/diff/220001/components/mus/public/...
components/mus/public/interfaces/input_events.mojom:108: int32 flags;
Please add comments.

https://codereview.chromium.org/1527183003/diff/220001/mash/browser_driver/br...
File mash/browser_driver/browser_driver_application_delegate.cc (right):

https://codereview.chromium.org/1527183003/diff/220001/mash/browser_driver/br...
mash/browser_driver/browser_driver_application_delegate.cc:27: int event_flags;
Please add comment.

https://codereview.chromium.org/1527183003/diff/220001/mash/wm/property_util.h
File mash/wm/property_util.h (right):

https://codereview.chromium.org/1527183003/diff/220001/mash/wm/property_util....
mash/wm/property_util.h:40: int32_t GetResizeBehavior(const mus::Window*
window);
Please add comment what does the return value mean.

https://codereview.chromium.org/1527183003/diff/220001/media/mojo/services/me...
File media/mojo/services/media_type_converters.cc (right):

https://codereview.chromium.org/1527183003/diff/220001/media/mojo/services/me...
media/mojo/services/media_type_converters.cc:122:
"media::interfaces::DemuxerStream::Type::LAST_TYPE");
+2?

https://codereview.chromium.org/1527183003/diff/220001/mojo/public/tools/bind...
File mojo/public/tools/bindings/generators/cpp_templates/enum_macros.tmpl
(right):

https://codereview.chromium.org/1527183003/diff/220001/mojo/public/tools/bind...
mojo/public/tools/bindings/generators/cpp_templates/enum_macros.tmpl:39:
{{values|map(attribute='name')|join(', ')}}
nit: does it make sense to also add indent for this line? (I understand that we
don't have a very consistent guideline for indentation in templates, though.)

https://codereview.chromium.org/1527183003/diff/220001/mojo/public/tools/bind...
File mojo/public/tools/bindings/generators/mojom_cpp_generator.py (right):

https://codereview.chromium.org/1527183003/diff/220001/mojo/public/tools/bind...
mojo/public/tools/bindings/generators/mojom_cpp_generator.py:465: "to_all_caps":
generator.CamelCaseToAllCaps,
I think now we no longer need "to_all_caps" as well as
generator.CamelCaseToAllCaps. Please remove both if that is true.

[yzshen1, 2016-01-07 19:24:52]

Thanks for the change!

One general comment is that when changing SomeEnumType to int32 for a
variable/parameter/method return, please always consider whether comments need
to be added to clarify what that field means. (I added review comments for all
places where I have found, but maybe I have missed some.)

https://codereview.chromium.org/1527183003/diff/220001/components/mus/public/...
File components/mus/public/interfaces/input_event_constants.mojom (right):

https://codereview.chromium.org/1527183003/diff/220001/components/mus/public/...
components/mus/public/interfaces/input_event_constants.mojom:25: const int32
kEventFlagLeftMouseButton    =     16;
Ah, this is because we don't support shift operators...
Is it better to use hex numbers?

https://codereview.chromium.org/1527183003/diff/220001/components/mus/public/...
File components/mus/public/interfaces/input_event_matcher.mojom (right):

https://codereview.chromium.org/1527183003/diff/220001/components/mus/public/...
components/mus/public/interfaces/input_event_matcher.mojom:28: int32 flags;
Please add comments about what flags is. Previously the type EventFlags at least
give hints about where to look for more info.

https://codereview.chromium.org/1527183003/diff/220001/components/mus/public/...
File components/mus/public/interfaces/input_events.mojom (right):

https://codereview.chromium.org/1527183003/diff/220001/components/mus/public/...
components/mus/public/interfaces/input_events.mojom:108: int32 flags;
Please add comments.

https://codereview.chromium.org/1527183003/diff/220001/mash/browser_driver/br...
File mash/browser_driver/browser_driver_application_delegate.cc (right):

https://codereview.chromium.org/1527183003/diff/220001/mash/browser_driver/br...
mash/browser_driver/browser_driver_application_delegate.cc:27: int event_flags;
Please add comment.

https://codereview.chromium.org/1527183003/diff/220001/mash/wm/property_util.h
File mash/wm/property_util.h (right):

https://codereview.chromium.org/1527183003/diff/220001/mash/wm/property_util....
mash/wm/property_util.h:40: int32_t GetResizeBehavior(const mus::Window*
window);
Please add comment what does the return value mean.

https://codereview.chromium.org/1527183003/diff/220001/media/mojo/services/me...
File media/mojo/services/media_type_converters.cc (right):

https://codereview.chromium.org/1527183003/diff/220001/media/mojo/services/me...
media/mojo/services/media_type_converters.cc:122:
"media::interfaces::DemuxerStream::Type::LAST_TYPE");
+2?

https://codereview.chromium.org/1527183003/diff/220001/mojo/public/tools/bind...
File mojo/public/tools/bindings/generators/cpp_templates/enum_macros.tmpl
(right):

https://codereview.chromium.org/1527183003/diff/220001/mojo/public/tools/bind...
mojo/public/tools/bindings/generators/cpp_templates/enum_macros.tmpl:39:
{{values|map(attribute='name')|join(', ')}}
nit: does it make sense to also add indent for this line? (I understand that we
don't have a very consistent guideline for indentation in templates, though.)

https://codereview.chromium.org/1527183003/diff/220001/mojo/public/tools/bind...
File mojo/public/tools/bindings/generators/mojom_cpp_generator.py (right):

https://codereview.chromium.org/1527183003/diff/220001/mojo/public/tools/bind...
mojo/public/tools/bindings/generators/mojom_cpp_generator.py:465: "to_all_caps":
generator.CamelCaseToAllCaps,
I think now we no longer need "to_all_caps" as well as
generator.CamelCaseToAllCaps. Please remove both if that is true.

[Sam McNally, 2016-01-08 05:12:52]

https://codereview.chromium.org/1527183003/diff/220001/components/mus/public/...
File components/mus/public/interfaces/input_event_constants.mojom (right):

https://codereview.chromium.org/1527183003/diff/220001/components/mus/public/...
components/mus/public/interfaces/input_event_constants.mojom:25: const int32
kEventFlagLeftMouseButton    =     16;
On 2016/01/07 19:24:52, yzshen1 wrote:
> Ah, this is because we don't support shift operators...
> Is it better to use hex numbers? 

Done.

https://codereview.chromium.org/1527183003/diff/220001/components/mus/public/...
File components/mus/public/interfaces/input_event_matcher.mojom (right):

https://codereview.chromium.org/1527183003/diff/220001/components/mus/public/...
components/mus/public/interfaces/input_event_matcher.mojom:28: int32 flags;
On 2016/01/07 19:24:52, yzshen1 wrote:
> Please add comments about what flags is. Previously the type EventFlags at
least
> give hints about where to look for more info.

Done.

https://codereview.chromium.org/1527183003/diff/220001/components/mus/public/...
File components/mus/public/interfaces/input_events.mojom (right):

https://codereview.chromium.org/1527183003/diff/220001/components/mus/public/...
components/mus/public/interfaces/input_events.mojom:108: int32 flags;
On 2016/01/07 19:24:52, yzshen1 wrote:
> Please add comments.

Done.

https://codereview.chromium.org/1527183003/diff/220001/mash/browser_driver/br...
File mash/browser_driver/browser_driver_application_delegate.cc (right):

https://codereview.chromium.org/1527183003/diff/220001/mash/browser_driver/br...
mash/browser_driver/browser_driver_application_delegate.cc:27: int event_flags;
On 2016/01/07 19:24:52, yzshen1 wrote:
> Please add comment.

Done.

https://codereview.chromium.org/1527183003/diff/220001/mash/wm/property_util.h
File mash/wm/property_util.h (right):

https://codereview.chromium.org/1527183003/diff/220001/mash/wm/property_util....
mash/wm/property_util.h:40: int32_t GetResizeBehavior(const mus::Window*
window);
On 2016/01/07 19:24:52, yzshen1 wrote:
> Please add comment what does the return value mean.

Done.

https://codereview.chromium.org/1527183003/diff/220001/media/mojo/services/me...
File media/mojo/services/media_type_converters.cc (right):

https://codereview.chromium.org/1527183003/diff/220001/media/mojo/services/me...
media/mojo/services/media_type_converters.cc:122:
"media::interfaces::DemuxerStream::Type::LAST_TYPE");
On 2016/01/07 19:24:52, yzshen1 wrote:
> +2?

Done.

https://codereview.chromium.org/1527183003/diff/220001/mojo/public/tools/bind...
File mojo/public/tools/bindings/generators/cpp_templates/enum_macros.tmpl
(right):

https://codereview.chromium.org/1527183003/diff/220001/mojo/public/tools/bind...
mojo/public/tools/bindings/generators/cpp_templates/enum_macros.tmpl:39:
{{values|map(attribute='name')|join(', ')}}
On 2016/01/07 19:24:52, yzshen1 wrote:
> nit: does it make sense to also add indent for this line? (I understand that
we
> don't have a very consistent guideline for indentation in templates, though.)

Done.

https://codereview.chromium.org/1527183003/diff/220001/mojo/public/tools/bind...
File mojo/public/tools/bindings/generators/mojom_cpp_generator.py (right):

https://codereview.chromium.org/1527183003/diff/220001/mojo/public/tools/bind...
mojo/public/tools/bindings/generators/mojom_cpp_generator.py:465: "to_all_caps":
generator.CamelCaseToAllCaps,
On 2016/01/07 19:24:52, yzshen1 wrote:
> I think now we no longer need "to_all_caps" as well as
> generator.CamelCaseToAllCaps. Please remove both if that is true.

Done.

[yzshen1, 2016-01-08 17:07:28]

LGTM

Thanks for making this change!

[Sam McNally, 2016-01-14 07:12:20]

Patchset #9 (id:370001) has been deleted

[Sam McNally, 2016-01-14 07:12:50]

Patchset #9 (id:390001) has been deleted

[Sam McNally, 2016-01-14 07:13:34]

Patchset #7 (id:330001) has been deleted

[Sam McNally, 2016-01-14 07:14:03]

Patchset #7 (id:350001) has been deleted

[Sam McNally, 2016-01-14 07:25:16]

jam: PTAL

[Sam McNally, 2016-01-20 03:02:49]

jam: ping

[jam, 2016-01-20 15:49:24]

On 2016/01/20 03:02:49, Sam McNally wrote:
> jam: ping

sorry I missed this. rubberstamp lgtm (i only looked at some of the files)

[Sam McNally, 2016-01-21 00:10:35]

sammc@chromium.org changed reviewers:
+ tsepez@chromium.org

[Sam McNally, 2016-01-21 00:10:36]

tsepez@chromium.org: Please review changes in
content/public/common/mojo_geoposition.mojom

[Tom Sepez, 2016-01-21 01:02:41]

RS LGTM

[Sam McNally, 2016-01-21 03:30:39]

The CQ bit was checked by sammc@chromium.org

[Sam McNally, 2016-01-21 03:30:40]

The patchset sent to the CQ was uploaded after l-g-t-m from yzshen@chromium.org,
tsepez@chromium.org, jam@chromium.org
Link to the patchset: https://codereview.chromium.org/1527183003/#ps440001
(title: "rebase")

[commit-bot: I haz the power, 2016-01-21 03:33:06]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1527183003/440001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1527183003/440001

[commit-bot: I haz the power, 2016-01-21 05:30:28]

Committed patchset #9 (id:440001)

[commit-bot: I haz the power, 2016-01-21 05:31:09]

Description was changed from

==========
Change mojo enums to be scoped enums in the generated C++ bindings.

Some mojo enums were previously not proper enums: they were constants
for bitfields. These have been replaced by const int32s in the mojoms:
- mus.mojom.EventFlags
- mus.mojom.MouseEventFlags
- mus.mojom.ResizeBehavior
- mus.mojom.WindowTree.AccessPolicy

Some mojo enum values now conflict with macros (mostly on Windows) and
needed to change:
- mus.mojom.Cursor.NULL became CURSOR_NULL (again)
- mus.mojom.KeyboardCode.DELETE became DELETE_KEY
- mus.mojom.WindowManagerErrorCode.ERROR_ACCESS_DENIED became
  ACCESS_DENIED
- device.usb.TransferDirection.IN became INBOUND
- device.usb.TransferDirection.OUT became OUTBOUND
- device.usb.TransferStatus.ERROR became TRANSFER_ERROR
- device.NFCRecordType.OPAQUE became OPAQUE_RECORD
- media.interfaces.Decryptor.Status.ERROR became DECRYPTION_ERROR
- skia.AlphaType.OPAQUE became ALPHA_TYPE_OPAQUE
==========

to

==========
Change mojo enums to be scoped enums in the generated C++ bindings.

Some mojo enums were previously not proper enums: they were constants
for bitfields. These have been replaced by const int32s in the mojoms:
- mus.mojom.EventFlags
- mus.mojom.MouseEventFlags
- mus.mojom.ResizeBehavior
- mus.mojom.WindowTree.AccessPolicy

Some mojo enum values now conflict with macros (mostly on Windows) and
needed to change:
- mus.mojom.Cursor.NULL became CURSOR_NULL (again)
- mus.mojom.KeyboardCode.DELETE became DELETE_KEY
- mus.mojom.WindowManagerErrorCode.ERROR_ACCESS_DENIED became
  ACCESS_DENIED
- device.usb.TransferDirection.IN became INBOUND
- device.usb.TransferDirection.OUT became OUTBOUND
- device.usb.TransferStatus.ERROR became TRANSFER_ERROR
- device.NFCRecordType.OPAQUE became OPAQUE_RECORD
- media.interfaces.Decryptor.Status.ERROR became DECRYPTION_ERROR
- skia.AlphaType.OPAQUE became ALPHA_TYPE_OPAQUE

Committed: https://crrev.com/16fb38a8b87fe700645055e472d84de7aefe48a5
Cr-Commit-Position: refs/heads/master@{#370632}
==========

[commit-bot: I haz the power, 2016-01-21 05:31:11]

Patchset 9 (id:??) landed as
https://crrev.com/16fb38a8b87fe700645055e472d84de7aefe48a5
Cr-Commit-Position: refs/heads/master@{#370632}