Skip to content

Commit f513a69

Browse files
Mark _LayoutBuilderElement as always clean (flutter#154694)
Fixes flutter#154060 The error message doesn't make sense to me since one can call `setState` during the idle phase, and I'm not sure what this is guarding against without the right error message. In the case of flutter#154060 the layout builder was never laid out: ``` ��child 1: _RenderLayoutBuilder#7c319 NEEDS-LAYOUT NEEDS-PAINT � creator: LayoutBuilder � _BodyBuilder � MediaQuery � � LayoutId-[<_ScaffoldSlot.body>] � CustomMultiChildLayout � � _ActionsScope � Actions � AnimatedBuilder � DefaultTextStyle � � AnimatedDefaultTextStyle � _InkFeatures-[GlobalKey#1f6eb ink � renderer] � NotificationListener<LayoutChangedNotification> � � � parentData: offset=Offset(0.0, 0.0); id=_ScaffoldSlot.body � constraints: MISSING � size: MISSING ``` So flutter#154681 doesn't really fix flutter#154060 since the layout callback cannot be run without a set of valid constraints. Before the `BuildScope` change all `_inDirtyList` flags were unset after the `BuildOwner` finishes rebuilding the widget tree, so `LayoutBuilder._inDirtyLst` is always set to false after a frame even for layout builders that were never laid out. With the `BuildScope` change, `LayoutBuilder` has its own `BuildScope` which is only flushed after LayoutBuilder gets its constraints.
1 parent 3f78a5b commit f513a69

File tree

4 files changed

+44
-7
lines changed

4 files changed

+44
-7
lines changed

packages/flutter/lib/src/widgets/framework.dart

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2911,15 +2911,14 @@ class BuildOwner {
29112911
'The dirty list for the current build scope is: ${buildScope._dirtyElements}',
29122912
);
29132913
}
2914-
// When reactivating an inactivate Element, _scheduleBuildFor should only be
2915-
// called within _flushDirtyElements.
29162914
if (!_debugBuilding && element._inDirtyList) {
29172915
throw FlutterError.fromParts(<DiagnosticsNode>[
29182916
ErrorSummary('BuildOwner.scheduleBuildFor() called inappropriately.'),
29192917
ErrorHint(
2920-
'The BuildOwner.scheduleBuildFor() method should only be called while the '
2921-
'buildScope() method is actively rebuilding the widget tree.',
2918+
'The BuildOwner.scheduleBuildFor() method called on an Element '
2919+
'that is already in the dirty list.',
29222920
),
2921+
element.describeElement('the dirty Element was'),
29232922
]);
29242923
}
29252924
return true;
@@ -5145,8 +5144,10 @@ abstract class Element extends DiagnosticableTree implements BuildContext {
51455144
/// Returns true if the element has been marked as needing rebuilding.
51465145
///
51475146
/// The flag is true when the element is first created and after
5148-
/// [markNeedsBuild] has been called. The flag is reset to false in the
5149-
/// [performRebuild] implementation.
5147+
/// [markNeedsBuild] has been called. The flag is typically reset to false in
5148+
/// the [performRebuild] implementation, but certain elements (that of the
5149+
/// [LayoutBuilder] widget, for example) may choose to override [markNeedsBuild]
5150+
/// such that it does not set the [dirty] flag to `true` when called.
51505151
bool get dirty => _dirty;
51515152
bool _dirty = true;
51525153

packages/flutter/lib/src/widgets/layout_builder.dart

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,10 @@ class _LayoutBuilderElement<ConstraintType extends Constraints> extends RenderOb
158158

159159
@override
160160
void markNeedsBuild() {
161-
super.markNeedsBuild();
161+
// Calling super.markNeedsBuild is not needed. This Element does not need
162+
// to performRebuild since this call already does what performRebuild does,
163+
// So the element is clean as soon as this method returns and does not have
164+
// to be added to the dirty list or marked as dirty.
162165
renderObject.markNeedsLayout();
163166
_needsBuild = true;
164167
}

packages/flutter/test/widgets/framework_test.dart

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1941,6 +1941,18 @@ The findRenderObject() method was called for the following element:
19411941
expect(scopeElement.dirty, isFalse);
19421942
expect(keyedWidget.dirty, isTrue);
19431943
});
1944+
1945+
testWidgets('Calling scheduleBuildFor on an Element already in dirty list throws', (WidgetTester tester) async {
1946+
await tester.pumpWidget(const SizedBox());
1947+
final Element element = tester.element(find.byType(SizedBox));
1948+
element.markNeedsBuild();
1949+
expect(
1950+
() => element.owner!.scheduleBuildFor(element),
1951+
throwsA(isFlutterError.having(
1952+
(FlutterError e) => e.message, 'message', contains('The BuildOwner.scheduleBuildFor() method called on an Element that is already in the dirty list.'),
1953+
)),
1954+
);
1955+
});
19441956
}
19451957

19461958
class _TestInheritedElement extends InheritedElement {

packages/flutter/test/widgets/layout_builder_test.dart

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -850,6 +850,27 @@ void main() {
850850
expect(paintCount, 3);
851851
expect(mostRecentOffset, const Offset(60, 60));
852852
});
853+
854+
testWidgets('LayoutBuilder in a subtree that skips layout does not throw during the initial treewalk', (WidgetTester tester) async {
855+
final OverlayEntry overlayEntry1 = OverlayEntry(maintainState: true, builder: (BuildContext context) => LayoutBuilder(builder: (BuildContext context, BoxConstraints constraints) => const Placeholder()));
856+
// OverlayEntry2 obstructs OverlayEntry1 and forces it to skip layout.
857+
final OverlayEntry overlayEntry2 = OverlayEntry(opaque: true, canSizeOverlay: true, builder: (BuildContext context) => Container());
858+
addTearDown(() => overlayEntry1..remove()..dispose());
859+
addTearDown(() => overlayEntry2..remove()..dispose());
860+
await tester.pumpWidget(
861+
Directionality(
862+
textDirection: TextDirection.ltr,
863+
// The UnconstrainedBox makes sure the OverlayEntries are not relayout boundaries.
864+
child: UnconstrainedBox(child: Overlay(initialEntries: <OverlayEntry>[overlayEntry1, overlayEntry2])),
865+
),
866+
);
867+
868+
WidgetsBinding.instance.buildOwner!.reassemble(WidgetsBinding.instance.rootElement!);
869+
await tester.pump();
870+
WidgetsBinding.instance.buildOwner!.reassemble(WidgetsBinding.instance.rootElement!);
871+
await tester.pump();
872+
expect(tester.takeException(), isNull);
873+
});
853874
}
854875

855876
class _SmartLayoutBuilder extends ConstrainedLayoutBuilder<BoxConstraints> {

0 commit comments

Comments
 (0)