[Skwasm] Correctly handle paragraphs with empty text. (flutter#51695)

eyebrowsoffire · web-flow · commit 2e0616a71b19 · 2024-03-27T20:18:57.000Z
Instead of just returning, if our paragraph builder has empty text, we still need to generate a set of line breaks for skia to use. Otherwise, it will actually cause subtle memory access errors.
diff --git a/lib/web_ui/lib/src/engine/skwasm/skwasm_impl/paragraph.dart b/lib/web_ui/lib/src/engine/skwasm/skwasm_impl/paragraph.dart
@@ -940,27 +940,29 @@ class SkwasmParagraphBuilder extends SkwasmObjectWrapper<RawParagraphBuilder> im
     // just create both up front here.
     final Pointer<Uint32> outSize = scope.allocUint32Array(1);
     final Pointer<Uint8> utf8Data = paragraphBuilderGetUtf8Text(handle, outSize);
+
+    final String text;
+    final JSString jsText;
     if (utf8Data == nullptr) {
-      return;
+      text = '';
+      jsText = ''.toJS;
+    } else {
+      final List<int> codeUnitList = List<int>.generate(
+        outSize.value,
+        (int index) => utf8Data[index]
+      );
+      text = utf8.decode(codeUnitList);
+      jsText = _utf8Decoder.decode(
+        // In an ideal world we would just use a subview of wasm memory rather
+        // than a slice, but the TextDecoder API doesn't work on shared buffer
+        // sources yet.
+        // See https://bugs.chromium.org/p/chromium/issues/detail?id=1012656
+        createUint8ArrayFromBuffer(skwasmInstance.wasmMemory.buffer).slice(
+          utf8Data.address.toJS,
+          (utf8Data.address + outSize.value).toJS
+        ));
     }
 
-    // TODO(jacksongardner): We could make a subclass of `List<int>` here to
-    // avoid this copy.
-    final List<int> codeUnitList = List<int>.generate(
-      outSize.value,
-      (int index) => utf8Data[index]
-    );
-    final String text = utf8.decode(codeUnitList);
-    final JSString jsText = _utf8Decoder.decode(
-      // In an ideal world we would just use a subview of wasm memory rather
-      // than a slice, but the TextDecoder API doesn't work on shared buffer
-      // sources yet.
-      // See https://bugs.chromium.org/p/chromium/issues/detail?id=1012656
-      createUint8ArrayFromBuffer(skwasmInstance.wasmMemory.buffer).slice(
-        utf8Data.address.toJS,
-        (utf8Data.address + outSize.value).toJS
-      ));
-
     _addGraphemeBreakData(text, jsText);
     _addWordBreakData(text, jsText);
     _addLineBreakData(text, jsText);
diff --git a/lib/web_ui/test/ui/line_metrics_test.dart b/lib/web_ui/test/ui/line_metrics_test.dart
@@ -176,5 +176,5 @@ Future<void> testMain() async {
 
     // In Roboto, the width should be 11 here. In the test font, it would be square (16 points)
     expect(metrics!.width, 11);
-  }, solo: true);
+  });
 }
diff --git a/lib/web_ui/test/ui/paragraph_builder_test.dart b/lib/web_ui/test/ui/paragraph_builder_test.dart
@@ -39,4 +39,14 @@ Future<void> testMain() async {
 
     expect(() => builder.build(), returnsNormally);
   });
+
+  test('build and layout a paragraph with an empty addText', () {
+    final ParagraphBuilder builder = ParagraphBuilder(ParagraphStyle());
+    builder.addText('');
+    final Paragraph paragraph = builder.build();
+    expect(
+      () => paragraph.layout(const ParagraphConstraints(width: double.infinity)),
+      returnsNormally,
+    );
+  });
 }

Original file line number	Diff line number	Diff line change
`@@ -176,5 +176,5 @@ Future<void> testMain() async {`
`176`	`176`
`177`	`177`	`// In Roboto, the width should be 11 here. In the test font, it would be square (16 points)`
`178`	`178`	`expect(metrics!.width, 11);`
`179`		`- }, solo: true);`
	`179`	`+ });`
`180`	`180`	`}`