Throw exceptions rather than crashing when Canvas API is misused. (#3811)

This attempts to make the Canvas API and some related features more
likely to throw a Dart exception than crash when exposed to bad input.

Depends on rolling tonic to
https://fuchsia-review.googlesource.com/c/35742/ which this patch does
not yet do, but I wanted to put it up for review to see if it was even
a reasonable approach.

lib/ui/painting/canvas.cc

@@ -17,6 +17,8 @@
 #include "third_party/skia/include/core/SkCanvas.h"
 #include "third_party/skia/include/core/SkRSXform.h"
 
+using tonic::ToDart;
+
 namespace blink {
 
 static void Canvas_constructor(Dart_NativeArguments args) {
@@ -70,8 +72,9 @@ ftl::RefPtr<Canvas> Canvas::Create(PictureRecorder* recorder,
                                    double top,
                                    double right,
                                    double bottom) {
-  FTL_DCHECK(recorder);
-  FTL_DCHECK(!recorder->isRecording());
+  if (!recorder)
+    Dart_ThrowException(ToDart("Canvas constructor called with non-genuine PictureRecorder."));
+  FTL_DCHECK(!recorder->isRecording()); // verified by Dart code
   ftl::RefPtr<Canvas> canvas = ftl::MakeRefCounted<Canvas>(
       recorder->BeginRecording(SkRect::MakeLTRB(left, top, right, bottom)));
   recorder->set_canvas(canvas);
@@ -164,6 +167,8 @@ void Canvas::clipRRect(const RRect& rrect) {
 void Canvas::clipPath(const CanvasPath* path) {
   if (!canvas_)
     return;
+  if (!path)
+    Dart_ThrowException(ToDart("Canvas.clipPath called with non-genuine Path."));
   canvas_->clipPath(path->path(), true);
 }
 
@@ -260,7 +265,8 @@ void Canvas::drawPath(const CanvasPath* path,
                       const PaintData& paint_data) {
   if (!canvas_)
     return;
-  FTL_DCHECK(path);
+  if (!path)
+    Dart_ThrowException(ToDart("Canvas.drawPath called with non-genuine Path."));
   canvas_->drawPath(path->path(), *paint.paint());
 }
 
@@ -271,7 +277,8 @@ void Canvas::drawImage(const CanvasImage* image,
                        const PaintData& paint_data) {
   if (!canvas_)
     return;
-  FTL_DCHECK(image);
+  if (!image)
+    Dart_ThrowException(ToDart("Canvas.drawImage called with non-genuine Image."));
   canvas_->drawImage(image->image(), x, y, paint.paint());
 }
 
@@ -288,7 +295,8 @@ void Canvas::drawImageRect(const CanvasImage* image,
                            const PaintData& paint_data) {
   if (!canvas_)
     return;
-  FTL_DCHECK(image);
+  if (!image)
+    Dart_ThrowException(ToDart("Canvas.drawImageRect called with non-genuine Image."));
   SkRect src = SkRect::MakeLTRB(src_left, src_top, src_right, src_bottom);
   SkRect dst = SkRect::MakeLTRB(dst_left, dst_top, dst_right, dst_bottom);
   canvas_->drawImageRect(image->image(), src, dst, paint.paint(),
@@ -308,7 +316,8 @@ void Canvas::drawImageNine(const CanvasImage* image,
                            const PaintData& paint_data) {
   if (!canvas_)
     return;
-  FTL_DCHECK(image);
+  if (!image)
+    Dart_ThrowException(ToDart("Canvas.drawImageNine called with non-genuine Image."));
   SkRect center =
       SkRect::MakeLTRB(center_left, center_top, center_right, center_bottom);
   SkIRect icenter;
@@ -320,7 +329,8 @@ void Canvas::drawImageNine(const CanvasImage* image,
 void Canvas::drawPicture(Picture* picture) {
   if (!canvas_)
     return;
-  FTL_DCHECK(picture);
+  if (!picture)
+    Dart_ThrowException(ToDart("Canvas.drawPicture called with non-genuine Picture."));
   canvas_->drawPicture(picture->picture().get());
 }
 
@@ -346,6 +356,8 @@ void Canvas::drawVertices(const Vertices* vertices,
                           const PaintData& paint_data) {
   if (!canvas_)
     return;
+  if (!vertices)
+    Dart_ThrowException(ToDart("Canvas.drawVertices called with non-genuine Vertices."));
 
   canvas_->drawVertices(vertices->vertices(),
                         blend_mode,
@@ -362,6 +374,8 @@ void Canvas::drawAtlas(const Paint& paint,
                        const tonic::Float32List& cull_rect) {
   if (!canvas_)
     return;
+  if (!atlas)
+    Dart_ThrowException(ToDart("Canvas.drawAtlas or Canvas.drawRawAtlas called with non-genuine Image."));
 
   sk_sp<SkImage> skImage = atlas->image();
 
@@ -383,6 +397,8 @@ void Canvas::drawShadow(const CanvasPath* path,
                         SkColor color,
                         double elevation,
                         bool transparentOccluder) {
+  if (!path)
+    Dart_ThrowException(ToDart("Canvas.drawShader called with non-genuine Path."));
   flow::PhysicalModelLayer::DrawShadow(canvas_,
                                        path->path(),
                                        color,

lib/ui/painting/image_shader.cc

@@ -9,6 +9,8 @@
 #include "lib/tonic/converter/dart_converter.h"
 #include "lib/tonic/dart_library_natives.h"
 
+using tonic::ToDart;
+
 namespace blink {
 
 static void ImageShader_constructor(Dart_NativeArguments args) {
@@ -35,7 +37,8 @@ void ImageShader::initWithImage(CanvasImage* image,
                                 SkShader::TileMode tmx,
                                 SkShader::TileMode tmy,
                                 const tonic::Float64List& matrix4) {
-  FTL_DCHECK(image != NULL);
+  if (!image)
+    Dart_ThrowException(ToDart("ImageShader constructor called with non-genuine Image."));
   SkMatrix sk_matrix = ToSkMatrix(matrix4);
   set_shader(image->image()->makeShader(tmx, tmy, &sk_matrix));
 }

lib/ui/painting/path.cc

@@ -12,6 +12,8 @@
 #include "lib/tonic/converter/dart_converter.h"
 #include "lib/tonic/dart_library_natives.h"
 
+using tonic::ToDart;
+
 namespace blink {
 
 typedef CanvasPath Path;
@@ -165,13 +167,15 @@ void CanvasPath::addRRect(const RRect& rrect) {
 }
 
 void CanvasPath::addPath(CanvasPath* path, double dx, double dy) {
-  if (path)
-    path_.addPath(path->path(), dx, dy, SkPath::kAppend_AddPathMode);
+  if (!path)
+    Dart_ThrowException(ToDart("Path.addPath called with non-genuine Path."));
+  path_.addPath(path->path(), dx, dy, SkPath::kAppend_AddPathMode);
 }
 
 void CanvasPath::extendWithPath(CanvasPath* path, double dx, double dy) {
-  if (path)
-    path_.addPath(path->path(), dx, dy, SkPath::kExtend_AddPathMode);
+  if (!path)
+    Dart_ThrowException(ToDart("Path.extendWithPath called with non-genuine Path."));
+  path_.addPath(path->path(), dx, dy, SkPath::kExtend_AddPathMode);
 }
 
 void CanvasPath::close() {

testing/dart/canvas_test.dart

+// Copyright 2015 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.
+
+import 'dart:ui';
+
+import 'package:test/test.dart';
+
+class FakeEverything implements Canvas, PictureRecorder, Color {
+  dynamic noSuchMethod(Invocation invocation) {
+    return new FakeEverything();
+  }
+}
+
+class NegativeSpace implements Canvas, PictureRecorder, Color {
+  dynamic noSuchMethod(Invocation invocation) {
+    return false;
+  }
+}
+
+void testCanvas(callback(Canvas canvas)) {
+  try {
+    callback(new Canvas(new PictureRecorder(), new Rect.fromLTRB(0.0, 0.0, 0.0, 0.0)));
+  } catch (error) { }
+}
+
+void main() {
+  test("canvas APIs should not crash", () {
+    dynamic fake = new FakeEverything();
+    dynamic no = new NegativeSpace();
+    Paint paint = new Paint();
+    Rect rect = new Rect.fromLTRB(double.NAN, double.NAN, double.NAN, double.NAN);
+    List<dynamic> list = <dynamic>[fake, fake];
+    Offset offset = new Offset(double.NAN, double.NAN);
+    Path path = new Path();
+
+    try { new Canvas(null, null); } catch (error) { }
+    try { new Canvas(null, rect); } catch (error) { }
+    try { new Canvas(null, fake); } catch (error) { }
+    try { new Canvas(fake, rect); } catch (error) { }
+    try { new Canvas(no, rect); } catch (error) { }
+
+    try {
+      new PictureRecorder()
+        ..endRecording()
+        ..endRecording()
+        ..endRecording();
+    } catch (error) { }
+
+    testCanvas((Canvas canvas) => canvas.clipPath(fake));
+    testCanvas((Canvas canvas) => canvas.clipRect(fake));
+    testCanvas((Canvas canvas) => canvas.clipRRect(fake));
+    testCanvas((Canvas canvas) => canvas.drawArc(fake, 0.0, 0.0, false, paint));
+    testCanvas((Canvas canvas) => canvas.drawArc(rect, 0.0, 0.0, false, fake));
+    testCanvas((Canvas canvas) => canvas.drawAtlas(fake, list, list, list, fake, rect, paint));
+    testCanvas((Canvas canvas) => canvas.drawCircle(offset, double.NAN, paint));
+    testCanvas((Canvas canvas) => canvas.drawColor(fake, fake));
+    testCanvas((Canvas canvas) => canvas.drawDRRect(fake, fake, fake));
+    testCanvas((Canvas canvas) => canvas.drawImage(fake, offset, paint));
+    testCanvas((Canvas canvas) => canvas.drawImageNine(fake, rect, rect, paint));
+    testCanvas((Canvas canvas) => canvas.drawImageRect(fake, rect, rect, paint));
+    testCanvas((Canvas canvas) => canvas.drawLine(offset, offset, paint));
+    testCanvas((Canvas canvas) => canvas.drawOval(rect, paint));
+    testCanvas((Canvas canvas) => canvas.drawPaint(paint));
+    testCanvas((Canvas canvas) => canvas.drawPaint(fake));
+    testCanvas((Canvas canvas) => canvas.drawPaint(no));
+    testCanvas((Canvas canvas) => canvas.drawParagraph(fake, offset));
+    testCanvas((Canvas canvas) => canvas.drawPath(fake, paint));
+    testCanvas((Canvas canvas) => canvas.drawPicture(fake));
+    testCanvas((Canvas canvas) => canvas.drawPoints(fake, list, fake));
+    testCanvas((Canvas canvas) => canvas.drawRawAtlas(fake, fake, fake, fake, fake, fake, fake));
+    testCanvas((Canvas canvas) => canvas.drawRawPoints(fake, list, paint));
+    testCanvas((Canvas canvas) => canvas.drawRect(rect, paint));
+    testCanvas((Canvas canvas) => canvas.drawRRect(fake, paint));
+    testCanvas((Canvas canvas) => canvas.drawShadow(path, color, double.NAN, null));
+    testCanvas((Canvas canvas) => canvas.drawShadow(path, color, double.NAN, false));
+    testCanvas((Canvas canvas) => canvas.drawShadow(path, color, double.NAN, true));
+    testCanvas((Canvas canvas) => canvas.drawShadow(path, color, double.NAN, no));
+    testCanvas((Canvas canvas) => canvas.drawShadow(path, color, double.NAN, fake));
+    testCanvas((Canvas canvas) => canvas.drawVertices(fake, null, paint));
+    testCanvas((Canvas canvas) => canvas.getSaveCount());
+    testCanvas((Canvas canvas) => canvas.restore());
+    testCanvas((Canvas canvas) => canvas.rotate(double.NAN));
+    testCanvas((Canvas canvas) => canvas.save());
+    testCanvas((Canvas canvas) => canvas.saveLayer(rect, paint));
+    testCanvas((Canvas canvas) => canvas.saveLayer(fake, fake));
+    testCanvas((Canvas canvas) => canvas.saveLayer(null, null));
+    testCanvas((Canvas canvas) => canvas.scale(double.NAN, double.NAN));
+    testCanvas((Canvas canvas) => canvas.skew(double.NAN, double.NAN));
+    testCanvas((Canvas canvas) => canvas.transform(fake));
+    testCanvas((Canvas canvas) => canvas.transform(no));
+    testCanvas((Canvas canvas) => canvas.transform(null));
+    testCanvas((Canvas canvas) => canvas.translate(double.NAN, double.NAN));
+  });
+}

testing/run_tests.sh

@@ -15,4 +15,5 @@ popd
 
 for TEST_SCRIPT in flutter/testing/dart/*.dart; do
     out/host_debug_unopt/flutter_tester --disable-observatory --disable-diagnostic --non-interactive --enable-checked-mode $TEST_SCRIPT
+    out/host_debug_unopt/flutter_tester --disable-observatory --disable-diagnostic --non-interactive $TEST_SCRIPT
 done