firka/flutter
firka/flutter
5
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

[CP-stable]Make _layoutBoundary a boolean 2 (#171106)

Browse Source 
This pull request is created by [automatic cherry pick workflow](https://github.com/flutter/flutter/blob/main/docs/releases/Flutter-Cherrypick-Process.md#automatically-creates-a-cherry-pick-request)
Please fill in the form below, and a flutter domain expert will evaluate this cherry pick request.

### Issue Link:
What is the link to the issue this cherry-pick is addressing?

https://github.com/flutter/flutter/issues/168936.

### Changelog Description:
Explain this cherry pick in one line that is accessible to most Flutter developers. See [best practices](https://github.com/flutter/flutter/blob/main/docs/releases/Hotfix-Documentation-Best-Practices.md) for examples

Fixes a "Null check operator used on a null value" crash when a scroll view contains a `LayoutBuilder`.

### Impact Description:
What is the impact (ex. visual jank on Samsung phones, app crash, cannot ship an iOS app)? Does it impact development (ex. flutter doctor crashes when Android Studio is installed), or the shipping production app (the app crashes on launch)

The app may crash if there a `LayoutBuilder` child inside of a `RenderSliverMultiBoxAdaptor` (which `SliverList` uses), and that child suddenly becomes offstage and kept-alive.

One example is when you have a TextField inside of a LayoutBuilder which is part of a `SliverList`. The app crashes ("Null check operator used on a null value") even in release mode when the user drags so that the text field becomes offscreen. All 3 are popular widgets so I think this combination will not be that uncommon.

### Workaround:
Is there a workaround for this issue?

There's no known workaround for this issue. You can increase the cache extent to prevent the child from becoming kept-alive but this is not really feasible for long / infinite list as it will go OOM fast.

### Risk:
What is the risk level of this cherry-pick?

This should be a relatively safe fix. 

Despite changing an important flag in RenderObject from `RenderObject?` to `bool?`, there's only one change in behavior:

in `RenderObject.dropChild`: https://github.com/flutter/flutter/pull/169958/files#diff-4c298197a8aa7831a26eece096c8b8b07773ba1f5376d848ea4ef2924b606e9fL2052
which originally set the flag to `null` for the entire subtree until a new relayout boundary is reached, now it only sets the flag to `null` for the root of the subtree.
In release mode, the only place the framework cares about whether the flag is null is [here](https://github.com/flutter/flutter/pull/169958/files#diff-4c298197a8aa7831a26eece096c8b8b07773ba1f5376d848ea4ef2924b606e9fR2342) (everywhere else `null` just means `false`), and that won't actually introduce any correctness issues because if the node originally enters the `if` block in line 2343 it will still enter the `if` block after the patch.

Also, the fix was merged a couple weeks ago and I haven't seen bug reports associated with the fix yet.

### Test Coverage:
Are you confident that your fix is well-tested by automated tests?

### Validation Steps:
What are the steps to validate that this fix works?

follow the repro steps in https://github.com/flutter/flutter/issues/168936, or 
run the test (already checked in in `master`)

```dart  
testWidgets('LayoutBuilder does not crash when it becomes kept-alive', (
    WidgetTester tester,
  ) async {
    final FocusNode focusNode = FocusNode();
    final TextEditingController controller = TextEditingController();
    addTearDown(focusNode.dispose);
    addTearDown(controller.dispose);
    final Widget layoutBuilderWithParent = SizedBox(
      key: GlobalKey(),
      child: LayoutBuilder(
        builder: (BuildContext _, BoxConstraints _) {
          // The text field keeps the widget alive in the SliverList.
          return EditableText(
            focusNode: focusNode,
            backgroundCursorColor: const Color(0xFFFFFFFF),
            cursorColor: const Color(0xFFFFFFFF),
            style: const TextStyle(),
            controller: controller,
          );
        },
      ),
    );

    await tester.pumpWidget(
      Directionality(
        textDirection: TextDirection.ltr,
        child: CustomScrollView(
          slivers: <Widget>[
            SliverList.list(
              addRepaintBoundaries: false,
              addSemanticIndexes: false,
              children: <Widget>[const SizedBox(height: 60), layoutBuilderWithParent],
            ),
          ],
        ),
      ),
    );
    focusNode.requestFocus();
    await tester.pumpWidget(
      Directionality(
        textDirection: TextDirection.ltr,
        child: CustomScrollView(
          slivers: <Widget>[
            SliverList.list(
              addRepaintBoundaries: false,
              addSemanticIndexes: false,
              children: <Widget>[const SizedBox(height: 6000), layoutBuilderWithParent],
            ),
          ],
        ),
      ),
    );
  });
```
This commit is contained in:
flutteractionsbot 2025-07-07 14:56:25 -07:00 committed by GitHub
parent fcf2c11572
commit d779fc779d
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 121 additions and 116 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

183
packages/flutter/lib/src/rendering/object.dart
View File

@ -2049,7 +2049,9 @@ abstract class RenderObject with DiagnosticableTreeMixin implements HitTestTarge
assert(child._parent == this);
assert(child.attached == attached);
assert(child.parentData != null);
_cleanChildRelayoutBoundary(child);
if (!(_isRelayoutBoundary ?? true)) {
child._isRelayoutBoundary = null;
}
child.parentData!.detach();
child.parentData = null;
child._parent = null;
@ -2337,7 +2339,7 @@ abstract class RenderObject with DiagnosticableTreeMixin implements HitTestTarge
_owner = owner;
// If the node was dirtied in some way while unattached, make sure to add
// it to the appropriate dirty list now that an owner is available
if (_needsLayout && _relayoutBoundary != null) {
if (_needsLayout && _isRelayoutBoundary != null) {
// Don't enter this block if we've never laid out at all;
// scheduleInitialLayout() will handle it
_needsLayout = false;
@ -2393,33 +2395,35 @@ abstract class RenderObject with DiagnosticableTreeMixin implements HitTestTarge
bool _needsLayout = true;
/// The nearest relayout boundary enclosing this render object, if known.
/// Whether this [RenderObject] is a known relayout boundary.
///
/// When a render object is marked as needing layout, its parent may
/// as a result also need to be marked as needing layout.
/// For details, see [markNeedsLayout].
/// A render object where relayout does not require relayout of the parent
/// (because its size cannot change on relayout, or because
/// its parent does not use the child's size for its own layout)
/// is a "relayout boundary".
/// A relayout boundary is a [RenderObject] whose parent does not rely on the
/// child [RenderObject]'s size in its own layout algorithm. In other words,
/// if a [RenderObject]'s [performLayout] implementation does not ask the child
/// for its size at all, **the child** is a relayout boundary.
///
/// This property is set in [layout], and consulted by [markNeedsLayout] in
/// deciding whether to recursively mark the parent as also needing layout.
/// The type of "size" is typically defined by the coordinate system a
/// [RenderObject] subclass uses. For instance, [RenderSliver]s produce
/// [SliverGeometry] and [RenderBox]es produce [Size]. A parent [RenderObject]
/// may not read the child's size but still depend on the child's layout (using
/// a [RenderBox] child's baseline location, for example), this flag does not
/// reflect such dependencies and the [RenderObject] subclass must handle those
/// cases in its own implementation. See [RenderBox.markNeedsLayout] for an
/// example.
///
/// This property is initially null, and becomes null again if this
/// render object is removed from the tree (with [dropChild]);
/// it remains null until the first layout of this render object
/// after it was most recently added to the tree.
/// This property can also be null while an ancestor in the tree is
/// currently doing layout, until this render object itself does layout.
/// Relayout boundaries enable an important layout optimization: the parent not
/// depending on the size of a child means the child changing size does not
/// affect the layout of the parent. When a relayout boundary is marked as
/// needing layout, its parent does not have to be marked as dirty, hence the
/// name. For details, see [markNeedsLayout].
///
/// When not null, the relayout boundary is either this render object itself
/// or one of its ancestors, and all the render objects in the ancestry chain
/// up through that ancestor have the same [_relayoutBoundary].
/// Equivalently: when not null, the relayout boundary is either this render
/// object itself or the same as that of its parent. (So [_relayoutBoundary]
/// is one of `null`, `this`, or `parent!._relayoutBoundary!`.)
RenderObject? _relayoutBoundary;
/// This flag is typically set in [RenderObject.layout], and consulted by
/// [markNeedsLayout] in deciding whether to recursively mark the parent as
/// also needing layout.
///
/// The flag is initially set to `null` when [layout] has yet been called, and
/// reset to `null` when the parent drops this child via [dropChild].
bool? _isRelayoutBoundary;
/// Whether [invokeLayoutCallback] for this render object is currently running.
bool get debugDoingThisLayoutWithCallback => _doingThisLayoutWithCallback;
@ -2458,20 +2462,19 @@ abstract class RenderObject with DiagnosticableTreeMixin implements HitTestTarge
static bool debugCheckingIntrinsics = false;
bool _debugRelayoutBoundaryAlreadyMarkedNeedsLayout() {
if (_relayoutBoundary == null) {
// We don't know where our relayout boundary is yet.
return true;
}
RenderObject node = this;
while (node != _relayoutBoundary) {
assert(node._relayoutBoundary == _relayoutBoundary);
assert(node.parent != null);
node = node.parent!;
if ((!node._needsLayout) && (!node._debugDoingThisLayout)) {
for (
RenderObject? node = this;
node != null && node._isRelayoutBoundary != null;
node = node.parent
) {
final bool alreadyMarkedNeedsLayout = node._needsLayout || node._debugDoingThisLayout;
if (!alreadyMarkedNeedsLayout) {
return false;
}
if (node._isRelayoutBoundary!) {
return true;
}
}
assert(node._relayoutBoundary == node);
return true;
}
@ -2519,30 +2522,18 @@ abstract class RenderObject with DiagnosticableTreeMixin implements HitTestTarge
assert(_debugRelayoutBoundaryAlreadyMarkedNeedsLayout());
return;
}
if (_relayoutBoundary == null) {
_needsLayout = true;
if (parent != null) {
// _relayoutBoundary is cleaned by an ancestor in RenderObject.layout.
// Conservatively mark everything dirty until it reaches the closest
// known relayout boundary.
markParentNeedsLayout();
}
return;
}
if (_relayoutBoundary != this) {
_needsLayout = true;
if (owner case final PipelineOwner owner? when (_isRelayoutBoundary ?? false)) {
assert(() {
if (debugPrintMarkNeedsLayoutStacks) {
debugPrintStack(label: 'markNeedsLayout() called for $this');
}
return true;
}());
owner._nodesNeedingLayout.add(this);
owner.requestVisualUpdate();
} else if (parent != null) {
markParentNeedsLayout();
} else {
_needsLayout = true;
if (owner != null) {
assert(() {
if (debugPrintMarkNeedsLayoutStacks) {
debugPrintStack(label: 'markNeedsLayout() called for $this');
}
return true;
}());
owner!._nodesNeedingLayout.add(this);
owner!.requestVisualUpdate();
}
}
}
@ -2581,38 +2572,6 @@ abstract class RenderObject with DiagnosticableTreeMixin implements HitTestTarge
markParentNeedsLayout();
}
/// Set [_relayoutBoundary] to null throughout this render object's subtree,
/// stopping at relayout boundaries.
// This is a static method to reduce closure allocation with visitChildren.
static void _cleanChildRelayoutBoundary(RenderObject child) {
if (child._relayoutBoundary != child) {
child.visitChildren(_cleanChildRelayoutBoundary);
child._relayoutBoundary = null;
}
}
// This is a static method to reduce closure allocation with visitChildren.
static void _propagateRelayoutBoundaryToChild(RenderObject child) {
if (child._relayoutBoundary == child) {
return;
}
final RenderObject? parentRelayoutBoundary = child.parent?._relayoutBoundary;
assert(parentRelayoutBoundary != null);
assert(parentRelayoutBoundary != child._relayoutBoundary);
child._setRelayoutBoundary(parentRelayoutBoundary!);
}
/// Set [_relayoutBoundary] to [value] throughout this render object's
/// subtree, including this render object but stopping at relayout boundaries
/// thereafter.
void _setRelayoutBoundary(RenderObject value) {
assert(value != _relayoutBoundary);
// This may temporarily break the _relayoutBoundary invariant at children;
// the visitChildren restores the invariant.
_relayoutBoundary = value;
visitChildren(_propagateRelayoutBoundaryToChild);
}
/// Bootstrap the rendering pipeline by scheduling the very first layout.
///
/// Requires this render object to be attached and that this render object
@ -2624,8 +2583,8 @@ abstract class RenderObject with DiagnosticableTreeMixin implements HitTestTarge
assert(attached);
assert(parent == null);
assert(!owner!._debugDoingLayout);
assert(_relayoutBoundary == null);
_relayoutBoundary = this;
assert(_isRelayoutBoundary == null);
_isRelayoutBoundary = true;
assert(() {
_debugCanParentUseSize = false;
return true;
@ -2636,7 +2595,7 @@ abstract class RenderObject with DiagnosticableTreeMixin implements HitTestTarge
@pragma('vm:notify-debugger-on-exception')
void _layoutWithoutResize() {
assert(_needsLayout);
assert(_relayoutBoundary == this || this is RenderObjectWithLayoutCallbackMixin);
assert((_isRelayoutBoundary ?? false) || this is RenderObjectWithLayoutCallbackMixin);
RenderObject? debugPreviousActiveLayout;
assert(!_debugMutationsLocked);
assert(!_doingThisLayoutWithCallback);
@ -2739,14 +2698,12 @@ abstract class RenderObject with DiagnosticableTreeMixin implements HitTestTarge
);
assert(!_debugDoingThisResize);
assert(!_debugDoingThisLayout);
final bool isRelayoutBoundary =
!parentUsesSize || sizedByParent || constraints.isTight || parent == null;
final RenderObject relayoutBoundary = isRelayoutBoundary ? this : parent!._relayoutBoundary!;
assert(() {
_debugCanParentUseSize = parentUsesSize;
return true;
}());
_isRelayoutBoundary = !parentUsesSize || sizedByParent || constraints.isTight || parent == null;
if (!_needsLayout && constraints == _constraints) {
assert(() {
// in case parentUsesSize changed since the last invocation, set size
@ -2761,11 +2718,6 @@ abstract class RenderObject with DiagnosticableTreeMixin implements HitTestTarge
_debugDoingThisResize = false;
return true;
}());
if (relayoutBoundary != _relayoutBoundary) {
_setRelayoutBoundary(relayoutBoundary);
}
if (!kReleaseMode && debugProfileLayoutsEnabled) {
FlutterTimeline.finishSync();
}
@ -2773,14 +2725,6 @@ abstract class RenderObject with DiagnosticableTreeMixin implements HitTestTarge
}
_constraints = constraints;
if (_relayoutBoundary != null && relayoutBoundary != _relayoutBoundary) {
// The local relayout boundary has changed, must notify children in case
// they also need updating. Otherwise, they will be confused about what
// their actual relayout boundary is later.
visitChildren(_cleanChildRelayoutBoundary);
}
_relayoutBoundary = relayoutBoundary;
assert(!_debugMutationsLocked);
assert(!_doingThisLayoutWithCallback);
assert(() {
@ -3901,13 +3845,20 @@ abstract class RenderObject with DiagnosticableTreeMixin implements HitTestTarge
header += ' DISPOSED';
return header;
}
if (_relayoutBoundary != null && _relayoutBoundary != this) {
int count = 1;
RenderObject? target = parent;
while (target != null && target != _relayoutBoundary) {
target = target.parent;
count += 1;
int count = 0;
for (
RenderObject? node = this;
node != null && !(node._isRelayoutBoundary ?? false);
node = node.parent
) {
if (node._isRelayoutBoundary == null) {
count = -1;
break;
}
count += 1;
}
if (count > 0) {
header += ' relayoutBoundary=up$count';
}
if (_needsLayout) {

54
packages/flutter/test/widgets/layout_builder_test.dart
View File

@ -984,6 +984,60 @@ void main() {
await tester.pump();
expect(rebuilt, isTrue);
});
testWidgets('LayoutBuilder does not crash when it becomes kept-alive', (
WidgetTester tester,
) async {
final FocusNode focusNode = FocusNode();
final TextEditingController controller = TextEditingController();
addTearDown(focusNode.dispose);
addTearDown(controller.dispose);
final Widget layoutBuilderWithParent = SizedBox(
key: GlobalKey(),
child: LayoutBuilder(
builder: (BuildContext _, BoxConstraints _) {
// The text field keeps the widget alive in the SliverList.
return EditableText(
focusNode: focusNode,
backgroundCursorColor: const Color(0xFFFFFFFF),
cursorColor: const Color(0xFFFFFFFF),
style: const TextStyle(),
controller: controller,
);
},
),
);
await tester.pumpWidget(
Directionality(
textDirection: TextDirection.ltr,
child: CustomScrollView(
slivers: <Widget>[
SliverList.list(
addRepaintBoundaries: false,
addSemanticIndexes: false,
children: <Widget>[const SizedBox(height: 60), layoutBuilderWithParent],
),
],
),
),
);
focusNode.requestFocus();
await tester.pumpWidget(
Directionality(
textDirection: TextDirection.ltr,
child: CustomScrollView(
slivers: <Widget>[
SliverList.list(
addRepaintBoundaries: false,
addSemanticIndexes: false,
children: <Widget>[const SizedBox(height: 6000), layoutBuilderWithParent],
),
],
),
),
);
});
}
class _SmartLayoutBuilder extends ConstrainedLayoutBuilder<BoxConstraints> {