Handle GlobalKey reparenting inside LayoutBuilder (#5673)

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

@@ -91,6 +91,7 @@ class _FlexibleSpaceBarState extends State<FlexibleSpaceBar> {
     if (config.background != null) {
       final double fadeStart = math.max(0.0, 1.0 - kToolBarHeight / deltaHeight);
       final double fadeEnd = 1.0;
+      assert(fadeStart <= fadeEnd);
       final double opacity = 1.0 - new Interval(fadeStart, fadeEnd).transform(t);
       final double parallax = new Tween<double>(begin: 0.0, end: deltaHeight / 4.0).lerp(t);
       if (opacity > 0.0) {

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

@@ -880,6 +880,25 @@ class PipelineOwner {
     }
   }
 
+  // This flag is used to allow the kinds of mutations performed by GlobalKey
+  // reparenting while a LayoutBuilder is being rebuilt and in so doing tries to
+  // move a node from another LayoutBuilder subtree that hasn't been updated
+  // yet. To set this, call [_enableMutationsToDirtySubtrees], which is called
+  // by [RenderObject.invokeLayoutCallback].
+  bool _debugAllowMutationsToDirtySubtrees = false;
+
+  // See [RenderObject.invokeLayoutCallback].
+  void _enableMutationsToDirtySubtrees(VoidCallback callback) {
+    assert(_debugDoingLayout);
+    bool oldState = _debugAllowMutationsToDirtySubtrees;
+    _debugAllowMutationsToDirtySubtrees = true;
+    try {
+      callback();
+    } finally {
+      _debugAllowMutationsToDirtySubtrees = oldState;
+    }
+  }
+
   List<RenderObject> _nodesNeedingCompositingBitsUpdate = <RenderObject>[];
   /// Updates the [needsCompositing] bits.
   ///
@@ -1174,6 +1193,10 @@ abstract class RenderObject extends AbstractNode implements HitTestTarget {
           result = true;
           break;
         }
+        if (owner != null && owner._debugAllowMutationsToDirtySubtrees && node._needsLayout) {
+          result = true;
+          break;
+        }
         if (node._debugMutationsLocked) {
           result = false;
           break;
@@ -1584,8 +1607,21 @@ abstract class RenderObject extends AbstractNode implements HitTestTarget {
   // this field always holds a closure.
   VoidCallback _performLayout = _doNothing;
 
-  /// Allows this render object to mutate its child list during layout and
-  /// calls callback.
+  /// Allows mutations to be made to this object's child list (and any
+  /// descendants) as well as to any other dirty nodes in the render tree owned
+  /// by the same [PipelineOwner] as this object. The `callback` argument is
+  /// invoked synchronously, and the mutations are allowed only during that
+  /// callback's execution.
+  ///
+  /// This exists to allow child lists to be built on-demand during layout (e.g.
+  /// based on the object's size), and to enable nodes to be moved around the
+  /// tree as this happens (e.g. to handle [GlobalKey] reparenting), while still
+  /// ensuring that any particular node is only laid out once per frame.
+  ///
+  /// Calling this function disables a number of assertions that are intended to
+  /// catch likely bugs. As such, using this function is generally discouraged.
+  ///
+  /// This function can only be called during layout.
   @protected
   void invokeLayoutCallback(LayoutCallback callback) {
     assert(_debugMutationsLocked);
@@ -1593,7 +1629,7 @@ abstract class RenderObject extends AbstractNode implements HitTestTarget {
     assert(!_doingThisLayoutWithCallback);
     _doingThisLayoutWithCallback = true;
     try {
-      callback(constraints);
+      owner._enableMutationsToDirtySubtrees(() { callback(constraints); });
     } finally {
       _doingThisLayoutWithCallback = false;
     }

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

@@ -48,10 +48,14 @@ class LayoutBuilder extends RenderObjectWidget {
 
   @override
   _RenderLayoutBuilder createRenderObject(BuildContext context) => new _RenderLayoutBuilder();
+
+  // updateRenderObject is redundant with the logic in the LayoutBuilderElement below.
 }
 
 class _RenderLayoutBuilder extends RenderBox with RenderObjectWithChildMixin<RenderBox> {
-  _RenderLayoutBuilder({ LayoutCallback callback }) : _callback = callback;
+  _RenderLayoutBuilder({
+    LayoutCallback callback,
+  }) : _callback = callback;
 
   LayoutCallback get callback => _callback;
   LayoutCallback _callback;
@@ -102,8 +106,8 @@ class _RenderLayoutBuilder extends RenderBox with RenderObjectWithChildMixin<Ren
 
   @override
   void performLayout() {
-    if (callback != null)
-      invokeLayoutCallback(callback);
+    assert(callback != null);
+    invokeLayoutCallback(callback);
     if (child != null) {
       child.layout(constraints, parentUsesSize: true);
       size = constraints.constrain(child.size);
@@ -124,6 +128,7 @@ class _RenderLayoutBuilder extends RenderBox with RenderObjectWithChildMixin<Ren
   }
 }
 
+// TODO(ianh): move this class up to just below its widget.
 class _LayoutBuilderElement extends RenderObjectElement {
   _LayoutBuilderElement(LayoutBuilder widget) : super(widget);
 
@@ -144,7 +149,7 @@ class _LayoutBuilderElement extends RenderObjectElement {
   @override
   void mount(Element parent, dynamic newSlot) {
     super.mount(parent, newSlot); // Creates the renderObject.
-    renderObject.callback = _layout; // The _child will be built during layout.
+    renderObject.callback = _layout;
   }
 
   @override
@@ -161,7 +166,7 @@ class _LayoutBuilderElement extends RenderObjectElement {
     // This gets called if markNeedsBuild() is called on us.
     // That might happen if, e.g., our builder uses Inherited widgets.
     renderObject.markNeedsLayout();
-    super.performRebuild(); // calls widget.updateRenderObject
+    super.performRebuild(); // Calls widget.updateRenderObject (a no-op in this case).
   }
 
   @override
@@ -171,18 +176,24 @@ class _LayoutBuilderElement extends RenderObjectElement {
   }
 
   void _layout(BoxConstraints constraints) {
-    if (widget.builder == null)
-      return;
+    Widget built;
+    if (widget.builder != null) {
+      owner.lockState(() {
+        try {
+          built = widget.builder(this, constraints);
+          debugWidgetBuilderValue(widget, built);
+        } catch (e, stack) {
+          _debugReportException('building $widget', e, stack);
+          built = new ErrorWidget(e);
+        }
+      });
+    }
     owner.lockState(() {
-      Widget built;
-      try {
-        built = widget.builder(this, constraints);
-        debugWidgetBuilderValue(widget, built);
-      } catch (e, stack) {
-        _debugReportException('building $widget', e, stack);
-        built = new ErrorWidget(e);
+      if (widget.builder == null) {
+        if (_child != null)
+          _child = updateChild(_child, null, null);
+        return;
       }
-
       try {
         _child = updateChild(_child, built, null);
         assert(_child != null);

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

@@ -66,7 +66,7 @@ abstract class ScrollBehavior<T, U> {
   /// from the given offset by the given delta.
   T applyCurve(T scrollOffset, T scrollDelta) => scrollOffset;
 
-  /// Whether this scroll behavior currently permits scrolling
+  /// Whether this scroll behavior currently permits scrolling.
   bool get isScrollable => true;
 
   /// The scroll drag constant to use for physics simulations created by this

packages/flutter/test/widget/layout_builder_and_parent_data_test.dart

+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+import 'package:flutter_test/flutter_test.dart' hide TypeMatcher;
+import 'package:flutter/widgets.dart';
+
+class SizeChanger extends StatefulWidget {
+  SizeChanger({
+    Key key,
+    this.child,
+  }) : super(key: key);
+
+  final Widget child;
+
+  @override
+  SizeChangerState createState() => new SizeChangerState();
+}
+
+class SizeChangerState extends State<SizeChanger> {
+  bool _flag = false;
+
+  void trigger() {
+    setState(() {
+      _flag = true;
+    });
+  }
+
+  @override
+  Widget build(BuildContext context) {
+    return new Row(
+      children: <Widget>[
+        new SizedBox(
+          height: _flag ? 50.0 : 100.0,
+          width: 100.0,
+          child: config.child
+        )
+      ],
+    );
+  }
+}
+
+class Wrapper extends StatelessWidget {
+  Wrapper({
+    Key key,
+    this.child,
+  }) : super(key: key);
+
+  final Widget child;
+
+  @override
+  Widget build(BuildContext context) {
+    return child;
+  }
+}
+
+void main() {
+  testWidgets('Applying parent data inside a LayoutBuilder', (WidgetTester tester) async {
+    int frame = 0;
+    await tester.pumpWidget(new SizeChanger( // when this is triggered, the child LayoutBuilder will build again
+      child: new LayoutBuilder(builder: (BuildContext context, BoxConstraints constraints) {
+        return new Column(children: <Widget>[new Flexible(
+          flex: frame, // this is different after the next pump, so that the parentData has to be applied again
+          child: new Container(height: 100.0),
+        )]);
+      })
+    ));
+    frame += 1;
+    SizeChangerState state = tester.state(find.byType(SizeChanger));
+    state.trigger();
+    await tester.pump();
+  });
+}

packages/flutter/test/widget/layout_builder_mutations_test.dart

+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+import 'package:flutter/src/widgets/basic.dart';
+import 'package:flutter/src/widgets/framework.dart';
+import 'package:flutter/src/widgets/layout_builder.dart';
+import 'package:flutter_test/flutter_test.dart' hide TypeMatcher;
+
+class Wrapper extends StatelessWidget {
+  Wrapper({
+    Key key,
+    this.child
+  }) : super(key: key) {
+    assert(child != null);
+  }
+
+  final Widget child;
+
+  @override
+  Widget build(BuildContext context) => child;
+}
+
+void main() {
+  testWidgets('Moving a global key from another LayoutBuilder at layout time', (WidgetTester tester) async {
+    final GlobalKey victimKey = new GlobalKey();
+
+    await tester.pumpWidget(new Row(children: <Widget>[
+      new Wrapper(
+        child: new LayoutBuilder(builder: (BuildContext context, BoxConstraints constraints) {
+          return new SizedBox();
+        }),
+      ),
+      new Wrapper(
+        child: new Wrapper(
+          child: new LayoutBuilder(builder: (BuildContext context, BoxConstraints constraints) {
+            return new Wrapper(
+              child: new SizedBox(key: victimKey)
+            );
+          })
+        )
+      ),
+    ]));
+
+    await tester.pumpWidget(new Row(children: <Widget>[
+      new Wrapper(
+        child: new LayoutBuilder(builder: (BuildContext context, BoxConstraints constraints) {
+          return new Wrapper(
+            child: new SizedBox(key: victimKey)
+          );
+        })
+      ),
+      new Wrapper(
+        child: new Wrapper(
+          child: new LayoutBuilder(builder: (BuildContext context, BoxConstraints constraints) {
+            return new SizedBox();
+          })
+        )
+      ),
+    ]));
+  });
+}