Mojo C++ bindings: support enum validation.

Mojo C++ bindings: support enum validation.

By default, enums are not extensible, which means any unknown value will fail validation. If an enum may grow in the future, it needs to have [Extensible=True] attribute specified. In that case, the user code is responsible for handling unknown values properly.

This CL doesn't add corresponding validation for JS or Java.

BUG=404186
Committed: https://crrev.com/dca229ec9a69564985d182a16c9f2068db4d1989
Cr-Commit-Position: refs/heads/master@{#371954}

[yzshen1, 2016-01-26 00:59:00]

Description was changed from

==========
Mojo C++ bindings: support enum validation.

BUG=404186
==========

to

==========
Mojo C++ bindings: support enum validation.

By default, enums are not extensible, which means any unknown value will fail
validation. If an enum may grow in the future, it needs to have
[Extensible=True] attribute specified. In that case, the user code is
responsible for handling unknown values properly.

This CL doesn't add corresponding validation for JS or Java.

BUG=404186
==========

[yzshen1, 2016-01-26 00:59:00]

yzshen@chromium.org changed reviewers:
+ jam@chromium.org, lhchavez@chromium.org, rockot@chromium.org

[yzshen1, 2016-01-26 00:59:48]

Hi, reviewers.

Would you please take a look? Thanks!

Ken: Everything
John: OWNER and high-level idea.
Luis: ARC related changes.

[Ken Rockot(use gerrit already), 2016-01-26 15:39:20]

awesome. everything lgtm

https://codereview.chromium.org/1618963006/diff/20001/mojo/public/cpp/binding...
File mojo/public/cpp/bindings/lib/bindings_internal.h (right):

https://codereview.chromium.org/1618963006/diff/20001/mojo/public/cpp/binding...
mojo/public/cpp/bindings/lib/bindings_internal.h:97: typedef char
(*EnsureTypeIsComplete)[sizeof(T)];
nit: any reason not to use a c++11 alias here and below? just convention?

[Luis Héctor Chávez, 2016-01-26 16:41:20]

On 2016/01/26 15:39:20, Ken Rockot wrote:
> awesome. everything lgtm
> 
>
https://codereview.chromium.org/1618963006/diff/20001/mojo/public/cpp/binding...
> File mojo/public/cpp/bindings/lib/bindings_internal.h (right):
> 
>
https://codereview.chromium.org/1618963006/diff/20001/mojo/public/cpp/binding...
> mojo/public/cpp/bindings/lib/bindings_internal.h:97: typedef char
> (*EnsureTypeIsComplete)[sizeof(T)];
> nit: any reason not to use a c++11 alias here and below? just convention?

the following file got added yesterday night and also needs to be extensible:
components/arc/common/net.mojom

otherwise lgtm.

[yzshen1, 2016-01-26 18:16:04]

> the following file got added yesterday night and also
> needs to be extensible:
> components/arc/common/net.mojom
Done. Thanks!

https://codereview.chromium.org/1618963006/diff/20001/mojo/public/cpp/binding...
File mojo/public/cpp/bindings/lib/bindings_internal.h (right):

https://codereview.chromium.org/1618963006/diff/20001/mojo/public/cpp/binding...
mojo/public/cpp/bindings/lib/bindings_internal.h:97: typedef char
(*EnsureTypeIsComplete)[sizeof(T)];
On 2016/01/26 15:39:20, Ken Rockot wrote:
> nit: any reason not to use a c++11 alias here and below? just convention?

Done. I also refactored a little bit. Thanks!

[jam, 2016-01-27 03:33:26]

looking at the consumers, they have to call IsKnownEnumValue to do the
verification? i.e. this isn't automatic (for the non-extensible case, which that
one isn't)?

[yzshen1, 2016-01-27 06:55:06]

On 2016/01/27 03:33:26, jam wrote:
> looking at the consumers, they have to call IsKnownEnumValue to do the
> verification? i.e. this isn't automatic (for the non-extensible case, which
that
> one isn't)?

Actually, it is automatic: by default mojo enums are not extensible, automatic
verification is done to reject any unknown values. The user code doesn't have to
do anything. Almost all chromium usage falls into this category.
If [Extensible=True] is specified, the user code needs to take responsibility of
handling unknown values. Only in that case, calling IsKnownEnumValue() is
necessary.

I have removed some unnecessary checks.

[jam, 2016-01-27 16:11:33]

On 2016/01/27 06:55:06, yzshen1 wrote:
> On 2016/01/27 03:33:26, jam wrote:
> > looking at the consumers, they have to call IsKnownEnumValue to do the
> > verification? i.e. this isn't automatic (for the non-extensible case, which
> that
> > one isn't)?
> 
> Actually, it is automatic: by default mojo enums are not extensible, automatic
> verification is done to reject any unknown values. The user code doesn't have
to
> do anything. Almost all chromium usage falls into this category.
> If [Extensible=True] is specified, the user code needs to take responsibility
of
> handling unknown values. Only in that case, calling IsKnownEnumValue() is
> necessary.
> 
> I have removed some unnecessary checks.

Great, that's what I thought from reading the bug but the DCHECKS threw me off
:)

