Add SkBitmap StructTraits for skia::mojo::Bitmap.

Add SkBitmap StructTraits for skia::mojo::Bitmap.

Prerequisite (as per dcheng's request) for new mojo Bitmap use in:
  https://codereview.chromium.org/2014013002

Add StructTraits to convert between SkBitmap and skia::mojo::Bitmap.
(add profile enum conversion functions, BitmapBuffer utility struct)

Update skia::mojo::BitmapPtr users throughout codebase.
Remove the now unused TypeConverter for these types.

Add public_deps as needed for skia header usage/mapping.
Propagate public_deps necessitated by typemap implementations.

BUG=557405
TEST=No regressions for existing mojo bitmap users.
R=yzshen@chromium.org,tsepez@chromium.org,reed@chromium.org,sky@chromium.org,avi@chromium.org
TBR=ben@chromium.org

Committed: https://crrev.com/b0123d15a8c1b957a5edd5baed30bfeeb8c10dd5
Cr-Commit-Position: refs/heads/master@{#397263}

[msw, 2016-05-25 21:06:00]

Description was changed from

==========
Add SkBitmap and skia::mojo::Bitmap StructTraits.

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

to

==========
Add SkBitmap and skia::mojo::Bitmap StructTraits.

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

[msw, 2016-05-25 21:06:15]

Description was changed from

==========
Add SkBitmap and skia::mojo::Bitmap StructTraits.

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

to

==========
Add SkBitmap StructTraits for skia::mojo::Bitmap.

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

[msw, 2016-05-26 20:18:49]

Patchset #10 (id:180001) has been deleted

[msw, 2016-05-26 21:51:38]

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

[commit-bot: I haz the power, 2016-05-26 21:52:46]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2014013002/220001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/2014013002/220001

[commit-bot: I haz the power, 2016-05-26 21:58:03]

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

[commit-bot: I haz the power, 2016-05-26 21:58:04]

Dry run: Try jobs failed on following builders:
  android_clang_dbg_recipe on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)
  android_compile_dbg on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  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...)
  linux_chromium_compile_dbg_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_gn_chromeos_rel on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_gn_rel on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_gn_r...)

[msw, 2016-05-26 22:02:15]

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

[commit-bot: I haz the power, 2016-05-26 22:03:14]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2014013002/240001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/2014013002/240001

[msw, 2016-05-26 22:04:59]

Description was changed from

==========
Add SkBitmap StructTraits for skia::mojo::Bitmap.

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

to

==========
Add SkBitmap StructTraits for skia::mojo::Bitmap.

Prerequisite (as per dcheng's request) for new mojo Bitmap use in:
  https://codereview.chromium.org/2014013002

Add StructTraits to convert between SkBitmap and skia::mojo::Bitmap.
(add profile enum conversion functions, BitmapBuffer utility struct)

Update skia::mojo::BitmapPtr users throughout codebase.
Remove the now unused TypeConverter for these types.

Add public_deps as needed for skia header usage/mapping.
Propagate public_deps necessitated by typemap implementations.

BUG=557405
TEST=No regressions for existing mojo bitmap users.
R=yzshen@chromium.org,sky@chromium.org
==========

[msw, 2016-05-26 22:04:59]

msw@chromium.org changed reviewers:
+ sky@chromium.org, yzshen@chromium.org

[msw, 2016-05-26 22:07:54]

Hey Yuzhu and Scott, please take a look; thanks!
Yuzhu: skia/public/interfaces. Scott: everything :)
Please review closely; it's likely that I've done something wrong.

[sky, 2016-05-26 22:41:36]

sky@chromium.org changed reviewers:
+ reed@chromium.org, tsepez@chromium.org

[sky, 2016-05-26 22:41:37]

I have little value to add here other than:
+tsepez for security
+reed for skia

[commit-bot: I haz the power, 2016-05-26 22:43:53]

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

[commit-bot: I haz the power, 2016-05-26 22:43:54]

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

[yzshen1, 2016-05-26 22:44:13]

Happy to see this change!

https://codereview.chromium.org/2014013002/diff/240001/chrome/utility/image_d...
File chrome/utility/image_decoder_impl.cc (right):

https://codereview.chromium.org/2014013002/diff/240001/chrome/utility/image_d...
chrome/utility/image_decoder_impl.cc:101: callback.Run(decoded_image.isNull() ?
SkBitmap() : decoded_image);
SkBitmap() creates a Null image, right?
So I think you can directly callback.Run(decoded_image);

https://codereview.chromium.org/2014013002/diff/240001/skia/public/interfaces...
File skia/public/interfaces/bitmap_skbitmap_struct_traits.h (right):

https://codereview.chromium.org/2014013002/diff/240001/skia/public/interfaces...
skia/public/interfaces/bitmap_skbitmap_struct_traits.h:9: #include
"third_party/skia/include/core/SkBitmap.h"
Please order them alphabetically.

https://codereview.chromium.org/2014013002/diff/240001/skia/public/interfaces...
skia/public/interfaces/bitmap_skbitmap_struct_traits.h:16: SkColorType
MojoColorTypeToSk(skia::mojom::ColorType type) {
Can you move the impl code of this file into a .cc file?

(You can even move the StructTraits/ArrayTraits impl code because they are fully
specialized.)

https://codereview.chromium.org/2014013002/diff/240001/skia/public/interfaces...
skia/public/interfaces/bitmap_skbitmap_struct_traits.h:34: default:
Please consider removing "default" and also line 37.
That way if later a new value is added to skia::mojom::ColorType, the code won't
compile so that people will find out the mismatch.

(Same for the other conversion functions.)

https://codereview.chromium.org/2014013002/diff/240001/skia/public/interfaces...
skia/public/interfaces/bitmap_skbitmap_struct_traits.h:143: struct
StructTraits<skia::mojom::Bitmap, SkBitmap> {
SkBitmap supports null state; besides, we do have code that pass null nullable
Bitmap (please see ImageDecoder.DecodeImage()).

Therefore, you need to implement IsNull() and SetToNull(). Please see the
corresponding section in struct_traits.h for what they are used for.

https://codereview.chromium.org/2014013002/diff/240001/skia/public/interfaces...
skia/public/interfaces/bitmap_skbitmap_struct_traits.h:154: return r.width() < 0
? 0 : static_cast<uint32_t>(r.width());
Out of curiosity: when will this be < 0?

https://codereview.chromium.org/2014013002/diff/240001/skia/public/interfaces...
skia/public/interfaces/bitmap_skbitmap_struct_traits.h:160: static
mojo::Array<uint8_t> pixel_data(const SkBitmap& r) {
Now that you have ArrayTraits<BitmapBuffer> defined, you can simply do:

// Return a BitmapBuffer value (instead of a reference) because it is cheap.
static BitmapBuffer pixel_data(const SkBitmap& r) {
}

That saves one extra copy.

https://codereview.chromium.org/2014013002/diff/240001/skia/public/interfaces...
skia/public/interfaces/bitmap_skbitmap_struct_traits.h:172: // TODO: Ensure
width and height are reasonable, eg. <= kMaxBitmapSize?
Should this be addressed in this CL?

https://codereview.chromium.org/2014013002/diff/240001/skia/public/interfaces...
File skia/public/interfaces/skbitmap.typemap (right):

https://codereview.chromium.org/2014013002/diff/240001/skia/public/interfaces...
skia/public/interfaces/skbitmap.typemap:8: deps = [ "//mojo/public/cpp/bindings"
]
I think this is not necessary. Right?

[Tom Sepez, 2016-05-26 22:57:34]

https://codereview.chromium.org/2014013002/diff/240001/skia/public/interfaces...
File skia/public/interfaces/bitmap_skbitmap_struct_traits.h (right):

https://codereview.chromium.org/2014013002/diff/240001/skia/public/interfaces...
skia/public/interfaces/bitmap_skbitmap_struct_traits.h:16: SkColorType
MojoColorTypeToSk(skia::mojom::ColorType type) {
So this is a header file and we risk instantiating copies of these in every file
that includes these.  I think these functions may have to be externs with the
impls in a .cpp file, or use the dreaded inline keyword.

[msw, 2016-05-27 19:34:11]

Please take another look; thanks!

https://codereview.chromium.org/2014013002/diff/240001/chrome/utility/image_d...
File chrome/utility/image_decoder_impl.cc (right):

https://codereview.chromium.org/2014013002/diff/240001/chrome/utility/image_d...
chrome/utility/image_decoder_impl.cc:101: callback.Run(decoded_image.isNull() ?
SkBitmap() : decoded_image);
On 2016/05/26 22:44:13, yzshen1 wrote:
> SkBitmap() creates a Null image, right?
> So I think you can directly callback.Run(decoded_image);

Done.

https://codereview.chromium.org/2014013002/diff/240001/skia/public/interfaces...
File skia/public/interfaces/bitmap_skbitmap_struct_traits.h (right):

https://codereview.chromium.org/2014013002/diff/240001/skia/public/interfaces...
skia/public/interfaces/bitmap_skbitmap_struct_traits.h:9: #include
"third_party/skia/include/core/SkBitmap.h"
On 2016/05/26 22:44:13, yzshen1 wrote:
> Please order them alphabetically.

Done.

https://codereview.chromium.org/2014013002/diff/240001/skia/public/interfaces...
skia/public/interfaces/bitmap_skbitmap_struct_traits.h:16: SkColorType
MojoColorTypeToSk(skia::mojom::ColorType type) {
On 2016/05/26 22:44:13, yzshen1 wrote:
> Can you move the impl code of this file into a .cc file?
> 
> (You can even move the StructTraits/ArrayTraits impl code because they are
fully
> specialized.)

I couldn't get forward decls working, but I split the definitions to a cc.

https://codereview.chromium.org/2014013002/diff/240001/skia/public/interfaces...
skia/public/interfaces/bitmap_skbitmap_struct_traits.h:16: SkColorType
MojoColorTypeToSk(skia::mojom::ColorType type) {
On 2016/05/26 22:57:34, Tom Sepez wrote:
> So this is a header file and we risk instantiating copies of these in every
file
> that includes these.  I think these functions may have to be externs with the
> impls in a .cpp file, or use the dreaded inline keyword.

Moved impls to a cc file.

https://codereview.chromium.org/2014013002/diff/240001/skia/public/interfaces...
skia/public/interfaces/bitmap_skbitmap_struct_traits.h:34: default:
On 2016/05/26 22:44:13, yzshen1 wrote:
> Please consider removing "default" and also line 37.
> That way if later a new value is added to skia::mojom::ColorType, the code
won't
> compile so that people will find out the mismatch.
> 
> (Same for the other conversion functions.)
> 
> 

Done.

https://codereview.chromium.org/2014013002/diff/240001/skia/public/interfaces...
skia/public/interfaces/bitmap_skbitmap_struct_traits.h:143: struct
StructTraits<skia::mojom::Bitmap, SkBitmap> {
On 2016/05/26 22:44:13, yzshen1 wrote:
> SkBitmap supports null state; besides, we do have code that pass null nullable
> Bitmap (please see ImageDecoder.DecodeImage()).
> 
> Therefore, you need to implement IsNull() and SetToNull(). Please see the
> corresponding section in struct_traits.h for what they are used for. 

Done.

https://codereview.chromium.org/2014013002/diff/240001/skia/public/interfaces...
skia/public/interfaces/bitmap_skbitmap_struct_traits.h:154: return r.width() < 0
? 0 : static_cast<uint32_t>(r.width());
On 2016/05/26 22:44:13, yzshen1 wrote:
> Out of curiosity: when will this be < 0?

I don't know of any legitimate cases, this is just a guard for int->uint
conversion; should I add a DCHECK/CHECK, change the Bitmap mojo struct to use
signed int, or do something else?

https://codereview.chromium.org/2014013002/diff/240001/skia/public/interfaces...
skia/public/interfaces/bitmap_skbitmap_struct_traits.h:160: static
mojo::Array<uint8_t> pixel_data(const SkBitmap& r) {
On 2016/05/26 22:44:13, yzshen1 wrote:
> Now that you have ArrayTraits<BitmapBuffer> defined, you can simply do:
> 
> // Return a BitmapBuffer value (instead of a reference) because it is cheap.
> static BitmapBuffer pixel_data(const SkBitmap& r) {
> }
> 
> That saves one extra copy.

Done.

https://codereview.chromium.org/2014013002/diff/240001/skia/public/interfaces...
skia/public/interfaces/bitmap_skbitmap_struct_traits.h:172: // TODO: Ensure
width and height are reasonable, eg. <= kMaxBitmapSize?
On 2016/05/26 22:44:13, yzshen1 wrote:
> Should this be addressed in this CL?

No, the converter didn't check, so it's fine to leave for followup.
(seems worth considering later, but I can remove the TODO if you want)

https://codereview.chromium.org/2014013002/diff/240001/skia/public/interfaces...
File skia/public/interfaces/skbitmap.typemap (right):

https://codereview.chromium.org/2014013002/diff/240001/skia/public/interfaces...
skia/public/interfaces/skbitmap.typemap:8: deps = [ "//mojo/public/cpp/bindings"
]
On 2016/05/26 22:44:13, yzshen1 wrote:
> I think this is not necessary. Right?

This fixes the analyze step failure from the prior patch set:
https://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium...
ERROR at //skia/public/interfaces/bitmap_skbitmap_struct_traits.h:8:11: Include
not allowed.
#include "mojo/public/cpp/bindings/array_traits.h"
          ^--------------------------------------
It is not in any dependency of
  //skia/public/interfaces:interfaces_cpp_sources
The include file is in the target(s):
  //mojo/public/cpp/bindings:bindings

[msw, 2016-05-27 20:19:55]

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

[commit-bot: I haz the power, 2016-05-27 20:20:28]

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

[commit-bot: I haz the power, 2016-05-27 20:33:28]

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

[commit-bot: I haz the power, 2016-05-27 20:33:29]

Dry run: Try jobs failed on following builders:
  android_compile_dbg on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  chromeos_daisy_chromium_compile_only_ng on tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
  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...)

[msw, 2016-05-27 20:47:33]

Patchset #16 (id:320001) has been deleted

[msw, 2016-05-27 20:51:52]

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

[commit-bot: I haz the power, 2016-05-27 20:52:13]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2014013002/340001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/2014013002/340001

[commit-bot: I haz the power, 2016-05-27 21:05:28]

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

[commit-bot: I haz the power, 2016-05-27 21:05:29]

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-...)
  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...)

[msw, 2016-05-27 21:29:42]

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

[commit-bot: I haz the power, 2016-05-27 21:30:04]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2014013002/360001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/2014013002/360001

[commit-bot: I haz the power, 2016-05-27 21:42:41]

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

[commit-bot: I haz the power, 2016-05-27 21:42:42]

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

[msw, 2016-05-27 21:52:32]

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

[commit-bot: I haz the power, 2016-05-27 21:52:58]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2014013002/380001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/2014013002/380001

[commit-bot: I haz the power, 2016-05-27 22:38:19]

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

[commit-bot: I haz the power, 2016-05-27 22:38:21]

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

[Fady Samuel, 2016-05-30 22:23:01]

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

[Fady Samuel, 2016-05-30 22:23:03]

https://codereview.chromium.org/2014013002/diff/380001/skia/public/interfaces...
File skia/public/interfaces/bitmap_skbitmap_struct_traits.cc (right):

https://codereview.chromium.org/2014013002/diff/380001/skia/public/interfaces...
skia/public/interfaces/bitmap_skbitmap_struct_traits.cc:116: return b.data !=
nullptr;
drive-by: b.data == nullptr;

[Tom Sepez, 2016-05-31 17:20:33]

lgtm

https://codereview.chromium.org/2014013002/diff/380001/skia/public/interfaces...
File skia/public/interfaces/bitmap_skbitmap_struct_traits.cc (right):

https://codereview.chromium.org/2014013002/diff/380001/skia/public/interfaces...
skia/public/interfaces/bitmap_skbitmap_struct_traits.cc:116: return b.data !=
nullptr;
or just 
  return !b.data;

[Tom Sepez, 2016-05-31 17:21:36]

On 2016/05/31 17:20:33, Tom Sepez wrote:
> lgtm
> 
>
https://codereview.chromium.org/2014013002/diff/380001/skia/public/interfaces...
> File skia/public/interfaces/bitmap_skbitmap_struct_traits.cc (right):
> 
>
https://codereview.chromium.org/2014013002/diff/380001/skia/public/interfaces...
> skia/public/interfaces/bitmap_skbitmap_struct_traits.cc:116: return b.data !=
> nullptr;
> or just 
>   return !b.data;
By that, I mean LGTM once the IsNull() bug is fixed.

[msw, 2016-05-31 19:22:08]

Patchset #19 (id:400001) has been deleted

[msw, 2016-05-31 19:28:17]

Patchset #19 (id:420001) has been deleted

[msw, 2016-05-31 19:28:59]

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

[commit-bot: I haz the power, 2016-05-31 19:29:38]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2014013002/440001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/2014013002/440001

[commit-bot: I haz the power, 2016-05-31 19:33:37]

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

[commit-bot: I haz the power, 2016-05-31 19:33:39]

Dry run: Try jobs failed on following builders:
  ios-device on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-gn on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-gn/bui...)
  ios-simulator on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  ios-simulator-gn on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-gn/...)
  mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_gn_rel on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_gn_r...)
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[msw, 2016-05-31 19:57:38]

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

[commit-bot: I haz the power, 2016-05-31 19:58:38]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2014013002/460001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/2014013002/460001

[commit-bot: I haz the power, 2016-05-31 20:44:22]

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

[commit-bot: I haz the power, 2016-05-31 20:44:26]

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...)

[msw, 2016-05-31 22:59:45]

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

[commit-bot: I haz the power, 2016-05-31 23:00:32]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2014013002/480001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/2014013002/480001

[msw, 2016-05-31 23:01:07]

Description was changed from

==========
Add SkBitmap StructTraits for skia::mojo::Bitmap.

Prerequisite (as per dcheng's request) for new mojo Bitmap use in:
  https://codereview.chromium.org/2014013002

Add StructTraits to convert between SkBitmap and skia::mojo::Bitmap.
(add profile enum conversion functions, BitmapBuffer utility struct)

Update skia::mojo::BitmapPtr users throughout codebase.
Remove the now unused TypeConverter for these types.

Add public_deps as needed for skia header usage/mapping.
Propagate public_deps necessitated by typemap implementations.

BUG=557405
TEST=No regressions for existing mojo bitmap users.
R=yzshen@chromium.org,sky@chromium.org
==========

to

==========
Add SkBitmap StructTraits for skia::mojo::Bitmap.

Prerequisite (as per dcheng's request) for new mojo Bitmap use in:
  https://codereview.chromium.org/2014013002

Add StructTraits to convert between SkBitmap and skia::mojo::Bitmap.
(add profile enum conversion functions, BitmapBuffer utility struct)

Update skia::mojo::BitmapPtr users throughout codebase.
Remove the now unused TypeConverter for these types.

Add public_deps as needed for skia header usage/mapping.
Propagate public_deps necessitated by typemap implementations.

BUG=557405
TEST=No regressions for existing mojo bitmap users.
R=yzshen@chromium.org,tsepez@chromium.org,reed@chromium.org
==========

[msw, 2016-05-31 23:02:48]

Hey Yuzhu, please take another look; thanks!

https://codereview.chromium.org/2014013002/diff/240001/skia/public/interfaces...
File skia/public/interfaces/bitmap_skbitmap_struct_traits.h (right):

https://codereview.chromium.org/2014013002/diff/240001/skia/public/interfaces...
skia/public/interfaces/bitmap_skbitmap_struct_traits.h:34: default:
On 2016/05/27 19:34:11, msw wrote:
> On 2016/05/26 22:44:13, yzshen1 wrote:
> > Please consider removing "default" and also line 37.
> > That way if later a new value is added to skia::mojom::ColorType, the code
> won't
> > compile so that people will find out the mismatch.
> > 
> > (Same for the other conversion functions.)
> > 
> > 
> 
> Done.

I had to restore the post-switch return statements (now with NOTREACHED) to
resolve build errors for some linux and android trybots:
https://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy...
../../skia/public/interfaces/bitmap_skbitmap_struct_traits.cc: In function
'skia::mojom::ColorType mojo::{anonymous}::SkColorTypeToMojo(SkColorType)':
../../skia/public/interfaces/bitmap_skbitmap_struct_traits.cc:76:1: error:
control reaches end of non-void function [-Werror=return-type]

https://codereview.chromium.org/2014013002/diff/380001/skia/public/interfaces...
File skia/public/interfaces/bitmap_skbitmap_struct_traits.cc (right):

https://codereview.chromium.org/2014013002/diff/380001/skia/public/interfaces...
skia/public/interfaces/bitmap_skbitmap_struct_traits.cc:116: return b.data !=
nullptr;
On 2016/05/30 22:23:03, Fady Samuel wrote:
> drive-by: b.data == nullptr;

Done.

https://codereview.chromium.org/2014013002/diff/380001/skia/public/interfaces...
skia/public/interfaces/bitmap_skbitmap_struct_traits.cc:116: return b.data !=
nullptr;
On 2016/05/31 17:20:33, Tom Sepez wrote:
> or just 
>   return !b.data; 

Done.

[yzshen1, 2016-05-31 23:17:36]

https://codereview.chromium.org/2014013002/diff/240001/skia/public/interfaces...
File skia/public/interfaces/skbitmap.typemap (right):

https://codereview.chromium.org/2014013002/diff/240001/skia/public/interfaces...
skia/public/interfaces/skbitmap.typemap:8: deps = [ "//mojo/public/cpp/bindings"
]
On 2016/05/27 19:34:11, msw wrote:
> On 2016/05/26 22:44:13, yzshen1 wrote:
> > I think this is not necessary. Right?
> 
> This fixes the analyze step failure from the prior patch set:
>
https://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium...
> ERROR at //skia/public/interfaces/bitmap_skbitmap_struct_traits.h:8:11:
Include
> not allowed.
> #include "mojo/public/cpp/bindings/array_traits.h"
>           ^--------------------------------------
> It is not in any dependency of
>   //skia/public/interfaces:interfaces_cpp_sources
> The include file is in the target(s):
>   //mojo/public/cpp/bindings:bindings

Ah, I see. Thanks!

https://codereview.chromium.org/2014013002/diff/480001/skia/public/interfaces...
File skia/public/interfaces/bitmap_skbitmap_struct_traits.cc (right):

https://codereview.chromium.org/2014013002/diff/480001/skia/public/interfaces...
skia/public/interfaces/bitmap_skbitmap_struct_traits.cc:121: b->data = nullptr;
I don't see how this is going to affect the resulting SkBitmap.

(Do we still have to allow nullable pixel data? If not, you could remove the '?'
suffix from the mojom file; and also remove IsNull and SetToNull for this
ArrayTraits.)

https://codereview.chromium.org/2014013002/diff/480001/skia/public/interfaces...
File skia/public/interfaces/bitmap_skbitmap_struct_traits.h (right):

https://codereview.chromium.org/2014013002/diff/480001/skia/public/interfaces...
skia/public/interfaces/bitmap_skbitmap_struct_traits.h:37: static
skia::mojom::ColorType color_type(const SkBitmap& r);
I had a previous comment about adding IsNull and SetToNull to this StructTraits.

Otherwise, the current usage in image decoder of sending a nullable bitmap will
break.

[msw, 2016-05-31 23:53:41]

https://codereview.chromium.org/2014013002/diff/480001/skia/public/interfaces...
File skia/public/interfaces/bitmap_skbitmap_struct_traits.cc (right):

https://codereview.chromium.org/2014013002/diff/480001/skia/public/interfaces...
skia/public/interfaces/bitmap_skbitmap_struct_traits.cc:121: b->data = nullptr;
On 2016/05/31 23:17:36, yzshen1 wrote:
> I don't see how this is going to affect the resulting SkBitmap.
> 
> (Do we still have to allow nullable pixel data? If not, you could remove the
'?'
> suffix from the mojom file; and also remove IsNull and SetToNull for this
> ArrayTraits.)
> 

Done. I don't think we need nullable pixel data, so I took your suggestions.

https://codereview.chromium.org/2014013002/diff/480001/skia/public/interfaces...
File skia/public/interfaces/bitmap_skbitmap_struct_traits.h (right):

https://codereview.chromium.org/2014013002/diff/480001/skia/public/interfaces...
skia/public/interfaces/bitmap_skbitmap_struct_traits.h:37: static
skia::mojom::ColorType color_type(const SkBitmap& r);
On 2016/05/31 23:17:36, yzshen1 wrote:
> I had a previous comment about adding IsNull and SetToNull to this
StructTraits.
> 
> Otherwise, the current usage in image decoder of sending a nullable bitmap
will
> break.

Done. Sorry I somehow neglected that! Though, I wonder if that interface
actually needs a nullable bitmap now... image_decoder_impl.cc instead returns a
default-constructed SkBitmap, and the client, image_decoder.cc, checks the
returned bitmap's isNull() function for valid pixels (isNull()==true for a
default-constructed SkBitmap). Since the only impl/client is in C++ and SkBitmap
has isNull(), we could probably remove the struct traits IsNull/SetToNull, and
make image_decoder.mojom always return an SkBitmap. WDYT?

[msw, 2016-05-31 23:53:52]

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

[commit-bot: I haz the power, 2016-05-31 23:54:35]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2014013002/500001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/2014013002/500001

[commit-bot: I haz the power, 2016-06-01 01:44:36]

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

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

Dry run: This issue passed the CQ dry run.

[yzshen1, 2016-06-01 17:53:21]

lgtm

[msw, 2016-06-01 18:15:11]

Description was changed from

==========
Add SkBitmap StructTraits for skia::mojo::Bitmap.

Prerequisite (as per dcheng's request) for new mojo Bitmap use in:
  https://codereview.chromium.org/2014013002

Add StructTraits to convert between SkBitmap and skia::mojo::Bitmap.
(add profile enum conversion functions, BitmapBuffer utility struct)

Update skia::mojo::BitmapPtr users throughout codebase.
Remove the now unused TypeConverter for these types.

Add public_deps as needed for skia header usage/mapping.
Propagate public_deps necessitated by typemap implementations.

BUG=557405
TEST=No regressions for existing mojo bitmap users.
R=yzshen@chromium.org,tsepez@chromium.org,reed@chromium.org
==========

to

==========
Add SkBitmap StructTraits for skia::mojo::Bitmap.

Prerequisite (as per dcheng's request) for new mojo Bitmap use in:
  https://codereview.chromium.org/2014013002

Add StructTraits to convert between SkBitmap and skia::mojo::Bitmap.
(add profile enum conversion functions, BitmapBuffer utility struct)

Update skia::mojo::BitmapPtr users throughout codebase.
Remove the now unused TypeConverter for these types.

Add public_deps as needed for skia header usage/mapping.
Propagate public_deps necessitated by typemap implementations.

BUG=557405
TEST=No regressions for existing mojo bitmap users.
R=yzshen@chromium.org,tsepez@chromium.org,ben@chromium.org
==========

[msw, 2016-06-01 18:15:11]

msw@chromium.org changed reviewers:
+ ben@chromium.org
- reed@chromium.org

[msw, 2016-06-01 18:20:34]

Description was changed from

==========
Add SkBitmap StructTraits for skia::mojo::Bitmap.

Prerequisite (as per dcheng's request) for new mojo Bitmap use in:
  https://codereview.chromium.org/2014013002

Add StructTraits to convert between SkBitmap and skia::mojo::Bitmap.
(add profile enum conversion functions, BitmapBuffer utility struct)

Update skia::mojo::BitmapPtr users throughout codebase.
Remove the now unused TypeConverter for these types.

Add public_deps as needed for skia header usage/mapping.
Propagate public_deps necessitated by typemap implementations.

BUG=557405
TEST=No regressions for existing mojo bitmap users.
R=yzshen@chromium.org,tsepez@chromium.org,ben@chromium.org
==========

to

==========
Add SkBitmap StructTraits for skia::mojo::Bitmap.

Prerequisite (as per dcheng's request) for new mojo Bitmap use in:
  https://codereview.chromium.org/2014013002

Add StructTraits to convert between SkBitmap and skia::mojo::Bitmap.
(add profile enum conversion functions, BitmapBuffer utility struct)

Update skia::mojo::BitmapPtr users throughout codebase.
Remove the now unused TypeConverter for these types.

Add public_deps as needed for skia header usage/mapping.
Propagate public_deps necessitated by typemap implementations.

BUG=557405
TEST=No regressions for existing mojo bitmap users.
R=yzshen@chromium.org,tsepez@chromium.org,reed@chromium.org,ben@chromium.org
==========

[msw, 2016-06-01 18:20:34]

msw@chromium.org changed reviewers:
+ reed@chromium.org

[msw, 2016-06-01 18:21:55]

Hey Mike and Ben, please take a look for OWNERS; thanks!
Mike: skia/*
Ben: everything else.

[reed1, 2016-06-01 19:33:56]

reed@google.com changed reviewers:
+ reed@google.com

[reed1, 2016-06-01 19:33:58]

skia lgtm

[msw, 2016-06-01 19:41:13]

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

[commit-bot: I haz the power, 2016-06-01 19:41:36]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2014013002/520001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/2014013002/520001

[commit-bot: I haz the power, 2016-06-01 19:46:49]

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

[commit-bot: I haz the power, 2016-06-01 19:46:51]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  chromeos_amd64-generic_chromium_compile_only_ng on tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
  chromeos_daisy_chromium_compile_only_ng on tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
  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...)
  linux_chromium_clobber_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_compile_dbg_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_gn_chromeos_rel on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[msw, 2016-06-01 19:51:56]

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

[commit-bot: I haz the power, 2016-06-01 19:52:45]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2014013002/540001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/2014013002/540001

[sky, 2016-06-01 20:03:35]

chrome LGTM

[Avi (use Gerrit), 2016-06-01 20:24:17]

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

[Avi (use Gerrit), 2016-06-01 20:24:19]

lgtm

content stamp

[msw, 2016-06-01 20:26:11]

Description was changed from

==========
Add SkBitmap StructTraits for skia::mojo::Bitmap.

Prerequisite (as per dcheng's request) for new mojo Bitmap use in:
  https://codereview.chromium.org/2014013002

Add StructTraits to convert between SkBitmap and skia::mojo::Bitmap.
(add profile enum conversion functions, BitmapBuffer utility struct)

Update skia::mojo::BitmapPtr users throughout codebase.
Remove the now unused TypeConverter for these types.

Add public_deps as needed for skia header usage/mapping.
Propagate public_deps necessitated by typemap implementations.

BUG=557405
TEST=No regressions for existing mojo bitmap users.
R=yzshen@chromium.org,tsepez@chromium.org,reed@chromium.org,ben@chromium.org
==========

to

==========
Add SkBitmap StructTraits for skia::mojo::Bitmap.

Prerequisite (as per dcheng's request) for new mojo Bitmap use in:
  https://codereview.chromium.org/2014013002

Add StructTraits to convert between SkBitmap and skia::mojo::Bitmap.
(add profile enum conversion functions, BitmapBuffer utility struct)

Update skia::mojo::BitmapPtr users throughout codebase.
Remove the now unused TypeConverter for these types.

Add public_deps as needed for skia header usage/mapping.
Propagate public_deps necessitated by typemap implementations.

BUG=557405
TEST=No regressions for existing mojo bitmap users.
R=yzshen@chromium.org,tsepez@chromium.org,reed@chromium.org,sky@chromium.org,...
TBR=ben@chromium.org
==========

[commit-bot: I haz the power, 2016-06-01 21:45:02]

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

[commit-bot: I haz the power, 2016-06-01 21:45:04]

Dry run: This issue passed the CQ dry run.

[msw, 2016-06-01 21:50:47]

The CQ bit was checked by msw@chromium.org

[msw, 2016-06-01 21:50:47]

The patchset sent to the CQ was uploaded after l-g-t-m from tsepez@chromium.org,
yzshen@chromium.org, reed@google.com
Link to the patchset: https://codereview.chromium.org/2014013002/#ps540001
(title: "Sync and rebase.")

[commit-bot: I haz the power, 2016-06-01 21:51:41]

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

[commit-bot: I haz the power, 2016-06-01 22:00:12]

Description was changed from

==========
Add SkBitmap StructTraits for skia::mojo::Bitmap.

Prerequisite (as per dcheng's request) for new mojo Bitmap use in:
  https://codereview.chromium.org/2014013002

Add StructTraits to convert between SkBitmap and skia::mojo::Bitmap.
(add profile enum conversion functions, BitmapBuffer utility struct)

Update skia::mojo::BitmapPtr users throughout codebase.
Remove the now unused TypeConverter for these types.

Add public_deps as needed for skia header usage/mapping.
Propagate public_deps necessitated by typemap implementations.

BUG=557405
TEST=No regressions for existing mojo bitmap users.
R=yzshen@chromium.org,tsepez@chromium.org,reed@chromium.org,sky@chromium.org,...
TBR=ben@chromium.org
==========

to

==========
Add SkBitmap StructTraits for skia::mojo::Bitmap.

Prerequisite (as per dcheng's request) for new mojo Bitmap use in:
  https://codereview.chromium.org/2014013002

Add StructTraits to convert between SkBitmap and skia::mojo::Bitmap.
(add profile enum conversion functions, BitmapBuffer utility struct)

Update skia::mojo::BitmapPtr users throughout codebase.
Remove the now unused TypeConverter for these types.

Add public_deps as needed for skia header usage/mapping.
Propagate public_deps necessitated by typemap implementations.

BUG=557405
TEST=No regressions for existing mojo bitmap users.
R=yzshen@chromium.org,tsepez@chromium.org,reed@chromium.org,sky@chromium.org,...
TBR=ben@chromium.org
==========

[commit-bot: I haz the power, 2016-06-01 22:00:14]

Committed patchset #24 (id:540001)

[commit-bot: I haz the power, 2016-06-01 22:08:04]

Description was changed from

==========
Add SkBitmap StructTraits for skia::mojo::Bitmap.

Prerequisite (as per dcheng's request) for new mojo Bitmap use in:
  https://codereview.chromium.org/2014013002

Add StructTraits to convert between SkBitmap and skia::mojo::Bitmap.
(add profile enum conversion functions, BitmapBuffer utility struct)

Update skia::mojo::BitmapPtr users throughout codebase.
Remove the now unused TypeConverter for these types.

Add public_deps as needed for skia header usage/mapping.
Propagate public_deps necessitated by typemap implementations.

BUG=557405
TEST=No regressions for existing mojo bitmap users.
R=yzshen@chromium.org,tsepez@chromium.org,reed@chromium.org,sky@chromium.org,...
TBR=ben@chromium.org
==========

to

==========
Add SkBitmap StructTraits for skia::mojo::Bitmap.

Prerequisite (as per dcheng's request) for new mojo Bitmap use in:
  https://codereview.chromium.org/2014013002

Add StructTraits to convert between SkBitmap and skia::mojo::Bitmap.
(add profile enum conversion functions, BitmapBuffer utility struct)

Update skia::mojo::BitmapPtr users throughout codebase.
Remove the now unused TypeConverter for these types.

Add public_deps as needed for skia header usage/mapping.
Propagate public_deps necessitated by typemap implementations.

BUG=557405
TEST=No regressions for existing mojo bitmap users.
R=yzshen@chromium.org,tsepez@chromium.org,reed@chromium.org,sky@chromium.org,...
TBR=ben@chromium.org

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

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

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