cc: Audit and static_assert the types in PaintOpBuffer can be moved.

cc/paint/paint_op_buffer.h

Index: cc/paint/paint_op_buffer.h
diff --git a/cc/paint/paint_op_buffer.h b/cc/paint/paint_op_buffer.h
index c400f51735799e7d7f3e092f54e2c8537f38954f..10be38069186b249e4f801d8c0a95eacf1a1fb4a 100644
--- a/cc/paint/paint_op_buffer.h
+++ b/cc/paint/paint_op_buffer.h
@@ -72,6 +72,29 @@ enum class PaintOpType : uint8_t {
   LastPaintOpType = Translate,
 };
 
+// It is a requirement for every field in a PaintOp that we can memcpy it to
+// another place in memory and free the only one (without calling any
+// constructors or destructors). std::is_trivially_copyable<> would guarantee
+// this, but is a superset of the guarantees that we need. For instance sk_sp<>
+// works correctly when we memcpy+free it, but it has a non-trivial move
+// constructor and destructor, which would make it fail a check for
+// std::is_trivially_copyable. std::is_trivially_move_constructible<> also
+// provides enough as we are moving the objects from one address to another
+// conceptually, but also is not satisfied by sk_sp<> since it modifies the
+// moved-from type.
+// TODO(danakj): std::is_trivially_move_constructible is not always available
+// for us yet, but we could use it when it is.
+// For every field in a PaintOp, then, provide either a static_assert or a
+// comment explaining why this field satisfies the ability to memcpy+free
+// its data.
+// sk_sp<T> pointers behave if their memory address is changed because there is
+// never a pointer back to the sk_sp<T> itself from inside the T. This is true
+// because multiple sk_sp<> may point to the same T and they are transient
+// compared to the lifetime of any T. Additionally, the move constructor doesn't
+// change any state that needs to be reflected in the new object.
+// NOTABLY std::vector does *not* satisfy this, as implementations can (and do)
+// sometimes have a pointer back to the vector from its internals, so running
+// its move-assignment operator is required.
 struct CC_PAINT_EXPORT PaintOp {
   uint32_t type : 8;
   uint32_t skip : 24;
@@ -124,9 +147,17 @@ struct CC_PAINT_EXPORT PaintOpWithFlags : PaintOp {
   // provide a modified PaintFlags. The RasterWithFlags() method is static with
   // a const PaintOpWithFlags* parameter so that it can be used as a function
   // pointer.
+  // PaintFlags is a collection of trivial data types and sk_sp<> ref pointers.
+  // The trivial data types satisfy std::is_trivially_copyable, and sk_sp<> is
+  // valid to have its memory address changed for reasons decribed at the top of
+  // PaintOp.
   PaintFlags flags;
 };
 
+// A PaintOp with an additional block of memory with variable size. The
+// additional data held in the PaintOpWithData must be able to be moved in
+// memory without consequences. As such it must not (directly or indirectly)
+// contain any pointers back to itself or to the PaintOp.
 struct CC_PAINT_EXPORT PaintOpWithData : PaintOpWithFlags {
   // Having data is just a helper for ops that have a varying amount of data and
   // want a way to store that inline.  This is for ops that pass in a
@@ -168,8 +199,16 @@ struct CC_PAINT_EXPORT PaintOpWithArrayBase : PaintOpWithFlags {
       : PaintOpWithFlags(flags) {}
 };
 
+// A PaintOp with an additional block of memory with variable size that is
+// an array of a known type. The types held in the array must be able to be
+// moved in memory without consequences. As such it must not (directly or
+// indirectly) contain any pointers back to itself or to the PaintOp. We use
+// std::is_trivially_copyable() to verify this (it is more strict but satisfies
+// our needs).
 template <typename M>
 struct CC_PAINT_EXPORT PaintOpWithArray : PaintOpWithArrayBase {
+  static_assert(base::is_trivially_copyable<M>::value, "");
+
   // Paint op that has a M[count] and a char[bytes].
   // Array data is stored first so that it can be aligned with T's alignment
   // with the arbitrary unaligned char data after it.
@@ -239,48 +278,66 @@ struct CC_PAINT_EXPORT AnnotateOp final : PaintOp {
                      SkCanvas* canvas,
                      const SkMatrix& original_ctm);
 
-  PaintCanvas::AnnotationType annotation_type;
+  // uint8_t is a trivial type.
+  uint8_t annotation_type;
+  // Verify that SkRect can be moved trivially.
+  static_assert(base::is_trivially_copyable<SkRect>::value, "");
   SkRect rect;
+  // sk_sp<> satisfies our data requirements as described at the top of PaintOp.
   sk_sp<SkData> data;
 };
 
 struct CC_PAINT_EXPORT ClipPathOp final : PaintOp {
   static constexpr PaintOpType kType = PaintOpType::ClipPath;
   ClipPathOp(SkPath path, SkClipOp op, bool antialias)
-      : path(path), op(op), antialias(antialias) {}
+      : path(path), op(static_cast<uint8_t>(op)), antialias(antialias) {}
   static void Raster(const PaintOp* op,
                      SkCanvas* canvas,
                      const SkMatrix& original_ctm);
   int CountSlowPaths() const;
 
+  // ThreadsafePath is an SkPath which contains trivial types, sk_sp<>, and
+  // SkAtomic<uint8_t>. sk_sp<> is valid as described at the top of PaintOp.
+  // SkAtomic<uint8_t> stores a trivial type internally, and takes its address
+  // but does not store it.
   ThreadsafePath path;
-  SkClipOp op;
+  // uint8_t is a trivial type.
+  uint8_t op;
+  // bool is a trivial type.
   bool antialias;
 };
 
 struct CC_PAINT_EXPORT ClipRectOp final : PaintOp {
   static constexpr PaintOpType kType = PaintOpType::ClipRect;
   ClipRectOp(const SkRect& rect, SkClipOp op, bool antialias)
-      : rect(rect), op(op), antialias(antialias) {}
+      : rect(rect), op(static_cast<uint8_t>(op)), antialias(antialias) {}
   static void Raster(const PaintOp* op,
                      SkCanvas* canvas,
                      const SkMatrix& original_ctm);
 
+  // Verify that SkRect can be moved trivially.
+  static_assert(base::is_trivially_copyable<SkRect>::value, "");
   SkRect rect;
-  SkClipOp op;
+  // uint8_t is a trivial type.
+  uint8_t op;
+  // bool is a trivial type.
   bool antialias;
 };
 
 struct CC_PAINT_EXPORT ClipRRectOp final : PaintOp {
   static constexpr PaintOpType kType = PaintOpType::ClipRRect;
   ClipRRectOp(const SkRRect& rrect, SkClipOp op, bool antialias)
-      : rrect(rrect), op(op), antialias(antialias) {}
+      : rrect(rrect), op(static_cast<uint8_t>(op)), antialias(antialias) {}
   static void Raster(const PaintOp* op,
                      SkCanvas* canvas,
                      const SkMatrix& original_ctm);
 
+  // Verify that SkRRect can be moved trivially.
+  static_assert(base::is_trivially_copyable<SkRRect>::value, "");
   SkRRect rrect;
-  SkClipOp op;
+  // uint8_t is a trivial type.
+  uint8_t op;
+  // bool is a trivial type.
   bool antialias;
 };
 
@@ -291,6 +348,8 @@ struct CC_PAINT_EXPORT ConcatOp final : PaintOp {
                      SkCanvas* canvas,
                      const SkMatrix& original_ctm);
 
+  // Verify that ThreadsafeMatrix can be moved trivially.
+  static_assert(base::is_trivially_copyable<ThreadsafeMatrix>::value, "");
   ThreadsafeMatrix matrix;
 };
 
@@ -298,8 +357,8 @@ struct CC_PAINT_EXPORT DrawArcOp final : PaintOpWithFlags {
   static constexpr PaintOpType kType = PaintOpType::DrawArc;
   static constexpr bool kIsDrawOp = true;
   DrawArcOp(const SkRect& oval,
-            SkScalar start_angle,
-            SkScalar sweep_angle,
+            float start_angle,
+            float sweep_angle,
             bool use_center,
             const PaintFlags& flags)
       : PaintOpWithFlags(flags),
@@ -318,19 +377,21 @@ struct CC_PAINT_EXPORT DrawArcOp final : PaintOpWithFlags {
                               SkCanvas* canvas,
                               const SkMatrix& original_ctm);
 
+  // Verify that SkRect can be moved trivially.
+  static_assert(base::is_trivially_copyable<SkRect>::value, "");
   SkRect oval;
-  SkScalar start_angle;
-  SkScalar sweep_angle;
+  // float is a trivial type.
+  float start_angle;
+  // float is a trivial type.
+  float sweep_angle;
+  // bool is a trivial type.
   bool use_center;
 };
 
 struct CC_PAINT_EXPORT DrawCircleOp final : PaintOpWithFlags {
   static constexpr PaintOpType kType = PaintOpType::DrawCircle;
   static constexpr bool kIsDrawOp = true;
-  DrawCircleOp(SkScalar cx,
-               SkScalar cy,
-               SkScalar radius,
-               const PaintFlags& flags)
+  DrawCircleOp(float cx, float cy, float radius, const PaintFlags& flags)
       : PaintOpWithFlags(flags), cx(cx), cy(cy), radius(radius) {}
   static void Raster(const PaintOp* op,
                      SkCanvas* canvas,
@@ -343,21 +404,27 @@ struct CC_PAINT_EXPORT DrawCircleOp final : PaintOpWithFlags {
                               SkCanvas* canvas,
                               const SkMatrix& original_ctm);
 
-  SkScalar cx;
-  SkScalar cy;
-  SkScalar radius;
+  // float is a trivial type.
+  float cx;
+  // float is a trivial type.
+  float cy;
+  // float is a trivial type.
+  float radius;
 };
 
 struct CC_PAINT_EXPORT DrawColorOp final : PaintOp {
   static constexpr PaintOpType kType = PaintOpType::DrawColor;
   static constexpr bool kIsDrawOp = true;
-  DrawColorOp(SkColor color, SkBlendMode mode) : color(color), mode(mode) {}
+  DrawColorOp(SkColor color, SkBlendMode mode)
+      : color(static_cast<uint32_t>(color)), mode(static_cast<uint8_t>(mode)) {}
   static void Raster(const PaintOp* op,
                      SkCanvas* canvas,
                      const SkMatrix& original_ctm);
 
-  SkColor color;
-  SkBlendMode mode;
+  // uint32_t is a trivial type.
+  uint32_t color;
+  // uint8_t is a trivial type.
+  uint8_t mode;
 };
 
 struct CC_PAINT_EXPORT DrawDisplayItemListOp final : PaintOp {
@@ -377,6 +444,8 @@ struct CC_PAINT_EXPORT DrawDisplayItemListOp final : PaintOp {
   bool HasDiscardableImages() const;
   // TODO(enne): DisplayItemList should know number of slow paths.
 
+  // scoped_refptr<> is able to be moved in memory for the same reasons as
+  // sk_sp<> which is described at the top of PaintOp.
   scoped_refptr<DisplayItemList> list;
 };
 
@@ -398,7 +467,10 @@ struct CC_PAINT_EXPORT DrawDRRectOp final : PaintOpWithFlags {
                               SkCanvas* canvas,
                               const SkMatrix& original_ctm);
 
+  // Verify that SkRRect can be moved trivially.
+  static_assert(base::is_trivially_copyable<SkRRect>::value, "");
   SkRRect outer;
+  // The above verifies SkRRect can be moved trivially.
   SkRRect inner;
 };
 
@@ -406,8 +478,8 @@ struct CC_PAINT_EXPORT DrawImageOp final : PaintOpWithFlags {
   static constexpr PaintOpType kType = PaintOpType::DrawImage;
   static constexpr bool kIsDrawOp = true;
   DrawImageOp(const PaintImage& image,
-              SkScalar left,
-              SkScalar top,
+              float left,
+              float top,
               const PaintFlags* flags);
   ~DrawImageOp();
   static void Raster(const PaintOp* op,
@@ -422,9 +494,14 @@ struct CC_PAINT_EXPORT DrawImageOp final : PaintOpWithFlags {
                               const SkMatrix& original_ctm);
   bool HasDiscardableImages() const;
 
+  // PaintImage is a set of trivial types and an sk_sp<> which is able to be
+  // moved in memory as described at the top of PaintOp. PaintImage does not
+  // hold any pointers back to itself directly or indirectly.
   PaintImage image;
-  SkScalar left;
-  SkScalar top;
+  // float is a trivial type.
+  float left;
+  // float is a trivial type.
+  float top;
 };
 
 struct CC_PAINT_EXPORT DrawImageRectOp final : PaintOpWithFlags {
@@ -448,10 +525,17 @@ struct CC_PAINT_EXPORT DrawImageRectOp final : PaintOpWithFlags {
                               const SkMatrix& original_ctm);
   bool HasDiscardableImages() const;
 
+  // PaintImage is a set of trivial types and an sk_sp<> which is able to be
+  // moved in memory as described at the top of PaintOp. PaintImage does not
+  // hold any pointers back to itself directly or indirectly.
   PaintImage image;
+  // Verify that SkRect can be moved trivially.
+  static_assert(base::is_trivially_copyable<SkRect>::value, "");
   SkRect src;
+  // The above verifies that SkRect can be moved trivially.
   SkRect dst;
-  PaintCanvas::SrcRectConstraint constraint;
+  // uint8_t is a trivial type.
+  uint8_t constraint;
 };
 
 struct CC_PAINT_EXPORT DrawIRectOp final : PaintOpWithFlags {
@@ -470,17 +554,15 @@ struct CC_PAINT_EXPORT DrawIRectOp final : PaintOpWithFlags {
                               SkCanvas* canvas,
                               const SkMatrix& original_ctm);
 
+  // Verify that SkIRect can be moved trivially.
+  static_assert(base::is_trivially_copyable<SkIRect>::value, "");
   SkIRect rect;
 };
 
 struct CC_PAINT_EXPORT DrawLineOp final : PaintOpWithFlags {
   static constexpr PaintOpType kType = PaintOpType::DrawLine;
   static constexpr bool kIsDrawOp = true;
-  DrawLineOp(SkScalar x0,
-             SkScalar y0,
-             SkScalar x1,
-             SkScalar y1,
-             const PaintFlags& flags)
+  DrawLineOp(float x0, float y0, float x1, float y1, const PaintFlags& flags)
       : PaintOpWithFlags(flags), x0(x0), y0(y0), x1(x1), y1(y1) {}
   static void Raster(const PaintOp* op,
                      SkCanvas* canvas,
@@ -495,10 +577,14 @@ struct CC_PAINT_EXPORT DrawLineOp final : PaintOpWithFlags {
 
   int CountSlowPaths() const;
 
-  SkScalar x0;
-  SkScalar y0;
-  SkScalar x1;
-  SkScalar y1;
+  // float is a trivial type.
+  float x0;
+  // float is a trivial type.
+  float y0;
+  // float is a trivial type.
+  float x1;
+  // float is a trivial type.
+  float y1;
 };
 
 struct CC_PAINT_EXPORT DrawOvalOp final : PaintOpWithFlags {
@@ -517,6 +603,8 @@ struct CC_PAINT_EXPORT DrawOvalOp final : PaintOpWithFlags {
                               SkCanvas* canvas,
                               const SkMatrix& original_ctm);
 
+  // Verify that SkRect can be moved trivially.
+  static_assert(base::is_trivially_copyable<SkRect>::value, "");
   SkRect oval;
 };
 
@@ -537,6 +625,10 @@ struct CC_PAINT_EXPORT DrawPathOp final : PaintOpWithFlags {
                               const SkMatrix& original_ctm);
   int CountSlowPaths() const;
 
+  // ThreadsafePath is an SkPath which contains trivial types, sk_sp<>, and
+  // SkAtomic<uint8_t>. sk_sp<> is valid as described at the top of PaintOp.
+  // SkAtomic<uint8_t> stores a trivial type internally, and takes its address
+  // but does not store it.
   ThreadsafePath path;
 };
 
@@ -574,6 +666,7 @@ struct CC_PAINT_EXPORT DrawRecordOp final : PaintOp {
   bool HasDiscardableImages() const;
   int CountSlowPaths() const;
 
+  // sk_sp<> can be moved in memory as decribed at the top of PaintOp.
   sk_sp<const PaintRecord> record;
 };
 
@@ -593,6 +686,8 @@ struct CC_PAINT_EXPORT DrawRectOp final : PaintOpWithFlags {
                               SkCanvas* canvas,
                               const SkMatrix& original_ctm);
 
+  // Verify that SkRect can be moved trivially.
+  static_assert(base::is_trivially_copyable<SkRect>::value, "");
   SkRect rect;
 };
 
@@ -612,13 +707,15 @@ struct CC_PAINT_EXPORT DrawRRectOp final : PaintOpWithFlags {
                               SkCanvas* canvas,
                               const SkMatrix& original_ctm);
 
+  // Verify that SkRRect can be moved trivially.
+  static_assert(base::is_trivially_copyable<SkRRect>::value, "");
   SkRRect rrect;
 };
 
 struct CC_PAINT_EXPORT DrawTextOp final : PaintOpWithData {
   static constexpr PaintOpType kType = PaintOpType::DrawText;
   static constexpr bool kIsDrawOp = true;
-  DrawTextOp(size_t bytes, SkScalar x, SkScalar y, const PaintFlags& flags)
+  DrawTextOp(size_t bytes, float x, float y, const PaintFlags& flags)
       : PaintOpWithData(flags, bytes), x(x), y(y) {}
   static void Raster(const PaintOp* op,
                      SkCanvas* canvas,
@@ -634,16 +731,18 @@ struct CC_PAINT_EXPORT DrawTextOp final : PaintOpWithData {
   void* GetData() { return GetDataForThis(this); }
   const void* GetData() const { return GetDataForThis(this); }
 
-  SkScalar x;
-  SkScalar y;
+  // float is a trivial type.
+  float x;
+  // float is a trivial type.
+  float y;
 };
 
 struct CC_PAINT_EXPORT DrawTextBlobOp final : PaintOpWithFlags {
   static constexpr PaintOpType kType = PaintOpType::DrawTextBlob;
   static constexpr bool kIsDrawOp = true;
   DrawTextBlobOp(sk_sp<SkTextBlob> blob,
-                 SkScalar x,
-                 SkScalar y,
+                 float x,
+                 float y,
                  const PaintFlags& flags);
   ~DrawTextBlobOp();
   static void Raster(const PaintOp* op,
@@ -657,9 +756,12 @@ struct CC_PAINT_EXPORT DrawTextBlobOp final : PaintOpWithFlags {
                               SkCanvas* canvas,
                               const SkMatrix& original_ctm);
 
+  // sk_sp<> can be moved in memory as decribed at the top of PaintOp.
   sk_sp<SkTextBlob> blob;
-  SkScalar x;
-  SkScalar y;
+  // float is a trivial type.
+  float x;
+  // float is a trivial type.
+  float y;
 };
 
 struct CC_PAINT_EXPORT NoopOp final : PaintOp {
@@ -678,12 +780,13 @@ struct CC_PAINT_EXPORT RestoreOp final : PaintOp {
 
 struct CC_PAINT_EXPORT RotateOp final : PaintOp {
   static constexpr PaintOpType kType = PaintOpType::Rotate;
-  explicit RotateOp(SkScalar degrees) : degrees(degrees) {}
+  explicit RotateOp(float degrees) : degrees(degrees) {}
   static void Raster(const PaintOp* op,
                      SkCanvas* canvas,
                      const SkMatrix& original_ctm);
 
-  SkScalar degrees;
+  // float is a trivial type.
+  float degrees;
 };
 
 struct CC_PAINT_EXPORT SaveOp final : PaintOp {
@@ -709,6 +812,8 @@ struct CC_PAINT_EXPORT SaveLayerOp final : PaintOpWithFlags {
                               SkCanvas* canvas,
                               const SkMatrix& original_ctm);
 
+  // Verify that SkRect can be moved trivially.
+  static_assert(base::is_trivially_copyable<SkRect>::value, "");
   SkRect bounds;
 };
 
@@ -720,19 +825,24 @@ struct CC_PAINT_EXPORT SaveLayerAlphaOp final : PaintOp {
                      SkCanvas* canvas,
                      const SkMatrix& original_ctm);
 
+  // Verify that SkRect can be moved trivially.
+  static_assert(base::is_trivially_copyable<SkRect>::value, "");
   SkRect bounds;
+  // uint8_t is a trivial type.
   uint8_t alpha;
 };
 
 struct CC_PAINT_EXPORT ScaleOp final : PaintOp {
   static constexpr PaintOpType kType = PaintOpType::Scale;
-  ScaleOp(SkScalar sx, SkScalar sy) : sx(sx), sy(sy) {}
+  ScaleOp(float sx, float sy) : sx(sx), sy(sy) {}
   static void Raster(const PaintOp* op,
                      SkCanvas* canvas,
                      const SkMatrix& original_ctm);
 
-  SkScalar sx;
-  SkScalar sy;
+  // float is a trivial type.
+  float sx;
+  // float is a trivial type.
+  float sy;
 };
 
 struct CC_PAINT_EXPORT SetMatrixOp final : PaintOp {
@@ -748,18 +858,22 @@ struct CC_PAINT_EXPORT SetMatrixOp final : PaintOp {
                      SkCanvas* canvas,
                      const SkMatrix& original_ctm);
 
+  // Verify that ThreadsafeMatrix can be moved trivially.
+  static_assert(base::is_trivially_copyable<ThreadsafeMatrix>::value, "");
   ThreadsafeMatrix matrix;
 };
 
 struct CC_PAINT_EXPORT TranslateOp final : PaintOp {
   static constexpr PaintOpType kType = PaintOpType::Translate;
-  TranslateOp(SkScalar dx, SkScalar dy) : dx(dx), dy(dy) {}
+  TranslateOp(float dx, float dy) : dx(dx), dy(dy) {}
   static void Raster(const PaintOp* op,
                      SkCanvas* canvas,
                      const SkMatrix& original_ctm);
 
-  SkScalar dx;
-  SkScalar dy;
+  // float is a trivial type.
+  float dx;
+  // float is a trivial type.
+  float dy;
 };
 
 using LargestPaintOp = DrawDRRectOp;