lgtm, this is awesome.

[yzshen1, 2016-01-27 19:10:35]

The CQ bit was checked by yzshen@chromium.org

[yzshen1, 2016-01-27 19:10:36]

The patchset sent to the CQ was uploaded after l-g-t-m from rockot@chromium.org,
lhchavez@chromium.org
Link to the patchset: https://codereview.chromium.org/1618963006/#ps80001
(title: " ")

[commit-bot: I haz the power, 2016-01-27 19:12:09]

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

[commit-bot: I haz the power, 2016-01-27 19:29:33]

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

[commit-bot: I haz the power, 2016-01-27 19:29:34]

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

[yzshen1, 2016-01-27 21:32:49]

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

[yzshen1, 2016-01-27 21:32:49]

Hi, Tom.

Would you please review components/arc/common/*.mojom?

This CL doesn't make any semantic changes to those interfaces. Adding
[Extensible=True] to some enum definitions so that their meaning remains the
same after this CL.

[Tom Sepez, 2016-01-27 23:56:54]

On 2016/01/27 21:32:49, yzshen1 wrote:
> Hi, Tom.
> 
> Would you please review components/arc/common/*.mojom?
> 
> This CL doesn't make any semantic changes to those interfaces. Adding
> [Extensible=True] to some enum definitions so that their meaning remains the
> same after this CL.

LGTM.

[yzshen1, 2016-01-28 01:18:44]

The CQ bit was checked by yzshen@chromium.org

[commit-bot: I haz the power, 2016-01-28 01:20:23]

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

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

Description was changed from

==========
Mojo C++ bindings: support enum validation.

By default, enums are not extensible, which means any unknown value will fail
validation. If an enum may grow in the future, it needs to have
[Extensible=True] attribute specified. In that case, the user code is
responsible for handling unknown values properly.

This CL doesn't add corresponding validation for JS or Java.

BUG=404186
==========

to

==========
Mojo C++ bindings: support enum validation.

By default, enums are not extensible, which means any unknown value will fail
validation. If an enum may grow in the future, it needs to have
[Extensible=True] attribute specified. In that case, the user code is
responsible for handling unknown values properly.

This CL doesn't add corresponding validation for JS or Java.

BUG=404186
==========

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

Committed patchset #5 (id:80001)

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

Description was changed from

==========
Mojo C++ bindings: support enum validation.

By default, enums are not extensible, which means any unknown value will fail
validation. If an enum may grow in the future, it needs to have
[Extensible=True] attribute specified. In that case, the user code is
responsible for handling unknown values properly.

This CL doesn't add corresponding validation for JS or Java.

BUG=404186
==========

to

==========
Mojo C++ bindings: support enum validation.

By default, enums are not extensible, which means any unknown value will fail
validation. If an enum may grow in the future, it needs to have
[Extensible=True] attribute specified. In that case, the user code is
responsible for handling unknown values properly.

This CL doesn't add corresponding validation for JS or Java.

BUG=404186

Committed: https://crrev.com/dca229ec9a69564985d182a16c9f2068db4d1989
Cr-Commit-Position: refs/heads/master@{#371954}
==========

[commit-bot: I haz the power, 2016-01-28 02:05:06]

Patchset 5 (id:??) landed as
https://crrev.com/dca229ec9a69564985d182a16c9f2068db4d1989
Cr-Commit-Position: refs/heads/master@{#371954}