detect cases when Skia nops filters by returning nullptr to prevent crashes (flutter/engine#31689)

engine/src/flutter/display_list/display_list_color_filter.h

@@ -86,8 +86,9 @@ class DlBlendColorFilter final : public DlColorFilter {
   size_t size() const override { return sizeof(*this); }
   bool modifies_transparent_black() const override {
     // Look at blend and color to make a faster determination?
-    return skia_object()->filterColor(SK_ColorTRANSPARENT) !=
-           SK_ColorTRANSPARENT;
+    sk_sp<SkColorFilter> sk_filter = skia_object();
+    return sk_filter &&
+           sk_filter->filterColor(SK_ColorTRANSPARENT) != SK_ColorTRANSPARENT;
   }
 
   std::shared_ptr<DlColorFilter> shared() const override {
@@ -142,8 +143,9 @@ class DlMatrixColorFilter final : public DlColorFilter {
   bool modifies_transparent_black() const override {
     // Look at the matrix to make a faster determination?
     // Basically, are the translation components all 0?
-    return skia_object()->filterColor(SK_ColorTRANSPARENT) !=
-           SK_ColorTRANSPARENT;
+    sk_sp<SkColorFilter> sk_filter = skia_object();
+    return sk_filter &&
+           sk_filter->filterColor(SK_ColorTRANSPARENT) != SK_ColorTRANSPARENT;
   }
 
   std::shared_ptr<DlColorFilter> shared() const override {

engine/src/flutter/display_list/display_list_color_filter_unittests.cc

@@ -124,6 +124,11 @@ TEST(DisplayListColorFilter, BlendNotEquals) {
   ASSERT_NE(filter3, filter1);
 }
 
+TEST(DisplayListColorFilter, NopBlendShouldNotCrash) {
+  DlBlendColorFilter filter(SK_ColorTRANSPARENT, SkBlendMode::kSrcOver);
+  ASSERT_FALSE(filter.modifies_transparent_black());
+}
+
 TEST(DisplayListColorFilter, MatrixConstructor) {
   DlMatrixColorFilter filter(matrix);
 }
@@ -178,6 +183,17 @@ TEST(DisplayListColorFilter, MatrixNotEquals) {
   ASSERT_NE(filter1, filter2);
 }
 
+TEST(DisplayListColorFilter, NopMatrixShouldNotCrash) {
+  float matrix[20] = {
+      1, 0, 0, 0, 0,  //
+      0, 1, 0, 0, 0,  //
+      0, 0, 1, 0, 0,  //
+      0, 0, 0, 1, 0,  //
+  };
+  DlMatrixColorFilter filter(matrix);
+  ASSERT_FALSE(filter.modifies_transparent_black());
+}
+
 TEST(DisplayListColorFilter, SrgbToLinearConstructor) {
   DlSrgbToLinearGammaColorFilter filter;
 }

engine/src/flutter/lib/ui/painting/color_filter.h

@@ -36,6 +36,9 @@ class ColorFilter : public RefCountedDartWrappable<ColorFilter> {
   ~ColorFilter() override;
 
   std::shared_ptr<const DlColorFilter> filter() const { return filter_; }
+  const DlColorFilter* dl_filter() const {
+    return (filter_ && filter_->skia_object()) ? filter_.get() : nullptr;
+  }
 
   static void RegisterNatives(tonic::DartLibraryNatives* natives);
 

engine/src/flutter/lib/ui/painting/paint.cc

@@ -234,7 +234,7 @@ bool Paint::sync_to(DisplayListBuilder* builder,
       } else {
         ColorFilter* decoded_color_filter =
             tonic::DartConverter<ColorFilter*>::FromDart(color_filter);
-        builder->setColorFilter(decoded_color_filter->filter().get());
+        builder->setColorFilter(decoded_color_filter->dl_filter());
       }
     }
 
@@ -302,7 +302,11 @@ bool Paint::sync_to(DisplayListBuilder* builder,
             static_cast<SkBlurStyle>(uint_data[kMaskFilterBlurStyleIndex]);
         double sigma = float_data[kMaskFilterSigmaIndex];
         DlBlurMaskFilter dl_filter(blur_style, sigma);
-        builder->setMaskFilter(&dl_filter);
+        if (dl_filter.skia_object()) {
+          builder->setMaskFilter(&dl_filter);
+        } else {
+          builder->setMaskFilter(nullptr);
+        }
         break;
     }
   }

engine/src/flutter/testing/dart/BUILD.gn

@@ -28,6 +28,7 @@ tests = [
   "isolate_test.dart",
   "lerp_test.dart",
   "locale_test.dart",
+  "mask_filter_test.dart",
   "paragraph_builder_test.dart",
   "paragraph_test.dart",
   "path_test.dart",

engine/src/flutter/testing/dart/color_filter_test.dart

@@ -7,6 +7,7 @@ import 'dart:ui';
 
 import 'package:litetest/litetest.dart';
 
+const Color transparent = Color(0x00000000);
 const Color red = Color(0xFFAA0000);
 const Color green = Color(0xFF00AA00);
 
@@ -27,6 +28,12 @@ const List<double> greyscaleColorMatrix = <double>[
   0.2126, 0.7152, 0.0722, 0, 0, //
   0,      0,      0,      1, 0, //
 ];
+const List<double> identityColorMatrix = <double>[
+  1, 0, 0, 0, 0,
+  0, 1, 0, 0, 0,
+  0, 0, 1, 0, 0,
+  0, 0, 0, 1, 0,
+];
 
 void main() {
   Future<Uint32List> getBytesForPaint(Paint paint, {int width = 1, int height = 1}) async {
@@ -54,6 +61,25 @@ void main() {
     expect(bytes[0], greenRedColorBlendInverted);
   });
 
+  test('ColorFilter - NOP mode does not crash', () async {
+    final PictureRecorder recorder = PictureRecorder();
+    final Canvas canvas = Canvas(recorder);
+    final Paint paint = Paint()
+      ..color = green
+      ..colorFilter = const ColorFilter.mode(transparent, BlendMode.srcOver);
+    canvas.saveLayer(const Rect.fromLTRB(-100, -100, 200, 200), paint);
+    canvas.drawRect(const Rect.fromLTRB(0, 0, 100, 100), Paint());
+    canvas.restore();
+    final Picture picture = recorder.endRecording();
+
+    final SceneBuilder builder = SceneBuilder();
+    builder.addPicture(Offset.zero, picture);
+
+    final Scene scene = builder.build();
+    expect(scene != null, true);
+    await scene.toImage(100, 100);
+  });
+
   test('ColorFilter - matrix', () async {
     final Paint paint = Paint()
       ..color = green
@@ -67,6 +93,25 @@ void main() {
     expect(bytes[0], greenInvertedGreyscaled);
   });
 
+  test('ColorFilter - NOP matrix does not crash', () async {
+    final PictureRecorder recorder = PictureRecorder();
+    final Canvas canvas = Canvas(recorder);
+    final Paint paint = Paint()
+      ..color = const Color(0xff00AA00)
+      ..colorFilter = const ColorFilter.matrix(identityColorMatrix);
+    canvas.saveLayer(const Rect.fromLTRB(-100, -100, 200, 200), paint);
+    canvas.drawRect(const Rect.fromLTRB(0, 0, 100, 100), Paint());
+    canvas.restore();
+    final Picture picture = recorder.endRecording();
+
+    final SceneBuilder builder = SceneBuilder();
+    builder.addPicture(Offset.zero, picture);
+
+    final Scene scene = builder.build();
+    expect(scene != null, true);
+    await scene.toImage(100, 100);
+  });
+
   test('ColorFilter - linearToSrgbGamma', () async {
     final Paint paint = Paint()
       ..color = green

engine/src/flutter/testing/dart/mask_filter_test.dart

@@ -0,0 +1,28 @@
+// Copyright 2013 The Flutter 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:litetest/litetest.dart';
+
+void main() {
+  test('MaskFilter - NOP blur does not crash', () async {
+    final PictureRecorder recorder = PictureRecorder();
+    final Canvas canvas = Canvas(recorder);
+    final Paint paint = Paint()
+      ..color = const Color(0xff00AA00)
+      ..maskFilter = const MaskFilter.blur(BlurStyle.normal, 0);
+    canvas.saveLayer(const Rect.fromLTRB(-100, -100, 200, 200), paint);
+    canvas.drawRect(const Rect.fromLTRB(0, 0, 100, 100), Paint());
+    canvas.restore();
+    final Picture picture = recorder.endRecording();
+
+    final SceneBuilder builder = SceneBuilder();
+    builder.addPicture(Offset.zero, picture);
+
+    final Scene scene = builder.build();
+    expect(scene != null, true);
+    await scene.toImage(100, 100);
+  });
+}