Commit cb9152ca authored by Ian Hickson's avatar Ian Hickson

Merge pull request #1637 from Hixie/asserts

Improve exceptions and asserts for rendering lib.
parents 79828ef2 1a0484cc
...@@ -79,16 +79,16 @@ abstract class RenderSector extends RenderObject { ...@@ -79,16 +79,16 @@ abstract class RenderSector extends RenderObject {
} }
SectorConstraints get constraints => super.constraints; SectorConstraints get constraints => super.constraints;
bool debugDoesMeetConstraints() { void debugAssertDoesMeetConstraints() {
assert(constraints != null); assert(constraints != null);
assert(deltaRadius != null); assert(deltaRadius != null);
assert(deltaRadius < double.INFINITY); assert(deltaRadius < double.INFINITY);
assert(deltaTheta != null); assert(deltaTheta != null);
assert(deltaTheta < double.INFINITY); assert(deltaTheta < double.INFINITY);
return constraints.minDeltaRadius <= deltaRadius && assert(constraints.minDeltaRadius <= deltaRadius);
deltaRadius <= math.max(constraints.minDeltaRadius, constraints.maxDeltaRadius) && assert(deltaRadius <= math.max(constraints.minDeltaRadius, constraints.maxDeltaRadius));
constraints.minDeltaTheta <= deltaTheta && assert(constraints.minDeltaTheta <= deltaTheta);
deltaTheta <= math.max(constraints.minDeltaTheta, constraints.maxDeltaTheta); assert(deltaTheta <= math.max(constraints.minDeltaTheta, constraints.maxDeltaTheta));
} }
void performResize() { void performResize() {
// default behaviour for subclasses that have sizedByParent = true // default behaviour for subclasses that have sizedByParent = true
......
...@@ -350,8 +350,9 @@ class RenderBlockViewport extends RenderBlockBase { ...@@ -350,8 +350,9 @@ class RenderBlockViewport extends RenderBlockBase {
double result; double result;
if (intrinsicCallback == null) { if (intrinsicCallback == null) {
assert(() { assert(() {
'RenderBlockViewport does not support returning intrinsic dimensions if the relevant callbacks have not been specified.'; if (!RenderObject.debugCheckingIntrinsics)
return RenderObject.debugInDebugDoesMeetConstraints; throw new UnsupportedError('$runtimeType does not support returning intrinsic dimensions if the relevant callbacks have not been specified.');
return true;
}); });
return constrainer(0.0); return constrainer(0.0);
} }
......
This diff is collapsed.
...@@ -724,11 +724,14 @@ abstract class RenderObject extends AbstractNode implements HitTestTarget { ...@@ -724,11 +724,14 @@ abstract class RenderObject extends AbstractNode implements HitTestTarget {
void visitChildren(RenderObjectVisitor visitor) { } void visitChildren(RenderObjectVisitor visitor) { }
dynamic debugOwner; dynamic debugOwner;
static int _debugPrintedExceptionCount = 0;
void _debugReportException(String method, dynamic exception, StackTrace stack) { void _debugReportException(String method, dynamic exception, StackTrace stack) {
try { try {
if (debugRenderingExceptionHandler != null) { if (debugRenderingExceptionHandler != null) {
debugRenderingExceptionHandler(this, method, exception, stack); debugRenderingExceptionHandler(this, method, exception, stack);
} else { } else {
_debugPrintedExceptionCount += 1;
if (_debugPrintedExceptionCount == 1) {
debugPrint('-- EXCEPTION CAUGHT BY RENDERING LIBRARY -------------------------------'); debugPrint('-- EXCEPTION CAUGHT BY RENDERING LIBRARY -------------------------------');
debugPrint('The following exception was raised during $method():'); debugPrint('The following exception was raised during $method():');
debugPrint('$exception'); debugPrint('$exception');
...@@ -754,22 +757,12 @@ abstract class RenderObject extends AbstractNode implements HitTestTarget { ...@@ -754,22 +757,12 @@ abstract class RenderObject extends AbstractNode implements HitTestTarget {
debugPrint('This RenderObject has no descendants.'); debugPrint('This RenderObject has no descendants.');
} }
descendants.forEach(debugPrint); descendants.forEach(debugPrint);
assert(() {
if (debugInDebugDoesMeetConstraints) {
debugPrint('This exception was thrown while debugDoesMeetConstraints() was running.');
debugPrint('debugDoesMeetConstraints() verifies that some invariants are not being');
debugPrint('violated. For example, it verifies that RenderBox objects are sized in');
debugPrint('a manner consistent with the constraints provided, and, in addition, that');
debugPrint('the getMinIntrinsicWidth(), getMaxIntrinsicWidth(), etc, functions all');
debugPrint('return consistent values within the same constraints.');
debugPrint('If you are not writing your own RenderObject subclass, then this is not');
debugPrint('your fault. Contact support: https://github.com/flutter/flutter/issues/new');
}
return true;
});
debugPrint('Stack trace:'); debugPrint('Stack trace:');
debugPrint('$stack'); debugPrint('$stack');
debugPrint('------------------------------------------------------------------------'); debugPrint('------------------------------------------------------------------------');
} else {
debugPrint('Another exception was raised: ${exception.toString().split("\n")[0]}');
}
} }
} catch (exception) { } catch (exception) {
debugPrint('(exception during exception handler: $exception)'); debugPrint('(exception during exception handler: $exception)');
...@@ -809,15 +802,23 @@ abstract class RenderObject extends AbstractNode implements HitTestTarget { ...@@ -809,15 +802,23 @@ abstract class RenderObject extends AbstractNode implements HitTestTarget {
Constraints _constraints; Constraints _constraints;
/// The layout constraints most recently supplied by the parent. /// The layout constraints most recently supplied by the parent.
Constraints get constraints => _constraints; Constraints get constraints => _constraints;
/// Override this function in a subclass to verify that your state matches the constraints object. /// Verify that the object's constraints are being met. Override
bool debugDoesMeetConstraints(); /// this function in a subclass to verify that your state matches
/// When true, debugDoesMeetConstraints() is currently executing. /// the constraints object. This function is only called in checked
/// /// mode. If the constraints are not met, it should assert or throw
/// This should be set by implementations of debugDoesMeetConstraints() so that /// an exception.
/// tests can selectively ignore custom layout callbacks. It should not be set void debugAssertDoesMeetConstraints();
/// outside of debugDoesMeetConstraints() implementations and should not be used
/// for purposes other than tests. /// When true, debugAssertDoesMeetConstraints() is currently
static bool debugInDebugDoesMeetConstraints = false; /// executing asserts for verifying the consistent behaviour of
/// intrinsic dimensions methods.
///
/// This should only be set by debugAssertDoesMeetConstraints()
/// implementations. It is used by tests to selectively ignore
/// custom layout callbacks. It should not be set outside of
/// debugAssertDoesMeetConstraints(), and should not be checked in
/// release mode (where it will always be false).
static bool debugCheckingIntrinsics = false;
bool debugAncestorsAlreadyMarkedNeedsLayout() { bool debugAncestorsAlreadyMarkedNeedsLayout() {
if (_relayoutSubtreeRoot == null) if (_relayoutSubtreeRoot == null)
return true; // we haven't yet done layout even once, so there's nothing for us to do return true; // we haven't yet done layout even once, so there's nothing for us to do
...@@ -1022,7 +1023,7 @@ abstract class RenderObject extends AbstractNode implements HitTestTarget { ...@@ -1022,7 +1023,7 @@ abstract class RenderObject extends AbstractNode implements HitTestTarget {
assert(() { _debugDoingThisResize = true; return true; }); assert(() { _debugDoingThisResize = true; return true; });
try { try {
performResize(); performResize();
assert(debugDoesMeetConstraints()); assert(() { debugAssertDoesMeetConstraints(); return true; });
} catch (e, stack) { } catch (e, stack) {
_debugReportException('performResize', e, stack); _debugReportException('performResize', e, stack);
} }
...@@ -1038,7 +1039,7 @@ abstract class RenderObject extends AbstractNode implements HitTestTarget { ...@@ -1038,7 +1039,7 @@ abstract class RenderObject extends AbstractNode implements HitTestTarget {
try { try {
performLayout(); performLayout();
markNeedsSemanticsUpdate(); markNeedsSemanticsUpdate();
assert(debugDoesMeetConstraints()); assert(() { debugAssertDoesMeetConstraints(); return true; });
} catch (e, stack) { } catch (e, stack) {
_debugReportException('performLayout', e, stack); _debugReportException('performLayout', e, stack);
} }
...@@ -2023,3 +2024,10 @@ abstract class ContainerRenderObjectMixin<ChildType extends RenderObject, Parent ...@@ -2023,3 +2024,10 @@ abstract class ContainerRenderObjectMixin<ChildType extends RenderObject, Parent
return result; return result;
} }
} }
/// Error thrown when the rendering library encounters a contract violation.
class RenderingError extends AssertionError {
RenderingError(this.message);
final String message;
String toString() => message;
}
...@@ -78,7 +78,7 @@ class RenderView extends RenderObject with RenderObjectWithChildMixin<RenderBox> ...@@ -78,7 +78,7 @@ class RenderView extends RenderObject with RenderObjectWithChildMixin<RenderBox>
// We never call layout() on this class, so this should never get // We never call layout() on this class, so this should never get
// checked. (This class is laid out using scheduleInitialLayout().) // checked. (This class is laid out using scheduleInitialLayout().)
bool debugDoesMeetConstraints() { assert(false); return false; } void debugAssertDoesMeetConstraints() { assert(false); }
void performResize() { void performResize() {
assert(false); assert(false);
......
...@@ -155,10 +155,14 @@ class _MixedViewportElement extends RenderObjectElement<MixedViewport> { ...@@ -155,10 +155,14 @@ class _MixedViewportElement extends RenderObjectElement<MixedViewport> {
double _noIntrinsicExtent(BoxConstraints constraints) { double _noIntrinsicExtent(BoxConstraints constraints) {
assert(() { assert(() {
'MixedViewport does not support returning intrinsic dimensions. ' + if (!RenderObject.debugCheckingIntrinsics) {
'Calculating the intrinsic dimensions would require walking the entire child list, ' + throw new UnsupportedError(
'which defeats the entire point of having a lazily-built list of children.'; 'MixedViewport does not support returning intrinsic dimensions.\n'
return RenderObject.debugInDebugDoesMeetConstraints; 'Calculating the intrinsic dimensions would require walking the entire child list,\n'
'which defeats the entire point of having a lazily-built list of children.'
);
}
return true;
}); });
return null; return null;
} }
......
...@@ -442,13 +442,14 @@ class ScrollableViewportState extends ScrollableState<ScrollableViewport> { ...@@ -442,13 +442,14 @@ class ScrollableViewportState extends ScrollableState<ScrollableViewport> {
class Block extends StatelessComponent { class Block extends StatelessComponent {
Block({ Block({
Key key, Key key,
this.children, this.children: const <Widget>[],
this.padding, this.padding,
this.initialScrollOffset, this.initialScrollOffset,
this.scrollDirection: Axis.vertical, this.scrollDirection: Axis.vertical,
this.onScroll, this.onScroll,
this.scrollableKey this.scrollableKey
}) : super(key: key) { }) : super(key: key) {
assert(children != null);
assert(!children.any((Widget child) => child == null)); assert(!children.any((Widget child) => child == null));
} }
......
...@@ -11,7 +11,7 @@ class TestMultiChildLayoutDelegate extends MultiChildLayoutDelegate { ...@@ -11,7 +11,7 @@ class TestMultiChildLayoutDelegate extends MultiChildLayoutDelegate {
BoxConstraints getSizeConstraints; BoxConstraints getSizeConstraints;
Size getSize(BoxConstraints constraints) { Size getSize(BoxConstraints constraints) {
if (!RenderObject.debugInDebugDoesMeetConstraints) if (!RenderObject.debugCheckingIntrinsics)
getSizeConstraints = constraints; getSizeConstraints = constraints;
return new Size(200.0, 300.0); return new Size(200.0, 300.0);
} }
...@@ -23,7 +23,7 @@ class TestMultiChildLayoutDelegate extends MultiChildLayoutDelegate { ...@@ -23,7 +23,7 @@ class TestMultiChildLayoutDelegate extends MultiChildLayoutDelegate {
bool performLayoutIsChild; bool performLayoutIsChild;
void performLayout(Size size, BoxConstraints constraints) { void performLayout(Size size, BoxConstraints constraints) {
assert(!RenderObject.debugInDebugDoesMeetConstraints); assert(!RenderObject.debugCheckingIntrinsics);
expect(() { expect(() {
performLayoutSize = size; performLayoutSize = size;
performLayoutConstraints = constraints; performLayoutConstraints = constraints;
...@@ -36,7 +36,7 @@ class TestMultiChildLayoutDelegate extends MultiChildLayoutDelegate { ...@@ -36,7 +36,7 @@ class TestMultiChildLayoutDelegate extends MultiChildLayoutDelegate {
bool shouldRelayoutCalled = false; bool shouldRelayoutCalled = false;
bool shouldRelayoutValue = false; bool shouldRelayoutValue = false;
bool shouldRelayout(_) { bool shouldRelayout(_) {
assert(!RenderObject.debugInDebugDoesMeetConstraints); assert(!RenderObject.debugCheckingIntrinsics);
shouldRelayoutCalled = true; shouldRelayoutCalled = true;
return shouldRelayoutValue; return shouldRelayoutValue;
} }
......
...@@ -14,13 +14,13 @@ class TestOneChildLayoutDelegate extends OneChildLayoutDelegate { ...@@ -14,13 +14,13 @@ class TestOneChildLayoutDelegate extends OneChildLayoutDelegate {
Size childSizeFromGetPositionForChild; Size childSizeFromGetPositionForChild;
Size getSize(BoxConstraints constraints) { Size getSize(BoxConstraints constraints) {
if (!RenderObject.debugInDebugDoesMeetConstraints) if (!RenderObject.debugCheckingIntrinsics)
constraintsFromGetSize = constraints; constraintsFromGetSize = constraints;
return new Size(200.0, 300.0); return new Size(200.0, 300.0);
} }
BoxConstraints getConstraintsForChild(BoxConstraints constraints) { BoxConstraints getConstraintsForChild(BoxConstraints constraints) {
assert(!RenderObject.debugInDebugDoesMeetConstraints); assert(!RenderObject.debugCheckingIntrinsics);
constraintsFromGetConstraintsForChild = constraints; constraintsFromGetConstraintsForChild = constraints;
return new BoxConstraints( return new BoxConstraints(
minWidth: 100.0, minWidth: 100.0,
...@@ -31,7 +31,7 @@ class TestOneChildLayoutDelegate extends OneChildLayoutDelegate { ...@@ -31,7 +31,7 @@ class TestOneChildLayoutDelegate extends OneChildLayoutDelegate {
} }
Offset getPositionForChild(Size size, Size childSize) { Offset getPositionForChild(Size size, Size childSize) {
assert(!RenderObject.debugInDebugDoesMeetConstraints); assert(!RenderObject.debugCheckingIntrinsics);
sizeFromGetPositionForChild = size; sizeFromGetPositionForChild = size;
childSizeFromGetPositionForChild = childSize; childSizeFromGetPositionForChild = childSize;
return Offset.zero; return Offset.zero;
...@@ -40,7 +40,7 @@ class TestOneChildLayoutDelegate extends OneChildLayoutDelegate { ...@@ -40,7 +40,7 @@ class TestOneChildLayoutDelegate extends OneChildLayoutDelegate {
bool shouldRelayoutCalled = false; bool shouldRelayoutCalled = false;
bool shouldRelayoutValue = false; bool shouldRelayoutValue = false;
bool shouldRelayout(_) { bool shouldRelayout(_) {
assert(!RenderObject.debugInDebugDoesMeetConstraints); assert(!RenderObject.debugCheckingIntrinsics);
shouldRelayoutCalled = true; shouldRelayoutCalled = true;
return shouldRelayoutValue; return shouldRelayoutValue;
} }
......
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