From a2243bc340384324c125ea6f0d7333f6ede74740 Mon Sep 17 00:00:00 2001
From: Yegor Jbanov <yjbanov@google.com>
Date: Mon, 9 Nov 2020 13:08:06 -0800
Subject: [PATCH] Simplify API for scheduling Skia object deletions

---
 .../src/engine/canvaskit/canvaskit_api.dart   | 44 +++++++++++--------
 .../engine/canvaskit/skia_object_cache.dart   |  3 +-
 2 files changed, 26 insertions(+), 21 deletions(-)

diff --git a/lib/web_ui/lib/src/engine/canvaskit/canvaskit_api.dart b/lib/web_ui/lib/src/engine/canvaskit/canvaskit_api.dart
index 09d67dbe25cbe..4d323b11846e9 100644
--- a/lib/web_ui/lib/src/engine/canvaskit/canvaskit_api.dart
+++ b/lib/web_ui/lib/src/engine/canvaskit/canvaskit_api.dart
@@ -1692,13 +1692,12 @@ List<SkDeletable> _skObjectDeleteQueue = <SkDeletable>[];
 
 final SkObjectFinalizationRegistry skObjectFinalizationRegistry =
     SkObjectFinalizationRegistry(js.allowInterop((SkDeletable deletable) {
-  _skObjectDeleteQueue.add(deletable);
-  _skObjectCollector ??= _scheduleSkObjectCollection();
+  _scheduleSkObjectCollection(deletable);
 }));
 
-/// Schedules an asap timer to delete garbage-collected Skia objects.
+/// Schedules a Skia object for deletion in an asap timer.
 ///
-/// We use a timer for the following reasons:
+/// A timer is used for the following reasons:
 ///
 ///  - Deleting the object immediately may lead to dangling pointer as the Skia
 ///    object may still be used by a function in the current frame. For example,
@@ -1709,21 +1708,28 @@ final SkObjectFinalizationRegistry skObjectFinalizationRegistry =
 ///  - A microtask, while solves the problem above, would prevent the event from
 ///    yielding to the graphics system to render the frame on the screen if there
 ///    is a large number of objects to delete, causing jank.
-Timer _scheduleSkObjectCollection() => Timer(Duration.zero, () {
-      html.window.performance.mark('SkObject collection-start');
-      final int length = _skObjectDeleteQueue.length;
-      for (int i = 0; i < length; i++) {
-        _skObjectDeleteQueue[i].delete();
-      }
-      _skObjectDeleteQueue = <SkDeletable>[];
-
-      // Null out the timer so we can schedule a new one next time objects are
-      // scheduled for deletion.
-      _skObjectCollector = null;
-      html.window.performance.mark('SkObject collection-end');
-      html.window.performance.measure('SkObject collection',
-          'SkObject collection-start', 'SkObject collection-end');
-    });
+///
+/// Because scheduling a timer is expensive, the timer is shared by all objects
+/// deleted this frame. No timer is created if no objects were scheduled for
+/// deletion.
+void _scheduleSkObjectCollection(SkDeletable deletable) {
+  _skObjectDeleteQueue.add(deletable);
+  _skObjectCollector ??= Timer(Duration.zero, () {
+    html.window.performance.mark('SkObject collection-start');
+    final int length = _skObjectDeleteQueue.length;
+    for (int i = 0; i < length; i++) {
+      _skObjectDeleteQueue[i].delete();
+    }
+    _skObjectDeleteQueue = <SkDeletable>[];
+
+    // Null out the timer so we can schedule a new one next time objects are
+    // scheduled for deletion.
+    _skObjectCollector = null;
+    html.window.performance.mark('SkObject collection-end');
+    html.window.performance.measure('SkObject collection',
+        'SkObject collection-start', 'SkObject collection-end');
+  });
+}
 
 /// Any Skia object that has a `delete` method.
 @JS()
diff --git a/lib/web_ui/lib/src/engine/canvaskit/skia_object_cache.dart b/lib/web_ui/lib/src/engine/canvaskit/skia_object_cache.dart
index 74f013ed0ebbb..828dec0ad5f34 100644
--- a/lib/web_ui/lib/src/engine/canvaskit/skia_object_cache.dart
+++ b/lib/web_ui/lib/src/engine/canvaskit/skia_object_cache.dart
@@ -316,8 +316,7 @@ class SkiaObjectBox {
     assert(removed);
     _isDeleted = true;
     if (_refs.isEmpty) {
-      _skObjectDeleteQueue.add(skObject);
-      _skObjectCollector ??= _scheduleSkObjectCollection();
+      _scheduleSkObjectCollection(skObject);
     }
   }
 }