Implement DrawQuad StructTraits

Implement DrawQuad StructTraits

This CL partially implements DrawQuad and QuadList StructTraits.

The code is a bit clumsy currently for a number of reasons:

1. QuadList does not provide O(1) random access and so we use an
iterator.
2. DrawQuads are created inline within a QuadList, and so we must
create them prior to invoking the ArrayTraits.

BUG=611802
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/6ea4b2c26e6f2d2a021b10c965c3d06adb493898
Cr-Commit-Position: refs/heads/master@{#400345}

[Fady Samuel, 2016-06-13 19:54:38]

Description was changed from

==========
WIP: Implement DrawQuad StructTraits


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

to

==========
WIP: Implement DrawQuad StructTraits


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

[Fady Samuel, 2016-06-14 01:42:41]

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

[Fady Samuel, 2016-06-14 01:42:42]

+yzshen@: Hi Yuzhu, this is still work in progress, but I'd appreciate some
early feedback. I believe this matches our conversations on Friday? Thanks!

[yzshen1, 2016-06-14 16:55:57]

https://codereview.chromium.org/2064833002/diff/80001/cc/ipc/quads_struct_tra...
File cc/ipc/quads_struct_traits.h (right):

https://codereview.chromium.org/2064833002/diff/80001/cc/ipc/quads_struct_tra...
cc/ipc/quads_struct_traits.h:124: struct
StructTraits<cc::mojom::SolidColorQuadState, uint8_t> {
can this be

struct StructTraits<cc::mojom::SolidColorQuadState, void*> ?

[Fady Samuel, 2016-06-14 21:56:49]

PTAL Yuzhu,

Once you're happy I'll pass this along to ownerships Thanks!

https://codereview.chromium.org/2064833002/diff/80001/cc/ipc/quads_struct_tra...
File cc/ipc/quads_struct_traits.h (right):

https://codereview.chromium.org/2064833002/diff/80001/cc/ipc/quads_struct_tra...
cc/ipc/quads_struct_traits.h:124: struct
StructTraits<cc::mojom::SolidColorQuadState, uint8_t> {
On 2016/06/14 16:55:57, yzshen1 wrote:
> can this be
> 
> struct StructTraits<cc::mojom::SolidColorQuadState, void*> ?

This doesn't work in general because ArrayTraits::GetValue returns an rvalue for
QuadLIst because QuadList::ElementAt returns an rvalue.

[Fady Samuel, 2016-06-14 22:01:04]

Description was changed from

==========
WIP: Implement DrawQuad StructTraits


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

to

==========
Implement DrawQuad StructTraits

This CL partially implements DrawQuad and QuadList StructTraits.

The code is a bit clumsy currently for a number of rasons:

1. QuadList does not provide O(1) random access and so we use an iterator.
2. DrawQuads are created inline within a QuadList, and so we must create them
prior to invoking the ArrayTraits.

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

[Fady Samuel, 2016-06-14 22:49:04]

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

[Fady Samuel, 2016-06-14 22:49:05]

+ben@ once Yuzhu is happy
+dcheng@ for cc/ipc (tsepez@ is OOO)
+enne@ for other cc changes.

This CL does not fully implement all DrawQuad serialization. I'll complete TODOs
in a subsequent CL. Thanks!

[dcheng, 2016-06-15 12:27:20]

https://codereview.chromium.org/2064833002/diff/180001/cc/base/list_container.h
File cc/base/list_container.h (right):

https://codereview.chromium.org/2064833002/diff/180001/cc/base/list_container...
cc/base/list_container.h:151: ListContainer<BaseElementType>& operator=(
<BaseElementType> shouldn't be needed here.

https://codereview.chromium.org/2064833002/diff/180001/cc/ipc/quads.mojom
File cc/ipc/quads.mojom (right):

https://codereview.chromium.org/2064833002/diff/180001/cc/ipc/quads.mojom#new...
cc/ipc/quads.mojom:105: MATERIAL_LAST = YUV_VIDEO_CONTENT,
mojom shouldn't need to define a _LAST enum value, since enums in mojom are
validated by default.

https://codereview.chromium.org/2064833002/diff/180001/cc/ipc/quads_struct_tr...
File cc/ipc/quads_struct_traits.h (right):

https://codereview.chromium.org/2064833002/diff/180001/cc/ipc/quads_struct_tr...
cc/ipc/quads_struct_traits.h:125: return input.material !=
cc::DrawQuad::DEBUG_BORDER;
This is an interesting application of nullable types. I'm not sure how I feel
about it...

https://codereview.chromium.org/2064833002/diff/180001/cc/ipc/quads_struct_tr...
cc/ipc/quads_struct_traits.h:203: cc::DrawQuad* out) {
Let's out-of-line the non-trivial methods (in this case, it looks like just the
Read() methods), unless there's a good (perf) reason not to.

[Fady Samuel, 2016-06-15 14:44:10]

Description was changed from

==========
Implement DrawQuad StructTraits

This CL partially implements DrawQuad and QuadList StructTraits.

The code is a bit clumsy currently for a number of rasons:

1. QuadList does not provide O(1) random access and so we use an iterator.
2. DrawQuads are created inline within a QuadList, and so we must create them
prior to invoking the ArrayTraits.

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

to

==========
Implement DrawQuad StructTraits

This CL partially implements DrawQuad and QuadList StructTraits.

The code is a bit clumsy currently for a number of rasons:

1. QuadList does not provide O(1) random access and so we use an 
iterator.
2. DrawQuads are created inline within a QuadList, and so we must 
create them prior to invoking the ArrayTraits.

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

[Fady Samuel, 2016-06-15 14:46:25]

PTAL Daniel! PTAL Yuzhu, Enne, Ben! Thanks! BTW, this
serialization/deserialization code definitely needs more unit testing than
what's included here. I'd like to do that separately as this CL is exploding in
size very rapidly. I prefer small incremental CLs.

Thanks!

https://codereview.chromium.org/2064833002/diff/180001/cc/base/list_container.h
File cc/base/list_container.h (right):

https://codereview.chromium.org/2064833002/diff/180001/cc/base/list_container...
cc/base/list_container.h:151: ListContainer<BaseElementType>& operator=(
On 2016/06/15 12:27:19, dcheng wrote:
> <BaseElementType> shouldn't be needed here.

Done. Also turns out I don't need the move constructor either. I just need the
move assignment operator.

https://codereview.chromium.org/2064833002/diff/180001/cc/ipc/quads.mojom
File cc/ipc/quads.mojom (right):

https://codereview.chromium.org/2064833002/diff/180001/cc/ipc/quads.mojom#new...
cc/ipc/quads.mojom:105: MATERIAL_LAST = YUV_VIDEO_CONTENT,
On 2016/06/15 12:27:19, dcheng wrote:
> mojom shouldn't need to define a _LAST enum value, since enums in mojom are
> validated by default.

Done.

https://codereview.chromium.org/2064833002/diff/180001/cc/ipc/quads_struct_tr...
File cc/ipc/quads_struct_traits.h (right):

https://codereview.chromium.org/2064833002/diff/180001/cc/ipc/quads_struct_tr...
cc/ipc/quads_struct_traits.h:125: return input.material !=
cc::DrawQuad::DEBUG_BORDER;
On 2016/06/15 12:27:19, dcheng wrote:
> This is an interesting application of nullable types. I'm not sure how I feel
> about it...

I'm not a fan either but I can't think of a better way to map optional fields to
derived type fields.

https://codereview.chromium.org/2064833002/diff/180001/cc/ipc/quads_struct_tr...
cc/ipc/quads_struct_traits.h:203: cc::DrawQuad* out) {
On 2016/06/15 12:27:19, dcheng wrote:
> Let's out-of-line the non-trivial methods (in this case, it looks like just
the
> Read() methods), unless there's a good (perf) reason not to.

Done.

[yzshen1, 2016-06-15 17:12:13]

https://codereview.chromium.org/2064833002/diff/220001/cc/ipc/quads.typemap
File cc/ipc/quads.typemap (right):

https://codereview.chromium.org/2064833002/diff/220001/cc/ipc/quads.typemap#n...
cc/ipc/quads.typemap:15: 
Unnecessary empty line.

https://codereview.chromium.org/2064833002/diff/220001/cc/ipc/quads_struct_tr...
File cc/ipc/quads_struct_traits.cc (right):

https://codereview.chromium.org/2064833002/diff/220001/cc/ipc/quads_struct_tr...
cc/ipc/quads_struct_traits.cc:11: cc::mojom::Material
CCMaterialToMojo(cc::DrawQuad::Material material) {
You can use EnumTraits to do this now.

https://codereview.chromium.org/2064833002/diff/220001/cc/ipc/quads_struct_tr...
cc/ipc/quads_struct_traits.cc:236: StructTraits<cc::mojom::QuadList,
cc::QuadList>::quad_types(
Please note that this getter will be called multtiple times. So the calculation
of |materials| will be done twice. If that is undesirable, please do it in
SetUpContext().

https://codereview.chromium.org/2064833002/diff/220001/cc/ipc/quads_struct_tr...
cc/ipc/quads_struct_traits.cc:258: }
It should be validated that the |quad_types| list agrees with the actual |quads|
list:
- the size of the two are the same. That could be checked in 
ArrayTraits<QuadListArray>::Resize().
- the types in |quad_types| matches those in |quads|. We can do that by checking
whether a cc::DrawQuad pointer (allocated according to |quad_types|) to a
subclass (determined by |quads|) in
StructTraits<cc::mojom::DebugBorderQuadState, cc::DrawQuad>::Read() and its
siblings.

It is sad that this is so complex. Hope the work of exposing DataView simplifies
it.

WDYT?

https://codereview.chromium.org/2064833002/diff/220001/cc/ipc/quads_struct_tr...
File cc/ipc/quads_struct_traits.h (right):

https://codereview.chromium.org/2064833002/diff/220001/cc/ipc/quads_struct_tr...
cc/ipc/quads_struct_traits.h:32: static void SetToNull(cc::DrawQuad* output) {}
Could you please explain why this can be no-op? (And also the other SetToNull()
methods in this file)

[yzshen1, 2016-06-15 17:15:07]

https://codereview.chromium.org/2064833002/diff/220001/cc/ipc/quads_struct_tr...
File cc/ipc/quads_struct_traits.cc (right):

https://codereview.chromium.org/2064833002/diff/220001/cc/ipc/quads_struct_tr...
cc/ipc/quads_struct_traits.cc:258: }
On 2016/06/15 17:12:13, yzshen1 wrote:
> It should be validated that the |quad_types| list agrees with the actual
|quads|
> list:
> - the size of the two are the same. That could be checked in 
> ArrayTraits<QuadListArray>::Resize().
> - the types in |quad_types| matches those in |quads|. We can do that by
checking
> whether a cc::DrawQuad pointer (allocated according to |quad_types|) to a

I mean, We can do that by checking whether a cc::DrawQuad pointer (allocated
according to |quad_types|) has the right type before casting it to a subclass
(determined by |quads|) in StructTraits<cc::mojom::DebugBorderQuadState,
cc::DrawQuad>::Read() and its siblings.

> It is sad that this is so complex. Hope the work of exposing DataView
simplifies
> it.
> 
> WDYT?

[Fady Samuel, 2016-06-15 19:16:00]

PTAL Yuzhu! I believe I've addressed all your comments!

https://codereview.chromium.org/2064833002/diff/220001/cc/ipc/quads.typemap
File cc/ipc/quads.typemap (right):

https://codereview.chromium.org/2064833002/diff/220001/cc/ipc/quads.typemap#n...
cc/ipc/quads.typemap:15: 
On 2016/06/15 17:12:12, yzshen1 wrote:
> Unnecessary empty line.

Done.

https://codereview.chromium.org/2064833002/diff/220001/cc/ipc/quads_struct_tr...
File cc/ipc/quads_struct_traits.cc (right):

https://codereview.chromium.org/2064833002/diff/220001/cc/ipc/quads_struct_tr...
cc/ipc/quads_struct_traits.cc:11: cc::mojom::Material
CCMaterialToMojo(cc::DrawQuad::Material material) {
On 2016/06/15 17:12:12, yzshen1 wrote:
> You can use EnumTraits to do this now.

Done.

https://codereview.chromium.org/2064833002/diff/220001/cc/ipc/quads_struct_tr...
cc/ipc/quads_struct_traits.cc:236: StructTraits<cc::mojom::QuadList,
cc::QuadList>::quad_types(
On 2016/06/15 17:12:12, yzshen1 wrote:
> Please note that this getter will be called multtiple times. So the
calculation
> of |materials| will be done twice. If that is undesirable, please do it in
> SetUpContext().

Done.

https://codereview.chromium.org/2064833002/diff/220001/cc/ipc/quads_struct_tr...
cc/ipc/quads_struct_traits.cc:258: }
On 2016/06/15 17:15:07, yzshen1 wrote:
> On 2016/06/15 17:12:13, yzshen1 wrote:
> > It should be validated that the |quad_types| list agrees with the actual
> |quads|
> > list:
> > - the size of the two are the same. That could be checked in 
> > ArrayTraits<QuadListArray>::Resize().
> > - the types in |quad_types| matches those in |quads|. We can do that by
> checking
> > whether a cc::DrawQuad pointer (allocated according to |quad_types|) to a
> 
> I mean, We can do that by checking whether a cc::DrawQuad pointer (allocated
> according to |quad_types|) has the right type before casting it to a subclass
> (determined by |quads|) in StructTraits<cc::mojom::DebugBorderQuadState,
> cc::DrawQuad>::Read() and its siblings.
> 
> > It is sad that this is so complex. Hope the work of exposing DataView
> simplifies
> > it.
> > 
> > WDYT?
> 

I have written the verification just once in ReadDrawQuad.

In AllocateAndConstruct we fill the material field in the DrawQuad. In
ReadDrawQuad we read the material field again but from the quads array. I verify
there that there's a match between the allocated DrawQuad and the one in the
quads array.

https://codereview.chromium.org/2064833002/diff/220001/cc/ipc/quads_struct_tr...
File cc/ipc/quads_struct_traits.h (right):

https://codereview.chromium.org/2064833002/diff/220001/cc/ipc/quads_struct_tr...
cc/ipc/quads_struct_traits.h:32: static void SetToNull(cc::DrawQuad* output) {}
On 2016/06/15 17:12:13, yzshen1 wrote:
> Could you please explain why this can be no-op? (And also the other
SetToNull()
> methods in this file)

Done.

[yzshen1, 2016-06-15 19:47:51]

LGTM

https://codereview.chromium.org/2064833002/diff/240001/cc/ipc/quads_struct_tr...
File cc/ipc/quads_struct_traits.h (right):

https://codereview.chromium.org/2064833002/diff/240001/cc/ipc/quads_struct_tr...
cc/ipc/quads_struct_traits.h:155: // SolidColorDrawQuad. If it is, then this
should not be called.
SurfaceDrawQuad?

[Fady Samuel, 2016-06-15 19:52:41]

Thanks Yuzhu! :D PTAL:

Ben: components/mus
Daniel: ipc signoff
Enne: cc changes

https://codereview.chromium.org/2064833002/diff/240001/cc/ipc/quads_struct_tr...
File cc/ipc/quads_struct_traits.h (right):

https://codereview.chromium.org/2064833002/diff/240001/cc/ipc/quads_struct_tr...
cc/ipc/quads_struct_traits.h:155: // SolidColorDrawQuad. If it is, then this
should not be called.
On 2016/06/15 19:47:51, yzshen1 wrote:
> SurfaceDrawQuad?

Done.

[Ben Goodger (Google), 2016-06-15 19:53:12]

On 2016/06/15 19:52:41, Fady Samuel wrote:
> Thanks Yuzhu! :D PTAL:
> 
> Ben: components/mus
> Daniel: ipc signoff
> Enne: cc changes
> 
>
https://codereview.chromium.org/2064833002/diff/240001/cc/ipc/quads_struct_tr...
> File cc/ipc/quads_struct_traits.h (right):
> 
>
https://codereview.chromium.org/2064833002/diff/240001/cc/ipc/quads_struct_tr...
> cc/ipc/quads_struct_traits.h:155: // SolidColorDrawQuad. If it is, then this
> should not be called.
> On 2016/06/15 19:47:51, yzshen1 wrote:
> > SurfaceDrawQuad?
> 
> Done.

components/mus lgtm

[enne (OOO), 2016-06-15 20:59:07]

lgtm

[Fady Samuel, 2016-06-15 21:04:24]

Description was changed from

==========
Implement DrawQuad StructTraits

This CL partially implements DrawQuad and QuadList StructTraits.

The code is a bit clumsy currently for a number of rasons:

1. QuadList does not provide O(1) random access and so we use an 
iterator.
2. DrawQuads are created inline within a QuadList, and so we must 
create them prior to invoking the ArrayTraits.

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

to

==========
Implement DrawQuad StructTraits

This CL partially implements DrawQuad and QuadList StructTraits.

The code is a bit clumsy currently for a number of reasons:

1. QuadList does not provide O(1) random access and so we use an 
iterator.
2. DrawQuads are created inline within a QuadList, and so we must 
create them prior to invoking the ArrayTraits.

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

[Fady Samuel, 2016-06-15 21:22:13]

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

[commit-bot: I haz the power, 2016-06-15 21:22:55]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2064833002/260001

[commit-bot: I haz the power, 2016-06-15 21:38:42]

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

[commit-bot: I haz the power, 2016-06-15 21:38:44]

Dry run: Try jobs failed on following builders:
  chromeos_amd64-generic_chromium_compile_only_ng on tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)

[Fady Samuel, 2016-06-15 21:56:46]

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

[commit-bot: I haz the power, 2016-06-15 21:57:40]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2064833002/280001

[commit-bot: I haz the power, 2016-06-15 22:40:09]

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

[commit-bot: I haz the power, 2016-06-15 22:40:10]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[Fady Samuel, 2016-06-16 00:29:43]

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

[commit-bot: I haz the power, 2016-06-16 00:30:30]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2064833002/300001

[commit-bot: I haz the power, 2016-06-16 01:25:22]

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

[commit-bot: I haz the power, 2016-06-16 01:25:23]

Dry run: Try jobs failed on following builders:
  mac_chromium_gn_rel on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_gn_r...)

[Fady Samuel, 2016-06-16 01:28:53]

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

[commit-bot: I haz the power, 2016-06-16 01:29:11]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2064833002/300001

[commit-bot: I haz the power, 2016-06-16 02:03:15]

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

[commit-bot: I haz the power, 2016-06-16 02:03:16]

Dry run: Try jobs failed on following builders:
  mac_chromium_gn_rel on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_gn_r...)

[Fady Samuel, 2016-06-16 02:06:30]

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

[commit-bot: I haz the power, 2016-06-16 02:06:49]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2064833002/300001

[commit-bot: I haz the power, 2016-06-16 02:35:30]

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

[commit-bot: I haz the power, 2016-06-16 02:35:32]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Fady Samuel, 2016-06-16 02:52:54]

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

