Moves all argument checking of Vertices() and Vertices.raw() to asserts, Adds arg checking to all web renderers -Fixes #123085

AUTHORS

@@ -24,4 +24,5 @@ TheOneWithTheBraid <the-one@with-the-braid.cf>
 Twin Sun, LLC <google-contrib@twinsunsolutions.com>
 Qixing Cao <qixing.cao.83@gmail.com>
 LinXunFeng <linxunfeng@yeah.net>
-Amir Panahandeh <amirpanahandeh@gmail.com>
+Amir Panahandeh <amirpanahandeh@gmail.com>
+Tim Maffett <timmaffett@gmail.com>

lib/ui/painting.dart

@@ -4737,24 +4737,9 @@ base class Vertices extends NativeFieldWrapperClass1 {
     List<Color>? colors,
     List<Offset>? textureCoordinates,
     List<int>? indices,
-  }) {
-    if (colors != null && colors.length != positions.length) {
-      throw ArgumentError('"positions" and "colors" lengths must match.');
-    }
-    if (textureCoordinates != null && textureCoordinates.length != positions.length) {
-      throw ArgumentError('"positions" and "textureCoordinates" lengths must match.');
-    }
-    if (indices != null) {
-      for (int index = 0; index < indices.length; index += 1) {
-        if (indices[index] >= positions.length) {
-          throw ArgumentError(
-            '"indices" values must be valid indices in the positions list '
-            '(i.e. numbers in the range 0..${positions.length - 1}), '
-            'but indices[$index] is ${indices[index]}, which is too big.',
-          );
-        }
-      }
-    }
+  }) : assert(textureCoordinates == null || textureCoordinates.length == positions.length,'"positions" and "textureCoordinates" lengths must match.'),
+    assert(colors == null || colors.length == positions.length,'"positions" and "colors" lengths must match.'),
+    assert(indices == null || indices.every((int i) => i >= 0 && i < positions.length),'"indices" values must be valid indices in the positions list.') {
     final Float32List encodedPositions = _encodePointList(positions);
     final Float32List? encodedTextureCoordinates = (textureCoordinates != null)
       ? _encodePointList(textureCoordinates)
@@ -4821,27 +4806,10 @@ base class Vertices extends NativeFieldWrapperClass1 {
     Int32List? colors,
     Float32List? textureCoordinates,
     Uint16List? indices,
-  }) {
-    if (positions.length % 2 != 0) {
-      throw ArgumentError('"positions" must have an even number of entries (each coordinate is an x,y pair).');
-    }
-    if (colors != null && colors.length * 2 != positions.length) {
-      throw ArgumentError('"positions" and "colors" lengths must match.');
-    }
-    if (textureCoordinates != null && textureCoordinates.length != positions.length) {
-      throw ArgumentError('"positions" and "textureCoordinates" lengths must match.');
-    }
-    if (indices != null) {
-      for (int index = 0; index < indices.length; index += 1) {
-        if (indices[index] * 2 >= positions.length) {
-          throw ArgumentError(
-            '"indices" values must be valid indices in the positions list '
-            '(i.e. numbers in the range 0..${positions.length ~/ 2 - 1}), '
-            'but indices[$index] is ${indices[index]}, which is too big.',
-          );
-        }
-      }
-    }
+  }) : assert(positions.length.isEven,'"positions" must have an even number of entries (each coordinate is an x,y pair).'),
+    assert(textureCoordinates == null || textureCoordinates.length == positions.length,'"positions" and "textureCoordinates" lengths must match.'),
+    assert(colors == null || colors.length * 2 == positions.length,'"colors" length must be half the length of "positions".'),
+    assert(indices == null || indices.every((int i) => i >= 0 && i*2 < positions.length),'"indices" values must be valid indices in the positions list.') {
     if (!_init(this, mode.index, positions, textureCoordinates, colors, indices)) {
       throw ArgumentError('Invalid configuration for vertices.');
     }

lib/web_ui/lib/src/engine/canvaskit/vertices.dart

@@ -17,20 +17,9 @@ class CkVertices implements ui.Vertices {
     List<ui.Color>? colors,
     List<int>? indices,
   }) {
-    if (textureCoordinates != null &&
-        textureCoordinates.length != positions.length) {
-      throw ArgumentError(
-          '"positions" and "textureCoordinates" lengths must match.');
-    }
-    if (colors != null && colors.length != positions.length) {
-      throw ArgumentError('"positions" and "colors" lengths must match.');
-    }
-    if (indices != null &&
-        indices.any((int i) => i < 0 || i >= positions.length)) {
-      throw ArgumentError(
-          '"indices" values must be valid indices in the positions list.');
-    }
-
+    assert(textureCoordinates == null || textureCoordinates.length == positions.length,'"positions" and "textureCoordinates" lengths must match.');
+    assert(colors == null || colors.length == positions.length,'"positions" and "colors" lengths must match.');
+    assert(indices == null || indices.every((int i) => i >= 0 && i < positions.length),'"indices" values must be valid indices in the positions list.');
     return CkVertices._(
       toSkVertexMode(mode),
       toFlatSkPoints(positions),
@@ -47,20 +36,10 @@ class CkVertices implements ui.Vertices {
     Int32List? colors,
     Uint16List? indices,
   }) {
-    if (textureCoordinates != null &&
-        textureCoordinates.length != positions.length) {
-      throw ArgumentError(
-          '"positions" and "textureCoordinates" lengths must match.');
-    }
-    if (colors != null && colors.length * 2 != positions.length) {
-      throw ArgumentError('"positions" and "colors" lengths must match.');
-    }
-    if (indices != null &&
-        indices.any((int i) => i < 0 || i >= positions.length)) {
-      throw ArgumentError(
-          '"indices" values must be valid indices in the positions list.');
-    }
-
+    assert(positions.length.isEven,'"positions" must have an even number of entries (each coordinate is an x,y pair).');
+    assert(textureCoordinates == null || textureCoordinates.length == positions.length,'"positions" and "textureCoordinates" lengths must match.');
+    assert(colors == null || colors.length * 2 == positions.length,'"colors" length must be half the length of "positions".');
+    assert(indices == null || indices.every((int i) => i >= 0 && i*2 < positions.length),'"indices" values must be valid indices in the positions list.');
     Uint32List? unsignedColors;
     if (colors != null) {
       unsignedColors = colors.buffer.asUint32List(colors.offsetInBytes, colors.length);

lib/web_ui/lib/src/engine/html/render_vertices.dart

@@ -26,7 +26,9 @@ class SurfaceVertices implements ui.Vertices {
     List<ui.Offset> positions, {
     List<ui.Color>? colors,
     List<int>? indices,
-  })  : colors = colors != null ? _int32ListFromColors(colors) : null,
+  })  : assert(colors == null || colors.length == positions.length,'"positions" and "colors" lengths must match.'),
+        assert(indices == null || indices.every((int i) => i >= 0 && i < positions.length),'"indices" values must be valid indices in the positions list.'),
+        colors = colors != null ? _int32ListFromColors(colors) : null,
         indices = indices != null ? Uint16List.fromList(indices) : null,
         positions = offsetListToFloat32List(positions) {
     initWebGl();
@@ -37,7 +39,9 @@ class SurfaceVertices implements ui.Vertices {
     this.positions, {
     this.colors,
     this.indices,
-  }) {
+  }) :  assert(positions.length.isEven,'"positions" must have an even number of entries (each coordinate is an x,y pair).'),
+        assert(colors == null || colors.length * 2 == positions.length,'"colors" length must be half the length of "positions".'),
+        assert(indices == null || indices.every((int i) => i >= 0 && i*2 < positions.length),'"indices" values must be valid indices in the positions list.') {
     initWebGl();
   }
 

lib/web_ui/lib/src/engine/skwasm/skwasm_impl/vertices.dart

@@ -18,6 +18,9 @@ class SkwasmVertices extends SkwasmObjectWrapper<RawVertices> implements ui.Vert
     List<ui.Color>? colors,
     List<int>? indices,
   }) => withStackScope((StackScope scope) {
+    assert(textureCoordinates == null || textureCoordinates.length == positions.length,'"positions" and "textureCoordinates" lengths must match.');
+    assert(colors == null || colors.length == positions.length,'"positions" and "colors" lengths must match.');
+    assert(indices == null || indices.every((int i) => i >= 0 && i < positions.length),'"indices" values must be valid indices in the positions list.');
     final RawPointArray rawPositions = scope.convertPointArrayToNative(positions);
     final RawPointArray rawTextureCoordinates = textureCoordinates != null
       ? scope.convertPointArrayToNative(textureCoordinates)
@@ -47,6 +50,10 @@ class SkwasmVertices extends SkwasmObjectWrapper<RawVertices> implements ui.Vert
     Int32List? colors,
     Uint16List? indices,
   }) => withStackScope((StackScope scope) {
+    assert(positions.length.isEven,'"positions" must have an even number of entries (each coordinate is an x,y pair).');
+    assert(textureCoordinates == null || textureCoordinates.length == positions.length,'"positions" and "textureCoordinates" lengths must match.');
+    assert(colors == null || colors.length * 2 == positions.length,'"colors" length must be half the length of "positions".');
+    assert(indices == null || indices.every((int i) => i >= 0 && i*2 < positions.length),'"indices" values must be valid indices in the positions list.');
     final RawPointArray rawPositions = scope.convertDoublesToNative(positions);
     final RawPointArray rawTextureCoordinates = textureCoordinates != null
       ? scope.convertDoublesToNative(textureCoordinates)

lib/web_ui/test/ui/vertices_test.dart

@@ -6,6 +6,7 @@ import 'dart:typed_data';
 
 import 'package:test/bootstrap/browser.dart';
 import 'package:test/test.dart';
+import 'package:ui/src/engine.dart' show renderer;
 import 'package:ui/ui.dart' as ui;
 import 'package:web_engine_tester/golden_tester.dart';
 
@@ -17,6 +18,12 @@ void main() {
 }
 
 void testMain() {
+  bool assertsEnabled = false;
+  assert(() {
+    assertsEnabled = true;
+    return true;
+  }());
+
   group('Vertices', () {
     setUpUnitTests(setUpTestViewDimensions: false);
 
@@ -58,6 +65,148 @@ void testMain() {
     await drawPictureUsingCurrentRenderer(recorder.endRecording());
     await matchGoldenFile('ui_vertices_antialiased.png', region: region);
   }, skip: isHtml); // https://github.com/flutter/flutter/issues/127454
+
+  test('Vertices assert checks', () {
+    // We don't test textureCoordinate assert checks on html render because HTML renderer's SurfaceVertices() does not support textureCoordinates
+    if (renderer.rendererTag != 'html') {
+      try {
+        final ui.Vertices invalidVertices = ui.Vertices(
+          ui.VertexMode.triangles,
+          const <ui.Offset>[ui.Offset.zero, ui.Offset.zero, ui.Offset.zero],
+          textureCoordinates: const <ui.Offset>[ui.Offset.zero],
+        );
+        if (assertsEnabled) {
+          throw AssertionError('Vertices did not throw the expected assert error.');
+        } else {
+          // Assertions are NOT ENABLED, so we will attempt to use this invalid ui.Vertices object
+          // we just created to check if that causes exceptions/problems.
+          useAndDisposeOfInvalidVerticesObject(invalidVertices);
+        }
+      } on AssertionError catch (e) {
+        expect('$e', contains(r'\"positions\" and \"textureCoordinates\" lengths must match.'));
+      }
+    }
+    try {
+      final ui.Vertices invalidVertices = ui.Vertices(
+        ui.VertexMode.triangles,
+        const <ui.Offset>[ui.Offset.zero, ui.Offset.zero, ui.Offset.zero],
+        colors: const <ui.Color>[ui.Color.fromRGBO(255, 0, 0, 1.0)],
+      );
+      if (assertsEnabled) {
+        throw AssertionError('Vertices did not throw the expected assert error.');
+      } else {
+        // Assertions are NOT ENABLED, so we will attempt to use this invalid ui.Vertices object
+        // we just created to check if that causes exceptions/problems.
+        useAndDisposeOfInvalidVerticesObject(invalidVertices);
+      }
+    } on AssertionError catch (e) {
+      expect('$e', contains(r'\"positions\" and \"colors\" lengths must match.'));
+    }
+    try {
+      final ui.Vertices invalidVertices = ui.Vertices(
+        ui.VertexMode.triangles,
+        const <ui.Offset>[ui.Offset.zero, ui.Offset.zero, ui.Offset.zero],
+        indices: Uint16List.fromList(const <int>[0, 2, 5]),
+      );
+      if (assertsEnabled) {
+        throw AssertionError('Vertices did not throw the expected assert error.');
+      } else {
+        // Assertions are NOT ENABLED, so we will attempt to use this invalid ui.Vertices object
+        // we just created to check if that causes exceptions/problems.
+        useAndDisposeOfInvalidVerticesObject(invalidVertices);
+      }
+    } on AssertionError catch (e) {
+      expect('$e', contains(r'\"indices\" values must be valid indices in the positions list.'));
+    }
+    ui.Vertices( // This one does not throw.
+      ui.VertexMode.triangles,
+      const <ui.Offset>[ui.Offset.zero],
+    ).dispose();
+    ui.Vertices( // This one should not throw.
+      ui.VertexMode.triangles,
+      const <ui.Offset>[ui.Offset.zero, ui.Offset.zero, ui.Offset.zero],
+      indices: Uint16List.fromList(const <int>[0, 2, 1, 2, 0, 1, 2, 0]), // Uint16List implements List<int> so this is ok.
+    ).dispose();
+  });
+
+  test('Vertices.raw assert checks', () {
+    try {
+      final ui.Vertices invalidVertices = ui.Vertices.raw(
+        ui.VertexMode.triangles,
+        Float32List.fromList(const <double>[0.0]),
+      );
+      if (assertsEnabled) {
+        throw AssertionError('Vertices.raw did not throw the expected assert error.');
+      } else {
+        // Assertions are NOT ENABLED, so we will attempt to use this invalid ui.Vertices object
+        // we just created to check if that causes exceptions/problems.
+        useAndDisposeOfInvalidVerticesObject(invalidVertices);
+      }
+    } on AssertionError catch (e) {
+      expect('$e', contains(r'\"positions\" must have an even number of entries (each coordinate is an x,y pair).'));
+    }
+    // We don't test textureCoordinate assert checks on html render because HTML renderer's SurfaceVertices() does not support textureCoordinates
+    if (renderer.rendererTag != 'html') {
+      try {
+        final ui.Vertices invalidVertices = ui.Vertices.raw(
+          ui.VertexMode.triangles,
+          Float32List.fromList(const <double>[0.0, 0.0, 0.0, 0.0, 0.0, 0.0]),
+          textureCoordinates: Float32List.fromList(const <double>[0.0, 0.0]),
+        );
+        if (assertsEnabled) {
+          throw AssertionError('Vertices did not throw the expected assert error.');
+        } else {
+          // Assertions are NOT ENABLED, so we will attempt to use this invalid ui.Vertices object
+          // we just created to check if that causes exceptions/problems.
+          useAndDisposeOfInvalidVerticesObject(invalidVertices);
+        }
+      } on AssertionError catch (e) {
+        expect('$e', contains(r'\"positions\" and \"textureCoordinates\" lengths must match.'));
+      }
+    }
+    try {
+      final ui.Vertices invalidVertices = ui.Vertices.raw(
+        ui.VertexMode.triangles,
+        Float32List.fromList(const <double>[0.0, 0.0, 0.0, 0.0, 0.0, 0.0]),
+        colors: Int32List.fromList(const <int>[0xffff0000]),
+      );
+      if (assertsEnabled) {
+        throw AssertionError('Vertices did not throw the expected assert error.');
+      } else {
+        // Assertions are NOT ENABLED, so we will attempt to use this invalid ui.Vertices object
+        // we just created to check if that causes exceptions/problems.
+        useAndDisposeOfInvalidVerticesObject(invalidVertices);
+      }
+    } on AssertionError catch (e) {
+      expect('$e', contains(r'\"colors\" length must be half the length of \"positions\".'));
+    }
+    try {
+      final ui.Vertices invalidVertices = ui.Vertices.raw(
+        ui.VertexMode.triangles,
+        Float32List.fromList(const <double>[0.0, 0.0, 0.0, 0.0, 0.0, 0.0]),
+        indices: Uint16List.fromList(const <int>[0, 2, 5]),
+      );
+      if (assertsEnabled) {
+        throw AssertionError('Vertices.raw did not throw the expected assert error.');
+      } else {
+        // Assertions are NOT ENABLED, so we will attempt to use this invalid ui.Vertices object
+        // we just created to check if that causes exceptions/problems.
+        useAndDisposeOfInvalidVerticesObject(invalidVertices);
+      }
+    } on AssertionError catch (e) {
+      expect('$e', contains(r'\"indices\" values must be valid indices in the positions list.'));
+    }
+    ui.Vertices.raw( // This one does not throw.
+      ui.VertexMode.triangles,
+      Float32List.fromList(const <double>[0.0, 0.0]),
+    ).dispose();
+    ui.Vertices.raw( // This one should not throw.
+      ui.VertexMode.triangles,
+      Float32List.fromList(const <double>[0.0, 0.0, 0.0, 0.0, 0.0, 0.0]),
+      indices: Uint16List.fromList(const <int>[0, 2, 1, 2, 0, 1, 2, 0]),
+    ).dispose();
+  });
+
 }
 
 ui.Vertices _testVertices() {
@@ -266,3 +415,23 @@ const List<int> _circularVertexIndices = <int>[
   34,
   36
 ];
+
+// This function is used when asserts are NOT enabled and vertices() allows invalid
+// vertices objects to be created.  Here we attempt to use these invalid ui.Vertices
+// objects to determine if they cause problems within drawVertices()
+void useAndDisposeOfInvalidVerticesObject( ui.Vertices vertices ) {
+  expect(vertices.debugDisposed, isFalse);
+
+  final ui.PictureRecorder recorder = ui.PictureRecorder();
+  final ui.Canvas canvas = ui.Canvas(
+    recorder,
+    const ui.Rect.fromLTRB(0, 0, 100, 100)
+  );
+  canvas.drawVertices(
+    vertices,
+    ui.BlendMode.srcOver,
+    ui.Paint(),
+  );
+  vertices.dispose();
+  expect(vertices.debugDisposed, isTrue);
+}