Provide StructTraits for a vector<SkBitmap>

Provide StructTraits for a vector<SkBitmap>

To retain an existing ozone API while making it accessible from mojo, add
StructTraits conversion for a vector<SkBitmap>

BUG=620927
Committed: https://crrev.com/5557c8bccbf59dbc24a499c8593a5caae117a46e
Cr-Commit-Position: refs/heads/master@{#403501}

[rjkroege, 2016-06-23 21:41:02]

rjkroege@chromium.org changed reviewers:
+ fsamuel@google.com

[rjkroege, 2016-06-23 21:41:03]

ptal

[Fady Samuel, 2016-06-23 21:45:11]

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

[Fady Samuel, 2016-06-23 21:45:12]

lgtm + comment.

https://codereview.chromium.org/2096763005/diff/1/skia/public/interfaces/bitm...
File skia/public/interfaces/bitmap_array_struct_traits.h (right):

https://codereview.chromium.org/2096763005/diff/1/skia/public/interfaces/bitm...
skia/public/interfaces/bitmap_array_struct_traits.h:25: if
(!data.ReadBitmaps(&bitmaps))
It seems like you could simply:

data.ReadBitmaps(out)?

[rjkroege, 2016-06-24 14:13:20]

rjkroege@chromium.org changed reviewers:
+ dcheng@chromium.org, junov@chromium.org
- fsamuel@google.com

[rjkroege, 2016-06-24 14:13:20]

ptal

dcheng@: security review please.
junov@: skia owner please.

https://codereview.chromium.org/2096763005/diff/1/skia/public/interfaces/bitm...
File skia/public/interfaces/bitmap_array_struct_traits.h (right):

https://codereview.chromium.org/2096763005/diff/1/skia/public/interfaces/bitm...
skia/public/interfaces/bitmap_array_struct_traits.h:25: if
(!data.ReadBitmaps(&bitmaps))
On 2016/06/23 21:45:12, Fady Samuel wrote:
> It seems like you could simply:
> 
> data.ReadBitmaps(out)?

Thanks. Done.

[dcheng, 2016-06-24 16:58:28]

Why do we need the wrapper struct instead of just having code write
array<skia.mojom.Bitmap> directly?

[rjkroege, 2016-06-24 21:55:36]

On 2016/06/24 16:58:28, dcheng wrote:
> Why do we need the wrapper struct instead of just having code write
> array<skia.mojom.Bitmap> directly?

I don't understand your question. I wrote this struct_trait for the following:

Having a mojo:

interface Bar {
  method Foo(array<skia.mojom.Bitmap> f);
}

Gives an abstract C++ base class of

class Bar {
  void Foo(mojo::Array<skia::mojom::Bitmap> f) = 0;
};

I have an existing class BarImpl already written and in use in Chrome that's
something like this:

class BarImpl {
  void Foo(vector<skia::Bitmap> f);
}

BarImpl::Foo is already invoked elsewhere in the code. Adding this type map here
means that to expose class BarImpl in a minimally intrusive fashion to mojo IPC
on BarPtr, it suffices to make BarImpl a subclass of mojom::Bar.

[dcheng, 2016-06-24 21:59:20]

On 2016/06/24 21:55:36, rjkroege wrote:
> On 2016/06/24 16:58:28, dcheng wrote:
> > Why do we need the wrapper struct instead of just having code write
> > array<skia.mojom.Bitmap> directly?
> 
> I don't understand your question. I wrote this struct_trait for the following:
> 
> Having a mojo:
> 
> interface Bar {
>   method Foo(array<skia.mojom.Bitmap> f);
> }
> 
> Gives an abstract C++ base class of
> 
> class Bar {
>   void Foo(mojo::Array<skia::mojom::Bitmap> f) = 0;
> };
> 
> I have an existing class BarImpl already written and in use in Chrome that's
> something like this:
> 
> class BarImpl {
>   void Foo(vector<skia::Bitmap> f);
> }
> 
> BarImpl::Foo is already invoked elsewhere in the code. Adding this type map
here
> means that to expose class BarImpl in a minimally intrusive fashion to mojo
IPC
> on BarPtr, it suffices to make BarImpl a subclass of mojom::Bar.

Basically, I don't understand why we need BitmapArray. I would have expected the
typemapping code to Just Work without the new mojo struct and its corresponding
StructTraits.

[rjkroege, 2016-06-24 22:07:10]

On 2016/06/24 21:59:20, dcheng wrote:
> On 2016/06/24 21:55:36, rjkroege wrote:
> > On 2016/06/24 16:58:28, dcheng wrote:
> > > Why do we need the wrapper struct instead of just having code write
> > > array<skia.mojom.Bitmap> directly?
> > 
> > I don't understand your question. I wrote this struct_trait for the
following:
> > 
> > Having a mojo:
> > 
> > interface Bar {
> >   method Foo(array<skia.mojom.Bitmap> f);
> > }
> > 
> > Gives an abstract C++ base class of
> > 
> > class Bar {
> >   void Foo(mojo::Array<skia::mojom::Bitmap> f) = 0;
> > };
> > 
> > I have an existing class BarImpl already written and in use in Chrome that's
> > something like this:
> > 
> > class BarImpl {
> >   void Foo(vector<skia::Bitmap> f);
> > }
> > 
> > BarImpl::Foo is already invoked elsewhere in the code. Adding this type map
> here
> > means that to expose class BarImpl in a minimally intrusive fashion to mojo
> IPC
> > on BarPtr, it suffices to make BarImpl a subclass of mojom::Bar.
> 
> Basically, I don't understand why we need BitmapArray. I would have expected
the
> typemapping code to Just Work without the new mojo struct and its
corresponding
> StructTraits.

Not done yet:
https://groups.google.com/a/chromium.org/d/topic/chromium-mojo/uf0-kUYzwoc/di...

[dcheng, 2016-06-24 22:17:35]

On 2016/06/24 22:07:10, rjkroege wrote:
> On 2016/06/24 21:59:20, dcheng wrote:
> > On 2016/06/24 21:55:36, rjkroege wrote:
> > > On 2016/06/24 16:58:28, dcheng wrote:
> > > > Why do we need the wrapper struct instead of just having code write
> > > > array<skia.mojom.Bitmap> directly?
> > > 
> > > I don't understand your question. I wrote this struct_trait for the
> following:
> > > 
> > > Having a mojo:
> > > 
> > > interface Bar {
> > >   method Foo(array<skia.mojom.Bitmap> f);
> > > }
> > > 
> > > Gives an abstract C++ base class of
> > > 
> > > class Bar {
> > >   void Foo(mojo::Array<skia::mojom::Bitmap> f) = 0;
> > > };
> > > 
> > > I have an existing class BarImpl already written and in use in Chrome
that's
> > > something like this:
> > > 
> > > class BarImpl {
> > >   void Foo(vector<skia::Bitmap> f);
> > > }
> > > 
> > > BarImpl::Foo is already invoked elsewhere in the code. Adding this type
map
> > here
> > > means that to expose class BarImpl in a minimally intrusive fashion to
mojo
> > IPC
> > > on BarPtr, it suffices to make BarImpl a subclass of mojom::Bar.
> > 
> > Basically, I don't understand why we need BitmapArray. I would have expected
> the
> > typemapping code to Just Work without the new mojo struct and its
> corresponding
> > StructTraits.
> 
> Not done yet:
>
https://groups.google.com/a/chromium.org/d/topic/chromium-mojo/uf0-kUYzwoc/di...

Hmm... I'd rather we just fix this at the right layer (in Mojo) rather than
forcing this on code using Mojo. I'll ping the thread later.

[rjkroege, 2016-06-27 17:50:49]

On 2016/06/24 22:17:35, dcheng wrote:
> On 2016/06/24 22:07:10, rjkroege wrote:
> > On 2016/06/24 21:59:20, dcheng wrote:
> > > On 2016/06/24 21:55:36, rjkroege wrote:
> > > > On 2016/06/24 16:58:28, dcheng wrote:
> > > > > Why do we need the wrapper struct instead of just having code write
> > > > > array<skia.mojom.Bitmap> directly?
> > > > 
> > > > I don't understand your question. I wrote this struct_trait for the
> > following:
> > > > 
> > > > Having a mojo:
> > > > 
> > > > interface Bar {
> > > >   method Foo(array<skia.mojom.Bitmap> f);
> > > > }
> > > > 
> > > > Gives an abstract C++ base class of
> > > > 
> > > > class Bar {
> > > >   void Foo(mojo::Array<skia::mojom::Bitmap> f) = 0;
> > > > };
> > > > 
> > > > I have an existing class BarImpl already written and in use in Chrome
> that's
> > > > something like this:
> > > > 
> > > > class BarImpl {
> > > >   void Foo(vector<skia::Bitmap> f);
> > > > }
> > > > 
> > > > BarImpl::Foo is already invoked elsewhere in the code. Adding this type
> map
> > > here
> > > > means that to expose class BarImpl in a minimally intrusive fashion to
> mojo
> > > IPC
> > > > on BarPtr, it suffices to make BarImpl a subclass of mojom::Bar.
> > > 
> > > Basically, I don't understand why we need BitmapArray. I would have
expected
> > the
> > > typemapping code to Just Work without the new mojo struct and its
> > corresponding
> > > StructTraits.
> > 
> > Not done yet:
> >
>
https://groups.google.com/a/chromium.org/d/topic/chromium-mojo/uf0-kUYzwoc/di...
> 
> Hmm... I'd rather we just fix this at the right layer (in Mojo) rather than
> forcing this on code using Mojo. I'll ping the thread later.

I too would rather that this code be unnecessary. It would after all have been
easier for me to not need to have written it. :-)

