refactor: use breadth-first search in widget-filter (#2481)

getsentry · Dec 11, 2024 · 04211b9 · 04211b9
1 parent 20a5bce
commit 04211b9
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 11 deletions.
diff --git a/flutter/lib/src/screenshot/widget_filter.dart b/flutter/lib/src/screenshot/widget_filter.dart
@@ -14,6 +14,7 @@ class WidgetFilter {
   late WidgetFilterColorScheme _scheme;
   late double _pixelRatio;
   late Rect _bounds;
+  late List<Element> _visitList;
   final _warnedWidgets = <int>{};
 
   /// Used to test _obscureElementOrParent
@@ -34,11 +35,24 @@ class WidgetFilter {
     assert(colorScheme.background.isOpaque);
     assert(colorScheme.defaultMask.isOpaque);
     assert(colorScheme.defaultTextMask.isOpaque);
+
+    // clear the output list
     items.clear();
-    if (context is Element) {
-      _process(context);
-    } else {
-      context.visitChildElements(_process);
+
+    // Reset the list of elements we're going to process.
+    // Then do a breadth-first tree traversal on all the widgets.
+    // TODO benchmark performance compared to to DoubleLinkedQueue.
+    _visitList = [];
+    context.visitChildElements(_visitList.add);
+    while (_visitList.isNotEmpty) {
+      // Get a handle on the items we're supposed to process in this step.
+      // Then _visitList (which is updated in _process()) with a new instance.
+      final currentList = _visitList;
+      _visitList = [];
+
+      for (final element in currentList) {
+        _process(element);
+      }
     }
   }
 
@@ -66,7 +80,7 @@ class WidgetFilter {
         break;
       case SentryMaskingDecision.continueProcessing:
         // If this element should not be obscured, visit and check its children.
-        element.visitChildElements(_process);
+        element.visitChildElements(_visitList.add);
         break;
     }
   }

diff --git a/flutter/test/screenshot/widget_filter_test.dart b/flutter/test/screenshot/widget_filter_test.dart
@@ -81,12 +81,16 @@ void main() async {
           bounds: defaultBounds,
           colorScheme: colorScheme);
       expect(sut.items.length, 6);
-      expect(boundsRect(sut.items[0]), '624x48');
-      expect(boundsRect(sut.items[1]), '169x20');
-      expect(boundsRect(sut.items[2]), '800x192');
-      expect(boundsRect(sut.items[3]), '800x24');
-      expect(boundsRect(sut.items[4]), '800x24');
-      expect(boundsRect(sut.items[5]), '50x20');
+      expect(
+          sut.items.map(boundsRect),
+          unorderedEquals([
+            '624x48',
+            '169x20',
+            '800x192',
+            '800x24',
+            '800x24',
+            '50x20',
+          ]));
     });
   });