From 1f15e06e45e820a5bc36cff1870e70e542f06f2c Mon Sep 17 00:00:00 2001 From: Ian Hickson Date: Wed, 7 Sep 2016 14:45:19 -0700 Subject: [PATCH] Handle stateful widgets in layout builders. (#5752) Previously, if a StatefulWidget was marked dirty, then removed from the build, then reinserted using the exact same widget under a widget under a LayoutBuilder, it wouldn't rebuild. This fixes that. It also introduces an assert that's supposed to catch SizeObserver-like behaviour. Rather than make this patch even bigger, I papered over two pre-existing bugs which this assert uncovered (and fixed the other problems it found): https://github.com/flutter/flutter/issues/5751 https://github.com/flutter/flutter/issues/5749 We should fix those before 1.0 though. --- .../lib/stocks/build_bench.dart | 2 +- .../flutter/lib/src/material/drop_down.dart | 7 +- packages/flutter/lib/src/material/tabs.dart | 18 +- packages/flutter/lib/src/scheduler/debug.dart | 12 ++ packages/flutter/lib/src/widgets/binding.dart | 85 +++++++-- .../flutter/lib/src/widgets/framework.dart | 165 +++++++++++------- .../lib/src/widgets/layout_builder.dart | 15 +- .../flutter/lib/src/widgets/lazy_block.dart | 8 +- .../flutter/lib/src/widgets/scrollable.dart | 14 +- .../lib/src/widgets/semantics_debugger.dart | 15 +- .../lib/src/widgets/virtual_viewport.dart | 2 +- .../independent_widget_layout_test.dart | 2 +- .../widget/layout_builder_and_state_test.dart | 87 +++++++++ packages/flutter_test/lib/src/binding.dart | 53 +++--- 14 files changed, 353 insertions(+), 132 deletions(-) create mode 100644 packages/flutter/test/widget/layout_builder_and_state_test.dart diff --git a/dev/benchmarks/microbenchmarks/lib/stocks/build_bench.dart b/dev/benchmarks/microbenchmarks/lib/stocks/build_bench.dart index d53421652da..709487d89a1 100644 --- a/dev/benchmarks/microbenchmarks/lib/stocks/build_bench.dart +++ b/dev/benchmarks/microbenchmarks/lib/stocks/build_bench.dart @@ -30,7 +30,7 @@ Future main() async { watch.start(); while (watch.elapsed < kBenchmarkTime) { appState.markNeedsBuild(); - buildOwner.buildDirtyElements(); + buildOwner.buildScope(WidgetsBinding.instance.renderViewElement); iterations += 1; } watch.stop(); diff --git a/packages/flutter/lib/src/material/drop_down.dart b/packages/flutter/lib/src/material/drop_down.dart index af46d266134..c107e335153 100644 --- a/packages/flutter/lib/src/material/drop_down.dart +++ b/packages/flutter/lib/src/material/drop_down.dart @@ -5,6 +5,7 @@ import 'dart:async'; import 'dart:math' as math; +import 'package:flutter/scheduler.dart'; import 'package:flutter/widgets.dart'; import 'package:meta/meta.dart'; @@ -249,7 +250,11 @@ class _DropDownMenuRouteLayout extends SingleChildLayoutDelegate { if (route.initialLayout) { route.initialLayout = false; final double scrollOffset = selectedItemOffset - (buttonTop - top); - scrollableKey.currentState.scrollTo(scrollOffset); + SchedulerBinding.instance.addPostFrameCallback((Duration timeStamp) { + // TODO(ianh): Compute and set this during layout instead of being + // lagged by one frame. https://github.com/flutter/flutter/issues/5751 + scrollableKey.currentState.scrollTo(scrollOffset); + }); } return new Offset(buttonRect.left, top); diff --git a/packages/flutter/lib/src/material/tabs.dart b/packages/flutter/lib/src/material/tabs.dart index 83d7c536597..227dd635c24 100644 --- a/packages/flutter/lib/src/material/tabs.dart +++ b/packages/flutter/lib/src/material/tabs.dart @@ -7,6 +7,7 @@ import 'dart:math' as math; import 'package:flutter/physics.dart'; import 'package:flutter/rendering.dart'; +import 'package:flutter/scheduler.dart'; import 'package:flutter/widgets.dart'; import 'package:meta/meta.dart'; @@ -968,11 +969,18 @@ class _TabBarState extends ScrollableState> implements TabBarSelect } void _layoutChanged(Size tabBarSize, List tabWidths) { - setState(() { - _tabBarSize = tabBarSize; - _tabWidths = tabWidths; - _indicatorRect = _selection != null ? _tabIndicatorRect(_selection.index) : Rect.zero; - _updateScrollBehavior(); + // This is bad. We should use a LayoutBuilder or CustomMultiChildLayout or some such. + // As designed today, tabs are always lagging one frame behind, taking two frames + // to handle a layout change. + _tabBarSize = tabBarSize; + _tabWidths = tabWidths; + _indicatorRect = _selection != null ? _tabIndicatorRect(_selection.index) : Rect.zero; + _updateScrollBehavior(); + SchedulerBinding.instance.addPostFrameCallback((Duration timeStamp) { + setState(() { + // the changes were made at layout time + // TODO(ianh): remove this setState: https://github.com/flutter/flutter/issues/5749 + }); }); } diff --git a/packages/flutter/lib/src/scheduler/debug.dart b/packages/flutter/lib/src/scheduler/debug.dart index 405551e72b2..2d8787f9a68 100644 --- a/packages/flutter/lib/src/scheduler/debug.dart +++ b/packages/flutter/lib/src/scheduler/debug.dart @@ -3,6 +3,18 @@ // found in the LICENSE file. /// Print a banner at the beginning of each frame. +/// +/// Frames triggered by the engine and handler by the scheduler binding will +/// have a banner saying "Begin Frame" and giving the time stamp of the frame. +/// +/// Frames triggered eagerly by the widget framework (e.g. when calling +/// [runApp]) will have a label saying "Begin Warm-Up Frame". +/// +/// To include a banner at the end of each frame as well, to distinguish +/// intra-frame output from inter-frame output, set [debugPrintEndFrameBanner] +/// to true as well. +/// +/// See [SchedulerBinding.beginFrame]. bool debugPrintBeginFrameBanner = false; /// Print a banner at the end of each frame. diff --git a/packages/flutter/lib/src/widgets/binding.dart b/packages/flutter/lib/src/widgets/binding.dart index ba93d82d026..45a3f5ab66c 100644 --- a/packages/flutter/lib/src/widgets/binding.dart +++ b/packages/flutter/lib/src/widgets/binding.dart @@ -10,6 +10,7 @@ import 'package:flutter/gestures.dart'; import 'package:flutter/rendering.dart'; import 'package:flutter/scheduler.dart'; import 'package:flutter/services.dart'; +import 'package:meta/meta.dart'; import 'app.dart'; import 'framework.dart'; @@ -191,15 +192,41 @@ abstract class WidgetsBinding extends BindingBase implements GestureBinding, Ren } void _handleBuildScheduled() { - // If we're in the process of building dirty elements, we're know that any - // builds that are scheduled will be run this frame, which means we don't - // need to schedule another frame. - if (_buildingDirtyElements) - return; + // If we're in the process of building dirty elements, then changes + // should not trigger a new frame. + assert(() { + if (debugBuildingDirtyElements) { + throw new FlutterError( + 'Build scheduled during frame.\n' + 'While the widget tree was being built, laid out, and painted, ' + 'a new frame was scheduled to rebuild the widget tree. ' + 'This might be because setState() was called from a layout or ' + 'paint callback. ' + 'If a change is needed to the widget tree, it should be applied ' + 'as the tree is being built. Scheduling a change for the subsequent ' + 'frame instead results in an interface that lags behind by one frame. ' + 'If this was done to make your build dependent on a size measured at ' + 'layout time, consider using a LayoutBuilder, CustomSingleChildLayout, ' + 'or CustomMultiChildLayout. If, on the other hand, the one frame delay ' + 'is the desired effect, for example because this is an ' + 'animation, consider scheduling the frame in a post-frame callback ' + 'using SchedulerBinding.addPostFrameCallback or ' + 'using an AnimationController to trigger the animation.' + ); + } + return true; + }); scheduleFrame(); } - bool _buildingDirtyElements = false; + /// Whether we are currently in a frame. This is used to verify + /// that frames are not scheduled redundantly. + /// + /// This is public so that test frameworks can change it. + /// + /// This flag is not used in release builds. + @protected + bool debugBuildingDirtyElements = false; /// Pump the build and rendering pipeline to generate a frame. /// @@ -260,12 +287,21 @@ abstract class WidgetsBinding extends BindingBase implements GestureBinding, Ren // When editing the above, also update rendering/binding.dart's copy. @override void beginFrame() { - assert(!_buildingDirtyElements); - _buildingDirtyElements = true; - buildOwner.buildDirtyElements(); - _buildingDirtyElements = false; - super.beginFrame(); - buildOwner.finalizeTree(); + assert(!debugBuildingDirtyElements); + assert(() { + debugBuildingDirtyElements = true; + return true; + }); + try { + buildOwner.buildScope(renderViewElement); + super.beginFrame(); + buildOwner.finalizeTree(); + } finally { + assert(() { + debugBuildingDirtyElements = false; + return true; + }); + } // TODO(ianh): Following code should not be included in release mode, only profile and debug modes. // See https://github.com/dart-lang/sdk/issues/27192 if (_needToReportFirstFrame) { @@ -291,7 +327,17 @@ abstract class WidgetsBinding extends BindingBase implements GestureBinding, Ren debugShortDescription: '[root]', child: app ).attachToRenderTree(buildOwner, renderViewElement); + assert(() { + if (debugPrintBeginFrameBanner) + debugPrint('━━━━━━━┫ Begin Warm-Up Frame ┣━━━━━━━'); + return true; + }); beginFrame(); + assert(() { + if (debugPrintEndFrameBanner) + debugPrint('━━━━━━━┫ End of Warm-Up Frame ┣━━━━━━━'); + return true; + }); } @override @@ -364,15 +410,20 @@ class RenderObjectToWidgetAdapter extends RenderObjectWi /// /// Used by [runApp] to bootstrap applications. RenderObjectToWidgetElement attachToRenderTree(BuildOwner owner, [RenderObjectToWidgetElement element]) { - owner.lockState(() { - if (element == null) { + if (element == null) { + owner.lockState(() { element = createElement(); + assert(element != null); element.assignOwner(owner); + }); + owner.buildScope(element, () { element.mount(null, null); - } else { + }); + } else { + owner.buildScope(element, () { element.update(this); - } - }, building: true); + }); + } return element; } diff --git a/packages/flutter/lib/src/widgets/framework.dart b/packages/flutter/lib/src/widgets/framework.dart index e0fd09a07b3..11a15486233 100644 --- a/packages/flutter/lib/src/widgets/framework.dart +++ b/packages/flutter/lib/src/widgets/framework.dart @@ -1410,12 +1410,16 @@ class BuildOwner { final _InactiveElements _inactiveElements = new _InactiveElements(); final List _dirtyElements = []; + bool _scheduledFlushDirtyElements = false; /// Adds an element to the dirty elements list so that it will be rebuilt - /// when [buildDirtyElements] is called. + /// when [WidgetsBinding.beginFrame] calls [buildScope]. void scheduleBuildFor(BuildableElement element) { + assert(element != null); + assert(element.owner == this); + assert(element._inDirtyList == _dirtyElements.contains(element)); assert(() { - if (_dirtyElements.contains(element)) { + if (element._inDirtyList) { throw new FlutterError( 'scheduleBuildFor() called for a widget for which a build was already scheduled.\n' 'The method was called for the following element:\n' @@ -1440,9 +1444,12 @@ class BuildOwner { } return true; }); - if (_dirtyElements.isEmpty && onBuildScheduled != null) + if (!_scheduledFlushDirtyElements && onBuildScheduled != null) { + _scheduledFlushDirtyElements = true; onBuildScheduled(); + } _dirtyElements.add(element); + element._inDirtyList = true; } int _debugStateLockLevel = 0; @@ -1450,24 +1457,15 @@ class BuildOwner { bool _debugBuilding = false; BuildableElement _debugCurrentBuildTarget; - /// Establishes a scope in which calls to [State.setState] are forbidden. + /// Establishes a scope in which calls to [State.setState] are forbidden, and + /// calls the given `callback`. /// - /// This mechanism prevents build functions from transitively requiring other - /// build functions to run, potentially causing infinite loops. - /// - /// If the building argument is true, then this function enables additional - /// asserts that check invariants that should apply during building. - /// - /// The context argument is used to describe the scope in case an exception is - /// caught while invoking the callback. - void lockState(void callback(), { bool building: false }) { - bool debugPreviouslyBuilding; + /// This mechanism is used to ensure that, for instance, [State.dispose] does + /// not call [State.setState]. + void lockState(void callback()) { + assert(callback != null); assert(_debugStateLockLevel >= 0); assert(() { - if (building) { - debugPreviouslyBuilding = _debugBuilding; - _debugBuilding = true; - } _debugStateLockLevel += 1; return true; }); @@ -1476,10 +1474,75 @@ class BuildOwner { } finally { assert(() { _debugStateLockLevel -= 1; - if (building) { - assert(_debugBuilding); - _debugBuilding = debugPreviouslyBuilding; + return true; + }); + } + assert(_debugStateLockLevel >= 0); + } + + /// Establishes a scope for updating the widget tree, and calls the given + /// `callback`, if any. Then, builds all the elements that were marked as + /// dirty using [scheduleBuildFor], in depth order. + /// + /// This mechanism prevents build functions from transitively requiring other + /// build functions to run, potentially causing infinite loops. + /// + /// The dirty list is processed after `callback` returns, building all the + /// elements that were marked as dirty using [scheduleBuildFor], in depth + /// order. If elements are marked as dirty while this method is running, they + /// must be deeper than the `context` node, and deeper than any + /// previously-built node in this pass. + /// + /// To flush the current dirty list without performing any other work, this + /// function can be called with no callback. This is what the framework does + /// each frame, in [WidgetsBinding.beginFrame]. + /// + /// Only one [buildScope] can be active at a time. + /// + /// A [buildScope] implies a [lockState] scope as well. + void buildScope(Element context, [VoidCallback callback]) { + if (callback == null && _dirtyElements.isEmpty) + return; + assert(context != null); + assert(_debugStateLockLevel >= 0); + assert(!_debugBuilding); + assert(() { + _debugStateLockLevel += 1; + _debugBuilding = true; + return true; + }); + Timeline.startSync('Build'); + try { + _scheduledFlushDirtyElements = true; + if (callback != null) + callback(); + _dirtyElements.sort(_elementSort); + int dirtyCount = _dirtyElements.length; + int index = 0; + while (index < dirtyCount) { + assert(_dirtyElements[index] != null); + assert(_dirtyElements[index]._inDirtyList); + assert(!_dirtyElements[index]._active || _dirtyElements[index]._debugIsInScope(context)); + _dirtyElements[index].rebuild(); + index += 1; + if (dirtyCount < _dirtyElements.length) { + _dirtyElements.sort(_elementSort); + dirtyCount = _dirtyElements.length; } + } + } finally { + assert(!_dirtyElements.any((BuildableElement element) => element._active && element.dirty)); + for (BuildableElement element in _dirtyElements) { + assert(element._inDirtyList); + element._inDirtyList = false; + } + _dirtyElements.clear(); + _scheduledFlushDirtyElements = false; + Timeline.finishSync(); + assert(_debugBuilding); + assert(() { + _debugBuilding = false; + _debugStateLockLevel -= 1; return true; }); } @@ -1498,36 +1561,6 @@ class BuildOwner { return 0; } - /// Builds all the elements that were marked as dirty using - /// [scheduleBuildFor], in depth order. If elements are marked as dirty while - /// this runs, they must be deeper than the algorithm has yet reached. - /// - /// This is called by [WidgetsBinding.beginFrame]. - void buildDirtyElements() { - if (_dirtyElements.isEmpty) - return; - Timeline.startSync('Build'); - try { - lockState(() { - _dirtyElements.sort(_elementSort); - int dirtyCount = _dirtyElements.length; - int index = 0; - while (index < dirtyCount) { - _dirtyElements[index].rebuild(); - index += 1; - if (dirtyCount < _dirtyElements.length) { - _dirtyElements.sort(_elementSort); - dirtyCount = _dirtyElements.length; - } - } - assert(!_dirtyElements.any((BuildableElement element) => element._active && element.dirty)); - }, building: true); - } finally { - _dirtyElements.clear(); - Timeline.finishSync(); - } - } - /// Complete the element build pass by unmounting any elements that are no /// longer active. /// @@ -1607,6 +1640,15 @@ abstract class Element implements BuildContext { }); } + bool _debugIsInScope(Element target) { + assert(target != null); + if (target == this) + return true; + if (_parent != null) + return _parent._debugIsInScope(target); + return false; + } + RenderObject get renderObject { RenderObject result; void visit(Element element) { @@ -1875,6 +1917,7 @@ abstract class Element implements BuildContext { }); assert(_debugLifecycleState == _ElementLifecycle.inactive); assert(widget != null); + assert(owner != null); assert(depth != null); assert(!_active); _active = true; @@ -2091,6 +2134,10 @@ abstract class BuildableElement extends Element { bool get dirty => _dirty; bool _dirty = true; + // Whether this is in owner._dirtyElements. This is used to know whether we + // should be adding the element back into the list when it's reactivated. + bool _inDirtyList = false; + // We let widget authors call setState from initState, didUpdateConfig, and // build even when state is locked because its convenient and a no-op anyway. // This flag ensures that this convenience is only allowed on the element @@ -2124,15 +2171,7 @@ abstract class BuildableElement extends Element { // a current build target when we're building. return true; } - bool foundTarget = false; - visitAncestorElements((Element element) { - if (element == owner._debugCurrentBuildTarget) { - foundTarget = true; - return false; - } - return true; - }); - if (foundTarget) + if (_debugIsInScope(owner._debugCurrentBuildTarget)) return true; } if (owner._debugStateLocked && (!_debugAllowIgnoredCallsToMarkNeedsBuild || !dirty)) { @@ -2212,7 +2251,9 @@ abstract class BuildableElement extends Element { void activate() { final bool shouldRebuild = ((_dependencies != null && _dependencies.length > 0) || _hadUnsatisfiedDependencies); super.activate(); // clears _dependencies, and sets active to true - if (shouldRebuild) { + if (_dirty && !_inDirtyList) { + owner.scheduleBuildFor(this); + } else if (shouldRebuild) { assert(_active); // otherwise markNeedsBuild is a no-op markNeedsBuild(); } @@ -2904,7 +2945,7 @@ abstract class RootRenderObjectElement extends RenderObjectElement { /// The [WidgetsBinding] introduces the primary owner, /// [WidgetsBinding.buildOwner], and assigns it to the widget tree in the call /// to [runApp]. The binding is responsible for driving the build pipeline by - /// calling the build owner's [BuildOwner.buildDirtyElements] method. See + /// calling the build owner's [BuildOwner.buildScope] method. See /// [WidgetsBinding.beginFrame]. void assignOwner(BuildOwner owner) { _owner = owner; diff --git a/packages/flutter/lib/src/widgets/layout_builder.dart b/packages/flutter/lib/src/widgets/layout_builder.dart index f41cff68c81..c233ec393b9 100644 --- a/packages/flutter/lib/src/widgets/layout_builder.dart +++ b/packages/flutter/lib/src/widgets/layout_builder.dart @@ -176,9 +176,9 @@ class _LayoutBuilderElement extends RenderObjectElement { } void _layout(BoxConstraints constraints) { - Widget built; - if (widget.builder != null) { - owner.lockState(() { + owner.buildScope(this, () { + Widget built; + if (widget.builder != null) { try { built = widget.builder(this, constraints); debugWidgetBuilderValue(widget, built); @@ -186,13 +186,6 @@ class _LayoutBuilderElement extends RenderObjectElement { _debugReportException('building $widget', e, stack); built = new ErrorWidget(e); } - }); - } - owner.lockState(() { - if (widget.builder == null) { - if (_child != null) - _child = updateChild(_child, null, null); - return; } try { _child = updateChild(_child, built, null); @@ -202,7 +195,7 @@ class _LayoutBuilderElement extends RenderObjectElement { built = new ErrorWidget(e); _child = updateChild(null, built, slot); } - }, building: true); + }); } @override diff --git a/packages/flutter/lib/src/widgets/lazy_block.dart b/packages/flutter/lib/src/widgets/lazy_block.dart index 2df11708f6c..9e3f5a86804 100644 --- a/packages/flutter/lib/src/widgets/lazy_block.dart +++ b/packages/flutter/lib/src/widgets/lazy_block.dart @@ -629,11 +629,11 @@ class _LazyBlockElement extends RenderObjectElement { while (currentLogicalIndex > 0 && currentLogicalOffset > startLogicalOffset) { currentLogicalIndex -= 1; Element newElement; - owner.lockState(() { + owner.buildScope(this, () { Widget newWidget = _callBuilder(builder, currentLogicalIndex, requireNonNull: true); newWidget = new RepaintBoundary.wrap(newWidget, currentLogicalIndex); newElement = inflateWidget(newWidget, null); - }, building: true); + }); newChildren.add(newElement); RenderBox child = block.firstChild; assert(child == newChildren.last.renderObject); @@ -698,14 +698,14 @@ class _LazyBlockElement extends RenderObjectElement { if (physicalIndex >= _children.length) { assert(physicalIndex == _children.length); Element newElement; - owner.lockState(() { + owner.buildScope(this, () { Widget newWidget = _callBuilder(builder, currentLogicalIndex); if (newWidget == null) return; newWidget = new RepaintBoundary.wrap(newWidget, currentLogicalIndex); Element previousChild = _children.isEmpty ? null : _children.last; newElement = inflateWidget(newWidget, previousChild); - }, building: true); + }); if (newElement == null) break; _children.add(newElement); diff --git a/packages/flutter/lib/src/widgets/scrollable.dart b/packages/flutter/lib/src/widgets/scrollable.dart index dba087213e8..411006ba5f0 100644 --- a/packages/flutter/lib/src/widgets/scrollable.dart +++ b/packages/flutter/lib/src/widgets/scrollable.dart @@ -373,6 +373,7 @@ class ScrollableState extends State { } void _handleAnimationStatusChanged(AnimationStatus status) { + // this is not called when stop() is called on the controller setState(() { if (!_controller.isAnimating) _simulation = null; @@ -454,7 +455,8 @@ class ScrollableState extends State { /// If there are no in-progress scrolling physics, this function scrolls to /// the given offset instead. void didUpdateScrollBehavior(double newScrollOffset) { - setState(() { /* The scroll behavior is part of our build state. */ }); + // This does not call setState, because if anything below actually + // changes our build, it will itself independently trigger a frame. assert(_controller.isAnimating || _simulation == null); if (_numberOfInProgressScrolls > 0) { if (_simulation != null) { @@ -583,16 +585,16 @@ class ScrollableState extends State { } void _handleDragDown(_) { - _stop(); + setState(() { + _stop(); + }); } void _stop() { assert(mounted); assert(_controller.isAnimating || _simulation == null); - setState(() { - _controller.stop(); - _simulation = null; - }); + _controller.stop(); // this does not trigger a status notification + _simulation = null; } void _handleDragStart(DragStartDetails details) { diff --git a/packages/flutter/lib/src/widgets/semantics_debugger.dart b/packages/flutter/lib/src/widgets/semantics_debugger.dart index c3bd2f5bf42..b651f2cf1b4 100644 --- a/packages/flutter/lib/src/widgets/semantics_debugger.dart +++ b/packages/flutter/lib/src/widgets/semantics_debugger.dart @@ -5,6 +5,8 @@ import 'dart:collection'; import 'dart:math' as math; +import 'package:flutter/foundation.dart'; +import 'package:flutter/scheduler.dart'; import 'package:flutter/rendering.dart'; import 'package:sky_services/semantics/semantics.mojom.dart' as mojom; @@ -53,8 +55,17 @@ class _SemanticsDebuggerState extends State { } void _update() { - setState(() { - // the generation of the _SemanticsDebuggerListener has changed + SchedulerBinding.instance.addPostFrameCallback((Duration timeStamp) { + // We want the update to take effect next frame, so to make that + // explicit we call setState() in a post-frame callback. + if (mounted) { + // If we got disposed this frame, we will still get an update, + // because the inactive list is flushed after the semantics updates + // are transmitted to the semantics clients. + setState(() { + // The generation of the _SemanticsDebuggerListener has changed. + }); + } }); } diff --git a/packages/flutter/lib/src/widgets/virtual_viewport.dart b/packages/flutter/lib/src/widgets/virtual_viewport.dart index 8eb1b0a2a05..06e6f13d40b 100644 --- a/packages/flutter/lib/src/widgets/virtual_viewport.dart +++ b/packages/flutter/lib/src/widgets/virtual_viewport.dart @@ -175,7 +175,7 @@ abstract class VirtualViewportElement extends RenderObjectElement { assert(startOffsetBase != null); assert(startOffsetLimit != null); _updatePaintOffset(); - owner.lockState(_materializeChildren, building: true); + owner.buildScope(this, _materializeChildren); } void _materializeChildren() { diff --git a/packages/flutter/test/widget/independent_widget_layout_test.dart b/packages/flutter/test/widget/independent_widget_layout_test.dart index 54661bc9208..5afc0d4ea23 100644 --- a/packages/flutter/test/widget/independent_widget_layout_test.dart +++ b/packages/flutter/test/widget/independent_widget_layout_test.dart @@ -38,7 +38,7 @@ class OffscreenWidgetTree { } void pumpFrame() { - buildOwner.buildDirtyElements(); + buildOwner.buildScope(root); pipelineOwner.flushLayout(); pipelineOwner.flushCompositingBits(); pipelineOwner.flushPaint(); diff --git a/packages/flutter/test/widget/layout_builder_and_state_test.dart b/packages/flutter/test/widget/layout_builder_and_state_test.dart new file mode 100644 index 00000000000..c1ad973bc1c --- /dev/null +++ b/packages/flutter/test/widget/layout_builder_and_state_test.dart @@ -0,0 +1,87 @@ +// Copyright 2016 The Chromium 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_test/flutter_test.dart' hide TypeMatcher; +import 'package:flutter/widgets.dart'; +import 'test_widgets.dart'; + +class StatefulWrapper extends StatefulWidget { + StatefulWrapper({ + Key key, + this.child, + }) : super(key: key); + + final Widget child; + + @override + StatefulWrapperState createState() => new StatefulWrapperState(); +} + +class StatefulWrapperState extends State { + + void trigger() { + setState(() { /* no-op setState */ }); + } + + bool built = false; + + @override + Widget build(BuildContext context) { + built = true; + return config.child; + } +} + +class Wrapper extends StatelessWidget { + Wrapper({ + Key key, + this.child, + }) : super(key: key); + + final Widget child; + + @override + Widget build(BuildContext context) { + return child; + } +} + +void main() { + testWidgets('Calling setState on a widget that moves into a LayoutBuilder in the same frame', (WidgetTester tester) async { + StatefulWrapperState statefulWrapper; + final Widget inner = new Wrapper( + child: new StatefulWrapper( + key: new GlobalKey(), + child: new Container(), + ), + ); + await tester.pumpWidget(new FlipWidget( + left: new LayoutBuilder(builder: (BuildContext context, BoxConstraints constraints) { + return inner; + }), + right: inner, + )); + statefulWrapper = tester.state(find.byType(StatefulWrapper)); + expect(statefulWrapper.built, true); + statefulWrapper.built = false; + + statefulWrapper.trigger(); + flipStatefulWidget(tester); + await tester.pump(); + expect(statefulWrapper.built, true); + statefulWrapper.built = false; + + statefulWrapper.trigger(); + flipStatefulWidget(tester); + await tester.pump(); + expect(statefulWrapper.built, true); + statefulWrapper.built = false; + + statefulWrapper.trigger(); + flipStatefulWidget(tester); + await tester.pump(); + expect(statefulWrapper.built, true); + statefulWrapper.built = false; + }); +} diff --git a/packages/flutter_test/lib/src/binding.dart b/packages/flutter_test/lib/src/binding.dart index 4d4d5907ae9..ae6a18db1f3 100644 --- a/packages/flutter_test/lib/src/binding.dart +++ b/packages/flutter_test/lib/src/binding.dart @@ -22,9 +22,12 @@ import 'stack_manipulation.dart'; /// Phases that can be reached by [WidgetTester.pumpWidget] and /// [TestWidgetsFlutterBinding.pump]. +/// +/// See [WidgetsBinding.beginFrame] for a more detailed description of some of +/// these phases. // TODO(ianh): Merge with near-identical code in the rendering test code. enum EnginePhase { - /// The build phase in the widgets library. See [BuildOwner.buildDirtyElements]. + /// The build phase in the widgets library. See [BuildOwner.buildScope]. build, /// The layout phase in the rendering library. See [PipelineOwner.flushLayout]. @@ -396,6 +399,9 @@ abstract class TestWidgetsFlutterBinding extends BindingBase /// /// This binding controls time, allowing tests to verify long /// animation sequences without having to execute them in real time. +/// +/// This class assumes it is always run in checked mode (since tests are always +/// run in checked mode). class AutomatedTestWidgetsFlutterBinding extends TestWidgetsFlutterBinding { @override void initInstances() { @@ -447,26 +453,31 @@ class AutomatedTestWidgetsFlutterBinding extends TestWidgetsFlutterBinding { @override void beginFrame() { assert(inTest); - buildOwner.buildDirtyElements(); - if (_phase == EnginePhase.build) - return; - assert(renderView != null); - pipelineOwner.flushLayout(); - if (_phase == EnginePhase.layout) - return; - pipelineOwner.flushCompositingBits(); - if (_phase == EnginePhase.compositingBits) - return; - pipelineOwner.flushPaint(); - if (_phase == EnginePhase.paint) - return; - renderView.compositeFrame(); // this sends the bits to the GPU - if (_phase == EnginePhase.composite) - return; - pipelineOwner.flushSemantics(); - if (_phase == EnginePhase.flushSemantics) - return; - buildOwner.finalizeTree(); + try { + debugBuildingDirtyElements = true; + buildOwner.buildScope(renderViewElement); + if (_phase == EnginePhase.build) + return; + assert(renderView != null); + pipelineOwner.flushLayout(); + if (_phase == EnginePhase.layout) + return; + pipelineOwner.flushCompositingBits(); + if (_phase == EnginePhase.compositingBits) + return; + pipelineOwner.flushPaint(); + if (_phase == EnginePhase.paint) + return; + renderView.compositeFrame(); // this sends the bits to the GPU + if (_phase == EnginePhase.composite) + return; + pipelineOwner.flushSemantics(); + if (_phase == EnginePhase.flushSemantics) + return; + } finally { + buildOwner.finalizeTree(); + debugBuildingDirtyElements = false; + } } @override