Re: [src/ui/metro_viewer ]Apply automatic range checks to enum types across IPC (issue 611753002 by kulkarni.a@samsung.com)

[na...@chromium.org]

Please be precise when you ask someone to review. You are missing any
specifics
when listing our aliases.


https://codereview.chromium.org/611753002/diff/1/ui/metro_viewer/metro_viewer_messages.h
File ui/metro_viewer/metro_viewer_messages.h (right):

https://codereview.chromium.org/611753002/diff/1/ui/metro_viewer/metro_viewer_messages.h#newcode18
ui/metro_viewer/metro_viewer_messages.h:18:
IPC_ENUM_TRAITS_MAX_VALUE(ui::EventFlags, ui::EventFlags::EF_MOD3_DOWN)
Please define a EF_LAST and use that.

https://codereview.chromium.org/611753002/

[dch...@chromium.org]

https://codereview.chromium.org/611753002/diff/1/ui/metro_viewer/metro_viewer_messages.h
File ui/metro_viewer/metro_viewer_messages.h (right):

https://codereview.chromium.org/611753002/diff/1/ui/metro_viewer/metro_viewer_messages.h#newcode18
ui/metro_viewer/metro_viewer_messages.h:18:
IPC_ENUM_TRAITS_MAX_VALUE(ui::EventFlags, ui::EventFlags::EF_MOD3_DOWN)

EventFlags is a bit set, so this validation fails in the case someone
passes the highest flag set with any other flag.

https://codereview.chromium.org/611753002/

[kulka...@samsung.com]

PTAL.

https://codereview.chromium.org/611753002/

[na...@chromium.org]

Tom, this is another enum as bitfield case. It seems it has a lot more bits
and
not sure if it is worth adding the range check.

https://codereview.chromium.org/611753002/

[tse...@chromium.org]

https://codereview.chromium.org/611753002/diff/20001/ui/metro_viewer/metro_viewer_messages.h
File ui/metro_viewer/metro_viewer_messages.h (right):

https://codereview.chromium.org/611753002/diff/20001/ui/metro_viewer/metro_viewer_messages.h#newcode17
ui/metro_viewer/metro_viewer_messages.h:17:
IPC_ENUM_TRAITS_MAX_VALUE(ui::EventType, ui::EventType::ET_LAST)
This may not work since we play games with ui::RegisterCustomEventType()
to "extend" the range of the enum (dubiously legal, BTW). Let's just add
a comment here about this quirk

IPC_ENUM_TRAITS(ui::EventType) // Extended by
ui::registerCustomEventType().

https://codereview.chromium.org/611753002/diff/20001/ui/metro_viewer/metro_viewer_messages.h#newcode18
ui/metro_viewer/metro_viewer_messages.h:18:
IPC_ENUM_TRAITS_MAX_VALUE(ui::EventFlags, ui::EventFlags::EF_LAST)
This doesn't work because you may need to pass, say, EF_ALTGR_DOWN |
EF_MOD3_DOWN which is greater then EF_LAST (itself equal to
EF_MOD3_DOWN). Let's just mark this as

IPC_ENUM_TRAITS(ui::EventFlags) // Bitmask.

https://codereview.chromium.org/611753002/