Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Commit 03609b4

Browse files
authored
[web] Fix canvas2d leaks in text measurement (#38640)
* [web] Fix canvas2d leaks in text measurement * add a test * set context.font correctly * change the test to workaround weird safari behavior
1 parent cda410c commit 03609b4

File tree

3 files changed

+98
-18
lines changed

3 files changed

+98
-18
lines changed

lib/web_ui/lib/src/engine/dom.dart

+11
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import 'dart:typed_data';
88

99
import 'package:js/js.dart';
1010
import 'package:js/js_util.dart' as js_util;
11+
import 'package:meta/meta.dart';
1112

1213
/// This file contains static interop classes for interacting with the DOM and
1314
/// some helpers. All of the classes in this file are named after their
@@ -584,7 +585,16 @@ class DomPerformanceMeasure extends DomPerformanceEntry {}
584585
@staticInterop
585586
class DomCanvasElement extends DomHTMLElement {}
586587

588+
@visibleForTesting
589+
int debugCanvasCount = 0;
590+
591+
@visibleForTesting
592+
void debugResetCanvasCount() {
593+
debugCanvasCount = 0;
594+
}
595+
587596
DomCanvasElement createDomCanvasElement({int? width, int? height}) {
597+
debugCanvasCount++;
588598
final DomCanvasElement canvas =
589599
domWindow.document.createElement('canvas') as DomCanvasElement;
590600
if (width != null) {
@@ -625,6 +635,7 @@ abstract class DomCanvasImageSource {}
625635
class DomCanvasRenderingContext2D {}
626636

627637
extension DomCanvasRenderingContext2DExtension on DomCanvasRenderingContext2D {
638+
external DomCanvasElement? get canvas;
628639
external Object? get fillStyle;
629640
external set fillStyle(Object? style);
630641
external String get font;

lib/web_ui/lib/src/engine/text/layout_service.dart

+27-18
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,16 @@ import 'paragraph.dart';
1616
import 'ruler.dart';
1717
import 'text_direction.dart';
1818

19+
/// A single canvas2d context to use for all text measurements.
20+
@visibleForTesting
21+
final DomCanvasRenderingContext2D textContext =
22+
// We don't use this canvas to draw anything, so let's make it as small as
23+
// possible to save memory.
24+
createDomCanvasElement(width: 0, height: 0).context2D;
25+
26+
/// The last font used in the [textContext].
27+
String? _lastContextFont;
28+
1929
/// Performs layout on a [CanvasParagraph].
2030
///
2131
/// It uses a [DomCanvasElement] to measure text.
@@ -24,9 +34,6 @@ class TextLayoutService {
2434

2535
final CanvasParagraph paragraph;
2636

27-
final DomCanvasRenderingContext2D context =
28-
createDomCanvasElement().context2D;
29-
3037
// *** Results of layout *** //
3138

3239
// Look at the Paragraph class for documentation of the following properties.
@@ -53,7 +60,7 @@ class TextLayoutService {
5360
ui.Rect get paintBounds => _paintBounds;
5461
ui.Rect _paintBounds = ui.Rect.zero;
5562

56-
late final Spanometer spanometer = Spanometer(paragraph, context);
63+
late final Spanometer spanometer = Spanometer(paragraph);
5764

5865
late final LayoutFragmenter layoutFragmenter =
5966
LayoutFragmenter(paragraph.plainText, paragraph.spans);
@@ -882,10 +889,9 @@ class LineBuilder {
882889
/// it's set, the [Spanometer] updates the underlying [context] so that
883890
/// subsequent measurements use the correct styles.
884891
class Spanometer {
885-
Spanometer(this.paragraph, this.context);
892+
Spanometer(this.paragraph);
886893

887894
final CanvasParagraph paragraph;
888-
final DomCanvasRenderingContext2D context;
889895

890896
static final RulerHost _rulerHost = RulerHost();
891897

@@ -904,21 +910,31 @@ class Spanometer {
904910
_rulers.clear();
905911
}
906912

907-
String _cssFontString = '';
908-
909913
double? get letterSpacing => currentSpan.style.letterSpacing;
910914

911915
TextHeightRuler? _currentRuler;
912916
ParagraphSpan? _currentSpan;
913917

914918
ParagraphSpan get currentSpan => _currentSpan!;
915919
set currentSpan(ParagraphSpan? span) {
920+
// Update the font string if it's different from the last applied font
921+
// string.
922+
//
923+
// Also, we need to update the font string even if the span isn't changing.
924+
// That's because `textContext` is shared across all spanometers.
925+
if (span != null) {
926+
final String newCssFontString = span.style.cssFontString;
927+
if (_lastContextFont != newCssFontString) {
928+
_lastContextFont = newCssFontString;
929+
textContext.font = newCssFontString;
930+
}
931+
}
932+
916933
if (span == _currentSpan) {
917934
return;
918935
}
919936
_currentSpan = span;
920937

921-
// No need to update css font string when `span` is null.
922938
if (span == null) {
923939
_currentRuler = null;
924940
return;
@@ -933,13 +949,6 @@ class Spanometer {
933949
_rulers[heightStyle] = ruler;
934950
}
935951
_currentRuler = ruler;
936-
937-
// Update the font string if it's different from the previous span.
938-
final String cssFontString = span.style.cssFontString;
939-
if (_cssFontString != cssFontString) {
940-
_cssFontString = cssFontString;
941-
context.font = cssFontString;
942-
}
943952
}
944953

945954
/// Whether the spanometer is ready to take measurements.
@@ -955,7 +964,7 @@ class Spanometer {
955964
double get height => _currentRuler!.height;
956965

957966
double measureText(String text) {
958-
return measureSubstring(context, text, 0, text.length);
967+
return measureSubstring(textContext, text, 0, text.length);
959968
}
960969

961970
double measureRange(int start, int end) {
@@ -1047,7 +1056,7 @@ class Spanometer {
10471056
assert(end >= currentSpan.start && end <= currentSpan.end);
10481057

10491058
return measureSubstring(
1050-
context,
1059+
textContext,
10511060
paragraph.plainText,
10521061
start,
10531062
end,

lib/web_ui/test/text/layout_service_plain_test.dart

+60
Original file line numberDiff line numberDiff line change
@@ -691,4 +691,64 @@ Future<void> testMain() async {
691691
l('i', 9, 10, hardBreak: true, width: 10.0, left: 40.0),
692692
]);
693693
});
694+
695+
test('uses a single minimal canvas', () {
696+
debugResetCanvasCount();
697+
698+
plain(ahemStyle, 'Lorem').layout(constrain(double.infinity));
699+
plain(ahemStyle, 'ipsum dolor').layout(constrain(150.0));
700+
// Try different styles too.
701+
plain(EngineParagraphStyle(fontWeight: ui.FontWeight.bold), 'sit amet').layout(constrain(300.0));
702+
703+
expect(textContext.canvas!.width, isZero);
704+
expect(textContext.canvas!.height, isZero);
705+
// This number is 0 instead of 1 because the canvas is created at the top
706+
// level as a global variable. So by the time this test runs, the canvas
707+
// would have been created already.
708+
//
709+
// So we just make sure that no new canvas is created after the above layout
710+
// calls.
711+
expect(debugCanvasCount, 0);
712+
});
713+
714+
test('does not leak styles across spanometers', () {
715+
// This prevents the Ahem font from being forced in all paragraphs.
716+
ui.debugEmulateFlutterTesterEnvironment = false;
717+
718+
final CanvasParagraph p1 = plain(
719+
EngineParagraphStyle(
720+
fontSize: 20.0,
721+
fontFamily: 'FontFamily1',
722+
),
723+
'Lorem',
724+
)..layout(constrain(double.infinity));
725+
// After the layout, the canvas should have the above style applied.
726+
expect(textContext.font, contains('20px'));
727+
expect(textContext.font, contains('FontFamily1'));
728+
729+
final CanvasParagraph p2 = plain(
730+
EngineParagraphStyle(
731+
fontSize: 40.0,
732+
fontFamily: 'FontFamily2',
733+
),
734+
'ipsum dolor',
735+
)..layout(constrain(double.infinity));
736+
// After the layout, the canvas should have the above style applied.
737+
expect(textContext.font, contains('40px'));
738+
expect(textContext.font, contains('FontFamily2'));
739+
740+
p1.getBoxesForRange(0, 2);
741+
// getBoxesForRange performs some text measurements. Let's make sure that it
742+
// applied the correct style.
743+
expect(textContext.font, contains('20px'));
744+
expect(textContext.font, contains('FontFamily1'));
745+
746+
p2.getBoxesForRange(0, 4);
747+
// getBoxesForRange performs some text measurements. Let's make sure that it
748+
// applied the correct style.
749+
expect(textContext.font, contains('40px'));
750+
expect(textContext.font, contains('FontFamily2'));
751+
752+
ui.debugEmulateFlutterTesterEnvironment = true;
753+
});
694754
}

0 commit comments

Comments
 (0)