From 8ce3bfb6ca4c2339b6eaed3a954547fc6498e8db Mon Sep 17 00:00:00 2001 From: LongCatIsLooong <31859944+LongCatIsLooong@users.noreply.github.com> Date: Wed, 5 Jun 2024 13:51:04 -0700 Subject: [PATCH] Remove temporary LayoutBuilder migration flag, defer `markNeedsLayout` (#149637) Looks like I did not need this flag since flutter_portal pinned flutter version to 3.10 in their presubmits. I'll create a g3 fix for this since there're only a handful of scuba failures. Also a g3 test revealed a problem with calling `renderObject.markNeedsLayout` directly. It has to be deferred so the render tree is clean during the idle phase and the postFrameCallback phase. --- .../lib/src/widgets/layout_builder.dart | 42 +++++++++++++++---- .../flutter/test/widgets/framework_test.dart | 3 -- .../layout_builder_and_global_keys_test.dart | 3 -- .../test/widgets/layout_builder_test.dart | 27 ++++++++++-- 4 files changed, 58 insertions(+), 17 deletions(-) diff --git a/packages/flutter/lib/src/widgets/layout_builder.dart b/packages/flutter/lib/src/widgets/layout_builder.dart index 69efb2dfaa7..a9a5066155c 100644 --- a/packages/flutter/lib/src/widgets/layout_builder.dart +++ b/packages/flutter/lib/src/widgets/layout_builder.dart @@ -4,6 +4,7 @@ import 'package:flutter/foundation.dart'; import 'package:flutter/rendering.dart'; +import 'package:flutter/scheduler.dart'; import 'debug.dart'; import 'framework.dart'; @@ -81,9 +82,40 @@ class _LayoutBuilderElement extends RenderOb Element? _child; @override - BuildScope get buildScope => LayoutBuilder.applyDoubleRebuildFix ? _buildScope : super.buildScope; + BuildScope get buildScope => _buildScope; - late final BuildScope _buildScope = BuildScope(scheduleRebuild: renderObject.markNeedsLayout); + late final BuildScope _buildScope = BuildScope(scheduleRebuild: _scheduleRebuild); + + // To schedule a rebuild, markNeedsLayout needs to be called on this Element's + // render object (as the rebuilding is done in its performLayout call). However, + // the render tree should typically be kept clean during the postFrameCallbacks + // and the idle phase, so the layout data can be safely read. + bool _deferredCallbackScheduled = false; + void _scheduleRebuild() { + if (_deferredCallbackScheduled) { + return; + } + + final bool deferMarkNeedsLayout = switch (SchedulerBinding.instance.schedulerPhase) { + SchedulerPhase.idle || SchedulerPhase.postFrameCallbacks => true, + SchedulerPhase.transientCallbacks || SchedulerPhase.midFrameMicrotasks || SchedulerPhase.persistentCallbacks => false, + }; + if (!deferMarkNeedsLayout) { + renderObject.markNeedsLayout(); + return; + } + _deferredCallbackScheduled = true; + SchedulerBinding.instance.scheduleFrameCallback(_frameCallback); + } + + void _frameCallback(Duration timestamp) { + _deferredCallbackScheduled = false; + // This method is only called when the render tree is stable, if the Element + // is deactivated it will never be reincorporated back to the tree. + if (mounted) { + renderObject.markNeedsLayout(); + } + } @override void visitChildren(ElementVisitor visitor) { @@ -284,12 +316,6 @@ class LayoutBuilder extends ConstrainedLayoutBuilder { required super.builder, }); - /// Temporary flag that controls whether [LayoutBuilder]s and - /// [SliverLayoutBuilder]s should apply the double rebuild fix. This flag is - /// for migration only and **SHOULD NOT BE USED**. - @Deprecated('This is a temporary migration flag. DO NOT USE THIS.') // flutter_ignore: deprecation_syntax (see analyze.dart) - static bool applyDoubleRebuildFix = false; - @override RenderObject createRenderObject(BuildContext context) => _RenderLayoutBuilder(); } diff --git a/packages/flutter/test/widgets/framework_test.dart b/packages/flutter/test/widgets/framework_test.dart index 2313e352c85..8d155eba657 100644 --- a/packages/flutter/test/widgets/framework_test.dart +++ b/packages/flutter/test/widgets/framework_test.dart @@ -23,9 +23,6 @@ class _MyGlobalObjectKey> extends GlobalObjectKe } void main() { - setUp(() { LayoutBuilder.applyDoubleRebuildFix = true; }); - tearDown(() { LayoutBuilder.applyDoubleRebuildFix = false; }); - testWidgets('UniqueKey control test', (WidgetTester tester) async { final Key key = UniqueKey(); expect(key, hasOneLineDescription); diff --git a/packages/flutter/test/widgets/layout_builder_and_global_keys_test.dart b/packages/flutter/test/widgets/layout_builder_and_global_keys_test.dart index 4082255f5bd..4552943f6ed 100644 --- a/packages/flutter/test/widgets/layout_builder_and_global_keys_test.dart +++ b/packages/flutter/test/widgets/layout_builder_and_global_keys_test.dart @@ -41,9 +41,6 @@ class StatefulWrapperState extends State { } void main() { - setUp(() { LayoutBuilder.applyDoubleRebuildFix = true; }); - tearDown(() { LayoutBuilder.applyDoubleRebuildFix = false; }); - testWidgets('Moving global key inside a LayoutBuilder', (WidgetTester tester) async { final GlobalKey key = GlobalKey(); await tester.pumpWidget( diff --git a/packages/flutter/test/widgets/layout_builder_test.dart b/packages/flutter/test/widgets/layout_builder_test.dart index 216957d2ed8..51fbf778e03 100644 --- a/packages/flutter/test/widgets/layout_builder_test.dart +++ b/packages/flutter/test/widgets/layout_builder_test.dart @@ -7,9 +7,6 @@ import 'package:flutter/widgets.dart'; import 'package:flutter_test/flutter_test.dart'; void main() { - setUp(() { LayoutBuilder.applyDoubleRebuildFix = true; }); - tearDown(() { LayoutBuilder.applyDoubleRebuildFix = false; }); - testWidgets('LayoutBuilder parent size', (WidgetTester tester) async { late Size layoutBuilderSize; final Key childKey = UniqueKey(); @@ -334,6 +331,30 @@ void main() { expect(built, 2); }); + testWidgets('LayoutBuilder does not dirty the render tree during the idle phase', (WidgetTester tester) async { + RenderObject? dirtyRenderObject; + void visitSubtree(RenderObject node) { + assert(dirtyRenderObject == null); + if (node.debugNeedsLayout) { + dirtyRenderObject = node; + return; + } + node.visitChildren(visitSubtree); + } + + final Widget target = LayoutBuilder( + builder: (BuildContext context, BoxConstraints constraints) => const Placeholder(), + ); + await tester.pumpWidget(target); + final RenderObject renderObject = tester.renderObject(find.byWidget(target)); + visitSubtree(renderObject); + expect(dirtyRenderObject, isNull); + + tester.element(find.byType(Placeholder)).markNeedsBuild(); + visitSubtree(renderObject); + expect(dirtyRenderObject, isNull); + }); + testWidgets('LayoutBuilder can change size without rebuild', (WidgetTester tester) async { int built = 0; final Widget target = LayoutBuilder(