However, I presume that dedicated members of the mojo team like yzhen@, jam@ and
rockot@ have correctly prioritized the implementation of the feature that we
both would like to exist behind other more important tasks precisely because
it's easy to work around in the way that I've written here. I'm sure that
they'll get around to fixing mojo in this fashion eventually. However, in the
intervening time, I would still very much like to make progress on the bugs
assigned to me and the least intrusive way to add a mojo interface to existing
ozone code today is to use this  struct trait.

So will you do a security review of this CL? Or if you are currently too busy,
do you have any objections to me asking a different security reviewer to look at
this change?

[yzshen1, 2016-06-27 18:00:49]

On 2016/06/27 17:50:49, rjkroege wrote:
> On 2016/06/24 22:17:35, dcheng wrote:
> > On 2016/06/24 22:07:10, rjkroege wrote:
> > > On 2016/06/24 21:59:20, dcheng wrote:
> > > > On 2016/06/24 21:55:36, rjkroege wrote:
> > > > > On 2016/06/24 16:58:28, dcheng wrote:
> > > > > > Why do we need the wrapper struct instead of just having code write
> > > > > > array<skia.mojom.Bitmap> directly?
> > > > > 
> > > > > I don't understand your question. I wrote this struct_trait for the
> > > following:
> > > > > 
> > > > > Having a mojo:
> > > > > 
> > > > > interface Bar {
> > > > >   method Foo(array<skia.mojom.Bitmap> f);
> > > > > }
> > > > > 
> > > > > Gives an abstract C++ base class of
> > > > > 
> > > > > class Bar {
> > > > >   void Foo(mojo::Array<skia::mojom::Bitmap> f) = 0;
> > > > > };
> > > > > 
> > > > > I have an existing class BarImpl already written and in use in Chrome
> > that's
> > > > > something like this:
> > > > > 
> > > > > class BarImpl {
> > > > >   void Foo(vector<skia::Bitmap> f);
> > > > > }
> > > > > 
> > > > > BarImpl::Foo is already invoked elsewhere in the code. Adding this
type
> > map
> > > > here
> > > > > means that to expose class BarImpl in a minimally intrusive fashion to
> > mojo
> > > > IPC
> > > > > on BarPtr, it suffices to make BarImpl a subclass of mojom::Bar.
> > > > 
> > > > Basically, I don't understand why we need BitmapArray. I would have
> expected
> > > the
> > > > typemapping code to Just Work without the new mojo struct and its
> > > corresponding
> > > > StructTraits.
> > > 
> > > Not done yet:
> > >
> >
>
https://groups.google.com/a/chromium.org/d/topic/chromium-mojo/uf0-kUYzwoc/di...
> > 
> > Hmm... I'd rather we just fix this at the right layer (in Mojo) rather than
> > forcing this on code using Mojo. I'll ping the thread later.
> 
> I too would rather that this code be unnecessary. It would after all have been
> easier for me to not need to have written it. :-)
> 
> However, I presume that dedicated members of the mojo team like yzhen@, jam@
and
> rockot@ have correctly prioritized the implementation of the feature that we
> both would like to exist behind other more important tasks precisely because
> it's easy to work around in the way that I've written here. I'm sure that
> they'll get around to fixing mojo in this fashion eventually. However, in the
> intervening time, I would still very much like to make progress on the bugs
> assigned to me and the least intrusive way to add a mojo interface to existing
> ozone code today is to use this  struct trait.
> 
> So will you do a security review of this CL? Or if you are currently too busy,
> do you have any objections to me asking a different security reviewer to look
at
> this change?

