Update IPC ParamTraits for SkBitmap to follow best practices.

Using memcpy() to serialize a POD struct is highly discouraged. Just use
the standard IPC param traits macros for doing it.

Bug: 779428
Change-Id: I48f52c1f5c245ba274d595829ed92e8b3cb41334
Reviewed-on: https://chromium-review.googlesource.com/899649Reviewed-by: Tom Sepez <tsepez@chromium.org>
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#534562}

ui/gfx/ipc/skia/gfx_skia_param_traits.cc

@@ -7,57 +7,64 @@
 #include <string>
 
 #include "base/pickle.h"
+#include "ipc/ipc_message_utils.h"
 #include "third_party/skia/include/core/SkBitmap.h"
 #include "third_party/skia/include/core/SkImageInfo.h"
+#include "ui/gfx/ipc/skia/gfx_skia_param_traits_macros.h"
 #include "ui/gfx/transform.h"
 
-namespace {
-
-struct SkBitmap_Data {
-  // The color type for the bitmap (bits per pixel, etc).
-  SkColorType color_type;
+// Generate param traits write methods.
+#include "ipc/param_traits_write_macros.h"
+namespace IPC {
+#undef UI_GFX_IPC_SKIA_GFX_SKIA_PARAM_TRAITS_MACROS_H_
+#include "ui/gfx/ipc/skia/gfx_skia_param_traits_macros.h"
+}  // namespace IPC
 
-  // The alpha type for the bitmap (opaque, premul, unpremul).
-  SkAlphaType alpha_type;
+// Generate param traits read methods.
+#include "ipc/param_traits_read_macros.h"
+namespace IPC {
+#undef UI_GFX_IPC_SKIA_GFX_SKIA_PARAM_TRAITS_MACROS_H_
+#include "ui/gfx/ipc/skia/gfx_skia_param_traits_macros.h"
+}  // namespace IPC
 
-  // The width of the bitmap in pixels.
-  uint32_t width;
+// Generate param traits log methods.
+#include "ipc/param_traits_log_macros.h"
+namespace IPC {
+#undef UI_GFX_IPC_SKIA_GFX_SKIA_PARAM_TRAITS_MACROS_H_
+#include "ui/gfx/ipc/skia/gfx_skia_param_traits_macros.h"
+}  // namespace IPC
 
-  // The height of the bitmap in pixels.
-  uint32_t height;
+namespace IPC {
 
-  void InitSkBitmapDataForTransfer(const SkBitmap& bitmap) {
-    const SkImageInfo& info = bitmap.info();
-    color_type = info.colorType();
-    alpha_type = info.alphaType();
-    width = info.width();
-    height = info.height();
-  }
+void ParamTraits<SkImageInfo>::Write(base::Pickle* m, const SkImageInfo& p) {
+  WriteParam(m, p.colorType());
+  WriteParam(m, p.alphaType());
+  WriteParam(m, p.width());
+  WriteParam(m, p.height());
+}
 
-  // Returns whether |bitmap| successfully initialized.
-  bool InitSkBitmapFromData(SkBitmap* bitmap,
-                            const char* pixels,
-                            size_t pixels_size) const {
-    if (!bitmap->tryAllocPixels(
-            SkImageInfo::Make(width, height, color_type, alpha_type)))
-      return false;
-    if (pixels_size != bitmap->computeByteSize())
-      return false;
-    memcpy(bitmap->getPixels(), pixels, pixels_size);
-    return true;
+bool ParamTraits<SkImageInfo>::Read(const base::Pickle* m,
+                                    base::PickleIterator* iter,
+                                    SkImageInfo* r) {
+  SkColorType color_type;
+  SkAlphaType alpha_type;
+  uint32_t width;
+  uint32_t height;
+  if (!ReadParam(m, iter, &color_type) || !ReadParam(m, iter, &alpha_type) ||
+      !ReadParam(m, iter, &width) || !ReadParam(m, iter, &height)) {
+    return false;
   }
-};
 
-}  // namespace
+  *r = SkImageInfo::Make(width, height, color_type, alpha_type);
+  return true;
+}
 
-namespace IPC {
+void ParamTraits<SkImageInfo>::Log(const SkImageInfo& p, std::string* l) {
+  l->append("<SkImageInfo>");
+}
 
 void ParamTraits<SkBitmap>::Write(base::Pickle* m, const SkBitmap& p) {
-  size_t fixed_size = sizeof(SkBitmap_Data);
-  SkBitmap_Data bmp_data;
-  bmp_data.InitSkBitmapDataForTransfer(p);
-  m->WriteData(reinterpret_cast<const char*>(&bmp_data),
-               static_cast<int>(fixed_size));
+  WriteParam(m, p.info());
   size_t pixel_size = p.computeByteSize();
   m->WriteData(reinterpret_cast<const char*>(p.getPixels()),
                static_cast<int>(pixel_size));
@@ -66,28 +73,28 @@ void ParamTraits<SkBitmap>::Write(base::Pickle* m, const SkBitmap& p) {
 bool ParamTraits<SkBitmap>::Read(const base::Pickle* m,
                                  base::PickleIterator* iter,
                                  SkBitmap* r) {
-  const char* fixed_data;
-  int fixed_data_size = 0;
-  if (!iter->ReadData(&fixed_data, &fixed_data_size) ||
-     (fixed_data_size <= 0)) {
+  SkImageInfo image_info;
+  if (!ReadParam(m, iter, &image_info))
     return false;
-  }
-  if (fixed_data_size != sizeof(SkBitmap_Data))
-    return false;  // Message is malformed.
 
-  const char* variable_data;
-  int variable_data_size = 0;
-  if (!iter->ReadData(&variable_data, &variable_data_size) ||
-     (variable_data_size < 0)) {
+  const char* bitmap_data;
+  int bitmap_data_size = 0;
+  if (!iter->ReadData(&bitmap_data, &bitmap_data_size))
     return false;
-  }
-  const SkBitmap_Data* bmp_data =
-      reinterpret_cast<const SkBitmap_Data*>(fixed_data);
-  return bmp_data->InitSkBitmapFromData(r, variable_data, variable_data_size);
+  // ReadData() only returns true if bitmap_data_size >= 0.
+
+  if (!r->tryAllocPixels(image_info))
+    return false;
+
+  if (static_cast<size_t>(bitmap_data_size) != r->computeByteSize())
+    return false;
+  memcpy(r->getPixels(), bitmap_data, bitmap_data_size);
+  return true;
 }
 
 void ParamTraits<SkBitmap>::Log(const SkBitmap& p, std::string* l) {
   l->append("<SkBitmap>");
+  LogParam(p.info(), l);
 }
 
 void ParamTraits<gfx::Transform>::Write(base::Pickle* m, const param_type& p) {

ui/gfx/ipc/skia/gfx_skia_param_traits.h

@@ -12,6 +12,7 @@
 #include "ui/gfx/ipc/skia/gfx_skia_ipc_export.h"
 
 class SkBitmap;
+struct SkImageInfo;
 
 namespace base {
 class Pickle;
@@ -24,6 +25,16 @@ class Transform;
 
 namespace IPC {
 
+template <>
+struct GFX_SKIA_IPC_EXPORT ParamTraits<SkImageInfo> {
+  using param_type = SkImageInfo;
+  static void Write(base::Pickle* m, const param_type& p);
+  static bool Read(const base::Pickle* m,
+                   base::PickleIterator* iter,
+                   param_type* r);
+  static void Log(const param_type& p, std::string* l);
+};
+
 template <>
 struct GFX_SKIA_IPC_EXPORT ParamTraits<SkBitmap> {
   using param_type = SkBitmap;

ui/gfx/ipc/skia/gfx_skia_param_traits_macros.h

+// Copyright 2018 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 UI_GFX_IPC_SKIA_GFX_SKIA_PARAM_TRAITS_MACROS_H_
+#define UI_GFX_IPC_SKIA_GFX_SKIA_PARAM_TRAITS_MACROS_H_
+
+#include <stdint.h>
+
+#include "ipc/ipc_message_macros.h"
+#include "third_party/skia/include/core/SkImageInfo.h"
+
+IPC_ENUM_TRAITS_VALIDATE(SkColorType, kLastEnum_SkColorType);
+IPC_ENUM_TRAITS_VALIDATE(SkAlphaType, kLastEnum_SkAlphaType);
+
+#endif  // UI_GFX_IPC_SKIA_GFX_SKIA_PARAM_TRAITS_MACROS_H_