diff --git a/sky/packages/sky/lib/src/widgets/framework.dart b/sky/packages/sky/lib/src/widgets/framework.dart index c565530574e0923f6f3ff32ae9ee7b9fb69f970f..25138bfe588e5c24e821a7d8acd6e7b25608126b 100644 --- a/sky/packages/sky/lib/src/widgets/framework.dart +++ b/sky/packages/sky/lib/src/widgets/framework.dart @@ -19,7 +19,6 @@ export 'package:sky/src/rendering/box.dart' show BoxConstraints, BoxDecoration, export 'package:sky/src/rendering/object.dart' show Point, Offset, Size, Rect, Color, Paint, Path; final bool _shouldLogRenderDuration = false; // see also 'enableProfilingLoop' argument to runApp() -final bool _shouldReparentDuringSync = false; // currently in development typedef Widget Builder(); typedef void WidgetTreeWalker(Widget); @@ -422,17 +421,26 @@ abstract class Widget { Widget deadNode = oldNode; Widget candidate = _getCandidateSingleChildFrom(oldNode); oldNode = null; - while (candidate != null && _shouldReparentDuringSync) { + + while (candidate != null) { if (_canSync(newNode, candidate)) { assert(candidate.parent != null); assert(candidate.parent.singleChild == candidate); - candidate.parent.takeChild(); - oldNode = candidate; + if (candidate.renderObject != deadNode.renderObject) { + // TODO(ianh): Handle removal across RenderNode boundaries + } else { + candidate.parent.takeChild(); + oldNode = candidate; + } break; } candidate = _getCandidateSingleChildFrom(candidate); } - deadNode.detachRenderObject(); + + // TODO(ianh): Handle insertion, too... + + if (oldNode == null) + deadNode.detachRenderObject(); deadNode.remove(); } if (oldNode != null) { @@ -542,13 +550,19 @@ abstract class TagNode extends Widget { Widget get singleChild => child; + bool _debugChildTaken = false; + void takeChild() { + assert(!_debugChildTaken); assert(singleChild == child); assert(child != null); child = null; + _renderObject = null; + assert(() { _debugChildTaken = true; return true; }); } void _sync(Widget old, dynamic slot) { + assert(!_debugChildTaken); Widget oldChild = (old as TagNode)?.child; child = syncChild(child, oldChild, slot); if (child != null) { @@ -567,6 +581,7 @@ abstract class TagNode extends Widget { } void detachRenderObject() { + assert(!_debugChildTaken); if (child != null) child.detachRenderObject(); } @@ -750,9 +765,11 @@ abstract class Component extends Widget { assert(_debugChildTaken || (_child != null && _renderObject != null)); super.remove(); _child = null; + _renderObject = null; } void detachRenderObject() { + assert(!_debugChildTaken); if (_child != null) _child.detachRenderObject(); } @@ -786,6 +803,7 @@ abstract class Component extends Widget { // 3) Syncing against an old version // assert(_child == null && old != null) void _sync(Component old, dynamic slot) { + assert(!_debugChildTaken); assert(_child == null || old == null); updateSlot(slot); @@ -1278,8 +1296,9 @@ abstract class RenderObjectWrapper extends Widget { } void detachRenderObject() { - assert(_ancestor != null); assert(renderObject != null); + assert(_ancestor != null); + assert(_ancestor.renderObject != null); _ancestor.detachChildRenderObject(this); } @@ -1314,13 +1333,18 @@ abstract class OneChildRenderObjectWrapper extends RenderObjectWrapper { Widget get singleChild => _child; + bool _debugChildTaken = false; + void takeChild() { + assert(!_debugChildTaken); assert(singleChild == child); assert(child != null); _child = null; + assert(() { _debugChildTaken = true; return true; }); } void syncRenderObject(RenderObjectWrapper old) { + assert(!_debugChildTaken); super.syncRenderObject(old); Widget oldChild = (old as OneChildRenderObjectWrapper)?.child; Widget newChild = child; diff --git a/sky/unit/test/widget/syncing_test.dart b/sky/unit/test/widget/syncing_test.dart index 4d882bf682b5617bf82b0a4035141e748dd70e73..0a4552e5ff4b7fc86f925d4f351801a899954b2d 100644 --- a/sky/unit/test/widget/syncing_test.dart +++ b/sky/unit/test/widget/syncing_test.dart @@ -59,40 +59,39 @@ void main() { }); - // Requires _shouldReparentDuringSync - // test('remove one', () { - // - // WidgetTester tester = new WidgetTester(); - // - // tester.pumpFrame(() { - // return new Container( - // child: new Container( - // child: new TestState( - // state: 10, - // child: new Container() - // ) - // ) - // ); - // }); - // - // TestState stateWidget = tester.findWidget((widget) => widget is TestState); - // - // expect(stateWidget.state, equals(10)); - // expect(stateWidget.syncs, equals(0)); - // - // tester.pumpFrame(() { - // return new Container( - // child: new TestState( - // state: 11, - // child: new Container() - // ) - // ); - // }); - // - // expect(stateWidget.state, equals(10)); - // expect(stateWidget.syncs, equals(1)); - // - // }); + test('remove one', () { + + WidgetTester tester = new WidgetTester(); + + tester.pumpFrame(() { + return new Container( + child: new Container( + child: new TestState( + persistentState: 10, + child: new Container() + ) + ) + ); + }); + + TestState stateWidget = tester.findWidget((widget) => widget is TestState); + + expect(stateWidget.persistentState, equals(10)); + expect(stateWidget.syncs, equals(0)); + + tester.pumpFrame(() { + return new Container( + child: new TestState( + persistentState: 11, + child: new Container() + ) + ); + }); + + expect(stateWidget.persistentState, equals(10)); + expect(stateWidget.syncs, equals(1)); + + }); test('swap instances around', () {