diff --git a/packages/flutter/lib/src/widgets/binding.dart b/packages/flutter/lib/src/widgets/binding.dart index 20f490e88b..aad5743dd5 100644 --- a/packages/flutter/lib/src/widgets/binding.dart +++ b/packages/flutter/lib/src/widgets/binding.dart @@ -471,6 +471,12 @@ class RenderObjectToWidgetElement extends RootRenderObje visitor(_child); } + @override + void detachChild(Element child) { + assert(child == _child); + _child = null; + } + @override void mount(Element parent, dynamic newSlot) { assert(parent == null); diff --git a/packages/flutter/lib/src/widgets/framework.dart b/packages/flutter/lib/src/widgets/framework.dart index c1dbff5ecb..a55923c4bf 100644 --- a/packages/flutter/lib/src/widgets/framework.dart +++ b/packages/flutter/lib/src/widgets/framework.dart @@ -1143,6 +1143,13 @@ class _InactiveElements { void _unmount(Element element) { assert(element._debugLifecycleState == _ElementLifecycle.inactive); + assert(() { + if (debugPrintGlobalKeyedWidgetLifecycle) { + if (element.widget.key is GlobalKey) + debugPrint('Discarding $element from inactive elements list.'); + } + return true; + }); element.unmount(); assert(element._debugLifecycleState == _ElementLifecycle.defunct); element.visitChildren((Element child) { @@ -1720,7 +1727,15 @@ abstract class Element implements BuildContext { visitChildren(visitor); } - bool detachChild(Element child) => false; + /// Remove the given child. + /// + /// This updates the child model such that [visitChildren] does not walk that + /// child anymore. + /// + /// The element must have already been deactivated when this is called, + /// meaning that its parent should be null. + @protected + void detachChild(Element child); /// This method is the core of the system. /// @@ -1868,11 +1883,14 @@ abstract class Element implements BuildContext { return null; assert(() { if (debugPrintGlobalKeyedWidgetLifecycle) - debugPrint('Attempting to take $element from ${element._parent ?? "inactive elements list"} to put in $this'); + debugPrint('Attempting to take $element from ${element._parent ?? "inactive elements list"} to put in $this.'); return true; }); - if (element._parent != null && !element._parent.detachChild(element)) - return null; + final Element parent = element._parent; + if (parent != null) { + parent.deactivateChild(element); + parent.detachChild(element); + } assert(element._parent == null); owner._inactiveElements.remove(element); return element; @@ -1930,6 +1948,11 @@ abstract class Element implements BuildContext { void _activateWithParent(Element parent, dynamic newSlot) { assert(_debugLifecycleState == _ElementLifecycle.inactive); _parent = parent; + assert(() { + if (debugPrintGlobalKeyedWidgetLifecycle) + debugPrint('Reactivating $this (now child of $_parent).'); + return true; + }); _updateDepth(_parent.depth); _activateRecursively(this); attachRenderObject(newSlot); @@ -1947,11 +1970,6 @@ abstract class Element implements BuildContext { /// instead of being unmounted (see [unmount]). @mustCallSuper void activate() { - assert(() { - if (debugPrintGlobalKeyedWidgetLifecycle) - debugPrint('Reactivating $this (child of $_parent).'); - return true; - }); assert(_debugLifecycleState == _ElementLifecycle.inactive); assert(widget != null); assert(owner != null); @@ -2387,11 +2405,9 @@ abstract class ComponentElement extends BuildableElement { } @override - bool detachChild(Element child) { + void detachChild(Element child) { assert(child == _child); - deactivateChild(_child); _child = null; - return true; } } @@ -3026,6 +3042,11 @@ abstract class RootRenderObjectElement extends RenderObjectElement { class LeafRenderObjectElement extends RenderObjectElement { LeafRenderObjectElement(LeafRenderObjectWidget widget): super(widget); + @override + void detachChild(Element child) { + assert(false); + } + @override void insertChildRenderObject(RenderObject child, dynamic slot) { assert(false); @@ -3064,11 +3085,9 @@ class SingleChildRenderObjectElement extends RenderObjectElement { } @override - bool detachChild(Element child) { + void detachChild(Element child) { assert(child == _child); - deactivateChild(_child); _child = null; - return true; } @override @@ -3157,10 +3176,10 @@ class MultiChildRenderObjectElement extends RenderObjectElement { } @override - bool detachChild(Element child) { + void detachChild(Element child) { + assert(_children.contains(child)); + assert(!_detachedChildren.contains(child)); _detachedChildren.add(child); - deactivateChild(child); - return true; } @override diff --git a/packages/flutter/lib/src/widgets/layout_builder.dart b/packages/flutter/lib/src/widgets/layout_builder.dart index 42578b0d95..8bd9d8a724 100644 --- a/packages/flutter/lib/src/widgets/layout_builder.dart +++ b/packages/flutter/lib/src/widgets/layout_builder.dart @@ -69,6 +69,12 @@ class _LayoutBuilderElement extends RenderObjectElement { visitor(_child); } + @override + void detachChild(Element child) { + assert(child == _child); + _child = null; + } + @override void mount(Element parent, dynamic newSlot) { super.mount(parent, newSlot); // Creates the renderObject. diff --git a/packages/flutter/lib/src/widgets/lazy_block.dart b/packages/flutter/lib/src/widgets/lazy_block.dart index 9e3f5a8680..0707495a01 100644 --- a/packages/flutter/lib/src/widgets/lazy_block.dart +++ b/packages/flutter/lib/src/widgets/lazy_block.dart @@ -510,6 +510,18 @@ class _LazyBlockElement extends RenderObjectElement { visitor(child); } + @override + void detachChild(Element child) { + assert(() { + // TODO(ianh): implement detachChild for LazyBlock + throw new FlutterError( + 'LazyBlock does not yet support GlobalKey reparenting of its children.\n' + 'As a temporary workaround, wrap the child with the GlobalKey in a ' + 'Container or other harmless child.' + ); + }); + } + @override void mount(Element parent, dynamic newSlot) { super.mount(parent, newSlot); @@ -585,7 +597,7 @@ class _LazyBlockElement extends RenderObjectElement { void performRebuild() { IndexedWidgetBuilder builder = widget.delegate.buildItem; List widgets = []; - for (int i = 0; i < _children.length; ++i) { + for (int i = 0; i < _children.length; i += 1) { int logicalIndex = _firstChildLogicalIndex + i; Widget childWidget = _callBuilder(builder, logicalIndex); if (childWidget == null) diff --git a/packages/flutter/lib/src/widgets/overlay.dart b/packages/flutter/lib/src/widgets/overlay.dart index 3877efb1d3..cdd914f535 100644 --- a/packages/flutter/lib/src/widgets/overlay.dart +++ b/packages/flutter/lib/src/widgets/overlay.dart @@ -438,9 +438,10 @@ class _TheatreElement extends RenderObjectElement { if (child == _onstage) { _onstage = null; } else { + assert(_offstage.contains(child)); + assert(!_detachedOffstageChildren.contains(child)); _detachedOffstageChildren.add(child); } - deactivateChild(child); return true; } diff --git a/packages/flutter/lib/src/widgets/table.dart b/packages/flutter/lib/src/widgets/table.dart index 5ed1205b0b..bf22ae11f9 100644 --- a/packages/flutter/lib/src/widgets/table.dart +++ b/packages/flutter/lib/src/widgets/table.dart @@ -279,7 +279,6 @@ class _TableElement extends RenderObjectElement { @override bool detachChild(Element child) { _detachedChildren.add(child); - deactivateChild(child); return true; } } diff --git a/packages/flutter/lib/src/widgets/virtual_viewport.dart b/packages/flutter/lib/src/widgets/virtual_viewport.dart index 06e6f13d40..f3bcc55bb9 100644 --- a/packages/flutter/lib/src/widgets/virtual_viewport.dart +++ b/packages/flutter/lib/src/widgets/virtual_viewport.dart @@ -100,6 +100,18 @@ abstract class VirtualViewportElement extends RenderObjectElement { visitor(child); } + @override + void detachChild(Element child) { + assert(() { + // TODO(ianh): implement detachChild for VirtualViewport + throw new FlutterError( + '$runtimeType does not yet support GlobalKey reparenting of its children.\n' + 'As a temporary workaround, wrap the child with the GlobalKey in a ' + 'Container or other harmless child.' + ); + }); + } + _WidgetProvider _widgetProvider; @override diff --git a/packages/flutter/test/widget/reparent_state_with_layout_builder_test.dart b/packages/flutter/test/widget/reparent_state_with_layout_builder_test.dart new file mode 100644 index 0000000000..a28720fb94 --- /dev/null +++ b/packages/flutter/test/widget/reparent_state_with_layout_builder_test.dart @@ -0,0 +1,72 @@ +// 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/material.dart'; +import 'package:flutter_test/flutter_test.dart' hide TypeMatcher; + +// This is a regression test for https://github.com/flutter/flutter/issues/5840. + +class Bar extends StatefulWidget { + @override + BarState createState() => new BarState(); +} + +class BarState extends State { + final GlobalKey _fooKey = new GlobalKey(); + + bool _mode = false; + + void trigger() { + setState(() { + _mode = !_mode; + }); + } + + @override + Widget build(BuildContext context) { + if (_mode) { + return new SizedBox( + child: new LayoutBuilder( + builder: (BuildContext context, BoxConstraints constraints) => new StatefulCreationCounter(key: _fooKey), + ), + ); + } else { + return new LayoutBuilder( + builder: (BuildContext context, BoxConstraints constraints) => new StatefulCreationCounter(key: _fooKey), + ); + } + } +} + +class StatefulCreationCounter extends StatefulWidget { + StatefulCreationCounter({ Key key }) : super(key: key); + + @override + StatefulCreationCounterState createState() => new StatefulCreationCounterState(); +} + +class StatefulCreationCounterState extends State { + static int creationCount = 0; + + @override + void initState() { + super.initState(); + creationCount += 1; + } + + @override + Widget build(BuildContext context) => new Container(); +} + +void main() { + testWidgets('reparent state with layout builder', (WidgetTester tester) async { + expect(StatefulCreationCounterState.creationCount, 0); + await tester.pumpWidget(new Bar()); + expect(StatefulCreationCounterState.creationCount, 1); + BarState s = tester.state/**/(find.byType(Bar)); + s.trigger(); + await tester.pump(); + expect(StatefulCreationCounterState.creationCount, 1); + }); +}