Migrate ArcBitmap to use typemapping

components/arc/bitmap/bitmap_struct_traits.h

+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef COMPONENT_ARC_BITMAP_BITMAP_STRUCT_TRAITS_H_
+#define COMPONENT_ARC_BITMAP_BITMAP_STRUCT_TRAITS_H_
+
+#include "components/arc/common/bitmap.mojom.h"
+#include "third_party/skia/include/core/SkBitmap.h"
+
+namespace mojo {
+
+template <>
+struct StructTraits<arc::mojom::ArcBitmapDataView, SkBitmap> {
+  static mojo::Array<uint8_t> pixel_data(const SkBitmap& r) {
+    DCHECK(!r.drawsNothing());
+
+    const SkImageInfo& info = r.info();
+    DCHECK_EQ(info.colorType(), kRGBA_8888_SkColorType);
+
+    mojo::Array<uint8_t> pixel_data(r.getSize());
+    r.readPixels(info, &pixel_data[0], r.rowBytes(), 0, 0);

[dcheng, 2016/09/09 21:46:32]

Let's return a mojo::CArray here to avoid copying the data. We can just use
getPixels() instead of readPixels() here as well. It's important to make this
cheap, because it's called multiple times during serialization. I would probably
out-of-line it as well, since it's not just a simple getter like width() and
height().

[yoshiki, 2016/09/12 17:35:43]

Done.

+    return pixel_data;
+  }
+
+  static uint32_t width(const SkBitmap& r) {
+    return r.width();
+  }
+
+  static uint32_t height(const SkBitmap& r) {
+    return r.height();
+  }
+
+  static bool Read(arc::mojom::ArcBitmapDataView data, SkBitmap* out) {
+    DCHECK_NE(out, nullptr);

[dcheng, 2016/09/09 21:46:32]

This should be out-of-lined, and the DCHECK is unnecessary (it means the mojo
bindings are really badly broken)

[yoshiki, 2016/09/12 17:35:43]

Done.

+
+    mojo::Array<uint8_t> pixel_data;
+    if (!data.ReadPixelData(&pixel_data)) {

[dcheng, 2016/09/09 21:46:33]

Let's use the data view to avoid copying the pixels twice. We can use
GetPixelDataView() to get a read-only view of the pixels and memcpy them
directly into the allocated bitmap pixels below.

[yoshiki, 2016/09/12 17:35:43]

Done.

+      // Can't read the pixel data.
+      out->reset();

[dcheng, 2016/09/09 21:46:33]

No need to reset here and elsewhere.

[yoshiki, 2016/09/12 17:35:44]

Done.

+      return false;
+    }
+
+    if (data.width() == 0 || data.height() == 0) {

[dcheng, 2016/09/09 21:46:32]

I don't think there's any particular need to make this special case: I would
just remove it (it's slightly wasteful, but I imagine sending 0x0 bitmaps should
be quite uncommon?)

[yoshiki, 2016/09/12 17:35:43]

Done.

+      // Valid empty data.
+      out->reset();
+      return true;
+    }
+
+    SkImageInfo info = SkImageInfo::Make(

[dcheng, 2016/09/09 21:46:33]

I would recommend structuring this like
https://cs.chromium.org/chromium/src/ui/gfx/ipc/skia/gfx_skia_param_traits.cc...

We can just write directly into |out|, since |out| won't ever be used if we fail
validation.

[yoshiki, 2016/09/12 17:35:43]

That code can't be reused here, since we need to convert color type
(kRGBA_8888_SkColorType to kN32_SkColorType) in copying.

[dcheng, 2016/09/13 01:13:09]

Can we make our input and output pixel formats match?

[yoshiki, 2016/09/13 18:01:43]

It's impossible. Android uses kRGBA_8888_SkColorType representative in
android.graphics.Bitmap, and the standard format of Chrome's bitmap is
kN32_SkColorType.

[dcheng, 2016/09/13 23:39:16]

What does that mean "standard format"? I don't think there's any standard
format... and I would expect the right conversion to happen when you end up
drawing the bitmap into a canvas.

+        data.width(), data.height(),
+        kRGBA_8888_SkColorType, kPremul_SkAlphaType);
+    if (info.getSafeSize(info.minRowBytes()) > pixel_data.size()) {
+      // Insufficient buffer size.
+      out->reset();
+      return false;
+    }
+
+    // Create the SkBitmap object which wraps the arc bitmap pixels.
+    SkBitmap bitmap;
+    if (!bitmap.installPixels(info,
+            const_cast<uint8_t*>(pixel_data.storage().data()),
+            info.minRowBytes())) {
+      // Error in installing pixels.
+      out->reset();
+      return false;
+    }
+
+    // Copy the pixels with converting color type.
+    if (!bitmap.copyTo(out, kN32_SkColorType)) {
+      // Error in copying.
+      out->reset();
+      return false;
+    }
+
+    return true;
+  }
+};
+
+}
+
+#endif  // COMPONENT_ARC_BITMAP_BITMAP_STRUCT_TRAITS_H_