ui/gfx/android: Simplify Java/Skia bitmap conversions

Replace the explicit memcpy loops for converting between Skia and Java
bitmaps with SkBitmap::{read,write}Pixels.

We also adjust some fields of JavaBitmap to make their data types more
obvious.

Bug: 1144462
Change-Id: Ibc07ec546b38cb5650dc9a1e272e834b3579216a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2532250
Auto-Submit: Sami Kyöstilä <skyostil@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: danakj <danakj@chromium.org>
Reviewed-by: Adrian Taylor <adetaylor@chromium.org>
Commit-Queue: Sami Kyöstilä <skyostil@chromium.org>
Cr-Commit-Position: refs/heads/master@{#826791}

ui/base/clipboard/clipboard_android.cc

@@ -76,7 +76,7 @@ SkBitmap GetImageData(
   }
 
   gfx::JavaBitmap java_bitmap(jbitmap);
-  if (java_bitmap.size().IsEmpty() || java_bitmap.stride() == 0U ||
+  if (java_bitmap.size().IsEmpty() || java_bitmap.bytes_per_row() == 0U ||
       java_bitmap.pixels() == nullptr) {
     return SkBitmap();
   }

ui/gfx/android/java_bitmap.cc

@@ -20,6 +20,51 @@ using base::android::ScopedJavaLocalRef;
 using base::android::JavaRef;
 
 namespace gfx {
+namespace {
+
+int SkColorTypeToBitmapFormat(SkColorType color_type) {
+  switch (color_type) {
+    case kN32_SkColorType:
+      return BITMAP_FORMAT_ARGB_8888;
+    case kRGB_565_SkColorType:
+      return BITMAP_FORMAT_RGB_565;
+    default:
+      // A bad format can cause out-of-bounds issues when copying pixels into or
+      // out of the java bitmap's pixel buffer.
+      CHECK_NE(color_type, color_type);
+  }
+  return BITMAP_FORMAT_NO_CONFIG;
+}
+
+SkColorType BitmapFormatToSkColorType(BitmapFormat bitmap_format) {
+  switch (bitmap_format) {
+    case BITMAP_FORMAT_ALPHA_8:
+      return kAlpha_8_SkColorType;
+    case BITMAP_FORMAT_ARGB_4444:
+      return kARGB_4444_SkColorType;
+    case BITMAP_FORMAT_ARGB_8888:
+      return kN32_SkColorType;
+    case BITMAP_FORMAT_RGB_565:
+      return kRGB_565_SkColorType;
+    case BITMAP_FORMAT_NO_CONFIG:
+    default:
+      CHECK_NE(bitmap_format, bitmap_format);
+      return kUnknown_SkColorType;
+  }
+}
+
+// Wraps a Java bitmap as an SkPixmap. Since the pixmap references the
+// underlying pixel data in the Java bitmap directly, it is only valid as long
+// as |bitmap| is.
+SkPixmap WrapJavaBitmapAsPixmap(const JavaBitmap& bitmap) {
+  auto color_type = BitmapFormatToSkColorType(bitmap.format());
+  auto image_info =
+      SkImageInfo::Make(bitmap.size().width(), bitmap.size().height(),
+                        color_type, kPremul_SkAlphaType);
+  return SkPixmap(image_info, bitmap.pixels(), bitmap.bytes_per_row());
+}
+
+}  // namespace
 
 #define ASSERT_ENUM_EQ(a, b) \
   static_assert(static_cast<int>(a) == static_cast<int>(b), "")
@@ -44,8 +89,8 @@ JavaBitmap::JavaBitmap(const JavaRef<jobject>& bitmap)
   err = AndroidBitmap_getInfo(AttachCurrentThread(), bitmap_.obj(), &info);
   DCHECK(!err);
   size_ = gfx::Size(info.width, info.height);
-  format_ = info.format;
-  stride_ = info.stride;
+  format_ = static_cast<BitmapFormat>(info.format);
+  bytes_per_row_ = info.stride;
   byte_count_ = Java_BitmapHelper_getByteCount(AttachCurrentThread(), bitmap_);
 }
 
@@ -54,20 +99,6 @@ JavaBitmap::~JavaBitmap() {
   DCHECK(!err);
 }
 
-static int SkColorTypeToBitmapFormat(SkColorType color_type) {
-  switch (color_type) {
-    case kN32_SkColorType:
-      return BITMAP_FORMAT_ARGB_8888;
-    case kRGB_565_SkColorType:
-      return BITMAP_FORMAT_RGB_565;
-    default:
-      // A bad format can cause out-of-bounds issues when copying pixels into or
-      // out of the java bitmap's pixel buffer.
-      CHECK_NE(color_type, color_type);
-  }
-  return BITMAP_FORMAT_NO_CONFIG;
-}
-
 ScopedJavaLocalRef<jobject> ConvertToJavaBitmap(const SkBitmap& skbitmap,
                                                 OomBehavior reaction) {
   DCHECK(!skbitmap.isNull());
@@ -85,82 +116,33 @@ ScopedJavaLocalRef<jobject> ConvertToJavaBitmap(const SkBitmap& skbitmap,
   }
 
   JavaBitmap dst_lock(jbitmap);
-  // If java creates a bitmap with a different stride, then memcpy() will
-  // do the wrong thing below and we can end up with an out-of-bounds write.
-  CHECK_EQ(skbitmap.rowBytes(), dst_lock.stride());
-  // If java creates a bitmap with a different format, then memcpy() may
-  // do the wrong thing below since the buffer sizes may differ, and we can
-  // end up with an out-of-bounds write.
-  CHECK_EQ(java_bitmap_format, dst_lock.format());
-  // This is mostly a corrolary of the above checks, however, the max number of
-  // bytes in the JavaBitmap is less than in the SkBitmap, since it is expressed
-  // as an int, instead of a size_t. If java capped the size, then the memcpy()
-  // below can cause an out-of-bounds write.
-  CHECK_EQ(base::checked_cast<size_t>(dst_lock.byte_count()),
-           skbitmap.computeByteSize());
-
-  memcpy(dst_lock.pixels(), skbitmap.getPixels(), skbitmap.computeByteSize());
-
+  SkPixmap dst = WrapJavaBitmapAsPixmap(dst_lock);
+  skbitmap.readPixels(dst);
   return jbitmap;
 }
 
 SkBitmap CreateSkBitmapFromJavaBitmap(const JavaBitmap& jbitmap) {
   DCHECK(!jbitmap.size().IsEmpty());
-  DCHECK_GT(jbitmap.stride(), 0U);
+  DCHECK_GT(jbitmap.bytes_per_row(), 0U);
   DCHECK(jbitmap.pixels());
 
-  gfx::Size src_size = jbitmap.size();
-
-  SkBitmap skbitmap;
-  SkImageInfo image_info;
-  switch (jbitmap.format()) {
-    case ANDROID_BITMAP_FORMAT_RGBA_8888:
-      image_info =
-          SkImageInfo::MakeN32Premul(src_size.width(), src_size.height());
-      break;
-    case ANDROID_BITMAP_FORMAT_A_8:
-      image_info =
-          SkImageInfo::SkImageInfo::MakeA8(src_size.width(), src_size.height());
-      break;
-    default:
-      CHECK(false) << "Invalid Java bitmap format: " << jbitmap.format();
-      break;
-  }
-
   // Ensure 4 byte stride alignment since the texture upload code in the
   // compositor relies on this.
-  const size_t min_row_bytes = image_info.minRowBytes();
-  DCHECK_GE(jbitmap.stride(), min_row_bytes);
-
+  SkPixmap src = WrapJavaBitmapAsPixmap(jbitmap);
+  const size_t min_row_bytes = src.info().minRowBytes();
   const size_t row_bytes = base::bits::Align(min_row_bytes, 4u);
-  skbitmap.allocPixels(image_info, row_bytes);
-
-  const char* src_pixels = static_cast<const char*>(jbitmap.pixels());
-  char* dst_pixels = static_cast<char*>(skbitmap.getPixels());
-  for (int i = 0; i < src_size.height(); ++i) {
-    memcpy(dst_pixels, src_pixels, min_row_bytes);
-    src_pixels += jbitmap.stride();
-    dst_pixels += row_bytes;
-  }
+
+  SkBitmap skbitmap;
+  skbitmap.allocPixels(src.info(), row_bytes);
+  skbitmap.writePixels(src);
   return skbitmap;
 }
 
 SkColorType ConvertToSkiaColorType(const JavaRef<jobject>& bitmap_config) {
-  int jbitmap_config = Java_BitmapHelper_getBitmapFormatForConfig(
-      AttachCurrentThread(), bitmap_config);
-  switch (jbitmap_config) {
-    case BITMAP_FORMAT_ALPHA_8:
-      return kAlpha_8_SkColorType;
-    case BITMAP_FORMAT_ARGB_4444:
-      return kARGB_4444_SkColorType;
-    case BITMAP_FORMAT_ARGB_8888:
-      return kN32_SkColorType;
-    case BITMAP_FORMAT_RGB_565:
-      return kRGB_565_SkColorType;
-    case BITMAP_FORMAT_NO_CONFIG:
-    default:
-      return kUnknown_SkColorType;
-  }
+  BitmapFormat jbitmap_format =
+      static_cast<BitmapFormat>(Java_BitmapHelper_getBitmapFormatForConfig(
+          AttachCurrentThread(), bitmap_config));
+  return BitmapFormatToSkColorType(jbitmap_format);
 }
 
 }  //  namespace gfx

ui/gfx/android/java_bitmap.h

@@ -39,17 +39,16 @@ class GFX_EXPORT JavaBitmap {
   inline void* pixels() { return pixels_; }
   inline const void* pixels() const { return pixels_; }
   inline const gfx::Size& size() const { return size_; }
-  // Formats are in android/bitmap.h; e.g. ANDROID_BITMAP_FORMAT_RGBA_8888
-  inline int format() const { return format_; }
-  inline uint32_t stride() const { return stride_; }
+  inline BitmapFormat format() const { return format_; }
+  inline uint32_t bytes_per_row() const { return bytes_per_row_; }
   inline int byte_count() const { return byte_count_; }
 
  private:
   base::android::ScopedJavaGlobalRef<jobject> bitmap_;
   void* pixels_;
   gfx::Size size_;
-  int format_;
-  uint32_t stride_;
+  BitmapFormat format_;
+  uint32_t bytes_per_row_;
   int byte_count_;
 
   DISALLOW_COPY_AND_ASSIGN(JavaBitmap);