Merge pull request #1851 from Hixie/MixedViewport-bug

Fix crash when removing a card in card_collection

examples/widgets/card_collection.dart

@@ -301,7 +301,7 @@ class CardCollectionState extends State<CardCollection> {
             )
           : new DefaultTextStyle(
               style: DefaultTextStyle.of(context).merge(cardLabelStyle).merge(_textStyle).copyWith(
-                fontSize: _varyFontSizes ? 5.0 + _cardModels.length.toDouble() : null
+                fontSize: _varyFontSizes ? _cardModels.length.toDouble() : null
               ),
               child: new Column(<Widget>[
                   new Text(cardModel.label)

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

@@ -140,7 +140,7 @@ class _HomogeneousViewportElement extends RenderObjectElement<HomogeneousViewpor
       renderObject.minExtent = getTotalExtent(null);
       renderObject.startOffset = offset;
       renderObject.overlayPainter = widget.overlayPainter;
-    });
+    }, building: true);
   }
 
   void _updateChildren() {

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

@@ -17,7 +17,7 @@ enum _ChangeDescription { none, scrolled, resized }
 class MixedViewport extends RenderObjectWidget {
   MixedViewport({
     Key key,
-    this.startOffset,
+    this.startOffset: 0.0,
     this.direction: ScrollDirection.vertical,
     this.builder,
     this.token,
@@ -28,9 +28,9 @@ class MixedViewport extends RenderObjectWidget {
   final double startOffset;
   final ScrollDirection direction;
   final IndexedBuilder builder;
-  final Object token;
+  final Object token; // change this if the list changed (i.e. there are added, removed, or resorted items)
   final ExtentsUpdateCallback onExtentsUpdate;
-  final InvalidatorAvailableCallback onInvalidatorAvailable;
+  final InvalidatorAvailableCallback onInvalidatorAvailable; // call the callback this gives to invalidate sizes
 
   _MixedViewportElement createElement() => new _MixedViewportElement(this);
 
@@ -171,7 +171,16 @@ class _MixedViewportElement extends RenderObjectElement<MixedViewport> {
     if (changes != _ChangeDescription.none || !isValid) {
       renderObject.markNeedsLayout();
     } else {
-      reinvokeBuilders();
+      // We have to reinvoke our builders because they might return new data.
+      // Consider a stateful component that owns us. The builder it gives us
+      // includes some of the state from that component. The component calls
+      // setState() on itself. It rebuilds. Part of that involves rebuilding
+      // us, but now what? If we don't reinvoke the builders. then they will
+      // not be rebuilt, and so the new state won't be used.
+      // Note that if the builders are to change so much that the _sizes_ of
+      // the children would change, then the parent must change the 'token'.
+      if (!renderObject.needsLayout)
+        reinvokeBuilders();
     }
   }
 
@@ -207,7 +216,7 @@ 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) {
@@ -239,7 +248,10 @@ class _MixedViewportElement extends RenderObjectElement<MixedViewport> {
     if (widget.builder == null)
       return null;
     final Widget newWidget = widget.builder(this, index);
-    assert(newWidget == null || newWidget.key != null); // every widget in a list must have a list-unique key
+    assert(() {
+      'Every widget in a list must have a list-unique key.';
+      return newWidget == null || newWidget.key != null;
+    });
     return newWidget;
   }
 

packages/unit/test/widget/scrollable_list_with_inherited_test.dart

+import 'package:flutter/widgets.dart';
+import 'package:test/test.dart';
+
+import 'widget_tester.dart';
+
+List<int> items = <int>[0, 1, 2, 3, 4, 5];
+
+Widget buildCard(BuildContext context, int index) {
+  if (index >= items.length)
+    return null;
+  return new Container(
+    key: new ValueKey<int>(items[index]),
+    height: 100.0,
+    child: new DefaultTextStyle(
+      style: new TextStyle(fontSize: 2.0 + items.length.toDouble()),
+      child: new Text('${items[index]}')
+    )
+  );
+}
+
+InvalidatorCallback invalidator;
+
+Widget buildFrame() {
+  return new ScrollableMixedWidgetList(
+    builder: buildCard,
+    token: items.length,
+    onInvalidatorAvailable: (InvalidatorCallback callback) { invalidator = callback; }
+  );
+}
+
+void main() {
+  test('MixedViewport is a build function (smoketest)', () {
+    testWidgets((WidgetTester tester) {
+      tester.pumpWidget(buildFrame());
+      expect(tester.findText('0'), isNotNull);
+      expect(tester.findText('1'), isNotNull);
+      expect(tester.findText('2'), isNotNull);
+      expect(tester.findText('3'), isNotNull);
+      items.removeAt(2);
+      tester.pumpWidget(buildFrame());
+      expect(tester.findText('0'), isNotNull);
+      expect(tester.findText('1'), isNotNull);
+      expect(tester.findText('2'), isNull);
+      expect(tester.findText('3'), isNotNull);
+    });
+  });
+}

packages/unit/test/widget/test_widgets.dart

@@ -15,6 +15,16 @@ final BoxDecoration kBoxDecorationC = new BoxDecoration(
   backgroundColor: const Color(0xFF0000FF)
 );
 
+class TestBuildCounter extends StatelessComponent {
+  static int buildCount = 0;
+
+  Widget build(BuildContext context) {
+    ++buildCount;
+    return new DecoratedBox(decoration: kBoxDecorationA);
+  }
+}
+
+
 class FlipComponent extends StatefulComponent {
   FlipComponent({ Key key, this.left, this.right }) : super(key: key);
 
@@ -38,15 +48,6 @@ class FlipComponentState extends State<FlipComponent> {
   }
 }
 
-class TestBuildCounter extends StatelessComponent {
-  static int buildCount = 0;
-
-  Widget build(BuildContext context) {
-    ++buildCount;
-    return new DecoratedBox(decoration: kBoxDecorationA);
-  }
-}
-
 void flipStatefulComponent(WidgetTester tester) {
   StatefulComponentElement stateElement =
       tester.findElement((Element element) => element is StatefulComponentElement);