From d0d84e1666b7ee394125418e2e1e2db6af2f521f Mon Sep 17 00:00:00 2001 From: Hixie Date: Fri, 2 Oct 2015 15:07:35 -0700 Subject: [PATCH] Sundry debugging aids and fixes (These are all the debugging-related fixes and trivial typo fixes that I extracted out of my heroes branch.) Fix rendering.dart import order. Introduce a debugLabel for Performances so that when you create a performance, you can tag it so that if later you print it out, you can figure out which performance it is. Allow the progress of a PerformanceView to be determined (but not set). Allow subclasses of PerformanceView that are constants to be created by defining a constant constructor for PerformanceView. Introduce a debugPrint() method that throttles its output. This is a test to see if it resolves the problems people have been having with debugDumpRenderTree() et al having their output corrupted on Android. It turns out (according to some things I read On The Internets) that Android only has a 64KB kernel buffer for its logs and and if you output to it too fast, it'll drop data on the floor. If this does in fact reliably resolve this problem, we should probably move the fix over to C++ land (where "print" is implemented) so that any use of print is handled (avoiding the interleaving problem we have now if you use both debugPrint() and print()). Fix a bug with the debugging code for "size". In the specific case of a RenderBox having a parent that doesn't set parentUsesSize, then later the parent setting parentUsesSize but the child having its layout short-circuited (e.g. because the constraints didn't change), we didn't update the _DebugSize object to know that now it's ok that the size be used by the parent, and we'd assert. Also, allow a _DebugSize to be used to set the size of yourself. Previously you could only set your size from a regular Size or from your child's _DebugSize. Add more debugging information to various Widgets where it might be helpful. Make GlobalKey's toString() include the runtimeType so that when subclassing it the new class doesn't claim to be a GlobalKey instance. Include the Widget's key in the Element's description since we don't include it in the detailed description normally (it's in the name part). Fix a test that was returning null from a route. --- packages/flutter/lib/rendering.dart | 2 +- .../lib/src/animation/performance.dart | 18 ++++++- packages/flutter/lib/src/material/drawer.dart | 2 +- .../flutter/lib/src/rendering/binding.dart | 3 +- packages/flutter/lib/src/rendering/box.dart | 23 +++++--- packages/flutter/lib/src/rendering/debug.dart | 33 ++++++++++++ .../flutter/lib/src/rendering/object.dart | 41 ++++++++++---- .../flutter/lib/src/rendering/proxy_box.dart | 2 +- packages/flutter/lib/src/widgets/basic.dart | 53 +++++++++++++++++++ packages/flutter/lib/src/widgets/binding.dart | 4 +- .../flutter/lib/src/widgets/framework.dart | 29 +++++----- .../lib/src/widgets/mixed_viewport.dart | 2 +- packages/unit/test/widget/drawer_test.dart | 4 +- 13 files changed, 179 insertions(+), 37 deletions(-) diff --git a/packages/flutter/lib/rendering.dart b/packages/flutter/lib/rendering.dart index f7ed579fcf..24e7b46054 100644 --- a/packages/flutter/lib/rendering.dart +++ b/packages/flutter/lib/rendering.dart @@ -7,6 +7,7 @@ library rendering; export 'src/rendering/auto_layout.dart'; export 'src/rendering/basic_types.dart'; +export 'src/rendering/binding.dart'; export 'src/rendering/block.dart'; export 'src/rendering/box.dart'; export 'src/rendering/debug.dart'; @@ -22,7 +23,6 @@ export 'src/rendering/object.dart'; export 'src/rendering/paragraph.dart'; export 'src/rendering/proxy_box.dart'; export 'src/rendering/shifted_box.dart'; -export 'src/rendering/binding.dart'; export 'src/rendering/stack.dart'; export 'src/rendering/statistics_box.dart'; export 'src/rendering/toggleable.dart'; diff --git a/packages/flutter/lib/src/animation/performance.dart b/packages/flutter/lib/src/animation/performance.dart index 0b8a020f17..e959d16961 100644 --- a/packages/flutter/lib/src/animation/performance.dart +++ b/packages/flutter/lib/src/animation/performance.dart @@ -31,6 +31,8 @@ typedef void PerformanceStatusListener(PerformanceStatus status); /// want to watch a performance but should not be able to change the /// performance's state. abstract class PerformanceView { + const PerformanceView(); + /// Update the given variable according to the current progress of the performance void updateVariable(Animatable variable); /// Calls the listener every time the progress of the performance changes @@ -45,6 +47,10 @@ abstract class PerformanceView { /// The current status of this animation PerformanceStatus get status; + /// The current progress of this animation (a value from 0.0 to 1.0). + /// This is the value that is used to update any variables when using updateVariable(). + double get progress; + /// Whether this animation is stopped at the beginning bool get isDismissed => status == PerformanceStatus.dismissed; @@ -61,12 +67,16 @@ abstract class PerformanceView { /// [fling] the timeline causing a physics-based simulation to take over the /// progression. class Performance extends PerformanceView { - Performance({ this.duration, double progress }) { + Performance({ this.duration, double progress, this.debugLabel }) { _timeline = new SimulationStepper(_tick); if (progress != null) _timeline.value = progress.clamp(0.0, 1.0); } + /// A label that is used in the toString() output. Intended to aid with + /// identifying performance instances in debug output. + final String debugLabel; + /// Returns a [PerformanceView] for this performance, /// so that a pointer to this object can be passed around without /// allowing users of that pointer to mutate the AnimationPerformance state. @@ -220,6 +230,12 @@ class Performance extends PerformanceView { _notifyListeners(); _checkStatusChanged(); } + + String toString() { + if (debugLabel != null) + return '$runtimeType at $progress for $debugLabel'; + return '$runtimeType at $progress'; + } } /// An animation performance with an animated variable with a concrete type diff --git a/packages/flutter/lib/src/material/drawer.dart b/packages/flutter/lib/src/material/drawer.dart index d41e0875de..35ea2ef596 100644 --- a/packages/flutter/lib/src/material/drawer.dart +++ b/packages/flutter/lib/src/material/drawer.dart @@ -106,7 +106,7 @@ class _DrawerRoute extends Route { final int level; PerformanceView get performance => _performance?.view; - Performance _performance = new Performance(duration: _kBaseSettleDuration); + Performance _performance = new Performance(duration: _kBaseSettleDuration, debugLabel: 'Drawer'); bool get opaque => false; diff --git a/packages/flutter/lib/src/rendering/binding.dart b/packages/flutter/lib/src/rendering/binding.dart index e501ff39fa..6b36f42150 100644 --- a/packages/flutter/lib/src/rendering/binding.dart +++ b/packages/flutter/lib/src/rendering/binding.dart @@ -6,6 +6,7 @@ import 'dart:ui' as ui; import 'package:flutter/animation.dart'; import 'package:flutter/gestures.dart'; +import 'package:flutter/rendering.dart'; import 'box.dart'; import 'hit_test.dart'; @@ -252,5 +253,5 @@ class FlutterBinding extends HitTestTarget { /// Prints a textual representation of the entire render tree void debugDumpRenderTree() { - FlutterBinding.instance.renderView.toStringDeep().split('\n').forEach(print); + debugPrint(FlutterBinding.instance.renderView.toStringDeep()); } diff --git a/packages/flutter/lib/src/rendering/box.dart b/packages/flutter/lib/src/rendering/box.dart index 968a8c80ca..b26a99adbb 100644 --- a/packages/flutter/lib/src/rendering/box.dart +++ b/packages/flutter/lib/src/rendering/box.dart @@ -359,9 +359,11 @@ abstract class RenderBox extends RenderObject { final _DebugSize _size = this._size; assert(_size._owner == this); if (RenderObject.debugActiveLayout != null) { - // we are always allowed to access our own size (for print debugging and asserts if nothing else) - // other than us, the only object that's allowed to read our size is our parent, if they're said they will - // if you hit this assert trying to access a child's size, pass parentUsesSize: true in layout() + // We are always allowed to access our own size (for print debugging + // and asserts if nothing else). Other than us, the only object that's + // allowed to read our size is our parent, if they've said they will. + // If you hit this assert trying to access a child's size, pass + // "parentUsesSize: true" to that child's layout(). assert(debugDoingThisResize || debugDoingThisLayout || (RenderObject.debugActiveLayout == parent && _size._canBeUsedByParent)); } @@ -377,8 +379,12 @@ abstract class RenderBox extends RenderObject { assert((sizedByParent && debugDoingThisResize) || (!sizedByParent && debugDoingThisLayout)); assert(() { - if (value is _DebugSize) - return value._canBeUsedByParent && value._owner.parent == this; + if (value is _DebugSize) { + if (value._owner != this) { + assert(value._owner.parent == this); + assert(value._canBeUsedByParent); + } + } return true; }); _size = value; @@ -389,6 +395,11 @@ abstract class RenderBox extends RenderObject { assert(debugDoesMeetConstraints()); } + void debugResetSize() { + // updates the value of size._canBeUsedByParent if necessary + size = size; + } + Map _cachedBaselines; bool _ancestorUsesBaseline = false; static bool _debugDoingBaseline = false; @@ -473,7 +484,7 @@ abstract class RenderBox extends RenderObject { }); bool result = constraints.isSatisfiedBy(_size); if (!result) - print("${this.runtimeType} does not meet its constraints. Constraints: $constraints, size: $_size"); + debugPrint("${this.runtimeType} does not meet its constraints. Constraints: $constraints, size: $_size"); return result; } diff --git a/packages/flutter/lib/src/rendering/debug.dart b/packages/flutter/lib/src/rendering/debug.dart index cf2df69323..49a6937ee8 100644 --- a/packages/flutter/lib/src/rendering/debug.dart +++ b/packages/flutter/lib/src/rendering/debug.dart @@ -3,6 +3,8 @@ // found in the LICENSE file. import 'dart:ui' as ui; +import 'dart:async'; +import 'dart:collection'; /// Causes each RenderBox to paint a box around its bounds. bool debugPaintSizeEnabled = false; @@ -30,3 +32,34 @@ bool debugPaintBoundsEnabled = false; /// The color to use when painting RenderError boxes in checked mode. ui.Color debugErrorBoxColor = const ui.Color(0xFFFF0000); + +/// Prints a message to the console, which you can access using the "flutter" +/// tool's "logs" command ("flutter logs"). +/// +/// This function very crudely attempts to throttle the rate at which messages +/// are sent to avoid data loss on Android. This means that interleaving calls +/// to this function (directly or indirectly via [debugDumpRenderTree] or +/// [debugDumpApp]) and to the Dart [print] method can result in out-of-order +/// messages in the logs. +void debugPrint(String message) { + _debugPrintBuffer.addAll(message.split('\n')); + if (!_debugPrintScheduled) + _debugPrintTask(); +} +int _debugPrintedCharacters = 0; +int _kDebugPrintCapacity = 32 * 1024; +Queue _debugPrintBuffer = new Queue(); +bool _debugPrintScheduled = false; +void _debugPrintTask() { + _debugPrintScheduled = false; + while (_debugPrintedCharacters < _kDebugPrintCapacity && _debugPrintBuffer.length > 0) { + String line = _debugPrintBuffer.removeFirst(); + _debugPrintedCharacters += line.length; // TODO(ianh): Use the UTF-8 byte length instead + print(line); + } + if (_debugPrintBuffer.length > 0) { + _debugPrintScheduled = true; + _debugPrintedCharacters = 0; + new Timer(new Duration(seconds: 1), _debugPrintTask); + } +} diff --git a/packages/flutter/lib/src/rendering/object.dart b/packages/flutter/lib/src/rendering/object.dart index 679ccecad5..a203e74dac 100644 --- a/packages/flutter/lib/src/rendering/object.dart +++ b/packages/flutter/lib/src/rendering/object.dart @@ -513,14 +513,14 @@ abstract class RenderObject extends AbstractNode implements HitTestTarget { dynamic debugExceptionContext = ''; void _debugReportException(String method, dynamic exception, StackTrace stack) { - print('-- EXCEPTION --'); - print('The following exception was raised during $method():'); - print('$exception'); - print('Stack trace:'); - print('$stack'); - print('The following RenderObject was being processed when the exception was fired:\n${this}'); + debugPrint('-- EXCEPTION --'); + debugPrint('The following exception was raised during $method():'); + debugPrint('$exception'); + debugPrint('Stack trace:'); + debugPrint('$stack'); + debugPrint('The following RenderObject was being processed when the exception was fired:\n${this}'); if (debugExceptionContext != '') - 'That RenderObject had the following exception context:\n$debugExceptionContext'.split('\n').forEach(print); + debugPrint('That RenderObject had the following exception context:\n$debugExceptionContext'); if (debugRenderingExceptionHandler != null) debugRenderingExceptionHandler(this, method, exception, stack); } @@ -723,15 +723,30 @@ abstract class RenderObject extends AbstractNode implements HitTestTarget { else relayoutSubtreeRoot = parent._relayoutSubtreeRoot; assert(parent == this.parent); - if (!needsLayout && constraints == _constraints && relayoutSubtreeRoot == _relayoutSubtreeRoot) + assert(() { + _debugCanParentUseSize = parentUsesSize; + return true; + }); + if (!needsLayout && constraints == _constraints && relayoutSubtreeRoot == _relayoutSubtreeRoot) { + assert(() { + // in case parentUsesSize changed since the last invocation, set size + // to itself, so it has the right internal debug values. + _debugDoingThisLayout = true; + RenderObject debugPreviousActiveLayout = _debugActiveLayout; + _debugActiveLayout = this; + debugResetSize(); + _debugActiveLayout = debugPreviousActiveLayout; + _debugDoingThisLayout = false; + return true; + }); return; + } _constraints = constraints; _relayoutSubtreeRoot = relayoutSubtreeRoot; assert(!_debugMutationsLocked); assert(!_doingThisLayoutWithCallback); assert(() { _debugMutationsLocked = true; - _debugCanParentUseSize = parentUsesSize; return true; }); if (sizedByParent) { @@ -768,6 +783,14 @@ abstract class RenderObject extends AbstractNode implements HitTestTarget { assert(parent == this.parent); } + /// If a subclass has a "size" (the state controlled by "parentUsesSize", + /// whatever it is in the subclass, e.g. the actual "size" property of + /// RenderBox), and the subclass verifies that in checked mode this "size" + /// property isn't used when debugCanParentUseSize isn't set, then that + /// subclass should override debugResetSize() to reapply the current values of + /// debugCanParentUseSize to that state. + void debugResetSize() { } + /// Whether the constraints are the only input to the sizing algorithm (in /// particular, child nodes have no impact) /// diff --git a/packages/flutter/lib/src/rendering/proxy_box.dart b/packages/flutter/lib/src/rendering/proxy_box.dart index 805d506321..198d834f04 100644 --- a/packages/flutter/lib/src/rendering/proxy_box.dart +++ b/packages/flutter/lib/src/rendering/proxy_box.dart @@ -1018,7 +1018,7 @@ class RenderTransform extends RenderProxyBox { String debugDescribeSettings(String prefix) { List result = _transform.toString().split('\n').map((String s) => '$prefix $s\n').toList(); result.removeLast(); - return '${super.debugDescribeSettings(prefix)}${prefix}transform matrix:\n${result.join()}\n${prefix}origin: $origin\nalignment: $alignment\n'; + return '${super.debugDescribeSettings(prefix)}${prefix}transform matrix:\n${result.join()}\n${prefix}origin: $origin\n${prefix}alignment: $alignment\n'; } } diff --git a/packages/flutter/lib/src/widgets/basic.dart b/packages/flutter/lib/src/widgets/basic.dart index eec0075f46..776e33bd4c 100644 --- a/packages/flutter/lib/src/widgets/basic.dart +++ b/packages/flutter/lib/src/widgets/basic.dart @@ -76,6 +76,11 @@ class Opacity extends OneChildRenderObjectWidget { void updateRenderObject(RenderOpacity renderObject, Opacity oldWidget) { renderObject.opacity = opacity; } + + void debugFillDescription(List description) { + super.debugFillDescription(description); + description.add('opacity: $opacity'); + } } class ColorFilter extends OneChildRenderObjectWidget { @@ -283,6 +288,14 @@ class SizedBox extends OneChildRenderObjectWidget { void updateRenderObject(RenderConstrainedBox renderObject, SizedBox oldWidget) { renderObject.additionalConstraints = _additionalConstraints; } + + void debugFillDescription(List description) { + super.debugFillDescription(description); + if (width != null) + description.add('width: $width'); + if (height != null) + description.add('height: $height'); + } } class ConstrainedBox extends OneChildRenderObjectWidget { @@ -298,6 +311,11 @@ class ConstrainedBox extends OneChildRenderObjectWidget { void updateRenderObject(RenderConstrainedBox renderObject, ConstrainedBox oldWidget) { renderObject.additionalConstraints = constraints; } + + void debugFillDescription(List description) { + super.debugFillDescription(description); + description.add('constraints: $constraints'); + } } class FractionallySizedBox extends OneChildRenderObjectWidget { @@ -316,6 +334,14 @@ class FractionallySizedBox extends OneChildRenderObjectWidget { renderObject.widthFactor = width; renderObject.heightFactor = height; } + + void debugFillDescription(List description) { + super.debugFillDescription(description); + if (width != null) + description.add('width: $width'); + if (height != null) + description.add('height: $height'); + } } class OverflowBox extends OneChildRenderObjectWidget { @@ -355,6 +381,11 @@ class AspectRatio extends OneChildRenderObjectWidget { void updateRenderObject(RenderAspectRatio renderObject, AspectRatio oldWidget) { renderObject.aspectRatio = aspectRatio; } + + void debugFillDescription(List description) { + super.debugFillDescription(description); + description.add('aspectRatio: $aspectRatio'); + } } class IntrinsicWidth extends OneChildRenderObjectWidget { @@ -644,6 +675,18 @@ class Positioned extends ParentDataWidget { if (needsLayout) renderObject.markNeedsLayout(); } + + void debugFillDescription(List description) { + super.debugFillDescription(description); + if (left != null) + description.add('left: $left'); + if (top != null) + description.add('top: $top'); + if (right != null) + description.add('right: $right'); + if (bottom != null) + description.add('bottom: $bottom'); + } } class Grid extends MultiChildRenderObjectWidget { @@ -728,6 +771,11 @@ class Flexible extends ParentDataWidget { renderObject.markNeedsLayout(); } } + + void debugFillDescription(List description) { + super.debugFillDescription(description); + description.add('flex: $flex'); + } } class Paragraph extends LeafRenderObjectWidget { @@ -1077,6 +1125,11 @@ class MetaData extends OneChildRenderObjectWidget { void updateRenderObject(RenderMetaData renderObject, MetaData oldWidget) { renderObject.metaData = metaData; } + + void debugFillDescription(List description) { + super.debugFillDescription(description); + description.add('$metaData'); + } } class KeyedSubtree extends StatelessComponent { diff --git a/packages/flutter/lib/src/widgets/binding.dart b/packages/flutter/lib/src/widgets/binding.dart index 8ceeb73244..dcead3da1c 100644 --- a/packages/flutter/lib/src/widgets/binding.dart +++ b/packages/flutter/lib/src/widgets/binding.dart @@ -94,8 +94,8 @@ void debugDumpApp() { assert(WidgetFlutterBinding.instance.renderViewElement != null); String mode = 'RELEASE MODE'; assert(() { mode = 'CHECKED MODE'; return true; }); - print('${WidgetFlutterBinding.instance.runtimeType} - $mode'); - WidgetFlutterBinding.instance.renderViewElement.toStringDeep().split('\n').forEach(print); + debugPrint('${WidgetFlutterBinding.instance.runtimeType} - $mode'); + debugPrint(WidgetFlutterBinding.instance.renderViewElement.toStringDeep()); } /// This class provides a bridge from a RenderObject to an Element tree. The diff --git a/packages/flutter/lib/src/widgets/framework.dart b/packages/flutter/lib/src/widgets/framework.dart index 1cfd022b49..891f7ebd71 100644 --- a/packages/flutter/lib/src/widgets/framework.dart +++ b/packages/flutter/lib/src/widgets/framework.dart @@ -7,6 +7,8 @@ import 'dart:collection'; import 'package:flutter/rendering.dart'; +export 'package:flutter/rendering.dart' show debugPrint; + // KEYS /// A Key is an identifier for [Widget]s and [Element]s. A new Widget will only @@ -184,7 +186,7 @@ class GlobalObjectKey extends GlobalKey { return identical(value, typedOther.value); } int get hashCode => identityHashCode(value); - String toString() => '[GlobalKey ${value.runtimeType}(${value.hashCode})]'; + String toString() => '[$runtimeType ${value.runtimeType}(${value.hashCode})]'; } @@ -823,10 +825,13 @@ abstract class Element implements BuildContext { void debugFillDescription(List description) { if (depth == null) description.add('no depth'); - if (widget == null) + if (widget == null) { description.add('no widget'); - else + } else { + if (widget.key != null) + description.add('${widget.key}'); widget.debugFillDescription(description); + } } String toStringDeep([String prefixLineOne = '', String prefixOtherLines = '']) { @@ -1090,7 +1095,7 @@ class StatefulComponentElement> assert(() { if (_state._debugLifecycleState == _StateLifecycle.initialized) return true; - print('${_state.runtimeType}.initState failed to call super.initState'); + debugPrint('${_state.runtimeType}.initState failed to call super.initState'); return false; }); assert(() { _state._debugLifecycleState = _StateLifecycle.ready; return true; }); @@ -1123,7 +1128,7 @@ class StatefulComponentElement> assert(() { if (_state._debugLifecycleState == _StateLifecycle.defunct) return true; - print('${_state.runtimeType}.dispose failed to call super.dispose'); + debugPrint('${_state.runtimeType}.dispose failed to call super.dispose'); return false; }); assert(!dirty); // See BuildableElement.unmount for why this is important. @@ -1267,7 +1272,7 @@ abstract class RenderObjectElement extends Buildab // dirty, e.g. if they have a builder callback. (Builder callbacks have a // 'BuildContext' argument which you can pass to Theme.of() and other // InheritedWidget APIs which eventually trigger a rebuild.) - print('$runtimeType failed to implement reinvokeBuilders(), but got marked dirty'); + debugPrint('$runtimeType failed to implement reinvokeBuilders(), but got marked dirty'); assert(() { 'reinvokeBuilders() not implemented'; return false; @@ -1609,11 +1614,11 @@ void _debugReportException(String context, dynamic exception, StackTrace stack) if (debugWidgetsExceptionHandler != null) { debugWidgetsExceptionHandler(context, exception, stack); } else { - print('------------------------------------------------------------------------'); - 'Exception caught while $context'.split('\n').forEach(print); - print('$exception'); - print('Stack trace:'); - '$stack'.split('\n').forEach(print); - print('------------------------------------------------------------------------'); + debugPrint('------------------------------------------------------------------------'); + debugPrint('Exception caught while $context'); + debugPrint('$exception'); + debugPrint('Stack trace:'); + debugPrint('$stack'); + debugPrint('------------------------------------------------------------------------'); } } diff --git a/packages/flutter/lib/src/widgets/mixed_viewport.dart b/packages/flutter/lib/src/widgets/mixed_viewport.dart index 2965054988..95e8cdfabc 100644 --- a/packages/flutter/lib/src/widgets/mixed_viewport.dart +++ b/packages/flutter/lib/src/widgets/mixed_viewport.dart @@ -333,7 +333,7 @@ class _MixedViewportElement extends RenderObjectElement { double newExtent = _getElementExtent(element, innerConstraints); bool result = _childExtents[index] == newExtent; if (!result) - print("Element $element at index $index was size ${_childExtents[index]} but is now size $newExtent yet no invalidate() was received to that effect"); + debugPrint("Element $element at index $index was size ${_childExtents[index]} but is now size $newExtent yet no invalidate() was received to that effect"); return result; } diff --git a/packages/unit/test/widget/drawer_test.dart b/packages/unit/test/widget/drawer_test.dart index 631ae43ef9..cc5fe28eae 100644 --- a/packages/unit/test/widget/drawer_test.dart +++ b/packages/unit/test/widget/drawer_test.dart @@ -14,7 +14,7 @@ void main() { routes: { '/': (RouteArguments args) { navigator = args.navigator; - new Container(); + return new Container(); } } ) @@ -43,7 +43,7 @@ void main() { routes: { '/': (RouteArguments args) { navigator = args.navigator; - new Container(); + return new Container(); } } )