[commit-bot: I haz the power, 2016-06-16 02:53:07]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2064833002/320001

[commit-bot: I haz the power, 2016-06-16 03:57:40]

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

[commit-bot: I haz the power, 2016-06-16 03:57:41]

Dry run: Exceeded global retry quota

[Fady Samuel, 2016-06-16 10:53:04]

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

[commit-bot: I haz the power, 2016-06-16 10:53:19]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2064833002/320001

[commit-bot: I haz the power, 2016-06-16 11:36:47]

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

[commit-bot: I haz the power, 2016-06-16 11:36:49]

Dry run: This issue passed the CQ dry run.

[dcheng, 2016-06-16 20:57:42]

I still think this patch is a bit too clever, but LGTM for now to get things
unblocked.

https://codereview.chromium.org/2064833002/diff/320001/cc/base/list_container.h
File cc/base/list_container.h (right):

https://codereview.chromium.org/2064833002/diff/320001/cc/base/list_container...
cc/base/list_container.h:147: ListContainer& operator=(ListContainer&& other) {
Even if you don't need it now, it's better form to define both a move
constructor *and* move assignment operator.

https://codereview.chromium.org/2064833002/diff/320001/cc/ipc/quads_struct_tr...
File cc/ipc/quads_struct_traits.h (right):

https://codereview.chromium.org/2064833002/diff/320001/cc/ipc/quads_struct_tr...
cc/ipc/quads_struct_traits.h:345: static void AdvanceIterator(ConstIterator&
iterator) { iterator++; }
Nit: ++iterator here and below

[Fady Samuel, 2016-06-16 22:56:29]

For what it's worth I think Yuzhu and I are in agreement with you, Daniel. This
code looks too magical. I think this highlights missing mojo features that we'll
address in subsequent patches. Thanks! CQ'ing.

https://codereview.chromium.org/2064833002/diff/320001/cc/base/list_container.h
File cc/base/list_container.h (right):

https://codereview.chromium.org/2064833002/diff/320001/cc/base/list_container...
cc/base/list_container.h:147: ListContainer& operator=(ListContainer&& other) {
On 2016/06/16 20:57:41, dcheng wrote:
> Even if you don't need it now, it's better form to define both a move
> constructor *and* move assignment operator.

Agreed. Done.

https://codereview.chromium.org/2064833002/diff/320001/cc/ipc/quads_struct_tr...
File cc/ipc/quads_struct_traits.h (right):

https://codereview.chromium.org/2064833002/diff/320001/cc/ipc/quads_struct_tr...
cc/ipc/quads_struct_traits.h:345: static void AdvanceIterator(ConstIterator&
iterator) { iterator++; }
On 2016/06/16 20:57:41, dcheng wrote:
> Nit: ++iterator here and below

Done.

[Fady Samuel, 2016-06-16 22:56:54]

The CQ bit was checked by fsamuel@chromium.org

[Fady Samuel, 2016-06-16 22:56:54]

The patchset sent to the CQ was uploaded after l-g-t-m from yzshen@chromium.org,
ben@chromium.org, enne@chromium.org, dcheng@chromium.org
Link to the patchset: https://codereview.chromium.org/2064833002/#ps340001
(title: "Addressed dcheng's comments")

[commit-bot: I haz the power, 2016-06-16 22:57:26]

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

[commit-bot: I haz the power, 2016-06-17 00:09:19]

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

[commit-bot: I haz the power, 2016-06-17 00:09:20]

Try jobs failed on following builders:
  linux_blink_rel on tryserver.blink (JOB_FAILED,
http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/8...)

[Fady Samuel, 2016-06-17 02:06:25]

The CQ bit was checked by fsamuel@chromium.org

[commit-bot: I haz the power, 2016-06-17 02:06:53]

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

[Fady Samuel, 2016-06-17 02:24:44]

The CQ bit was checked by fsamuel@chromium.org

[Fady Samuel, 2016-06-17 02:24:44]

The patchset sent to the CQ was uploaded after l-g-t-m from yzshen@chromium.org,
ben@chromium.org, dcheng@chromium.org, enne@chromium.org
Link to the patchset: https://codereview.chromium.org/2064833002/#ps360001
(title: "Fix move constructor")

[commit-bot: I haz the power, 2016-06-17 02:25:05]

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

[commit-bot: I haz the power, 2016-06-17 02:40:36]

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

[commit-bot: I haz the power, 2016-06-17 02:40:38]

Try jobs failed on following builders:
  chromeos_x86-generic_chromium_compile_only_ng on tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_x86-ge...)

[Fady Samuel, 2016-06-17 02:43:00]

The CQ bit was checked by fsamuel@chromium.org

[commit-bot: I haz the power, 2016-06-17 02:43:20]

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

[commit-bot: I haz the power, 2016-06-17 03:40:24]

Description was changed from

==========
Implement DrawQuad StructTraits

This CL partially implements DrawQuad and QuadList StructTraits.

The code is a bit clumsy currently for a number of reasons:

1. QuadList does not provide O(1) random access and so we use an 
iterator.
2. DrawQuads are created inline within a QuadList, and so we must 
create them prior to invoking the ArrayTraits.

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

to

==========
Implement DrawQuad StructTraits

This CL partially implements DrawQuad and QuadList StructTraits.

The code is a bit clumsy currently for a number of reasons:

1. QuadList does not provide O(1) random access and so we use an 
iterator.
2. DrawQuads are created inline within a QuadList, and so we must 
create them prior to invoking the ArrayTraits.

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

[commit-bot: I haz the power, 2016-06-17 03:40:26]

Committed patchset #19 (id:360001)

[commit-bot: I haz the power, 2016-06-17 03:42:55]

Description was changed from

==========
Implement DrawQuad StructTraits

This CL partially implements DrawQuad and QuadList StructTraits.

The code is a bit clumsy currently for a number of reasons:

1. QuadList does not provide O(1) random access and so we use an 
iterator.
2. DrawQuads are created inline within a QuadList, and so we must 
create them prior to invoking the ArrayTraits.

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

to

==========
Implement DrawQuad StructTraits

This CL partially implements DrawQuad and QuadList StructTraits.

The code is a bit clumsy currently for a number of reasons:

1. QuadList does not provide O(1) random access and so we use an
iterator.
2. DrawQuads are created inline within a QuadList, and so we must
create them prior to invoking the ArrayTraits.

BUG=611802
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/6ea4b2c26e6f2d2a021b10c965c3d06adb493898
Cr-Commit-Position: refs/heads/master@{#400345}
==========

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

Patchset 19 (id:??) landed as
https://crrev.com/6ea4b2c26e6f2d2a021b10c965c3d06adb493898
Cr-Commit-Position: refs/heads/master@{#400345}