Hi,

With your Bar/BarImpl example above, I feel that the easiest short-term solution
is to have BarImpl as a member of MojoBarimpl, which is a subclass of the mojo
Bar interface:

class MojoBarImpl {
 public:
  void Foo(mojo::Array<blah> input) override {
    impl_->Foo(input.PassStorage());
  }

 private:
  BarImpl impl_;
};

Eventually, we want to get rid of mojo::Array<> and use std::vector (or
base::Optional<std::vector<>> if nullable).
(Please see:
https://docs.google.com/document/d/1dutUW6XUlbSJzsqmtmPnN-J7R8z-iEL-QJ_GR9hWKHg)

[rjkroege, 2016-06-27 23:41:33]

On 2016/06/27 18:00:49, yzshen1 wrote:
> On 2016/06/27 17:50:49, rjkroege wrote:
> > On 2016/06/24 22:17:35, dcheng wrote:
> > > On 2016/06/24 22:07:10, rjkroege wrote:
> > > > On 2016/06/24 21:59:20, dcheng wrote:
> > > > > On 2016/06/24 21:55:36, rjkroege wrote:
> > > > > > On 2016/06/24 16:58:28, dcheng wrote:
> > > > > > > Why do we need the wrapper struct instead of just having code
write
> > > > > > > array<skia.mojom.Bitmap> directly?
> > > > > > 
> > > > > > I don't understand your question. I wrote this struct_trait for the
> > > > following:
> > > > > > 
> > > > > > Having a mojo:
> > > > > > 
> > > > > > interface Bar {
> > > > > >   method Foo(array<skia.mojom.Bitmap> f);
> > > > > > }
> > > > > > 
> > > > > > Gives an abstract C++ base class of
> > > > > > 
> > > > > > class Bar {
> > > > > >   void Foo(mojo::Array<skia::mojom::Bitmap> f) = 0;
> > > > > > };
> > > > > > 
> > > > > > I have an existing class BarImpl already written and in use in
Chrome
> > > that's
> > > > > > something like this:
> > > > > > 
> > > > > > class BarImpl {
> > > > > >   void Foo(vector<skia::Bitmap> f);
> > > > > > }
> > > > > > 
> > > > > > BarImpl::Foo is already invoked elsewhere in the code. Adding this
> type
> > > map
> > > > > here
> > > > > > means that to expose class BarImpl in a minimally intrusive fashion
to
> > > mojo
> > > > > IPC
> > > > > > on BarPtr, it suffices to make BarImpl a subclass of mojom::Bar.
> > > > > 
> > > > > Basically, I don't understand why we need BitmapArray. I would have
> > expected
> > > > the
> > > > > typemapping code to Just Work without the new mojo struct and its
> > > > corresponding
> > > > > StructTraits.
> > > > 
> > > > Not done yet:
> > > >
> > >
> >
>
https://groups.google.com/a/chromium.org/d/topic/chromium-mojo/uf0-kUYzwoc/di...
> > > 
> > > Hmm... I'd rather we just fix this at the right layer (in Mojo) rather
than
> > > forcing this on code using Mojo. I'll ping the thread later.
> > 
> > I too would rather that this code be unnecessary. It would after all have
been
> > easier for me to not need to have written it. :-)
> > 
> > However, I presume that dedicated members of the mojo team like yzhen@, jam@
> and
> > rockot@ have correctly prioritized the implementation of the feature that we
> > both would like to exist behind other more important tasks precisely because
> > it's easy to work around in the way that I've written here. I'm sure that
> > they'll get around to fixing mojo in this fashion eventually. However, in
the
> > intervening time, I would still very much like to make progress on the bugs
> > assigned to me and the least intrusive way to add a mojo interface to
existing
> > ozone code today is to use this  struct trait.
> > 
> > So will you do a security review of this CL? Or if you are currently too
busy,
> > do you have any objections to me asking a different security reviewer to
look
> at
> > this change?
> 
> Hi,
> 
yzshen@: thanks for the suggestion. This approach could be done but would
require a significant amount of unnecessary refactoring in the ozone code --
refactoring that would probably end up being a larger (and more likely to
introduce a regression) patch than this CL.

