Merge pull request #2165 from Hixie/size-obs-3

SizeObserver crusade: ScrollableViewport and tabs

packages/flutter/lib/src/material/tabs.dart

@@ -699,10 +699,10 @@ class _TabBarState<T> extends ScrollableState<TabBar<T>> implements TabBarSelect
   }
 
   void _updateScrollBehavior() {
-    scrollBehavior.updateExtents(
+    scrollTo(scrollBehavior.updateExtents(
       containerExtent: config.scrollDirection == Axis.vertical ? _viewportSize.height : _viewportSize.width,
       contentExtent: _tabWidths.reduce((double sum, double width) => sum + width)
-    );
+    ));
   }
 
   void _layoutChanged(Size tabBarSize, List<double> tabWidths) {
@@ -713,11 +713,16 @@ class _TabBarState<T> extends ScrollableState<TabBar<T>> implements TabBarSelect
     });
   }
 
-  void _handleViewportSizeChanged(Size newSize) {
-    _viewportSize = newSize;
+  Offset _handlePaintOffsetUpdateNeeded(ViewportDimensions dimensions) {
+    // We make various state changes here but don't have to do so in a
+    // setState() callback because we are called during layout and all
+    // we're updating is the new offset, which we are providing to the
+    // render object via our return value.
+    _viewportSize = dimensions.containerSize;
     _updateScrollBehavior();
     if (config.isScrollable)
       scrollTo(_centeredTabScrollOffset(_selection.index), duration: _kTabBarScroll);
+    return scrollOffsetToPixelDelta(scrollOffset);
   }
 
   Widget buildContent(BuildContext context) {
@@ -772,13 +777,11 @@ class _TabBarState<T> extends ScrollableState<TabBar<T>> implements TabBarSelect
     );
 
     if (config.isScrollable) {
-      contents = new SizeObserver(
-        onSizeChanged: _handleViewportSizeChanged,
-        child: new Viewport(
-          scrollDirection: Axis.horizontal,
-          paintOffset: scrollOffsetToPixelDelta(scrollOffset),
-          child: contents
-        )
+      child: new Viewport(
+        scrollDirection: Axis.horizontal,
+        paintOffset: scrollOffsetToPixelDelta(scrollOffset),
+        onPaintOffsetUpdateNeeded: _handlePaintOffsetUpdateNeeded,
+        child: contents
       );
     }
 

packages/flutter/lib/src/rendering/viewport.dart

@@ -39,6 +39,20 @@ class ViewportDimensions {
         return paintOffset + (containerSize - contentSize);
     }
   }
+
+  bool operator ==(dynamic other) {
+    if (identical(this, other))
+      return true;
+    if (other is! ViewportDimensions)
+      return false;
+    final ViewportDimensions typedOther = other;
+    return contentSize == typedOther.contentSize &&
+           containerSize == typedOther.containerSize;
+  }
+
+  int get hashCode => hashValues(contentSize, containerSize);
+
+  String toString() => 'ViewportDimensions(container: $containerSize, content: $contentSize)';
 }
 
 abstract class HasScrollDirection {
@@ -163,6 +177,8 @@ class RenderViewportBase extends RenderBox implements HasScrollDirection {
 
 }
 
+typedef Offset ViewportDimensionsChangeCallback(ViewportDimensions dimensions);
+
 /// A render object that's bigger on the inside.
 ///
 /// The child of a viewport can layout to a larger size than the viewport
@@ -176,11 +192,16 @@ class RenderViewport extends RenderViewportBase with RenderObjectWithChildMixin<
     Offset paintOffset: Offset.zero,
     Axis scrollDirection: Axis.vertical,
     ViewportAnchor scrollAnchor: ViewportAnchor.start,
-    Painter overlayPainter
+    Painter overlayPainter,
+    this.onPaintOffsetUpdateNeeded
   }) : super(paintOffset, scrollDirection, scrollAnchor, overlayPainter) {
     this.child = child;
   }
 
