Validate rrects in PaintOpBuffer serialization

Also, consolidate all PaintOp validation into new IsValid
functions.

Bug: 749023, 750010
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I1f5b695fe71c50d5ae126937c15a8957285a5487
Reviewed-on: https://chromium-review.googlesource.com/590749Reviewed-by: Vladimir Levin <vmpstr@chromium.org>
Reviewed-by: Khushal <khushalsagar@chromium.org>
Commit-Queue: enne <enne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#491171}

cc/paint/paint_flags.cc

@@ -4,6 +4,8 @@
 
 #include "cc/paint/paint_flags.h"
 
+#include "cc/paint/paint_op_buffer.h"
+
 namespace {
 
 static bool affects_alpha(const SkColorFilter* cf) {
@@ -122,4 +124,8 @@ SkPaint PaintFlags::ToSkPaint() const {
   return paint;
 }
 
+bool PaintFlags::IsValid() const {
+  return PaintOp::IsValidPaintFlagsSkBlendMode(getBlendMode());
+}
+
 }  // namespace cc

cc/paint/paint_flags.h

@@ -217,6 +217,8 @@ class CC_PAINT_EXPORT PaintFlags {
 
   SkPaint ToSkPaint() const;
 
+  bool IsValid() const;
+
  private:
   friend class PaintOpReader;
   friend class PaintOpWriter;

cc/paint/paint_op_buffer.cc

@@ -214,6 +214,7 @@ struct Rasterizer {
     static_assert(
         !T::kHasPaintFlags,
         "This function should not be used for a PaintOp that has PaintFlags");
+    DCHECK(op->IsValid());
     NOTREACHED();
   }
   static void Raster(const T* op,
@@ -222,6 +223,7 @@ struct Rasterizer {
     static_assert(
         !T::kHasPaintFlags,
         "This function should not be used for a PaintOp that has PaintFlags");
+    DCHECK(op->IsValid());
     T::Raster(op, canvas, params);
   }
 };
@@ -234,6 +236,7 @@ struct Rasterizer<T, true> {
                               const PlaybackParams& params) {
     static_assert(T::kHasPaintFlags,
                   "This function expects the PaintOp to have PaintFlags");
+    DCHECK(op->IsValid());
     T::RasterWithFlags(op, flags, canvas, params);
   }
 
@@ -242,6 +245,7 @@ struct Rasterizer<T, true> {
                      const PlaybackParams& params) {
     static_assert(T::kHasPaintFlags,
                   "This function expects the PaintOp to have PaintFlags");
+    DCHECK(op->IsValid());
     T::RasterWithFlags(op, &op->flags, canvas, params);
   }
 };
@@ -733,6 +737,8 @@ T* SimpleDeserialize(const void* input,
   memcpy(output, input, sizeof(T));
 
   T* op = reinterpret_cast<T*>(output);
+  if (!op->IsValid())
+    return nullptr;
   // Type and skip were already read once, so could have been changed.
   // Don't trust them and clobber them with something valid.
   UpdateTypeAndSkip(op);
@@ -750,7 +756,7 @@ PaintOp* AnnotateOp::Deserialize(const void* input,
   helper.Read(&op->annotation_type);
   helper.Read(&op->rect);
   helper.Read(&op->data);
-  if (!helper.valid()) {
+  if (!helper.valid() || !op->IsValid()) {
     op->~AnnotateOp();
     return nullptr;
   }
@@ -778,7 +784,7 @@ PaintOp* ClipPathOp::Deserialize(const void* input,
   helper.Read(&op->path);
   helper.Read(&op->op);
   helper.Read(&op->antialias);
-  if (!helper.valid() || !IsValidSkClipOp(op->op)) {
+  if (!helper.valid() || !op->IsValid()) {
     op->~ClipPathOp();
     return nullptr;
   }
@@ -791,18 +797,14 @@ PaintOp* ClipRectOp::Deserialize(const void* input,
                                  size_t input_size,
                                  void* output,
                                  size_t output_size) {
-  ClipRectOp* op =
-      SimpleDeserialize<ClipRectOp>(input, input_size, output, output_size);
-  return op && IsValidSkClipOp(op->op) ? op : nullptr;
+  return SimpleDeserialize<ClipRectOp>(input, input_size, output, output_size);
 }
 
 PaintOp* ClipRRectOp::Deserialize(const void* input,
                                   size_t input_size,
                                   void* output,
                                   size_t output_size) {
-  ClipRRectOp* op =
-      SimpleDeserialize<ClipRRectOp>(input, input_size, output, output_size);
-  return op && IsValidSkClipOp(op->op) ? op : nullptr;
+  return SimpleDeserialize<ClipRRectOp>(input, input_size, output, output_size);
 }
 
 PaintOp* ConcatOp::Deserialize(const void* input,
@@ -825,7 +827,7 @@ PaintOp* DrawArcOp::Deserialize(const void* input,
   helper.Read(&op->start_angle);
   helper.Read(&op->sweep_angle);
   helper.Read(&op->use_center);
-  if (!helper.valid()) {
+  if (!helper.valid() || !op->IsValid()) {
     op->~DrawArcOp();
     return nullptr;
   }
@@ -845,7 +847,7 @@ PaintOp* DrawCircleOp::Deserialize(const void* input,
   helper.Read(&op->cx);
   helper.Read(&op->cy);
   helper.Read(&op->radius);
-  if (!helper.valid()) {
+  if (!helper.valid() || !op->IsValid()) {
     op->~DrawCircleOp();
     return nullptr;
   }
@@ -857,9 +859,7 @@ PaintOp* DrawColorOp::Deserialize(const void* input,
                                   size_t input_size,
                                   void* output,
                                   size_t output_size) {
-  DrawColorOp* op =
-      SimpleDeserialize<DrawColorOp>(input, input_size, output, output_size);
-  return op && IsValidDrawColorSkBlendMode(op->mode) ? op : nullptr;
+  return SimpleDeserialize<DrawColorOp>(input, input_size, output, output_size);
 }
 
 PaintOp* DrawDRRectOp::Deserialize(const void* input,
@@ -873,7 +873,7 @@ PaintOp* DrawDRRectOp::Deserialize(const void* input,
   helper.Read(&op->flags);
   helper.Read(&op->outer);
   helper.Read(&op->inner);
-  if (!helper.valid()) {
+  if (!helper.valid() || !op->IsValid()) {
     op->~DrawDRRectOp();
     return nullptr;
   }
@@ -893,7 +893,7 @@ PaintOp* DrawImageOp::Deserialize(const void* input,
   helper.Read(&op->image);
   helper.Read(&op->left);
   helper.Read(&op->top);
-  if (!helper.valid()) {
+  if (!helper.valid() || !op->IsValid()) {
     op->~DrawImageOp();
     return nullptr;
   }
@@ -914,7 +914,7 @@ PaintOp* DrawImageRectOp::Deserialize(const void* input,
   helper.Read(&op->src);
   helper.Read(&op->dst);
   helper.Read(&op->constraint);
-  if (!helper.valid()) {
+  if (!helper.valid() || !op->IsValid()) {
     op->~DrawImageRectOp();
     return nullptr;
   }
@@ -932,7 +932,7 @@ PaintOp* DrawIRectOp::Deserialize(const void* input,
   PaintOpReader helper(input, input_size);
   helper.Read(&op->flags);
   helper.Read(&op->rect);
-  if (!helper.valid()) {
+  if (!helper.valid() || !op->IsValid()) {
     op->~DrawIRectOp();
     return nullptr;
   }
@@ -953,7 +953,7 @@ PaintOp* DrawLineOp::Deserialize(const void* input,
   helper.Read(&op->y0);
   helper.Read(&op->x1);
   helper.Read(&op->y1);
-  if (!helper.valid()) {
+  if (!helper.valid() || !op->IsValid()) {
     op->~DrawLineOp();
     return nullptr;
   }
@@ -971,7 +971,7 @@ PaintOp* DrawOvalOp::Deserialize(const void* input,
   PaintOpReader helper(input, input_size);
   helper.Read(&op->flags);
   helper.Read(&op->oval);
-  if (!helper.valid()) {
+  if (!helper.valid() || !op->IsValid()) {
     op->~DrawOvalOp();
     return nullptr;
   }
@@ -989,7 +989,7 @@ PaintOp* DrawPathOp::Deserialize(const void* input,
   PaintOpReader helper(input, input_size);
   helper.Read(&op->flags);
   helper.Read(&op->path);
-  if (!helper.valid()) {
+  if (!helper.valid() || !op->IsValid()) {
     op->~DrawPathOp();
     return nullptr;
   }
@@ -1018,7 +1018,7 @@ PaintOp* DrawPosTextOp::Deserialize(const void* input,
     helper.ReadArray(op->count, op->GetArray());
     helper.ReadData(op->bytes, op->GetData());
   }
-  if (!helper.valid()) {
+  if (!helper.valid() || !op->IsValid()) {
     op->~DrawPosTextOp();
     return nullptr;
   }
@@ -1050,7 +1050,7 @@ PaintOp* DrawRectOp::Deserialize(const void* input,
   PaintOpReader helper(input, input_size);
   helper.Read(&op->flags);
   helper.Read(&op->rect);
-  if (!helper.valid()) {
+  if (!helper.valid() || !op->IsValid()) {
     op->~DrawRectOp();
     return nullptr;
   }
@@ -1068,7 +1068,7 @@ PaintOp* DrawRRectOp::Deserialize(const void* input,
   PaintOpReader helper(input, input_size);
   helper.Read(&op->flags);
   helper.Read(&op->rrect);
-  if (!helper.valid()) {
+  if (!helper.valid() || !op->IsValid()) {
     op->~DrawRRectOp();
     return nullptr;
   }
@@ -1090,7 +1090,7 @@ PaintOp* DrawTextOp::Deserialize(const void* input,
   helper.Read(&op->bytes);
   if (helper.valid())
     helper.ReadData(op->bytes, op->GetData());
-  if (!helper.valid()) {
+  if (!helper.valid() || !op->IsValid()) {
     op->~DrawTextOp();
     return nullptr;
   }
@@ -1113,7 +1113,7 @@ PaintOp* DrawTextBlobOp::Deserialize(const void* input,
   helper.Read(&op->x);
   helper.Read(&op->y);
   helper.Read(&op->blob);
-  if (!helper.valid()) {
+  if (!helper.valid() || !op->IsValid()) {
     op->~DrawTextBlobOp();
     return nullptr;
   }
@@ -1159,7 +1159,7 @@ PaintOp* SaveLayerOp::Deserialize(const void* input,
   PaintOpReader helper(input, input_size);
   helper.Read(&op->flags);
   helper.Read(&op->bounds);
-  if (!helper.valid()) {
+  if (!helper.valid() || !op->IsValid()) {
     op->~SaveLayerOp();
     return nullptr;
   }

cc/paint/paint_op_buffer_unittest.cc

@@ -1598,6 +1598,8 @@ void CompareFlags(const PaintFlags& original, const PaintFlags& written) {
 void CompareImages(const PaintImage& original, const PaintImage& written) {}
 
 void CompareAnnotateOp(const AnnotateOp* original, const AnnotateOp* written) {
+  EXPECT_TRUE(original->IsValid());
+  EXPECT_TRUE(written->IsValid());
   EXPECT_EQ(original->annotation_type, written->annotation_type);
   EXPECT_EQ(original->rect, written->rect);
   EXPECT_EQ(!!original->data, !!written->data);
@@ -1616,12 +1618,16 @@ void CompareClipDeviceRectOp(const ClipDeviceRectOp* original,
 }
 
 void CompareClipPathOp(const ClipPathOp* original, const ClipPathOp* written) {
+  EXPECT_TRUE(original->IsValid());
+  EXPECT_TRUE(written->IsValid());
   EXPECT_TRUE(original->path == written->path);
   EXPECT_EQ(original->op, written->op);
   EXPECT_EQ(original->antialias, written->antialias);
 }
 
 void CompareClipRectOp(const ClipRectOp* original, const ClipRectOp* written) {
+  EXPECT_TRUE(original->IsValid());
+  EXPECT_TRUE(written->IsValid());
   EXPECT_EQ(original->rect, written->rect);
   EXPECT_EQ(original->op, written->op);
   EXPECT_EQ(original->antialias, written->antialias);
@@ -1629,17 +1635,23 @@ void CompareClipRectOp(const ClipRectOp* original, const ClipRectOp* written) {
 
 void CompareClipRRectOp(const ClipRRectOp* original,
                         const ClipRRectOp* written) {
+  EXPECT_TRUE(original->IsValid());
+  EXPECT_TRUE(written->IsValid());
   EXPECT_EQ(original->rrect, written->rrect);
   EXPECT_EQ(original->op, written->op);
   EXPECT_EQ(original->antialias, written->antialias);
 }
 
 void CompareConcatOp(const ConcatOp* original, const ConcatOp* written) {
+  EXPECT_TRUE(original->IsValid());
+  EXPECT_TRUE(written->IsValid());
   EXPECT_EQ(original->matrix, written->matrix);
   EXPECT_EQ(original->matrix.getType(), written->matrix.getType());
 }
 
 void CompareDrawArcOp(const DrawArcOp* original, const DrawArcOp* written) {
+  EXPECT_TRUE(original->IsValid());
+  EXPECT_TRUE(written->IsValid());
   CompareFlags(original->flags, written->flags);
   EXPECT_EQ(original->oval, written->oval);
   EXPECT_EQ(original->start_angle, written->start_angle);
@@ -1649,6 +1661,8 @@ void CompareDrawArcOp(const DrawArcOp* original, const DrawArcOp* written) {
 
 void CompareDrawCircleOp(const DrawCircleOp* original,
                          const DrawCircleOp* written) {
+  EXPECT_TRUE(original->IsValid());
+  EXPECT_TRUE(written->IsValid());
   CompareFlags(original->flags, written->flags);
   EXPECT_EQ(original->cx, written->cx);
   EXPECT_EQ(original->cy, written->cy);
@@ -1657,11 +1671,15 @@ void CompareDrawCircleOp(const DrawCircleOp* original,
 
 void CompareDrawColorOp(const DrawColorOp* original,
                         const DrawColorOp* written) {
+  EXPECT_TRUE(original->IsValid());
+  EXPECT_TRUE(written->IsValid());
   EXPECT_EQ(original->color, written->color);
 }
 
 void CompareDrawDRRectOp(const DrawDRRectOp* original,
                          const DrawDRRectOp* written) {
+  EXPECT_TRUE(original->IsValid());
+  EXPECT_TRUE(written->IsValid());
   CompareFlags(original->flags, written->flags);
   EXPECT_EQ(original->outer, written->outer);
   EXPECT_EQ(original->inner, written->inner);
@@ -1669,6 +1687,8 @@ void CompareDrawDRRectOp(const DrawDRRectOp* original,
 
 void CompareDrawImageOp(const DrawImageOp* original,
                         const DrawImageOp* written) {
+  EXPECT_TRUE(original->IsValid());
+  EXPECT_TRUE(written->IsValid());
   CompareFlags(original->flags, written->flags);
   CompareImages(original->image, written->image);
   EXPECT_EQ(original->left, written->left);
@@ -1677,6 +1697,8 @@ void CompareDrawImageOp(const DrawImageOp* original,
 
 void CompareDrawImageRectOp(const DrawImageRectOp* original,
                             const DrawImageRectOp* written) {
+  EXPECT_TRUE(original->IsValid());
+  EXPECT_TRUE(written->IsValid());
   CompareFlags(original->flags, written->flags);
   CompareImages(original->image, written->image);
   EXPECT_EQ(original->src, written->src);
@@ -1685,11 +1707,15 @@ void CompareDrawImageRectOp(const DrawImageRectOp* original,
 
 void CompareDrawIRectOp(const DrawIRectOp* original,
                         const DrawIRectOp* written) {
+  EXPECT_TRUE(original->IsValid());
+  EXPECT_TRUE(written->IsValid());
   CompareFlags(original->flags, written->flags);
   EXPECT_EQ(original->rect, written->rect);
 }
 
 void CompareDrawLineOp(const DrawLineOp* original, const DrawLineOp* written) {
+  EXPECT_TRUE(original->IsValid());
+  EXPECT_TRUE(written->IsValid());
   CompareFlags(original->flags, written->flags);
   EXPECT_EQ(original->x0, written->x0);
   EXPECT_EQ(original->y0, written->y0);
@@ -1698,17 +1724,23 @@ void CompareDrawLineOp(const DrawLineOp* original, const DrawLineOp* written) {
 }
 
 void CompareDrawOvalOp(const DrawOvalOp* original, const DrawOvalOp* written) {
+  EXPECT_TRUE(original->IsValid());
+  EXPECT_TRUE(written->IsValid());
   CompareFlags(original->flags, written->flags);
   EXPECT_EQ(original->oval, written->oval);
 }
 
 void CompareDrawPathOp(const DrawPathOp* original, const DrawPathOp* written) {
+  EXPECT_TRUE(original->IsValid());
+  EXPECT_TRUE(written->IsValid());
   CompareFlags(original->flags, written->flags);
   EXPECT_TRUE(original->path == written->path);
 }
 
 void CompareDrawPosTextOp(const DrawPosTextOp* original,
                           const DrawPosTextOp* written) {
+  EXPECT_TRUE(original->IsValid());
+  EXPECT_TRUE(written->IsValid());
   CompareFlags(original->flags, written->flags);
   ASSERT_EQ(original->bytes, written->bytes);
   EXPECT_EQ(std::string(static_cast<const char*>(original->GetData())),
@@ -1719,17 +1751,23 @@ void CompareDrawPosTextOp(const DrawPosTextOp* original,
 }
 
 void CompareDrawRectOp(const DrawRectOp* original, const DrawRectOp* written) {
+  EXPECT_TRUE(original->IsValid());
+  EXPECT_TRUE(written->IsValid());
   CompareFlags(original->flags, written->flags);
   EXPECT_EQ(original->rect, written->rect);
 }
 
 void CompareDrawRRectOp(const DrawRRectOp* original,
                         const DrawRRectOp* written) {
+  EXPECT_TRUE(original->IsValid());
+  EXPECT_TRUE(written->IsValid());
   CompareFlags(original->flags, written->flags);
   EXPECT_EQ(original->rrect, written->rrect);
 }
 
 void CompareDrawTextOp(const DrawTextOp* original, const DrawTextOp* written) {
+  EXPECT_TRUE(original->IsValid());
+  EXPECT_TRUE(written->IsValid());
   CompareFlags(original->flags, written->flags);
   EXPECT_EQ(original->x, written->x);
   EXPECT_EQ(original->y, written->y);
@@ -1740,6 +1778,8 @@ void CompareDrawTextOp(const DrawTextOp* original, const DrawTextOp* written) {
 
 void CompareDrawTextBlobOp(const DrawTextBlobOp* original,
                            const DrawTextBlobOp* written) {
+  EXPECT_TRUE(original->IsValid());
+  EXPECT_TRUE(written->IsValid());
   CompareFlags(original->flags, written->flags);
   EXPECT_EQ(original->x, written->x);
   EXPECT_EQ(original->y, written->y);
@@ -1772,28 +1812,40 @@ void CompareDrawTextBlobOp(const DrawTextBlobOp* original,
 
 void CompareNoopOp(const NoopOp* original, const NoopOp* written) {
   // Nothing to compare.
+  EXPECT_TRUE(original->IsValid());
+  EXPECT_TRUE(written->IsValid());
 }
 
 void CompareRestoreOp(const RestoreOp* original, const RestoreOp* written) {
   // Nothing to compare.
+  EXPECT_TRUE(original->IsValid());
+  EXPECT_TRUE(written->IsValid());
 }
 
 void CompareRotateOp(const RotateOp* original, const RotateOp* written) {
+  EXPECT_TRUE(original->IsValid());
+  EXPECT_TRUE(written->IsValid());
   EXPECT_EQ(original->degrees, written->degrees);
 }
 
 void CompareSaveOp(const SaveOp* original, const SaveOp* written) {
   // Nothing to compare.
+  EXPECT_TRUE(original->IsValid());
+  EXPECT_TRUE(written->IsValid());
 }
 
 void CompareSaveLayerOp(const SaveLayerOp* original,
                         const SaveLayerOp* written) {
+  EXPECT_TRUE(original->IsValid());
+  EXPECT_TRUE(written->IsValid());
   CompareFlags(original->flags, written->flags);
   EXPECT_EQ(original->bounds, written->bounds);
 }
 
 void CompareSaveLayerAlphaOp(const SaveLayerAlphaOp* original,
                              const SaveLayerAlphaOp* written) {
+  EXPECT_TRUE(original->IsValid());
+  EXPECT_TRUE(written->IsValid());
   EXPECT_EQ(original->bounds, written->bounds);
   EXPECT_EQ(original->alpha, written->alpha);
   EXPECT_EQ(original->preserve_lcd_text_requests,
@@ -1801,23 +1853,35 @@ void CompareSaveLayerAlphaOp(const SaveLayerAlphaOp* original,
 }
 
 void CompareScaleOp(const ScaleOp* original, const ScaleOp* written) {
+  EXPECT_TRUE(original->IsValid());
+  EXPECT_TRUE(written->IsValid());
   EXPECT_EQ(original->sx, written->sx);
   EXPECT_EQ(original->sy, written->sy);
 }
 
 void CompareSetMatrixOp(const SetMatrixOp* original,
                         const SetMatrixOp* written) {
+  EXPECT_TRUE(original->IsValid());
+  EXPECT_TRUE(written->IsValid());
   EXPECT_EQ(original->matrix, written->matrix);
 }
 
 void CompareTranslateOp(const TranslateOp* original,
                         const TranslateOp* written) {
+  EXPECT_TRUE(original->IsValid());
+  EXPECT_TRUE(written->IsValid());
   EXPECT_EQ(original->dx, written->dx);
   EXPECT_EQ(original->dy, written->dy);
 }
 
 class PaintOpSerializationTest : public ::testing::TestWithParam<uint8_t> {
  public:
+  PaintOpSerializationTest() {
+    // Verify test data.
+    for (size_t i = 0; i < test_rrects.size(); ++i)
+      EXPECT_TRUE(test_rrects[i].isValid());
+  }
+
   PaintOpType GetParamType() const {
     return static_cast<PaintOpType>(GetParam());
   }

cc/paint/paint_op_reader.cc

@@ -7,7 +7,6 @@
 #include <stddef.h>
 
 #include "cc/paint/paint_flags.h"
-#include "cc/paint/paint_op_buffer.h"
 #include "third_party/skia/include/core/SkFlattenableSerialization.h"
 #include "third_party/skia/include/core/SkPath.h"
 #include "third_party/skia/include/core/SkRRect.h"
@@ -121,8 +120,6 @@ void PaintOpReader::Read(PaintFlags* flags) {
   Read(&flags->width_);
   Read(&flags->miter_limit_);
   ReadSimple(&flags->blend_mode_);
-  if (!PaintOp::IsValidPaintFlagsSkBlendMode(flags->getBlendMode()))
-    valid_ = false;
   ReadSimple(&flags->bitfields_uint_);
 
   // TODO(enne): ReadTypeface, http://crbug.com/737629