Don't paint on unattached layers. (#9519)

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

@@ -100,10 +100,14 @@ abstract class Layer extends AbstractNode with TreeDiagnosticsMixin {
   /// that created this layer. Used in debug messages.
   dynamic debugCreator;
 
+  @override
+  String toString() => '${super.toString()}${ owner == null ? " DETACHED" : ""}';
+
   @override
   void debugFillDescription(List<String> description) {
     super.debugFillDescription(description);
-    description.add('${ owner != null ? "owner: $owner" : "DETACHED" }');
+    if (parent == null && owner != null)
+      description.add('owner: $owner');
     if (debugCreator != null)
       description.add('creator: $debugCreator');
   }

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

@@ -67,18 +67,28 @@ class PaintingContext {
 
   /// Repaint the given render object.
   ///
-  /// The render object must have a composited layer and must be in need of
-  /// painting. The render object's layer is re-used, along with any layers in
-  /// the subtree that don't need to be repainted.
+  /// The render object must be attached to a [PipelineOwner], must have a
+  /// composited layer, and must be in need of painting. The render object's
+  /// layer, if any, is re-used, along with any layers in the subtree that don't
+  /// need to be repainted.
   static void repaintCompositedChild(RenderObject child, { bool debugAlsoPaintedParent: false }) {
     assert(child.isRepaintBoundary);
     assert(child._needsPaint);
     assert(() {
-      child.debugRegisterRepaintBoundaryPaint(includedParent: debugAlsoPaintedParent, includedChild: true);
+      // register the call for RepaintBoundary metrics
+      child.debugRegisterRepaintBoundaryPaint(
+        includedParent: debugAlsoPaintedParent,
+        includedChild: true,
+      );
       return true;
     });
-    child._layer ??= new OffsetLayer();
-    child._layer.removeAllChildren();
+    if (child._layer == null) {
+      assert(debugAlsoPaintedParent);
+      child._layer = new OffsetLayer();
+    } else {
+      assert(debugAlsoPaintedParent || child._layer.attached);
+      child._layer.removeAllChildren();
+    }
     assert(() {
       child._layer.debugCreator = child.debugCreator ?? child.runtimeType;
       return true;
@@ -125,7 +135,11 @@ class PaintingContext {
     } else {
       assert(child._layer != null);
       assert(() {
-        child.debugRegisterRepaintBoundaryPaint(includedParent: true, includedChild: false);
+        // register the call for RepaintBoundary metrics
+        child.debugRegisterRepaintBoundaryPaint(
+          includedParent: true,
+          includedChild: false,
+        );
         child._layer.debugCreator = child.debugCreator ?? child;
         return true;
       });
@@ -1096,8 +1110,14 @@ class PipelineOwner {
       _nodesNeedingPaint = <RenderObject>[];
       // Sort the dirty nodes in reverse order (deepest first).
       for (RenderObject node in dirtyNodes..sort((RenderObject a, RenderObject b) => b.depth - a.depth)) {
-        if (node._needsPaint && node.owner == this)
-          PaintingContext.repaintCompositedChild(node);
+        assert(node._layer != null);
+        if (node._needsPaint && node.owner == this) {
+          if (node._layer.attached) {
+            PaintingContext.repaintCompositedChild(node);
+          } else {
+            node._skippedPaintingOnLayer();
+          }
+        }
       }
       assert(_nodesNeedingPaint.isEmpty);
     } finally {
@@ -2132,6 +2152,31 @@ abstract class RenderObject extends AbstractNode implements HitTestTarget {
     }
   }
 
+  // Called when flushPaint() tries to make us paint but our layer is detached.
+  // To make sure that our subtree is repainted when it's finally reattached,
+  // even in the case where some ancestor layer is itself never marked dirty, we
+  // have to mark our entire detached subtree as dirty and needing to be
+  // repainted. That way, we'll eventually be repainted.
+  void _skippedPaintingOnLayer() {
+    assert(attached);
+    assert(isRepaintBoundary);
+    assert(_needsPaint);
+    assert(_layer != null);
+    assert(!_layer.attached);
+    AbstractNode ancestor = parent;
+    while (ancestor is RenderObject) {
+      final RenderObject node = ancestor;
+      if (node.isRepaintBoundary) {
+        if (node._layer == null)
+          break; // looks like the subtree here has never been painted. let it handle itself.
+        if (node._layer.attached)
+          break; // it's the one that detached us, so it's the one that will decide to repaint us.
+        node._needsPaint = true;
+      }
+      ancestor = node.parent;
+    }
+  }
+
   /// Bootstrap the rendering pipeline by scheduling the very first paint.
   ///
   /// Requires that this render object is attached, is the root of the render
@@ -2549,6 +2594,8 @@ abstract class RenderObject extends AbstractNode implements HitTestTarget {
     }
     if (_needsLayout)
       header += ' NEEDS-LAYOUT';
+    if (_needsPaint)
+      header += ' NEEDS-PAINT';
     if (!attached)
       header += ' DETACHED';
     return header;
@@ -2606,6 +2653,8 @@ abstract class RenderObject extends AbstractNode implements HitTestTarget {
       description.add('creator: $debugCreator');
     description.add('parentData: $parentData');
     description.add('constraints: $constraints');
+    if (_layer != null) // don't access it via the "layer" getter since that's only valid when we don't need paint
+      description.add('layer: $_layer');
   }
 
   /// Returns a string describing the current node's descendants. Each line of

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

@@ -15,7 +15,7 @@ export 'dart:ui' show hashValues, hashList;
 
 export 'package:flutter/foundation.dart' show FlutterError, debugPrint, debugPrintStack;
 export 'package:flutter/foundation.dart' show VoidCallback, ValueChanged, ValueGetter, ValueSetter;
-export 'package:flutter/rendering.dart' show RenderObject, RenderBox, debugDumpRenderTree;
+export 'package:flutter/rendering.dart' show RenderObject, RenderBox, debugDumpRenderTree, debugDumpLayerTree;
 
 // KEYS
 

packages/flutter/test/widgets/navigator_and_layers_test.dart

@@ -0,0 +1,88 @@
+// Copyright 2015 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';
+import 'package:flutter/material.dart';
+
+import 'test_widgets.dart';
+
+class TestCustomPainter extends CustomPainter {
+  TestCustomPainter({ this.log, this.name });
+
+  final List<String> log;
+  final String name;
+
+  @override
+  void paint(Canvas canvas, Size size) {
+    log.add(name);
+  }
+
+  @override
+  bool shouldRepaint(TestCustomPainter oldPainter) {
+    return name != oldPainter.name
+        || log != oldPainter.log;
+  }
+}
+
+void main() {
+  testWidgets('Do we paint when coming back from a navigation', (WidgetTester tester) async {
+    final List<String> log = <String>[];
+    log.add('0');
+    await tester.pumpWidget(
+      new MaterialApp(
+        routes: <String, WidgetBuilder>{
+          '/': (BuildContext context) => new RepaintBoundary(
+            child: new Container(
+              child: new RepaintBoundary(
+                child: new FlipWidget(
+                  left: new CustomPaint(
+                    painter: new TestCustomPainter(
+                      log: log,
+                      name: 'left'
+                    ),
+                  ),
+                  right: new CustomPaint(
+                    painter: new TestCustomPainter(
+                      log: log,
+                      name: 'right'
+                    ),
+                  ),
+                ),
+              ),
+            ),
+          ),
+          '/second': (BuildContext context) => new Container(),
+        },
+      ),
+    );
+    log.add('1');
+    final NavigatorState navigator = tester.state<NavigatorState>(find.byType(Navigator));
+    navigator.pushNamed('/second');
+    log.add('2');
+    expect(await tester.pumpAndSettle(const Duration(minutes: 1)), 2);
+    log.add('3');
+    flipStatefulWidget(tester, skipOffstage: false);
+    log.add('4');
+    navigator.pop();
+    log.add('5');
+    expect(await tester.pumpAndSettle(const Duration(minutes: 1)), 2);
+    log.add('6');
+    flipStatefulWidget(tester);
+    expect(await tester.pumpAndSettle(), 1);
+    log.add('7');
+    expect(log, <String>[
+      '0',
+      'left',
+      '1',
+      '2',
+      '3',
+      '4',
+      '5',
+      'right',
+      '6',
+      'left',
+      '7',
+    ]);
+  });
+}

packages/flutter/test/widgets/test_widgets.dart

@@ -54,6 +54,6 @@ class FlipWidgetState extends State<FlipWidget> {
   }
 }
 
-void flipStatefulWidget(WidgetTester tester) {
-  tester.state<FlipWidgetState>(find.byType(FlipWidget)).flip();
+void flipStatefulWidget(WidgetTester tester, { bool skipOffstage: true }) {
+  tester.state<FlipWidgetState>(find.byType(FlipWidget, skipOffstage: skipOffstage)).flip();
 }