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) {
+    if (r.drawsNothing())
+      return mojo::Array<uint8_t>();

[dcheng, 2016/09/07 22:47:17]

These methods are pretty complex. We should out-of-line things that aren't just
simple accessors.

Also, we don't need to do all these defensive checks on the sending side: they
can just be DCHECKs. Hopfeully you know that you're not sending bad data.

[yoshiki, 2016/09/09 16:45:30]

Made this simpler. Is it ok?

+
+    const SkImageInfo& info = r.info();
+    if (info.colorType() !=  kRGBA_8888_SkColorType)
+      return mojo::Array<uint8_t>();
+
+    mojo::Array<uint8_t> pixel_data(r.getSize());
+    if (!r.readPixels(info, &pixel_data[0], r.rowBytes(), 0, 0))
+      return mojo::Array<uint8_t>();
+
+    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) {

[dcheng, 2016/09/07 22:47:17]

SkBitmap is an out param. In effect, Read() is called like this:

  // Generated mojo code.
  SkBitmap native_bitmap;
  ReadStruct(bitmap, &native_bitmap);

So we should just be mutating |out| directly.

[yoshiki, 2016/09/09 16:45:30]

I was misunderstood. Fixed.

+    mojo::Array<uint8_t> pixel_data;
+    if (!data.ReadPixelData(&pixel_data)) {
+      // Can't read the pixel data.
+      out = new SkBitmap();
+      return false;
+    }
+
+    if (data.width() == 0 || data.height() == 0) {
+      // Valid empty data.
+      out = new SkBitmap();
+      return true;
+    }
+
+    SkImageInfo info = SkImageInfo::Make(
+        data.width(), data.height(),
+        kRGBA_8888_SkColorType, kPremul_SkAlphaType);
+    if (info.getSafeSize(info.minRowBytes()) > pixel_data.size()) {
+      // Insufficient buffer size.
+      out = new SkBitmap();

[xiyuan, 2016/09/07 22:09:00]

This looks wrong. It would leak and created SkBitmap is not passed back.

Think it should be
  *out = SkBitmap();
or do nothing and leave |out| untouched.

[yoshiki, 2016/09/09 16:45:30]

Good catch. Let me use out->reset() here.

+      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 = new SkBitmap();

[xiyuan, 2016/09/07 22:09:00]

ditto

[yoshiki, 2016/09/09 16:45:30]

Done.

+      return false;
+    }
+
+    // Copy the pixels with converting color type.
+    if (!bitmap.copyTo(out, kN32_SkColorType)) {
+      // Error in copying.
+      out = new SkBitmap();

[xiyuan, 2016/09/07 22:09:00]

ditto

[yoshiki, 2016/09/09 16:45:30]

Done.

+      return false;
+    }
+
+    return true;
+  }
+};
+
+}
+
+#endif  // COMPONENT_ARC_BITMAP_BITMAP_STRUCT_TRAITS_H_