+  /// Called during [layout] to report the dimensions of the viewport
+  /// and its child.
+  ViewportDimensionsChangeCallback onPaintOffsetUpdateNeeded;
+
   BoxConstraints _getInnerConstraints(BoxConstraints constraints) {
     BoxConstraints innerConstraints;
     switch (scrollDirection) {
@@ -228,6 +249,7 @@ class RenderViewport extends RenderViewportBase with RenderObjectWithChildMixin<
   // parent was baseline-aligned, which makes no sense.
 
   void performLayout() {
+    ViewportDimensions oldDimensions = dimensions;
     if (child != null) {
       child.layout(_getInnerConstraints(constraints), parentUsesSize: true);
       size = constraints.constrain(child.size);
@@ -238,6 +260,9 @@ class RenderViewport extends RenderViewportBase with RenderObjectWithChildMixin<
       performResize();
       dimensions = new ViewportDimensions(containerSize: size);
     }
+    if (onPaintOffsetUpdateNeeded != null && dimensions != oldDimensions)
+      paintOffset = onPaintOffsetUpdateNeeded(dimensions);
+    assert(paintOffset != null);
   }
 
   bool _shouldClipAtPaintOffset(Offset paintOffset) {

packages/flutter/lib/src/services/binding.dart

@@ -37,6 +37,8 @@ abstract class BindingBase {
   void initInstances() {
     assert(() { _debugInitialized = true; return true; });
   }
+
+  String toString() => '<$runtimeType>';
 }
 
 // A replacement for shell.connectToService.  Implementations should return true

packages/flutter/lib/src/widgets/basic.dart

@@ -43,7 +43,9 @@ export 'package:flutter/rendering.dart' show
     RelativeRect,
     ShaderCallback,
     ValueChanged,
-    ViewportAnchor;
+    ViewportAnchor,
+    ViewportDimensions,
+    ViewportDimensionsChangeCallback;
 
 // PAINTING NODES
 
@@ -777,6 +779,7 @@ class Viewport extends OneChildRenderObjectWidget {
     this.scrollDirection: Axis.vertical,
     this.scrollAnchor: ViewportAnchor.start,
     this.overlayPainter,
+    this.onPaintOffsetUpdateNeeded,
     Widget child
   }) : super(key: key, child: child) {
     assert(scrollDirection != null);
@@ -802,11 +805,14 @@ class Viewport extends OneChildRenderObjectWidget {
   /// Often used to paint scroll bars.
   final Painter overlayPainter;
 
+  final ViewportDimensionsChangeCallback onPaintOffsetUpdateNeeded;
+
   RenderViewport createRenderObject() {
     return new RenderViewport(
       paintOffset: paintOffset,
       scrollDirection: scrollDirection,
       scrollAnchor: scrollAnchor,
+      onPaintOffsetUpdateNeeded: onPaintOffsetUpdateNeeded,
       overlayPainter: overlayPainter
     );
   }
@@ -817,6 +823,7 @@ class Viewport extends OneChildRenderObjectWidget {
       ..scrollDirection = scrollDirection
       ..scrollAnchor = scrollAnchor
       ..paintOffset = paintOffset
+      ..onPaintOffsetUpdateNeeded = onPaintOffsetUpdateNeeded
       ..overlayPainter = overlayPainter;
   }
 }

packages/flutter/lib/src/widgets/gesture_detector.dart

@@ -280,29 +280,29 @@ class RawGestureDetectorState extends State<RawGestureDetector> {
   void replaceGestureRecognizers(Map<Type, GestureRecognizerFactory> gestures) {
     assert(() {
       RenderObject renderObject = context.findRenderObject();
-      assert(renderObject is RenderPointerListener);
-      RenderPointerListener listener = renderObject;
-      RenderBox descendant = listener.child;
       if (!config.excludeFromSemantics) {
-        assert(descendant is RenderSemanticsGestureHandler);
-        RenderSemanticsGestureHandler semanticsGestureHandler = descendant;
-        descendant = semanticsGestureHandler.child;
+        assert(renderObject is RenderSemanticsGestureHandler);
+        RenderSemanticsGestureHandler semanticsGestureHandler = renderObject;
+        renderObject = semanticsGestureHandler.child;
       }
-      assert(descendant != null);
-      if (!descendant.debugDoingThisLayout) {
+      assert(renderObject is RenderPointerListener);
+      RenderPointerListener pointerListener = renderObject;
+      renderObject = pointerListener.child;
+      if (!renderObject.debugDoingThisLayout) {
         throw new WidgetError(
           'replaceGestureRecognizers() can only be called during the layout phase of the GestureDetector\'s nearest descendant RenderObjectWidget.\n'
           'In this particular case, that is:\n'
-          '  $descendant'
+          '  $renderObject'
         );
       }
       return true;
     });
     _syncAll(gestures);
     if (!config.excludeFromSemantics) {
-      RenderPointerListener listener = context.findRenderObject();
-      RenderSemanticsGestureHandler semanticsGestureHandler = listener.child;
-      context.visitChildElements((RenderObjectElement element) => element.widget.updateRenderObject(semanticsGestureHandler, null));
+      RenderSemanticsGestureHandler semanticsGestureHandler = context.findRenderObject();
+      context.visitChildElements((RenderObjectElement element) {
+        element.widget.updateRenderObject(semanticsGestureHandler, null);
+      });
     }
   }
 

packages/flutter/lib/src/widgets/mixed_viewport.dart

@@ -10,7 +10,6 @@ import 'framework.dart';
 import 'basic.dart';
 
 typedef Widget IndexedBuilder(BuildContext context, int index); // return null if index is greater than index of last entry
-typedef void ExtentsUpdateCallback(double newExtents);
 typedef void InvalidatorCallback(Iterable<int> indices);
 typedef void InvalidatorAvailableCallback(InvalidatorCallback invalidator);
 
@@ -23,7 +22,7 @@ class MixedViewport extends RenderObjectWidget {
     this.direction: Axis.vertical,
     this.builder,
     this.token,
-    this.onExtentsUpdate,
+    this.onExtentChanged,
     this.onInvalidatorAvailable
   }) : super(key: key);
 
@@ -31,7 +30,7 @@ class MixedViewport extends RenderObjectWidget {
   final Axis direction;
   final IndexedBuilder builder;
   final Object token; // change this if the list changed (i.e. there are added, removed, or resorted items)
-  final ExtentsUpdateCallback onExtentsUpdate;
+  final ValueChanged<double> onExtentChanged;
   final InvalidatorAvailableCallback onInvalidatorAvailable; // call the callback this gives to invalidate sizes
 
   _MixedViewportElement createElement() => new _MixedViewportElement(this);
@@ -108,8 +107,8 @@ class _MixedViewportElement extends RenderObjectElement<MixedViewport> {
   /// The constraints for which the current offsets are valid.
   BoxConstraints _lastLayoutConstraints;
 
-  /// The last value that was sent to onExtentsUpdate.
-  double _lastReportedExtents;
+  /// The last value that was sent to onExtentChanged.
+  double _lastReportedExtent;
 
   RenderBlockViewport get renderObject => super.renderObject;
 
@@ -227,11 +226,11 @@ class _MixedViewportElement extends RenderObjectElement<MixedViewport> {
     BuildableElement.lockState(() {
       _doLayout(constraints);
     }, building: true);
-    if (widget.onExtentsUpdate != null) {
-      final double newExtents = _didReachLastChild ? _childOffsets.last : null;
-      if (newExtents != _lastReportedExtents) {
-        _lastReportedExtents = newExtents;
-        widget.onExtentsUpdate(_lastReportedExtents);
+    if (widget.onExtentChanged != null) {
+      final double newExtent = _didReachLastChild ? _childOffsets.last : null;
+      if (newExtent != _lastReportedExtent) {
+        _lastReportedExtent = newExtent;
+        widget.onExtentChanged(_lastReportedExtent);
       }
     }
   }

packages/flutter/lib/src/widgets/scroll_behavior.dart

@@ -35,6 +35,15 @@ abstract class ScrollBehavior<T, U> {
 
   /// Whether this scroll behavior currently permits scrolling
   bool get isScrollable => true;
+
+  String toString() {
+    List<String> description = <String>[];
+    debugFillDescription(description);
+    return '$runtimeType(${description.join("; ")})';
+  }
+  void debugFillDescription(List<String> description) {
+    description.add(isScrollable ? 'scrollable' : 'not scrollable');
+  }
 }
 
 /// A scroll behavior for a scrollable widget with linear extent (i.e.
@@ -74,6 +83,13 @@ abstract class ExtentScrollBehavior extends ScrollBehavior<double, double> {
 
   /// The maximum value the scroll offset can obtain.
   double get maxScrollOffset;
+
+  void debugFillDescription(List<String> description) {
+    super.debugFillDescription(description);
+    description.add('content: ${contentExtent.toStringAsFixed(1)}');
+    description.add('container: ${contentExtent.toStringAsFixed(1)}');
+    description.add('range: ${minScrollOffset?.toStringAsFixed(1)} .. ${maxScrollOffset?.toStringAsFixed(1)}');
+  }
 }
 
 /// A scroll behavior that prevents the user from exceeding scroll bounds.

packages/flutter/lib/src/widgets/scrollable.dart

@@ -237,32 +237,42 @@ abstract class ScrollableState<T extends Scrollable> extends State<T> {
     return _scrollBehavior;
   }
 
-  GestureDragStartCallback _getDragStartHandler(Axis direction) {
-    if (config.scrollDirection != direction || !scrollBehavior.isScrollable)
-      return null;
-    return _handleDragStart;
+  Map<Type, GestureRecognizerFactory> buildGestureDetectors() {
+    if (scrollBehavior.isScrollable) {
+      switch (config.scrollDirection) {
+        case Axis.vertical:
+          return <Type, GestureRecognizerFactory>{
+            VerticalDragGestureRecognizer: (VerticalDragGestureRecognizer recognizer) {
+              return (recognizer ??= new VerticalDragGestureRecognizer())
+                ..onStart = _handleDragStart
+                ..onUpdate = _handleDragUpdate
+                ..onEnd = _handleDragEnd;
+            }
+          };
+        case Axis.horizontal:
+          return <Type, GestureRecognizerFactory>{
+            HorizontalDragGestureRecognizer: (HorizontalDragGestureRecognizer recognizer) {
+              return (recognizer ??= new HorizontalDragGestureRecognizer())
+                ..onStart = _handleDragStart
+                ..onUpdate = _handleDragUpdate
+                ..onEnd = _handleDragEnd;
+            }
+          };
+      }
+    }
+    return const <Type, GestureRecognizerFactory>{};
   }
 
-  GestureDragUpdateCallback _getDragUpdateHandler(Axis direction) {
-    if (config.scrollDirection != direction || !scrollBehavior.isScrollable)
-      return null;
-    return _handleDragUpdate;
-  }
+  final GlobalKey _gestureDetectorKey = new GlobalKey();
 
-  GestureDragEndCallback _getDragEndHandler(Axis direction) {
-    if (config.scrollDirection != direction || !scrollBehavior.isScrollable)
-      return null;
-    return _handleDragEnd;
+  void updateGestureDetector() {
+    _gestureDetectorKey.currentState.replaceGestureRecognizers(buildGestureDetectors());
   }
 
   Widget build(BuildContext context) {
-    return new GestureDetector(
-      onVerticalDragStart: _getDragStartHandler(Axis.vertical),
-      onVerticalDragUpdate: _getDragUpdateHandler(Axis.vertical),
-      onVerticalDragEnd: _getDragEndHandler(Axis.vertical),
-      onHorizontalDragStart: _getDragStartHandler(Axis.horizontal),
-      onHorizontalDragUpdate: _getDragUpdateHandler(Axis.horizontal),
-      onHorizontalDragEnd: _getDragEndHandler(Axis.horizontal),
+    return new RawGestureDetector(
+      key: _gestureDetectorKey,
+      gestures: buildGestureDetectors(),
       behavior: HitTestBehavior.opaque,
       child: new Listener(
         child: buildContent(context),
@@ -321,7 +331,7 @@ abstract class ScrollableState<T extends Scrollable> extends State<T> {
     if (endScrollOffset.isNaN)
       return null;
 
-    final double snappedScrollOffset = snapScrollOffset(endScrollOffset);
+    final double snappedScrollOffset = snapScrollOffset(endScrollOffset); // invokes the config.snapOffsetCallback callback
     if (!_scrollOffsetIsInBounds(snappedScrollOffset))
       return null;
 
@@ -443,7 +453,7 @@ abstract class ScrollableState<T extends Scrollable> extends State<T> {
   }
 
   void _handleDragStart(_) {
-    scheduleMicrotask(dispatchOnScrollStart);
+    dispatchOnScrollStart();
   }
 
   void _handleDragUpdate(double delta) {
@@ -503,18 +513,19 @@ class _ScrollableViewportState extends ScrollableState<ScrollableViewport> {
 
   double _viewportSize = 0.0;
   double _childSize = 0.0;
-  void _handleViewportSizeChanged(Size newSize) {
-    _viewportSize = config.scrollDirection == Axis.vertical ? newSize.height : newSize.width;
-    setState(() {
-      _updateScrollBehavior();
-    });
-  }
-  void _handleChildSizeChanged(Size newSize) {
-    _childSize = config.scrollDirection == Axis.vertical ? newSize.height : newSize.width;
-    setState(() {
-      _updateScrollBehavior();
-    });
+
+  Offset _handlePaintOffsetUpdateNeeded(ViewportDimensions dimensions) {
+    // We make various state changes here but don't have to do so in a
+    // setState() callback because we are called during layout and all
+    // we're updating is the new offset, which we are providing to the
+    // render object via our return value.
+    _viewportSize = config.scrollDirection == Axis.vertical ? dimensions.containerSize.height : dimensions.containerSize.width;
+    _childSize = config.scrollDirection == Axis.vertical ? dimensions.contentSize.height : dimensions.contentSize.width;
+    _updateScrollBehavior();
+    updateGestureDetector();
+    return scrollOffsetToPixelDelta(scrollOffset);
   }
+
   void _updateScrollBehavior() {
     // if you don't call this from build(), you must call it from setState().
     scrollTo(scrollBehavior.updateExtents(
@@ -525,17 +536,12 @@ class _ScrollableViewportState extends ScrollableState<ScrollableViewport> {
   }
 
   Widget buildContent(BuildContext context) {
-    return new SizeObserver(
-      onSizeChanged: _handleViewportSizeChanged,
-      child: new Viewport(
-        paintOffset: scrollOffsetToPixelDelta(scrollOffset),
-        scrollDirection: config.scrollDirection,
-        scrollAnchor: config.scrollAnchor,
-        child: new SizeObserver(
-          onSizeChanged: _handleChildSizeChanged,
-          child: config.child
-        )
-      )
+    return new Viewport(
+      paintOffset: scrollOffsetToPixelDelta(scrollOffset),
+      scrollDirection: config.scrollDirection,
+      scrollAnchor: config.scrollAnchor,
+      onPaintOffsetUpdateNeeded: _handlePaintOffsetUpdateNeeded,
+      child: config.child
     );
   }
 }
@@ -690,11 +696,11 @@ class ScrollableMixedWidgetListState extends ScrollableState<ScrollableMixedWidg
     }
   }
 
-  void _handleExtentsUpdate(double newExtents) {
+  void _handleExtentChanged(double newExtent) {
     double newScrollOffset;
     setState(() {
       newScrollOffset = scrollBehavior.updateExtents(
-        contentExtent: newExtents ?? double.INFINITY,
+        contentExtent: newExtent ?? double.INFINITY,
         scrollOffset: scrollOffset
       );
     });
@@ -712,7 +718,7 @@ class ScrollableMixedWidgetListState extends ScrollableState<ScrollableMixedWidg
         builder: config.builder,
         token: config.token,
         onInvalidatorAvailable: config.onInvalidatorAvailable,
-        onExtentsUpdate: _handleExtentsUpdate
+        onExtentChanged: _handleExtentChanged
       )
     );
   }

packages/flutter/test/widget/block_test.dart

@@ -50,16 +50,18 @@ void main() {
           ]
         )
       );
-      tester.pump(); // for SizeObservers
 
       Point middleOfContainer = tester.getCenter(tester.findText('Hello'));
+      expect(middleOfContainer.x, equals(400.0));
+      expect(middleOfContainer.y, equals(1000.0));
+
       Point target = tester.getCenter(tester.findElementByKey(blockKey));
       TestGesture gesture = tester.startGesture(target);
       gesture.moveBy(const Offset(0.0, -10.0));
 
-      tester.pump(const Duration(milliseconds: 1));
+      tester.pump(); // redo layout
 
-      expect(tester.getCenter(tester.findText('Hello')) == middleOfContainer, isFalse);
+      expect(tester.getCenter(tester.findText('Hello')), isNot(equals(middleOfContainer)));
 
       gesture.up();
     });

packages/flutter_test/lib/src/widget_tester.dart

@@ -6,6 +6,7 @@ import 'dart:ui' as ui show window;
 
 import 'package:quiver/testing/async.dart';
 import 'package:quiver/time.dart';
+import 'package:flutter/rendering.dart';
 import 'package:flutter/scheduler.dart';
 import 'package:flutter/widgets.dart';
 
@@ -57,6 +58,11 @@ class WidgetTester extends Instrumentation {
     );
     async.flushMicrotasks();
   }
+
+  void dispatchEvent(PointerEvent event, HitTestResult result) {
+    super.dispatchEvent(event, result);
+    async.flushMicrotasks();
+  }
 }
 
 void testWidgets(callback(WidgetTester tester)) {