Merge pull request #2599 from abarth/rebuild_render_object_widget

Prepare to make RenderObjectElement buildable

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

@@ -2319,9 +2319,17 @@ class KeyedSubtree extends StatelessComponent {
   Widget build(BuildContext context) => child;
 }
 
+/// A platonic widget that invokes a closure to obtain its child widget.
 class Builder extends StatelessComponent {
   Builder({ Key key, this.builder }) : super(key: key);
+
+  /// Called to obtain the child widget.
+  ///
+  /// This function is invoked whether this widget is included in its parent's
+  /// build and the old widget (if any) that it synchronizes with has a distinct
+  /// object identity.
   final WidgetBuilder builder;
+
   Widget build(BuildContext context) => builder(context);
 }
 

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

@@ -1482,14 +1482,11 @@ class InheritedElement extends _ProxyElement<InheritedWidget> {
 
 /// Base class for instantiations of RenderObjectWidget subclasses
 abstract class RenderObjectElement<T extends RenderObjectWidget> extends BuildableElement<T> {
-  RenderObjectElement(T widget)
-    : _renderObject = widget.createRenderObject(), super(widget) {
-    assert(() { debugUpdateRenderObjectOwner(); return true; });
-  }
+  RenderObjectElement(T widget) : super(widget);
 
   /// The underlying [RenderObject] for this element
   RenderObject get renderObject => _renderObject;
-  final RenderObject _renderObject;
+  RenderObject _renderObject;
 
   RenderObjectElement _ancestorRenderObjectElement;
 
@@ -1512,8 +1509,9 @@ abstract class RenderObjectElement<T extends RenderObjectWidget> extends Buildab
 
   void mount(Element parent, dynamic newSlot) {
     super.mount(parent, newSlot);
-    assert(_slot == newSlot);
+    _renderObject = widget.createRenderObject();
     assert(() { debugUpdateRenderObjectOwner(); return true; });
+    assert(_slot == newSlot);
     attachRenderObject(newSlot);
     _dirty = false;
   }
@@ -1532,29 +1530,9 @@ abstract class RenderObjectElement<T extends RenderObjectWidget> extends Buildab
   }
 
   void performRebuild() {
-    reinvokeBuilders();
     _dirty = false;
   }
 
-  void reinvokeBuilders() {
-    // There's no way to mark a normal RenderObjectElement dirty.
-    // We inherit from BuildableElement so that subclasses can themselves
-    // implement reinvokeBuilders() if they do provide a way to mark themeselves
-    // 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.)
-    assert(() {
-      throw new WidgetError(
-        '$runtimeType failed to implement reinvokeBuilders(), but got marked dirty.\n'
-        'If a RenderObjectElement subclass supports being marked dirty, then the '
-        'reinvokeBuilders() method must be implemented.\n'
-        'If a RenderObjectElement uses a builder callback, it must support being '
-        'marked dirty, because builder callbacks can register the object as having '
-        'an Inherited dependency.'
-      );
-    });
-  }
-
   /// Utility function for subclasses that have one or more lists of children.
   /// Attempts to update the given old children list using the given new
   /// widgets, removing obsolete elements and introducing new ones as necessary,

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

@@ -192,15 +192,18 @@ class _MixedViewportElement extends RenderObjectElement<MixedViewport> {
       // 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
+      // not be rebuilt, and so the new state won't be used. Therefore, we use
+      // the object identity of the widget to determine whether to reinvoke the
+      // builders.
+      //
+      // 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();
+        performRebuild();
     }
   }
 
-  void reinvokeBuilders() {
+  void performRebuild() {
     // we just need to redraw our existing widgets as-is
     if (_childrenByKey.length > 0) {
       assert(_firstVisibleChildIndex >= 0);
@@ -223,6 +226,7 @@ class _MixedViewportElement extends RenderObjectElement<MixedViewport> {
         previousChild = newElement;
       }
     }
+    super.performRebuild();
   }
 
   void layout(BoxConstraints constraints) {
@@ -242,7 +246,7 @@ class _MixedViewportElement extends RenderObjectElement<MixedViewport> {
       final double newExtent = _didReachLastChild ? _childOffsets.last : double.INFINITY;
       Size contentSize;
       switch (widget.direction) {
-        case Axis.vertical: 
+        case Axis.vertical:
           contentSize = new Size(containerSize.width, newExtent);
           break;
         case Axis.horizontal:
@@ -257,7 +261,7 @@ class _MixedViewportElement extends RenderObjectElement<MixedViewport> {
         _lastReportedDimensions = dimensions;
         Offset overrideOffset = widget.onPaintOffsetUpdateNeeded(dimensions);
         switch (widget.direction) {
-          case Axis.vertical: 
+          case Axis.vertical:
             assert(overrideOffset.dx == 0.0);
             _overrideStartOffset = overrideOffset.dy;
             break;

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

@@ -256,6 +256,8 @@ class _LazyWidgetProvider extends _WidgetProvider {
   List<Widget> _widgets;
 
   void didUpdateWidget(VirtualViewportFromBuilder oldWidget, VirtualViewportFromBuilder newWidget) {
+    // TODO(abarth): We shouldn't check the itemBuilder closure for equality with.
+    // instead, we should use the widget's identity to decide whether to rebuild.
     if (_length != newWidget.itemCount || oldWidget?.itemBuilder != newWidget.itemBuilder) {
       _length = newWidget.itemCount;
       _base = null;

packages/flutter/test/widget/mixed_viewport_test.dart

@@ -3,7 +3,7 @@
 // found in the LICENSE file.
 
 import 'package:flutter_test/flutter_test.dart';
-import 'package:flutter/widgets.dart';
+import 'package:flutter/material.dart';
 import 'package:test/test.dart';
 
 import 'test_widgets.dart';
@@ -196,4 +196,48 @@ void main() {
       text.clear();
     });
   });
+
+  test('MixedViewport reinvoke builders', () {
+    testWidgets((WidgetTester tester) {
+      StateSetter setState;
+      ThemeData themeData = new ThemeData.light();
+
+      IndexedBuilder itemBuilder = (BuildContext context, int i) {
+        return new Container(
+          key: new ValueKey<int>(i),
+          width: 500.0, // this should be ignored
+          height: 220.0,
+          decoration: new BoxDecoration(
+            backgroundColor: Theme.of(context).primaryColor
+          ),
+          child: new Text("$i")
+        );
+      };
+
+      Widget viewport = new MixedViewport(builder: itemBuilder);
+
+      tester.pumpWidget(
+        new StatefulBuilder(
+          builder: (BuildContext context, StateSetter setter) {
+            setState = setter;
+            return new Theme(data: themeData, child: viewport);
+          }
+        )
+      );
+
+      DecoratedBox widget = tester.findWidgetOfType(DecoratedBox);
+      BoxDecoration decoraton = widget.decoration;
+      expect(decoraton.backgroundColor, equals(Colors.blue[500]));
+
+      setState(() {
+        themeData = new ThemeData(primarySwatch: Colors.green);
+      });
+
+      tester.pump();
+
+      widget = tester.findWidgetOfType(DecoratedBox);
+      decoraton = widget.decoration;
+      expect(decoraton.backgroundColor, equals(Colors.green[500]));
+    });
+  });
 }