I still think that the StructTraits implementation here is the least intrusive
solution that permits forward progress on mus until the work you mention below
is completed.


> With your Bar/BarImpl example above, I feel that the easiest short-term
solution
> is to have BarImpl as a member of MojoBarimpl, which is a subclass of the mojo
> Bar interface:
> 
> class MojoBarImpl {
>  public:
>   void Foo(mojo::Array<blah> input) override {
>     impl_->Foo(input.PassStorage());
>   }
> 
>  private:
>   BarImpl impl_;
> };
> 
> Eventually, we want to get rid of mojo::Array<> and use std::vector (or
> base::Optional<std::vector<>> if nullable).
> (Please see:
>
https://docs.google.com/document/d/1dutUW6XUlbSJzsqmtmPnN-J7R8z-iEL-QJ_GR9hWKHg)

[dcheng, 2016-06-28 02:23:17]

On 2016/06/27 17:50:49, rjkroege wrote:
> On 2016/06/24 22:17:35, dcheng wrote:
> > On 2016/06/24 22:07:10, rjkroege wrote:
> > > On 2016/06/24 21:59:20, dcheng wrote:
> > > > On 2016/06/24 21:55:36, rjkroege wrote:
> > > > > On 2016/06/24 16:58:28, dcheng wrote:
> > > > > > Why do we need the wrapper struct instead of just having code write
> > > > > > array<skia.mojom.Bitmap> directly?
> > > > > 
> > > > > I don't understand your question. I wrote this struct_trait for the
> > > following:
> > > > > 
> > > > > Having a mojo:
> > > > > 
> > > > > interface Bar {
> > > > >   method Foo(array<skia.mojom.Bitmap> f);
> > > > > }
> > > > > 
> > > > > Gives an abstract C++ base class of
> > > > > 
> > > > > class Bar {
> > > > >   void Foo(mojo::Array<skia::mojom::Bitmap> f) = 0;
> > > > > };
> > > > > 
> > > > > I have an existing class BarImpl already written and in use in Chrome
> > that's
> > > > > something like this:
> > > > > 
> > > > > class BarImpl {
> > > > >   void Foo(vector<skia::Bitmap> f);
> > > > > }
> > > > > 
> > > > > BarImpl::Foo is already invoked elsewhere in the code. Adding this
type
> > map
> > > > here
> > > > > means that to expose class BarImpl in a minimally intrusive fashion to
> > mojo
> > > > IPC
> > > > > on BarPtr, it suffices to make BarImpl a subclass of mojom::Bar.
> > > > 
> > > > Basically, I don't understand why we need BitmapArray. I would have
> expected
> > > the
> > > > typemapping code to Just Work without the new mojo struct and its
> > > corresponding
> > > > StructTraits.
> > > 
> > > Not done yet:
> > >
> >
>
https://groups.google.com/a/chromium.org/d/topic/chromium-mojo/uf0-kUYzwoc/di...
> > 
> > Hmm... I'd rather we just fix this at the right layer (in Mojo) rather than
> > forcing this on code using Mojo. I'll ping the thread later.
> 
> I too would rather that this code be unnecessary. It would after all have been
> easier for me to not need to have written it. :-)
> 
> However, I presume that dedicated members of the mojo team like yzhen@, jam@
and
> rockot@ have correctly prioritized the implementation of the feature that we
> both would like to exist behind other more important tasks precisely because
> it's easy to work around in the way that I've written here. I'm sure that
> they'll get around to fixing mojo in this fashion eventually. However, in the
> intervening time, I would still very much like to make progress on the bugs
> assigned to me and the least intrusive way to add a mojo interface to existing
> ozone code today is to use this  struct trait.
> 

