Promote Layer to full AbstractNode status (#9456)

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

@@ -39,7 +39,7 @@ class AbstractNode {
   /// The depth of this node in the tree.
   ///
   /// The depth of nodes in a tree monotonically increases as you traverse down
-  /// the trees.
+  /// the tree.
   int get depth => _depth;
   int _depth = 0;
 
@@ -80,8 +80,9 @@ class AbstractNode {
   /// Typically called only from the [parent]'s [attach] method, and by the
   /// [owner] to mark the root of a tree as attached.
   ///
-  /// Subclasses with children should [attach] all their children to the same
-  /// [owner] whenever this method is called.
+  /// Subclasses with children should override this method to first call their
+  /// inherited [attach] method, and then [attach] all their children to the
+  /// same [owner].
   @mustCallSuper
   void attach(covariant Object owner) {
     assert(owner != null);
@@ -94,12 +95,13 @@ class AbstractNode {
   /// Typically called only from the [parent]'s [detach], and by the [owner] to
   /// mark the root of a tree as detached.
   ///
-  /// Subclasses with children should [detach] all their children whenever this
-  /// method is called.
+  /// Subclasses with children should override this method to first call their
+  /// inherited [detach] method, and then [detach] all their children.
   @mustCallSuper
   void detach() {
     assert(_owner != null);
     _owner = null;
+    assert(parent == null || attached == parent.attached);
   }
 
   /// The parent of this node in the tree.

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

@@ -126,7 +126,7 @@ class PaintingContext {
       assert(child._layer != null);
       assert(() {
         child.debugRegisterRepaintBoundaryPaint(includedParent: true, includedChild: false);
-        child._layer.debugCreator = child.debugCreator ?? child.runtimeType;
+        child._layer.debugCreator = child.debugCreator ?? child;
         return true;
       });
     }
@@ -136,7 +136,7 @@ class PaintingContext {
 
   void _appendLayer(Layer layer) {
     assert(!_isRecording);
-    layer.detach();
+    layer.remove();
     _containerLayer.append(layer);
   }
 
@@ -1998,7 +1998,7 @@ abstract class RenderObject extends AbstractNode implements HitTestTarget {
   /// To access the layer in debug code, even when it might be inappropriate to
   /// access it (e.g. because it is dirty), consider [debugLayer].
   OffsetLayer get layer {
-    assert(isRepaintBoundary);
+    assert(isRepaintBoundary, 'You can only access RenderObject.layer for render objects that are repaint boundaries.');
     assert(!_needsPaint);
     return _layer;
   }
@@ -2141,6 +2141,7 @@ abstract class RenderObject extends AbstractNode implements HitTestTarget {
   ///
   /// See [RenderView] for an example of how this function is used.
   void scheduleInitialPaint(ContainerLayer rootLayer) {
+    assert(rootLayer.attached);
     assert(attached);
     assert(parent is! RenderObject);
     assert(!owner._debugDoingPaint);
@@ -2157,11 +2158,13 @@ abstract class RenderObject extends AbstractNode implements HitTestTarget {
   ///
   /// This might be called if, e.g., the device pixel ratio changed.
   void replaceRootLayer(OffsetLayer rootLayer) {
+    assert(rootLayer.attached);
     assert(attached);
     assert(parent is! RenderObject);
     assert(!owner._debugDoingPaint);
     assert(isRepaintBoundary);
     assert(_layer != null); // use scheduleInitialPaint the first time
+    _layer.detach();
     _layer = rootLayer;
     markNeedsPaint();
   }

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

@@ -812,6 +812,7 @@ class RenderTable extends RenderBox {
 
   @override
   void detach() {
+    super.detach();
     if (_rowDecorationPainters != null) {
       for (BoxPainter painter in _rowDecorationPainters)
         painter?.dispose();
@@ -819,7 +820,6 @@ class RenderTable extends RenderBox {
     }
     for (RenderBox child in _children)
       child?.detach();
-    super.detach();
   }
 
   @override

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

@@ -80,7 +80,9 @@ class RenderView extends RenderObject with RenderObjectWithChildMixin<RenderBox>
     if (configuration == value)
       return;
     _configuration = value;
-    replaceRootLayer(new TransformLayer(transform: configuration.toMatrix()));
+    final ContainerLayer rootLayer = new TransformLayer(transform: configuration.toMatrix());
+    rootLayer.attach(this);
+    replaceRootLayer(rootLayer);
     markNeedsLayout();
   }
 
@@ -88,7 +90,9 @@ class RenderView extends RenderObject with RenderObjectWithChildMixin<RenderBox>
   void scheduleInitialFrame() {
     assert(owner != null);
     scheduleInitialLayout();
-    scheduleInitialPaint(new TransformLayer(transform: configuration.toMatrix()));
+    final ContainerLayer rootLayer = new TransformLayer(transform: configuration.toMatrix());
+    rootLayer.attach(this);
+    scheduleInitialPaint(rootLayer);
     owner.requestVisualUpdate();
   }
 

packages/flutter/test/rendering/layers_test.dart

+// Copyright 2017 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/rendering.dart';
+import 'package:flutter_test/flutter_test.dart';
+
+import 'rendering_tester.dart';
+
+void main() {
+  test('non-painted layers are detached', () {
+    RenderObject boundary, inner;
+    final RenderOpacity root = new RenderOpacity(
+      child: boundary = new RenderRepaintBoundary(
+        child: inner = new RenderDecoratedBox(
+          decoration: const BoxDecoration(),
+        ),
+      ),
+    );
+    layout(root, phase: EnginePhase.paint);
+    expect(inner.isRepaintBoundary, isFalse);
+    expect(() => inner.layer, throwsAssertionError);
+    expect(boundary.isRepaintBoundary, isTrue);
+    expect(boundary.layer, isNotNull);
+    expect(boundary.layer.attached, isTrue); // this time it painted...
+
+    root.opacity = 0.0;
+    pumpFrame(phase: EnginePhase.paint);
+    expect(inner.isRepaintBoundary, isFalse);
+    expect(() => inner.layer, throwsAssertionError);
+    expect(boundary.isRepaintBoundary, isTrue);
+    expect(boundary.layer, isNotNull);
+    expect(boundary.layer.attached, isFalse); // this time it did not.
+
+    root.opacity = 0.5;
+    pumpFrame(phase: EnginePhase.paint);
+    expect(inner.isRepaintBoundary, isFalse);
+    expect(() => inner.layer, throwsAssertionError);
+    expect(boundary.isRepaintBoundary, isTrue);
+    expect(boundary.layer, isNotNull);
+    expect(boundary.layer.attached, isTrue); // this time it did again!
+  });
+}

packages/flutter/test/rendering/rendering_tester.dart

@@ -8,19 +8,15 @@ import 'package:flutter/rendering.dart';
 import 'package:flutter/scheduler.dart';
 import 'package:flutter/services.dart';
 
-enum EnginePhase {
-  layout,
-  compositingBits,
-  paint,
-  composite,
-  flushSemantics
-}
+import 'package:flutter_test/flutter_test.dart' show EnginePhase;
+export 'package:flutter_test/flutter_test.dart' show EnginePhase;
 
 class TestRenderingFlutterBinding extends BindingBase with SchedulerBinding, ServicesBinding, RendererBinding, GestureBinding {
   EnginePhase phase = EnginePhase.composite;
 
   @override
   void beginFrame() {
+    assert(phase != EnginePhase.build, 'rendering_tester does not support testing the build phase; use flutter_test instead');
     pipelineOwner.flushLayout();
     if (phase == EnginePhase.layout)
       return;
@@ -34,7 +30,9 @@ class TestRenderingFlutterBinding extends BindingBase with SchedulerBinding, Ser
     if (phase == EnginePhase.composite)
       return;
     pipelineOwner.flushSemantics();
-    assert(phase == EnginePhase.flushSemantics);
+    if (phase == EnginePhase.flushSemantics)
+      return;
+    assert(phase == EnginePhase.flushSemantics || phase == EnginePhase.sendSemanticsTree);
   }
 }
 
@@ -53,10 +51,13 @@ TestRenderingFlutterBinding get renderer {
 /// be put in a different place in the tree or passed to [layout] again, because
 /// [layout] places the given object into another [RenderBox] which you would
 /// need to unparent it from (but that box isn't itself made available).
+///
+/// The EnginePhase must not be [EnginePhase.build], since the rendering layer
+/// has no build phase.
 void layout(RenderBox box, {
   BoxConstraints constraints,
   FractionalOffset alignment: FractionalOffset.center,
-  EnginePhase phase: EnginePhase.layout
+  EnginePhase phase: EnginePhase.layout,
 }) {
   assert(box != null); // If you want to just repump the last box, call pumpFrame().
   assert(box.parent == null); // We stick the box in another, so you can't reuse it easily, sorry.

packages/flutter_test/lib/src/binding.dart

@@ -30,7 +30,6 @@ import 'test_text_input.dart';
 ///
 /// See [WidgetsBinding.beginFrame] for a more detailed description of some of
 /// these phases.
-// TODO(ianh): Merge with near-identical code in the rendering test code.
 enum EnginePhase {
   /// The build phase in the widgets library. See [BuildOwner.buildScope].
   build,