Commit 0a37e06b authored by Hixie's avatar Hixie

Fix breakage caused by sync-across-removal patch.

This makes the sync code stop if it would have to rearrange the
RenderObjects. I'll make it handle the cross-RenderObject case, as well
as the insertion-sync case, in subsequent patches.
parent 318b69be
...@@ -19,7 +19,6 @@ export 'package:sky/src/rendering/box.dart' show BoxConstraints, BoxDecoration, ...@@ -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; 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 _shouldLogRenderDuration = false; // see also 'enableProfilingLoop' argument to runApp()
final bool _shouldReparentDuringSync = false; // currently in development
typedef Widget Builder(); typedef Widget Builder();
typedef void WidgetTreeWalker(Widget); typedef void WidgetTreeWalker(Widget);
...@@ -422,16 +421,25 @@ abstract class Widget { ...@@ -422,16 +421,25 @@ abstract class Widget {
Widget deadNode = oldNode; Widget deadNode = oldNode;
Widget candidate = _getCandidateSingleChildFrom(oldNode); Widget candidate = _getCandidateSingleChildFrom(oldNode);
oldNode = null; oldNode = null;
while (candidate != null && _shouldReparentDuringSync) {
while (candidate != null) {
if (_canSync(newNode, candidate)) { if (_canSync(newNode, candidate)) {
assert(candidate.parent != null); assert(candidate.parent != null);
assert(candidate.parent.singleChild == candidate); assert(candidate.parent.singleChild == candidate);
if (candidate.renderObject != deadNode.renderObject) {
// TODO(ianh): Handle removal across RenderNode boundaries
} else {
candidate.parent.takeChild(); candidate.parent.takeChild();
oldNode = candidate; oldNode = candidate;
}
break; break;
} }
candidate = _getCandidateSingleChildFrom(candidate); candidate = _getCandidateSingleChildFrom(candidate);
} }
// TODO(ianh): Handle insertion, too...
if (oldNode == null)
deadNode.detachRenderObject(); deadNode.detachRenderObject();
deadNode.remove(); deadNode.remove();
} }
...@@ -542,13 +550,19 @@ abstract class TagNode extends Widget { ...@@ -542,13 +550,19 @@ abstract class TagNode extends Widget {
Widget get singleChild => child; Widget get singleChild => child;
bool _debugChildTaken = false;
void takeChild() { void takeChild() {
assert(!_debugChildTaken);
assert(singleChild == child); assert(singleChild == child);
assert(child != null); assert(child != null);
child = null; child = null;
_renderObject = null;
assert(() { _debugChildTaken = true; return true; });
} }
void _sync(Widget old, dynamic slot) { void _sync(Widget old, dynamic slot) {
assert(!_debugChildTaken);
Widget oldChild = (old as TagNode)?.child; Widget oldChild = (old as TagNode)?.child;
child = syncChild(child, oldChild, slot); child = syncChild(child, oldChild, slot);
if (child != null) { if (child != null) {
...@@ -567,6 +581,7 @@ abstract class TagNode extends Widget { ...@@ -567,6 +581,7 @@ abstract class TagNode extends Widget {
} }
void detachRenderObject() { void detachRenderObject() {
assert(!_debugChildTaken);
if (child != null) if (child != null)
child.detachRenderObject(); child.detachRenderObject();
} }
...@@ -750,9 +765,11 @@ abstract class Component extends Widget { ...@@ -750,9 +765,11 @@ abstract class Component extends Widget {
assert(_debugChildTaken || (_child != null && _renderObject != null)); assert(_debugChildTaken || (_child != null && _renderObject != null));
super.remove(); super.remove();
_child = null; _child = null;
_renderObject = null;
} }
void detachRenderObject() { void detachRenderObject() {
assert(!_debugChildTaken);
if (_child != null) if (_child != null)
_child.detachRenderObject(); _child.detachRenderObject();
} }
...@@ -786,6 +803,7 @@ abstract class Component extends Widget { ...@@ -786,6 +803,7 @@ abstract class Component extends Widget {
// 3) Syncing against an old version // 3) Syncing against an old version
// assert(_child == null && old != null) // assert(_child == null && old != null)
void _sync(Component old, dynamic slot) { void _sync(Component old, dynamic slot) {
assert(!_debugChildTaken);
assert(_child == null || old == null); assert(_child == null || old == null);
updateSlot(slot); updateSlot(slot);
...@@ -1278,8 +1296,9 @@ abstract class RenderObjectWrapper extends Widget { ...@@ -1278,8 +1296,9 @@ abstract class RenderObjectWrapper extends Widget {
} }
void detachRenderObject() { void detachRenderObject() {
assert(_ancestor != null);
assert(renderObject != null); assert(renderObject != null);
assert(_ancestor != null);
assert(_ancestor.renderObject != null);
_ancestor.detachChildRenderObject(this); _ancestor.detachChildRenderObject(this);
} }
...@@ -1314,13 +1333,18 @@ abstract class OneChildRenderObjectWrapper extends RenderObjectWrapper { ...@@ -1314,13 +1333,18 @@ abstract class OneChildRenderObjectWrapper extends RenderObjectWrapper {
Widget get singleChild => _child; Widget get singleChild => _child;
bool _debugChildTaken = false;
void takeChild() { void takeChild() {
assert(!_debugChildTaken);
assert(singleChild == child); assert(singleChild == child);
assert(child != null); assert(child != null);
_child = null; _child = null;
assert(() { _debugChildTaken = true; return true; });
} }
void syncRenderObject(RenderObjectWrapper old) { void syncRenderObject(RenderObjectWrapper old) {
assert(!_debugChildTaken);
super.syncRenderObject(old); super.syncRenderObject(old);
Widget oldChild = (old as OneChildRenderObjectWrapper)?.child; Widget oldChild = (old as OneChildRenderObjectWrapper)?.child;
Widget newChild = child; Widget newChild = child;
......
...@@ -59,40 +59,39 @@ void main() { ...@@ -59,40 +59,39 @@ void main() {
}); });
// Requires _shouldReparentDuringSync test('remove one', () {
// test('remove one', () {
// WidgetTester tester = new WidgetTester();
// WidgetTester tester = new WidgetTester();
// tester.pumpFrame(() {
// tester.pumpFrame(() { return new Container(
// return new Container( child: new Container(
// child: new Container( child: new TestState(
// child: new TestState( persistentState: 10,
// state: 10, child: new Container()
// child: new Container() )
// ) )
// ) );
// ); });
// });
// TestState stateWidget = tester.findWidget((widget) => widget is TestState);
// TestState stateWidget = tester.findWidget((widget) => widget is TestState);
// expect(stateWidget.persistentState, equals(10));
// expect(stateWidget.state, equals(10)); expect(stateWidget.syncs, equals(0));
// expect(stateWidget.syncs, equals(0));
// tester.pumpFrame(() {
// tester.pumpFrame(() { return new Container(
// return new Container( child: new TestState(
// child: new TestState( persistentState: 11,
// state: 11, child: new Container()
// child: new Container() )
// ) );
// ); });
// });
// expect(stateWidget.persistentState, equals(10));
// expect(stateWidget.state, equals(10)); expect(stateWidget.syncs, equals(1));
// expect(stateWidget.syncs, equals(1));
// });
// });
test('swap instances around', () { test('swap instances around', () {
......
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