This has already come up in several CLs: off the top of my head, this CL and one
other. Yes, it works but it's a bit of a kludge as well.

If it's really that critical to proceed without this portion of Mojo being
ready, this struct needs a TODO and a corresponding bug marked as blocking on
mojo::Array stuff, with an owner for cleaning this up.

> So will you do a security review of this CL? Or if you are currently too busy,
> do you have any objections to me asking a different security reviewer to look
at
> this change?

My comments were the security review...

[Ben Goodger (Google), 2016-06-28 22:26:21]

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

[Ben Goodger (Google), 2016-06-28 22:26:22]

dcheng: FTR, if we blocked development on every feature request from Mojo
system/bindings layers we'd never get anything done :-) As in many projects,
there are independent sub-components all of which are progressing
asynchronously.

We're in the process of correcting a bunch of this stuff as quickly as possible.
You're right, cases like this should have a TODO() that references the relevant
typemapping work. I think that's the extent of the burden that should exist on
folk like Rob though.

[rjkroege, 2016-06-29 19:27:11]

On 2016/06/28 02:23:17, dcheng wrote:
> On 2016/06/27 17:50:49, rjkroege wrote:
> > On 2016/06/24 22:17:35, dcheng wrote:
> > > On 2016/06/24 22:07:10, rjkroege wrote:
> > > > On 2016/06/24 21:59:20, dcheng wrote:
> > > > > On 2016/06/24 21:55:36, rjkroege wrote:
> > > > > > On 2016/06/24 16:58:28, dcheng wrote:
> > > > > > > Why do we need the wrapper struct instead of just having code
write
> > > > > > > array<skia.mojom.Bitmap> directly?
> > > > > > 
> > > > > > I don't understand your question. I wrote this struct_trait for the
> > > > following:
> > > > > > 
> > > > > > Having a mojo:
> > > > > > 
> > > > > > interface Bar {
> > > > > >   method Foo(array<skia.mojom.Bitmap> f);
> > > > > > }
> > > > > > 
> > > > > > Gives an abstract C++ base class of
> > > > > > 
> > > > > > class Bar {
> > > > > >   void Foo(mojo::Array<skia::mojom::Bitmap> f) = 0;
> > > > > > };
> > > > > > 
> > > > > > I have an existing class BarImpl already written and in use in
Chrome
> > > that's
> > > > > > something like this:
> > > > > > 
> > > > > > class BarImpl {
> > > > > >   void Foo(vector<skia::Bitmap> f);
> > > > > > }
> > > > > > 
> > > > > > BarImpl::Foo is already invoked elsewhere in the code. Adding this
> type
> > > map
> > > > > here
> > > > > > means that to expose class BarImpl in a minimally intrusive fashion
to
> > > mojo
> > > > > IPC
> > > > > > on BarPtr, it suffices to make BarImpl a subclass of mojom::Bar.
> > > > > 
> > > > > Basically, I don't understand why we need BitmapArray. I would have
> > expected
> > > > the
> > > > > typemapping code to Just Work without the new mojo struct and its
> > > > corresponding
> > > > > StructTraits.
> > > > 
> > > > Not done yet:
> > > >
> > >
> >
>
https://groups.google.com/a/chromium.org/d/topic/chromium-mojo/uf0-kUYzwoc/di...
> > > 
> > > Hmm... I'd rather we just fix this at the right layer (in Mojo) rather
than
> > > forcing this on code using Mojo. I'll ping the thread later.
> > 
> > I too would rather that this code be unnecessary. It would after all have
been
> > easier for me to not need to have written it. :-)
> > 
> > However, I presume that dedicated members of the mojo team like yzhen@, jam@
> and
> > rockot@ have correctly prioritized the implementation of the feature that we
> > both would like to exist behind other more important tasks precisely because
> > it's easy to work around in the way that I've written here. I'm sure that
> > they'll get around to fixing mojo in this fashion eventually. However, in
the
> > intervening time, I would still very much like to make progress on the bugs
> > assigned to me and the least intrusive way to add a mojo interface to
existing
> > ozone code today is to use this  struct trait.
> > 
> 
> This has already come up in several CLs: off the top of my head, this CL and
one
> other. Yes, it works but it's a bit of a kludge as well.
> 
> If it's really that critical to proceed without this portion of Mojo being
> ready, this struct needs a TODO and a corresponding bug marked as blocking on
> mojo::Array stuff, with an owner for cleaning this up.

