Implement YUVVideoDrawQuad StructTraits

Implement YUVVideoDrawQuad StructTraits

BUG=611802
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/651929c6a678ed0d8c4ea218979f51a88bc056d4
Cr-Commit-Position: refs/heads/master@{#400512}

[Fady Samuel, 2016-06-17 15:11:37]

Description was changed from

==========
Implement YUVVideoDrawQuad StructTraits

BUG=
==========

to

==========
Implement YUVVideoDrawQuad StructTraits

BUG=
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[Fady Samuel, 2016-06-17 15:46:35]

Description was changed from

==========
Implement YUVVideoDrawQuad StructTraits

BUG=
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
Implement YUVVideoDrawQuad StructTraits

BUG=611802
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[Fady Samuel, 2016-06-17 15:48:34]

fsamuel@chromium.org changed reviewers:
+ dcheng@chromium.org, enne@chromium.org

[Fady Samuel, 2016-06-17 15:48:34]

+dcheng@ for ipc.
+enne@ for other cc change.

[enne (OOO), 2016-06-17 16:54:31]

On 2016/06/17 at 15:48:34, fsamuel wrote:
> +dcheng@ for ipc.
> +enne@ for other cc change.

cc lgtm

[dcheng, 2016-06-17 20:26:15]

mojo lgtm

https://codereview.chromium.org/2072273002/diff/20001/cc/ipc/quads_struct_tra...
File cc/ipc/quads_struct_traits.cc (right):

https://codereview.chromium.org/2072273002/diff/20001/cc/ipc/quads_struct_tra...
cc/ipc/quads_struct_traits.cc:232: cc::YUVVideoDrawQuad::ColorSpace* out) {
I wonder if we really need to return a bool for non-extensible enums (since Mojo
should have already verified it). I'll ask yzshen@ about this.

I can't decide if this is confusing (because it won't be consistent with
extensible enums) or useful documentation outside the mojom. WDYT?

[yzshen1, 2016-06-17 20:37:49]

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

[yzshen1, 2016-06-17 20:37:50]

https://codereview.chromium.org/2072273002/diff/20001/cc/ipc/quads_struct_tra...
File cc/ipc/quads_struct_traits.cc (right):

https://codereview.chromium.org/2072273002/diff/20001/cc/ipc/quads_struct_tra...
cc/ipc/quads_struct_traits.cc:232: cc::YUVVideoDrawQuad::ColorSpace* out) {
On 2016/06/17 20:26:15, dcheng wrote:
> I wonder if we really need to return a bool for non-extensible enums (since
Mojo
> should have already verified it). I'll ask yzshen@ about this.
> 
> I can't decide if this is confusing (because it won't be consistent with
> extensible enums) or useful documentation outside the mojom. WDYT?

I think it is consistent to return a bool because all custom type deseralization
code (such as StructTraits::Read, ArrayTraits::Resize, etc.) is allowed to fail.
Besides, as you said, it would be a little confusing if
extensible/non-extensible enums have different signatures for EnumTraits.


WDYT? Thanks!

[Fady Samuel, 2016-06-17 20:48:14]

The CQ bit was checked by fsamuel@chromium.org

[commit-bot: I haz the power, 2016-06-17 20:48:28]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2072273002/20001

[Fady Samuel, 2016-06-17 21:11:03]

On 2016/06/17 20:37:50, yzshen1 wrote:
>
https://codereview.chromium.org/2072273002/diff/20001/cc/ipc/quads_struct_tra...
> File cc/ipc/quads_struct_traits.cc (right):
> 
>
https://codereview.chromium.org/2072273002/diff/20001/cc/ipc/quads_struct_tra...
> cc/ipc/quads_struct_traits.cc:232: cc::YUVVideoDrawQuad::ColorSpace* out) {
> On 2016/06/17 20:26:15, dcheng wrote:
> > I wonder if we really need to return a bool for non-extensible enums (since
> Mojo
> > should have already verified it). I'll ask yzshen@ about this.
> > 
> > I can't decide if this is confusing (because it won't be consistent with
> > extensible enums) or useful documentation outside the mojom. WDYT?
> 
> I think it is consistent to return a bool because all custom type
deseralization
> code (such as StructTraits::Read, ArrayTraits::Resize, etc.) is allowed to
fail.
> Besides, as you said, it would be a little confusing if
> extensible/non-extensible enums have different signatures for EnumTraits.
> 
> 
> WDYT? Thanks!

I've CQ'ed. FWIW, I agree with Yuzhu, let's keep the bool for consistency.

[commit-bot: I haz the power, 2016-06-17 21:41:27]

Description was changed from

==========
Implement YUVVideoDrawQuad StructTraits

BUG=611802
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
Implement YUVVideoDrawQuad StructTraits

BUG=611802
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[commit-bot: I haz the power, 2016-06-17 21:41:28]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-06-17 21:42:57]

Description was changed from

==========
Implement YUVVideoDrawQuad StructTraits

BUG=611802
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
Implement YUVVideoDrawQuad StructTraits

BUG=611802
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/651929c6a678ed0d8c4ea218979f51a88bc056d4
Cr-Commit-Position: refs/heads/master@{#400512}
==========

[commit-bot: I haz the power, 2016-06-17 21:42:58]

Patchset 2 (id:??) landed as
https://crrev.com/651929c6a678ed0d8c4ea218979f51a88bc056d4
Cr-Commit-Position: refs/heads/master@{#400512}