mirror of
https://github.com/flutter/flutter.git
synced 2026-01-14 17:35:51 +08:00
## Description This PR fixes a division by zero crash in `RenderTable` when intrinsic size methods are called on empty tables (0 rows × 0 columns) with non-zero constraints. ### The Problem When `RenderTable` has 0 columns and intrinsic size methods (`computeMinIntrinsicHeight`, `computeMaxIntrinsicHeight`, `computeMinIntrinsicWidth`, `computeMaxIntrinsicWidth`) are called with non-zero width constraints, the code calls `_computeColumnWidths()` without checking if the table is empty. This leads to division by zero at line 1140: ```dart final double delta = (minWidthConstraint - tableWidth) / columns; // When columns = 0, this crashes with division by zero ``` ### The Solution Added early return checks `if (rows * columns == 0) return 0.0;` to all four intrinsic size methods, matching the pattern already used by other methods that call `_computeColumnWidths()`: - `computeDryBaseline` (line 1242) - already has the check - `computeDryLayout` (line 1274) - already has the check - `performLayout` (line 1318) - already has the check - `paint` (line 1461) - already has the check Now all four intrinsic methods are consistent with the rest of the codebase. ### Testing Added comprehensive test coverage (`Empty table intrinsic dimensions should not crash`) that: - Creates an empty table (0×0) - Calls all four intrinsic size methods with various constraints - Verifies they return 0.0 without crashing ### Changes Made **packages/flutter/lib/src/rendering/table.dart:** - Added empty table check to `computeMinIntrinsicWidth` (line 963-965) - Added empty table check to `computeMaxIntrinsicWidth` (line 978-980) - Added empty table check to `computeMinIntrinsicHeight` (line 995-997) - Added empty table check to `computeMaxIntrinsicHeight` (line 1016-1019) **packages/flutter/test/rendering/table_test.dart:** - Added test case `Empty table intrinsic dimensions should not crash` - Tests all four intrinsic methods with both finite and infinite constraints ## Related Issues This fix addresses a potential crash when using `RenderTable` with intrinsic sizing widgets (like `IntrinsicHeight`, `IntrinsicWidth`) on empty tables. ## Checklist Before you create this PR, confirm all the requirements listed below by checking the relevant checkboxes (`[x]`). This will ensure a smooth review process. - [x] I have read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I have read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I have read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I have signed the [CLA]. - [x] I have listed at least one issue that this PR fixes (defensive programming for edge case). - [x] I have updated/added relevant documentation (doc comments with `///`). - [x] I have added new tests that verify the fix works. - [x] All existing and new tests are passing (`flutter analyze` shows no issues). - [x] I have followed the [breaking change policy] and added [Data Driven Fixes] where applicable (no breaking changes in this PR). [Contributor Guide]: https://github.com/flutter/flutter/blob/master/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/master/docs/contributing/Tree-hygiene.md [Flutter Style Guide]: https://github.com/flutter/flutter/blob/master/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/master/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [breaking change policy]: https://github.com/flutter/flutter/blob/master/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Data Driven Fixes]: https://github.com/flutter/flutter/blob/master/docs/contributing/Data-driven-Fixes.md --------- Co-authored-by: Kate Lovett <katelovett@google.com>
427 lines
14 KiB
Dart
427 lines
14 KiB
Dart
// Copyright 2014 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 'package:flutter/rendering.dart';
|
||
import 'package:flutter_test/flutter_test.dart';
|
||
|
||
import 'rendering_tester.dart';
|
||
|
||
RenderBox sizedBox(double width, double height) {
|
||
return RenderConstrainedBox(additionalConstraints: BoxConstraints.tight(Size(width, height)));
|
||
}
|
||
|
||
void main() {
|
||
TestRenderingFlutterBinding.ensureInitialized();
|
||
|
||
test('Table control test; tight', () {
|
||
RenderTable table;
|
||
layout(table = RenderTable(textDirection: TextDirection.ltr));
|
||
|
||
expect(table.size.width, equals(800.0));
|
||
expect(table.size.height, equals(600.0));
|
||
|
||
expect(table, hasAGoodToStringDeep);
|
||
expect(
|
||
table.toStringDeep(minLevel: DiagnosticLevel.info),
|
||
equalsIgnoringHashCodes(
|
||
'RenderTable#00000 NEEDS-PAINT\n'
|
||
' │ parentData: <none>\n'
|
||
' │ constraints: BoxConstraints(w=800.0, h=600.0)\n'
|
||
' │ semantic boundary\n'
|
||
' │ size: Size(800.0, 600.0)\n'
|
||
' │ default column width: FlexColumnWidth(1.0)\n'
|
||
' │ table size: 0×0\n'
|
||
' │ column offsets: unknown\n'
|
||
' │ row offsets: []\n'
|
||
' │\n'
|
||
' └─table is empty\n',
|
||
),
|
||
);
|
||
});
|
||
|
||
test('Table control test; loose', () {
|
||
RenderTable table;
|
||
layout(RenderPositionedBox(child: table = RenderTable(textDirection: TextDirection.ltr)));
|
||
|
||
expect(table.size, equals(Size.zero));
|
||
});
|
||
|
||
test('Table control test: constrained flex columns', () {
|
||
final table = RenderTable(textDirection: TextDirection.ltr);
|
||
final children = List<RenderBox>.generate(6, (_) => RenderPositionedBox());
|
||
|
||
table.setFlatChildren(6, children);
|
||
layout(table, constraints: const BoxConstraints.tightFor(width: 100.0));
|
||
|
||
const double expectedWidth = 100.0 / 6;
|
||
for (final child in children) {
|
||
expect(child.size.width, moreOrLessEquals(expectedWidth));
|
||
}
|
||
});
|
||
|
||
test('Table test: combinations', () {
|
||
RenderTable table;
|
||
layout(
|
||
RenderPositionedBox(
|
||
child: table = RenderTable(
|
||
columns: 5,
|
||
rows: 5,
|
||
defaultColumnWidth: const IntrinsicColumnWidth(),
|
||
textDirection: TextDirection.ltr,
|
||
defaultVerticalAlignment: TableCellVerticalAlignment.baseline,
|
||
textBaseline: TextBaseline.alphabetic,
|
||
),
|
||
),
|
||
);
|
||
|
||
expect(table.size, equals(Size.zero));
|
||
|
||
table.setChild(2, 4, sizedBox(100.0, 200.0));
|
||
|
||
pumpFrame();
|
||
|
||
expect(table.size, equals(const Size(100.0, 200.0)));
|
||
|
||
table.setChild(0, 0, sizedBox(10.0, 30.0));
|
||
table.setChild(1, 0, sizedBox(20.0, 20.0));
|
||
table.setChild(2, 0, sizedBox(30.0, 10.0));
|
||
|
||
pumpFrame();
|
||
|
||
expect(table.size, equals(const Size(130.0, 230.0)));
|
||
|
||
expect(table, hasAGoodToStringDeep);
|
||
expect(
|
||
table.toStringDeep(minLevel: DiagnosticLevel.info),
|
||
equalsIgnoringHashCodes(
|
||
'RenderTable#00000 relayoutBoundary=up1 NEEDS-PAINT NEEDS-COMPOSITING-BITS-UPDATE\n'
|
||
' │ parentData: offset=Offset(335.0, 185.0) (can use size)\n'
|
||
' │ constraints: BoxConstraints(0.0<=w<=800.0, 0.0<=h<=600.0)\n'
|
||
' │ semantic boundary\n'
|
||
' │ size: Size(130.0, 230.0)\n'
|
||
' │ default column width: IntrinsicColumnWidth(flex: null)\n'
|
||
' │ table size: 5×5\n'
|
||
' │ column offsets: 0.0, 10.0, 30.0, 130.0, 130.0\n'
|
||
' │ row offsets: 0.0, 30.0, 30.0, 30.0, 30.0, 230.0\n'
|
||
' │\n'
|
||
' ├─child (0, 0): RenderConstrainedBox#00000 relayoutBoundary=up2 NEEDS-PAINT\n'
|
||
' │ parentData: offset=Offset(0.0, 0.0); default vertical alignment\n'
|
||
' │ (can use size)\n'
|
||
' │ constraints: BoxConstraints(w=10.0, 0.0<=h<=Infinity)\n'
|
||
' │ size: Size(10.0, 30.0)\n'
|
||
' │ additionalConstraints: BoxConstraints(w=10.0, h=30.0)\n'
|
||
' │\n'
|
||
' ├─child (1, 0): RenderConstrainedBox#00000 relayoutBoundary=up2 NEEDS-PAINT\n'
|
||
' │ parentData: offset=Offset(10.0, 0.0); default vertical alignment\n'
|
||
' │ (can use size)\n'
|
||
' │ constraints: BoxConstraints(w=20.0, 0.0<=h<=Infinity)\n'
|
||
' │ size: Size(20.0, 20.0)\n'
|
||
' │ additionalConstraints: BoxConstraints(w=20.0, h=20.0)\n'
|
||
' │\n'
|
||
' ├─child (2, 0): RenderConstrainedBox#00000 relayoutBoundary=up2 NEEDS-PAINT\n'
|
||
' │ parentData: offset=Offset(30.0, 0.0); default vertical alignment\n'
|
||
' │ (can use size)\n'
|
||
' │ constraints: BoxConstraints(w=100.0, 0.0<=h<=Infinity)\n'
|
||
' │ size: Size(100.0, 10.0)\n'
|
||
' │ additionalConstraints: BoxConstraints(w=30.0, h=10.0)\n'
|
||
' │\n'
|
||
' ├─child (3, 0) is null\n'
|
||
' ├─child (4, 0) is null\n'
|
||
' ├─child (0, 1) is null\n'
|
||
' ├─child (1, 1) is null\n'
|
||
' ├─child (2, 1) is null\n'
|
||
' ├─child (3, 1) is null\n'
|
||
' ├─child (4, 1) is null\n'
|
||
' ├─child (0, 2) is null\n'
|
||
' ├─child (1, 2) is null\n'
|
||
' ├─child (2, 2) is null\n'
|
||
' ├─child (3, 2) is null\n'
|
||
' ├─child (4, 2) is null\n'
|
||
' ├─child (0, 3) is null\n'
|
||
' ├─child (1, 3) is null\n'
|
||
' ├─child (2, 3) is null\n'
|
||
' ├─child (3, 3) is null\n'
|
||
' ├─child (4, 3) is null\n'
|
||
' ├─child (0, 4) is null\n'
|
||
' ├─child (1, 4) is null\n'
|
||
' ├─child (2, 4): RenderConstrainedBox#00000 relayoutBoundary=up2 NEEDS-PAINT\n'
|
||
' │ parentData: offset=Offset(30.0, 30.0); default vertical alignment\n'
|
||
' │ (can use size)\n'
|
||
' │ constraints: BoxConstraints(w=100.0, 0.0<=h<=Infinity)\n'
|
||
' │ size: Size(100.0, 200.0)\n'
|
||
' │ additionalConstraints: BoxConstraints(w=100.0, h=200.0)\n'
|
||
' │\n'
|
||
' ├─child (3, 4) is null\n'
|
||
' └─child (4, 4) is null\n',
|
||
),
|
||
);
|
||
});
|
||
|
||
test('Table test: removing cells', () {
|
||
RenderTable table;
|
||
RenderBox child;
|
||
table = RenderTable(columns: 5, rows: 5, textDirection: TextDirection.ltr);
|
||
table.setChild(4, 4, child = sizedBox(10.0, 10.0));
|
||
|
||
layout(table);
|
||
|
||
expect(child.attached, isTrue);
|
||
table.rows = 4;
|
||
expect(child.attached, isFalse);
|
||
});
|
||
|
||
test('Table test: replacing cells', () {
|
||
RenderTable table;
|
||
final RenderBox child1 = RenderPositionedBox();
|
||
final RenderBox child2 = RenderPositionedBox();
|
||
final RenderBox child3 = RenderPositionedBox();
|
||
table = RenderTable(textDirection: TextDirection.ltr);
|
||
table.setFlatChildren(3, <RenderBox>[
|
||
child1,
|
||
RenderPositionedBox(),
|
||
child2,
|
||
RenderPositionedBox(),
|
||
child3,
|
||
RenderPositionedBox(),
|
||
]);
|
||
expect(table.rows, equals(2));
|
||
layout(table);
|
||
table.setFlatChildren(3, <RenderBox>[
|
||
RenderPositionedBox(),
|
||
child1,
|
||
RenderPositionedBox(),
|
||
child2,
|
||
RenderPositionedBox(),
|
||
child3,
|
||
]);
|
||
pumpFrame();
|
||
table.setFlatChildren(3, <RenderBox>[
|
||
RenderPositionedBox(),
|
||
child1,
|
||
RenderPositionedBox(),
|
||
child2,
|
||
RenderPositionedBox(),
|
||
child3,
|
||
]);
|
||
pumpFrame();
|
||
expect(table.columns, equals(3));
|
||
expect(table.rows, equals(2));
|
||
});
|
||
|
||
test('Table border painting', () {
|
||
final table = RenderTable(textDirection: TextDirection.rtl, border: TableBorder.all());
|
||
layout(table);
|
||
table.setFlatChildren(1, <RenderBox>[]);
|
||
pumpFrame();
|
||
expect(
|
||
table,
|
||
paints
|
||
..path()
|
||
..path()
|
||
..path()
|
||
..path(),
|
||
);
|
||
table.setFlatChildren(1, <RenderBox>[RenderPositionedBox()]);
|
||
pumpFrame();
|
||
expect(
|
||
table,
|
||
paints
|
||
..path()
|
||
..path()
|
||
..path()
|
||
..path(),
|
||
);
|
||
table.setFlatChildren(1, <RenderBox>[RenderPositionedBox(), RenderPositionedBox()]);
|
||
pumpFrame();
|
||
expect(
|
||
table,
|
||
paints
|
||
..path()
|
||
..path()
|
||
..path()
|
||
..path()
|
||
..path(),
|
||
);
|
||
table.setFlatChildren(2, <RenderBox>[RenderPositionedBox(), RenderPositionedBox()]);
|
||
pumpFrame();
|
||
expect(
|
||
table,
|
||
paints
|
||
..path()
|
||
..path()
|
||
..path()
|
||
..path()
|
||
..path(),
|
||
);
|
||
table.setFlatChildren(2, <RenderBox>[
|
||
RenderPositionedBox(),
|
||
RenderPositionedBox(),
|
||
RenderPositionedBox(),
|
||
RenderPositionedBox(),
|
||
]);
|
||
pumpFrame();
|
||
expect(
|
||
table,
|
||
paints
|
||
..path()
|
||
..path()
|
||
..path()
|
||
..path()
|
||
..path()
|
||
..path(),
|
||
);
|
||
table.setFlatChildren(3, <RenderBox>[
|
||
RenderPositionedBox(),
|
||
RenderPositionedBox(),
|
||
RenderPositionedBox(),
|
||
RenderPositionedBox(),
|
||
RenderPositionedBox(),
|
||
RenderPositionedBox(),
|
||
]);
|
||
pumpFrame();
|
||
expect(
|
||
table,
|
||
paints
|
||
..path()
|
||
..path()
|
||
..path()
|
||
..path()
|
||
..path()
|
||
..path(),
|
||
);
|
||
});
|
||
|
||
test('Table flex sizing', () {
|
||
const cellConstraints = BoxConstraints.tightFor(width: 100, height: 100);
|
||
final table = RenderTable(
|
||
textDirection: TextDirection.rtl,
|
||
children: <List<RenderBox>>[
|
||
List<RenderBox>.generate(
|
||
7,
|
||
(int _) => RenderConstrainedBox(additionalConstraints: cellConstraints),
|
||
),
|
||
],
|
||
columnWidths: const <int, TableColumnWidth>{
|
||
0: FlexColumnWidth(),
|
||
1: FlexColumnWidth(0.123),
|
||
2: FlexColumnWidth(0.123),
|
||
3: FlexColumnWidth(0.123),
|
||
4: FlexColumnWidth(0.123),
|
||
5: FlexColumnWidth(0.123),
|
||
6: FlexColumnWidth(0.123),
|
||
},
|
||
);
|
||
|
||
layout(table, constraints: BoxConstraints.tight(const Size(800.0, 600.0)));
|
||
expect(table.hasSize, true);
|
||
});
|
||
|
||
test('Table paints a borderRadius', () {
|
||
final table = RenderTable(
|
||
textDirection: TextDirection.ltr,
|
||
border: TableBorder.all(borderRadius: const BorderRadius.all(Radius.circular(8.0))),
|
||
);
|
||
layout(table);
|
||
table.setFlatChildren(2, <RenderBox>[
|
||
RenderPositionedBox(),
|
||
RenderPositionedBox(),
|
||
RenderPositionedBox(),
|
||
RenderPositionedBox(),
|
||
]);
|
||
pumpFrame();
|
||
expect(
|
||
table,
|
||
paints
|
||
..path()
|
||
..path()
|
||
..drrect(
|
||
outer: RRect.fromLTRBR(0.0, 0.0, 800.0, 0.0, const Radius.circular(8.0)),
|
||
inner: RRect.fromLTRBR(1.0, 1.0, 799.0, -1.0, const Radius.circular(7.0)),
|
||
),
|
||
);
|
||
});
|
||
|
||
test('MaxColumnWidth.flex returns the correct result', () {
|
||
var columnWidth = const MaxColumnWidth(
|
||
FixedColumnWidth(100), // returns null from .flex
|
||
FlexColumnWidth(), // returns 1 from .flex
|
||
);
|
||
final double? flexValue = columnWidth.flex(<RenderBox>[]);
|
||
expect(flexValue, 1.0);
|
||
|
||
// Swap a and b, check for same result.
|
||
columnWidth = const MaxColumnWidth(
|
||
FlexColumnWidth(), // returns 1 from .flex
|
||
FixedColumnWidth(100), // returns null from .flex
|
||
);
|
||
// Same result.
|
||
expect(columnWidth.flex(<RenderBox>[]), flexValue);
|
||
});
|
||
|
||
test('MinColumnWidth.flex returns the correct result', () {
|
||
var columnWidth = const MinColumnWidth(
|
||
FixedColumnWidth(100), // returns null from .flex
|
||
FlexColumnWidth(), // returns 1 from .flex
|
||
);
|
||
final double? flexValue = columnWidth.flex(<RenderBox>[]);
|
||
expect(flexValue, 1.0);
|
||
|
||
// Swap a and b, check for same result.
|
||
columnWidth = const MinColumnWidth(
|
||
FlexColumnWidth(), // returns 1 from .flex
|
||
FixedColumnWidth(100), // returns null from .flex
|
||
);
|
||
// Same result.
|
||
expect(columnWidth.flex(<RenderBox>[]), flexValue);
|
||
});
|
||
|
||
test('TableRows with different constraints, but vertically with intrinsicHeight', () {
|
||
const firstConstraints = BoxConstraints.tightFor(width: 100, height: 100);
|
||
const secondConstraints = BoxConstraints.tightFor(width: 200, height: 200);
|
||
|
||
final table = RenderTable(
|
||
textDirection: TextDirection.rtl,
|
||
defaultVerticalAlignment: TableCellVerticalAlignment.intrinsicHeight,
|
||
children: <List<RenderBox>>[
|
||
<RenderBox>[
|
||
RenderConstrainedBox(additionalConstraints: firstConstraints),
|
||
RenderConstrainedBox(additionalConstraints: secondConstraints),
|
||
],
|
||
],
|
||
columnWidths: const <int, TableColumnWidth>{0: FlexColumnWidth(), 1: FlexColumnWidth()},
|
||
);
|
||
|
||
const size = Size(300.0, 300.0);
|
||
|
||
// Layout the table with a fixed size.
|
||
layout(table, constraints: BoxConstraints.tight(size));
|
||
|
||
// Make sure the table has a size and that the children are filled vertically to the highest cell.
|
||
expect(table.size, equals(size));
|
||
expect(table.defaultVerticalAlignment, TableCellVerticalAlignment.intrinsicHeight);
|
||
});
|
||
|
||
test('Empty table intrinsic dimensions should not crash', () {
|
||
// Test that empty tables (0 rows x 0 columns) don't cause division by zero
|
||
// when intrinsic size methods are called with non-zero constraints.
|
||
final table = RenderTable(textDirection: TextDirection.ltr);
|
||
|
||
// Verify table is empty
|
||
expect(table.rows, equals(0));
|
||
expect(table.columns, equals(0));
|
||
|
||
// These should all return 0.0 without crashing (previously caused division by zero)
|
||
expect(table.getMinIntrinsicWidth(100.0), equals(0.0));
|
||
expect(table.getMaxIntrinsicWidth(100.0), equals(0.0));
|
||
expect(table.getMinIntrinsicHeight(100.0), equals(0.0));
|
||
expect(table.getMaxIntrinsicHeight(100.0), equals(0.0));
|
||
|
||
// Also test with infinite constraints
|
||
expect(table.getMinIntrinsicWidth(double.infinity), equals(0.0));
|
||
expect(table.getMaxIntrinsicWidth(double.infinity), equals(0.0));
|
||
expect(table.getMinIntrinsicHeight(double.infinity), equals(0.0));
|
||
expect(table.getMaxIntrinsicHeight(double.infinity), equals(0.0));
|
||
});
|
||
}
|