Done. PTAL.

> 
> > So will you do a security review of this CL? Or if you are currently too
busy,
> > do you have any objections to me asking a different security reviewer to
look
> at
> > this change?
> 
> My comments were the security review...

[dcheng, 2016-06-29 22:30:49]

mojom lgtm

[rjkroege, 2016-06-29 23:20:52]

On 2016/06/29 22:30:49, dcheng wrote:
> mojom lgtm

ping junov@ for OWNERS in skia

[rjkroege, 2016-06-29 23:21:02]

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

[commit-bot: I haz the power, 2016-06-29 23:22:03]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

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

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

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

Dry run: This issue passed the CQ dry run.

[rjkroege, 2016-06-30 18:31:45]

rjkroege@chromium.org changed reviewers:
+ mtklein@chromium.org
- junov@chromium.org

[rjkroege, 2016-06-30 18:31:45]

junov@ is on vacation. mtklein@: ptal for skia OWNERs review.

[mtklein_C, 2016-07-01 16:32:28]

lgtm

[rjkroege, 2016-07-01 18:27:09]

The CQ bit was checked by rjkroege@chromium.org

[rjkroege, 2016-07-01 18:27:10]

The patchset sent to the CQ was uploaded after l-g-t-m from fsamuel@chromium.org
Link to the patchset: https://codereview.chromium.org/2096763005/#ps40001
(title: "review comments")

[commit-bot: I haz the power, 2016-07-01 18:27:43]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-01 19:26:50]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-07-01 19:27:03]

CQ bit was unchecked.

[commit-bot: I haz the power, 2016-07-01 19:29:14]

Description was changed from

==========
Provide StructTraits for a vector<SkBitmap>

To retain an existing ozone API while making it accessible from mojo, add
StructTraits conversion for a vector<SkBitmap>

BUG=620927
==========

to

==========
Provide StructTraits for a vector<SkBitmap>

To retain an existing ozone API while making it accessible from mojo, add
StructTraits conversion for a vector<SkBitmap>

BUG=620927

Committed: https://crrev.com/5557c8bccbf59dbc24a499c8593a5caae117a46e
Cr-Commit-Position: refs/heads/master@{#403501}
==========

[commit-bot: I haz the power, 2016-07-01 19:29:16]

Patchset 3 (id:??) landed as
https://crrev.com/5557c8bccbf59dbc24a499c8593a5caae117a46e
Cr-Commit-Position: refs/heads/master@{#403501}