Commit 1f15e06e authored by Ian Hickson's avatar Ian Hickson Committed by GitHub

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.
parent 691c25fa
...@@ -30,7 +30,7 @@ Future<Null> main() async { ...@@ -30,7 +30,7 @@ Future<Null> main() async {
watch.start(); watch.start();
while (watch.elapsed < kBenchmarkTime) { while (watch.elapsed < kBenchmarkTime) {
appState.markNeedsBuild(); appState.markNeedsBuild();
buildOwner.buildDirtyElements(); buildOwner.buildScope(WidgetsBinding.instance.renderViewElement);
iterations += 1; iterations += 1;
} }
watch.stop(); watch.stop();
......
...@@ -5,6 +5,7 @@ ...@@ -5,6 +5,7 @@
import 'dart:async'; import 'dart:async';
import 'dart:math' as math; import 'dart:math' as math;
import 'package:flutter/scheduler.dart';
import 'package:flutter/widgets.dart'; import 'package:flutter/widgets.dart';
import 'package:meta/meta.dart'; import 'package:meta/meta.dart';
...@@ -249,7 +250,11 @@ class _DropDownMenuRouteLayout<T> extends SingleChildLayoutDelegate { ...@@ -249,7 +250,11 @@ class _DropDownMenuRouteLayout<T> extends SingleChildLayoutDelegate {
if (route.initialLayout) { if (route.initialLayout) {
route.initialLayout = false; route.initialLayout = false;
final double scrollOffset = selectedItemOffset - (buttonTop - top); final double scrollOffset = selectedItemOffset - (buttonTop - top);
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); scrollableKey.currentState.scrollTo(scrollOffset);
});
} }
return new Offset(buttonRect.left, top); return new Offset(buttonRect.left, top);
......
...@@ -7,6 +7,7 @@ import 'dart:math' as math; ...@@ -7,6 +7,7 @@ import 'dart:math' as math;
import 'package:flutter/physics.dart'; import 'package:flutter/physics.dart';
import 'package:flutter/rendering.dart'; import 'package:flutter/rendering.dart';
import 'package:flutter/scheduler.dart';
import 'package:flutter/widgets.dart'; import 'package:flutter/widgets.dart';
import 'package:meta/meta.dart'; import 'package:meta/meta.dart';
...@@ -968,11 +969,18 @@ class _TabBarState<T> extends ScrollableState<TabBar<T>> implements TabBarSelect ...@@ -968,11 +969,18 @@ class _TabBarState<T> extends ScrollableState<TabBar<T>> implements TabBarSelect
} }
void _layoutChanged(Size tabBarSize, List<double> tabWidths) { void _layoutChanged(Size tabBarSize, List<double> tabWidths) {
setState(() { // 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; _tabBarSize = tabBarSize;
_tabWidths = tabWidths; _tabWidths = tabWidths;
_indicatorRect = _selection != null ? _tabIndicatorRect(_selection.index) : Rect.zero; _indicatorRect = _selection != null ? _tabIndicatorRect(_selection.index) : Rect.zero;
_updateScrollBehavior(); _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
});
}); });
} }
......
...@@ -3,6 +3,18 @@ ...@@ -3,6 +3,18 @@
// found in the LICENSE file. // found in the LICENSE file.
/// Print a banner at the beginning of each frame. /// 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; bool debugPrintBeginFrameBanner = false;
/// Print a banner at the end of each frame. /// Print a banner at the end of each frame.
......
...@@ -10,6 +10,7 @@ import 'package:flutter/gestures.dart'; ...@@ -10,6 +10,7 @@ import 'package:flutter/gestures.dart';
import 'package:flutter/rendering.dart'; import 'package:flutter/rendering.dart';
import 'package:flutter/scheduler.dart'; import 'package:flutter/scheduler.dart';
import 'package:flutter/services.dart'; import 'package:flutter/services.dart';
import 'package:meta/meta.dart';
import 'app.dart'; import 'app.dart';
import 'framework.dart'; import 'framework.dart';
...@@ -191,15 +192,41 @@ abstract class WidgetsBinding extends BindingBase implements GestureBinding, Ren ...@@ -191,15 +192,41 @@ abstract class WidgetsBinding extends BindingBase implements GestureBinding, Ren
} }
void _handleBuildScheduled() { void _handleBuildScheduled() {
// If we're in the process of building dirty elements, we're know that any // If we're in the process of building dirty elements, then changes
// builds that are scheduled will be run this frame, which means we don't // should not trigger a new frame.
// need to schedule another frame. assert(() {
if (_buildingDirtyElements) if (debugBuildingDirtyElements) {
return; 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(); 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. /// Pump the build and rendering pipeline to generate a frame.
/// ///
...@@ -260,12 +287,21 @@ abstract class WidgetsBinding extends BindingBase implements GestureBinding, Ren ...@@ -260,12 +287,21 @@ abstract class WidgetsBinding extends BindingBase implements GestureBinding, Ren
// When editing the above, also update rendering/binding.dart's copy. // When editing the above, also update rendering/binding.dart's copy.
@override @override
void beginFrame() { void beginFrame() {
assert(!_buildingDirtyElements); assert(!debugBuildingDirtyElements);
_buildingDirtyElements = true; assert(() {
buildOwner.buildDirtyElements(); debugBuildingDirtyElements = true;
_buildingDirtyElements = false; return true;
});
try {
buildOwner.buildScope(renderViewElement);
super.beginFrame(); super.beginFrame();
buildOwner.finalizeTree(); buildOwner.finalizeTree();
} finally {
assert(() {
debugBuildingDirtyElements = false;
return true;
});
}
// TODO(ianh): Following code should not be included in release mode, only profile and debug modes. // 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 // See https://github.com/dart-lang/sdk/issues/27192
if (_needToReportFirstFrame) { if (_needToReportFirstFrame) {
...@@ -291,7 +327,17 @@ abstract class WidgetsBinding extends BindingBase implements GestureBinding, Ren ...@@ -291,7 +327,17 @@ abstract class WidgetsBinding extends BindingBase implements GestureBinding, Ren
debugShortDescription: '[root]', debugShortDescription: '[root]',
child: app child: app
).attachToRenderTree(buildOwner, renderViewElement); ).attachToRenderTree(buildOwner, renderViewElement);
assert(() {
if (debugPrintBeginFrameBanner)
debugPrint('━━━━━━━┫ Begin Warm-Up Frame ┣━━━━━━━');
return true;
});
beginFrame(); beginFrame();
assert(() {
if (debugPrintEndFrameBanner)
debugPrint('━━━━━━━┫ End of Warm-Up Frame ┣━━━━━━━');
return true;
});
} }
@override @override
...@@ -364,15 +410,20 @@ class RenderObjectToWidgetAdapter<T extends RenderObject> extends RenderObjectWi ...@@ -364,15 +410,20 @@ class RenderObjectToWidgetAdapter<T extends RenderObject> extends RenderObjectWi
/// ///
/// Used by [runApp] to bootstrap applications. /// Used by [runApp] to bootstrap applications.
RenderObjectToWidgetElement<T> attachToRenderTree(BuildOwner owner, [RenderObjectToWidgetElement<T> element]) { RenderObjectToWidgetElement<T> attachToRenderTree(BuildOwner owner, [RenderObjectToWidgetElement<T> element]) {
owner.lockState(() {
if (element == null) { if (element == null) {
owner.lockState(() {
element = createElement(); element = createElement();
assert(element != null);
element.assignOwner(owner); element.assignOwner(owner);
});
owner.buildScope(element, () {
element.mount(null, null); element.mount(null, null);
});
} else { } else {
owner.buildScope(element, () {
element.update(this); element.update(this);
});
} }
}, building: true);
return element; return element;
} }
......
...@@ -1410,12 +1410,16 @@ class BuildOwner { ...@@ -1410,12 +1410,16 @@ class BuildOwner {
final _InactiveElements _inactiveElements = new _InactiveElements(); final _InactiveElements _inactiveElements = new _InactiveElements();
final List<BuildableElement> _dirtyElements = <BuildableElement>[]; final List<BuildableElement> _dirtyElements = <BuildableElement>[];
bool _scheduledFlushDirtyElements = false;
/// Adds an element to the dirty elements list so that it will be rebuilt /// 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) { void scheduleBuildFor(BuildableElement element) {
assert(element != null);
assert(element.owner == this);
assert(element._inDirtyList == _dirtyElements.contains(element));
assert(() { assert(() {
if (_dirtyElements.contains(element)) { if (element._inDirtyList) {
throw new FlutterError( throw new FlutterError(
'scheduleBuildFor() called for a widget for which a build was already scheduled.\n' 'scheduleBuildFor() called for a widget for which a build was already scheduled.\n'
'The method was called for the following element:\n' 'The method was called for the following element:\n'
...@@ -1440,9 +1444,12 @@ class BuildOwner { ...@@ -1440,9 +1444,12 @@ class BuildOwner {
} }
return true; return true;
}); });
if (_dirtyElements.isEmpty && onBuildScheduled != null) if (!_scheduledFlushDirtyElements && onBuildScheduled != null) {
_scheduledFlushDirtyElements = true;
onBuildScheduled(); onBuildScheduled();
}
_dirtyElements.add(element); _dirtyElements.add(element);
element._inDirtyList = true;
} }
int _debugStateLockLevel = 0; int _debugStateLockLevel = 0;
...@@ -1450,24 +1457,15 @@ class BuildOwner { ...@@ -1450,24 +1457,15 @@ class BuildOwner {
bool _debugBuilding = false; bool _debugBuilding = false;
BuildableElement _debugCurrentBuildTarget; 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 /// This mechanism is used to ensure that, for instance, [State.dispose] does
/// caught while invoking the callback. /// not call [State.setState].
void lockState(void callback(), { bool building: false }) { void lockState(void callback()) {
bool debugPreviouslyBuilding; assert(callback != null);
assert(_debugStateLockLevel >= 0); assert(_debugStateLockLevel >= 0);
assert(() { assert(() {
if (building) {
debugPreviouslyBuilding = _debugBuilding;
_debugBuilding = true;
}
_debugStateLockLevel += 1; _debugStateLockLevel += 1;
return true; return true;
}); });
...@@ -1476,43 +1474,55 @@ class BuildOwner { ...@@ -1476,43 +1474,55 @@ class BuildOwner {
} finally { } finally {
assert(() { assert(() {
_debugStateLockLevel -= 1; _debugStateLockLevel -= 1;
if (building) {
assert(_debugBuilding);
_debugBuilding = debugPreviouslyBuilding;
}
return true; return true;
}); });
} }
assert(_debugStateLockLevel >= 0); assert(_debugStateLockLevel >= 0);
} }
static int _elementSort(BuildableElement a, BuildableElement b) { /// Establishes a scope for updating the widget tree, and calls the given
if (a.depth < b.depth) /// `callback`, if any. Then, builds all the elements that were marked as
return -1; /// dirty using [scheduleBuildFor], in depth order.
if (b.depth < a.depth)
return 1;
if (b.dirty && !a.dirty)
return -1;
if (a.dirty && !b.dirty)
return 1;
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]. /// This mechanism prevents build functions from transitively requiring other
void buildDirtyElements() { /// build functions to run, potentially causing infinite loops.
if (_dirtyElements.isEmpty) ///
/// 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; return;
assert(context != null);
assert(_debugStateLockLevel >= 0);
assert(!_debugBuilding);
assert(() {
_debugStateLockLevel += 1;
_debugBuilding = true;
return true;
});
Timeline.startSync('Build'); Timeline.startSync('Build');
try { try {
lockState(() { _scheduledFlushDirtyElements = true;
if (callback != null)
callback();
_dirtyElements.sort(_elementSort); _dirtyElements.sort(_elementSort);
int dirtyCount = _dirtyElements.length; int dirtyCount = _dirtyElements.length;
int index = 0; int index = 0;
while (index < dirtyCount) { while (index < dirtyCount) {
assert(_dirtyElements[index] != null);
assert(_dirtyElements[index]._inDirtyList);
assert(!_dirtyElements[index]._active || _dirtyElements[index]._debugIsInScope(context));
_dirtyElements[index].rebuild(); _dirtyElements[index].rebuild();
index += 1; index += 1;
if (dirtyCount < _dirtyElements.length) { if (dirtyCount < _dirtyElements.length) {
...@@ -1520,12 +1530,35 @@ class BuildOwner { ...@@ -1520,12 +1530,35 @@ class BuildOwner {
dirtyCount = _dirtyElements.length; dirtyCount = _dirtyElements.length;
} }
} }
assert(!_dirtyElements.any((BuildableElement element) => element._active && element.dirty));
}, building: true);
} finally { } finally {
assert(!_dirtyElements.any((BuildableElement element) => element._active && element.dirty));
for (BuildableElement element in _dirtyElements) {
assert(element._inDirtyList);
element._inDirtyList = false;
}
_dirtyElements.clear(); _dirtyElements.clear();
_scheduledFlushDirtyElements = false;
Timeline.finishSync(); Timeline.finishSync();
assert(_debugBuilding);
assert(() {
_debugBuilding = false;
_debugStateLockLevel -= 1;
return true;
});
}
assert(_debugStateLockLevel >= 0);
} }
static int _elementSort(BuildableElement a, BuildableElement b) {
if (a.depth < b.depth)
return -1;
if (b.depth < a.depth)
return 1;
if (b.dirty && !a.dirty)
return -1;
if (a.dirty && !b.dirty)
return 1;
return 0;
} }
/// Complete the element build pass by unmounting any elements that are no /// Complete the element build pass by unmounting any elements that are no
...@@ -1607,6 +1640,15 @@ abstract class Element implements BuildContext { ...@@ -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 get renderObject {
RenderObject result; RenderObject result;
void visit(Element element) { void visit(Element element) {
...@@ -1875,6 +1917,7 @@ abstract class Element implements BuildContext { ...@@ -1875,6 +1917,7 @@ abstract class Element implements BuildContext {
}); });
assert(_debugLifecycleState == _ElementLifecycle.inactive); assert(_debugLifecycleState == _ElementLifecycle.inactive);
assert(widget != null); assert(widget != null);
assert(owner != null);
assert(depth != null); assert(depth != null);
assert(!_active); assert(!_active);
_active = true; _active = true;
...@@ -2091,6 +2134,10 @@ abstract class BuildableElement extends Element { ...@@ -2091,6 +2134,10 @@ abstract class BuildableElement extends Element {
bool get dirty => _dirty; bool get dirty => _dirty;
bool _dirty = true; 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 // We let widget authors call setState from initState, didUpdateConfig, and
// build even when state is locked because its convenient and a no-op anyway. // 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 // This flag ensures that this convenience is only allowed on the element
...@@ -2124,15 +2171,7 @@ abstract class BuildableElement extends Element { ...@@ -2124,15 +2171,7 @@ abstract class BuildableElement extends Element {
// a current build target when we're building. // a current build target when we're building.
return true; return true;
} }
bool foundTarget = false; if (_debugIsInScope(owner._debugCurrentBuildTarget))
visitAncestorElements((Element element) {
if (element == owner._debugCurrentBuildTarget) {
foundTarget = true;
return false;
}
return true;
});
if (foundTarget)
return true; return true;
} }
if (owner._debugStateLocked && (!_debugAllowIgnoredCallsToMarkNeedsBuild || !dirty)) { if (owner._debugStateLocked && (!_debugAllowIgnoredCallsToMarkNeedsBuild || !dirty)) {
...@@ -2212,7 +2251,9 @@ abstract class BuildableElement extends Element { ...@@ -2212,7 +2251,9 @@ abstract class BuildableElement extends Element {
void activate() { void activate() {
final bool shouldRebuild = ((_dependencies != null && _dependencies.length > 0) || _hadUnsatisfiedDependencies); final bool shouldRebuild = ((_dependencies != null && _dependencies.length > 0) || _hadUnsatisfiedDependencies);
super.activate(); // clears _dependencies, and sets active to true 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 assert(_active); // otherwise markNeedsBuild is a no-op
markNeedsBuild(); markNeedsBuild();
} }
...@@ -2904,7 +2945,7 @@ abstract class RootRenderObjectElement extends RenderObjectElement { ...@@ -2904,7 +2945,7 @@ abstract class RootRenderObjectElement extends RenderObjectElement {
/// The [WidgetsBinding] introduces the primary owner, /// The [WidgetsBinding] introduces the primary owner,
/// [WidgetsBinding.buildOwner], and assigns it to the widget tree in the call /// [WidgetsBinding.buildOwner], and assigns it to the widget tree in the call
/// to [runApp]. The binding is responsible for driving the build pipeline by /// 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]. /// [WidgetsBinding.beginFrame].
void assignOwner(BuildOwner owner) { void assignOwner(BuildOwner owner) {
_owner = owner; _owner = owner;
......
...@@ -176,9 +176,9 @@ class _LayoutBuilderElement extends RenderObjectElement { ...@@ -176,9 +176,9 @@ class _LayoutBuilderElement extends RenderObjectElement {
} }
void _layout(BoxConstraints constraints) { void _layout(BoxConstraints constraints) {
owner.buildScope(this, () {
Widget built; Widget built;
if (widget.builder != null) { if (widget.builder != null) {
owner.lockState(() {
try { try {
built = widget.builder(this, constraints); built = widget.builder(this, constraints);
debugWidgetBuilderValue(widget, built); debugWidgetBuilderValue(widget, built);
...@@ -186,13 +186,6 @@ class _LayoutBuilderElement extends RenderObjectElement { ...@@ -186,13 +186,6 @@ class _LayoutBuilderElement extends RenderObjectElement {
_debugReportException('building $widget', e, stack); _debugReportException('building $widget', e, stack);
built = new ErrorWidget(e); built = new ErrorWidget(e);
} }
});
}
owner.lockState(() {
if (widget.builder == null) {
if (_child != null)
_child = updateChild(_child, null, null);
return;
} }
try { try {
_child = updateChild(_child, built, null); _child = updateChild(_child, built, null);
...@@ -202,7 +195,7 @@ class _LayoutBuilderElement extends RenderObjectElement { ...@@ -202,7 +195,7 @@ class _LayoutBuilderElement extends RenderObjectElement {
built = new ErrorWidget(e); built = new ErrorWidget(e);
_child = updateChild(null, built, slot); _child = updateChild(null, built, slot);
} }
}, building: true); });
} }
@override @override
......
...@@ -629,11 +629,11 @@ class _LazyBlockElement extends RenderObjectElement { ...@@ -629,11 +629,11 @@ class _LazyBlockElement extends RenderObjectElement {
while (currentLogicalIndex > 0 && currentLogicalOffset > startLogicalOffset) { while (currentLogicalIndex > 0 && currentLogicalOffset > startLogicalOffset) {
currentLogicalIndex -= 1; currentLogicalIndex -= 1;
Element newElement; Element newElement;
owner.lockState(() { owner.buildScope(this, () {
Widget newWidget = _callBuilder(builder, currentLogicalIndex, requireNonNull: true); Widget newWidget = _callBuilder(builder, currentLogicalIndex, requireNonNull: true);
newWidget = new RepaintBoundary.wrap(newWidget, currentLogicalIndex); newWidget = new RepaintBoundary.wrap(newWidget, currentLogicalIndex);
newElement = inflateWidget(newWidget, null); newElement = inflateWidget(newWidget, null);
}, building: true); });
newChildren.add(newElement); newChildren.add(newElement);
RenderBox child = block.firstChild; RenderBox child = block.firstChild;
assert(child == newChildren.last.renderObject); assert(child == newChildren.last.renderObject);
...@@ -698,14 +698,14 @@ class _LazyBlockElement extends RenderObjectElement { ...@@ -698,14 +698,14 @@ class _LazyBlockElement extends RenderObjectElement {
if (physicalIndex >= _children.length) { if (physicalIndex >= _children.length) {
assert(physicalIndex == _children.length); assert(physicalIndex == _children.length);
Element newElement; Element newElement;
owner.lockState(() { owner.buildScope(this, () {
Widget newWidget = _callBuilder(builder, currentLogicalIndex); Widget newWidget = _callBuilder(builder, currentLogicalIndex);
if (newWidget == null) if (newWidget == null)
return; return;
newWidget = new RepaintBoundary.wrap(newWidget, currentLogicalIndex); newWidget = new RepaintBoundary.wrap(newWidget, currentLogicalIndex);
Element previousChild = _children.isEmpty ? null : _children.last; Element previousChild = _children.isEmpty ? null : _children.last;
newElement = inflateWidget(newWidget, previousChild); newElement = inflateWidget(newWidget, previousChild);
}, building: true); });
if (newElement == null) if (newElement == null)
break; break;
_children.add(newElement); _children.add(newElement);
......
...@@ -373,6 +373,7 @@ class ScrollableState<T extends Scrollable> extends State<T> { ...@@ -373,6 +373,7 @@ class ScrollableState<T extends Scrollable> extends State<T> {
} }
void _handleAnimationStatusChanged(AnimationStatus status) { void _handleAnimationStatusChanged(AnimationStatus status) {
// this is not called when stop() is called on the controller
setState(() { setState(() {
if (!_controller.isAnimating) if (!_controller.isAnimating)
_simulation = null; _simulation = null;
...@@ -454,7 +455,8 @@ class ScrollableState<T extends Scrollable> extends State<T> { ...@@ -454,7 +455,8 @@ class ScrollableState<T extends Scrollable> extends State<T> {
/// If there are no in-progress scrolling physics, this function scrolls to /// If there are no in-progress scrolling physics, this function scrolls to
/// the given offset instead. /// the given offset instead.
void didUpdateScrollBehavior(double newScrollOffset) { 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); assert(_controller.isAnimating || _simulation == null);
if (_numberOfInProgressScrolls > 0) { if (_numberOfInProgressScrolls > 0) {
if (_simulation != null) { if (_simulation != null) {
...@@ -583,16 +585,16 @@ class ScrollableState<T extends Scrollable> extends State<T> { ...@@ -583,16 +585,16 @@ class ScrollableState<T extends Scrollable> extends State<T> {
} }
void _handleDragDown(_) { void _handleDragDown(_) {
setState(() {
_stop(); _stop();
});
} }
void _stop() { void _stop() {
assert(mounted); assert(mounted);
assert(_controller.isAnimating || _simulation == null); assert(_controller.isAnimating || _simulation == null);
setState(() { _controller.stop(); // this does not trigger a status notification
_controller.stop();
_simulation = null; _simulation = null;
});
} }
void _handleDragStart(DragStartDetails details) { void _handleDragStart(DragStartDetails details) {
......
...@@ -5,6 +5,8 @@ ...@@ -5,6 +5,8 @@
import 'dart:collection'; import 'dart:collection';
import 'dart:math' as math; import 'dart:math' as math;
import 'package:flutter/foundation.dart';
import 'package:flutter/scheduler.dart';
import 'package:flutter/rendering.dart'; import 'package:flutter/rendering.dart';
import 'package:sky_services/semantics/semantics.mojom.dart' as mojom; import 'package:sky_services/semantics/semantics.mojom.dart' as mojom;
...@@ -53,8 +55,17 @@ class _SemanticsDebuggerState extends State<SemanticsDebugger> { ...@@ -53,8 +55,17 @@ class _SemanticsDebuggerState extends State<SemanticsDebugger> {
} }
void _update() { void _update() {
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(() { setState(() {
// the generation of the _SemanticsDebuggerListener has changed // The generation of the _SemanticsDebuggerListener has changed.
});
}
}); });
} }
......
...@@ -175,7 +175,7 @@ abstract class VirtualViewportElement extends RenderObjectElement { ...@@ -175,7 +175,7 @@ abstract class VirtualViewportElement extends RenderObjectElement {
assert(startOffsetBase != null); assert(startOffsetBase != null);
assert(startOffsetLimit != null); assert(startOffsetLimit != null);
_updatePaintOffset(); _updatePaintOffset();
owner.lockState(_materializeChildren, building: true); owner.buildScope(this, _materializeChildren);
} }
void _materializeChildren() { void _materializeChildren() {
......
...@@ -38,7 +38,7 @@ class OffscreenWidgetTree { ...@@ -38,7 +38,7 @@ class OffscreenWidgetTree {
} }
void pumpFrame() { void pumpFrame() {
buildOwner.buildDirtyElements(); buildOwner.buildScope(root);
pipelineOwner.flushLayout(); pipelineOwner.flushLayout();
pipelineOwner.flushCompositingBits(); pipelineOwner.flushCompositingBits();
pipelineOwner.flushPaint(); pipelineOwner.flushPaint();
......
// 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<StatefulWrapper> {
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;
});
}
...@@ -22,9 +22,12 @@ import 'stack_manipulation.dart'; ...@@ -22,9 +22,12 @@ import 'stack_manipulation.dart';
/// Phases that can be reached by [WidgetTester.pumpWidget] and /// Phases that can be reached by [WidgetTester.pumpWidget] and
/// [TestWidgetsFlutterBinding.pump]. /// [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. // TODO(ianh): Merge with near-identical code in the rendering test code.
enum EnginePhase { enum EnginePhase {
/// The build phase in the widgets library. See [BuildOwner.buildDirtyElements]. /// The build phase in the widgets library. See [BuildOwner.buildScope].
build, build,
/// The layout phase in the rendering library. See [PipelineOwner.flushLayout]. /// The layout phase in the rendering library. See [PipelineOwner.flushLayout].
...@@ -396,6 +399,9 @@ abstract class TestWidgetsFlutterBinding extends BindingBase ...@@ -396,6 +399,9 @@ abstract class TestWidgetsFlutterBinding extends BindingBase
/// ///
/// This binding controls time, allowing tests to verify long /// This binding controls time, allowing tests to verify long
/// animation sequences without having to execute them in real time. /// 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 { class AutomatedTestWidgetsFlutterBinding extends TestWidgetsFlutterBinding {
@override @override
void initInstances() { void initInstances() {
...@@ -447,7 +453,9 @@ class AutomatedTestWidgetsFlutterBinding extends TestWidgetsFlutterBinding { ...@@ -447,7 +453,9 @@ class AutomatedTestWidgetsFlutterBinding extends TestWidgetsFlutterBinding {
@override @override
void beginFrame() { void beginFrame() {
assert(inTest); assert(inTest);
buildOwner.buildDirtyElements(); try {
debugBuildingDirtyElements = true;
buildOwner.buildScope(renderViewElement);
if (_phase == EnginePhase.build) if (_phase == EnginePhase.build)
return; return;
assert(renderView != null); assert(renderView != null);
...@@ -466,7 +474,10 @@ class AutomatedTestWidgetsFlutterBinding extends TestWidgetsFlutterBinding { ...@@ -466,7 +474,10 @@ class AutomatedTestWidgetsFlutterBinding extends TestWidgetsFlutterBinding {
pipelineOwner.flushSemantics(); pipelineOwner.flushSemantics();
if (_phase == EnginePhase.flushSemantics) if (_phase == EnginePhase.flushSemantics)
return; return;
} finally {
buildOwner.finalizeTree(); buildOwner.finalizeTree();
debugBuildingDirtyElements = false;
}
} }
@override @override
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment