Fix crash when a RenderObject tree is rooted in a non-RenderObject environment (#5933)

dev/devicelab/test/run_test.dart

@@ -35,7 +35,7 @@ void main() {
 
     test('Exits with code 1 when fails to connect', () async {
       expect(await runScript(<String>['smoke_test_setup_failure']), 1);
-    });
+    }, skip: true); // https://github.com/flutter/flutter/issues/5901
 
     test('Exits with code 1 when results are mixed', () async {
       expect(

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

@@ -26,7 +26,11 @@ abstract class RendererBinding extends BindingBase implements SchedulerBinding,
   void initInstances() {
     super.initInstances();
     _instance = this;
-    _pipelineOwner = new PipelineOwner(onNeedVisualUpdate: ensureVisualUpdate);
+    _pipelineOwner = new PipelineOwner(
+      onNeedVisualUpdate: ensureVisualUpdate,
+      onScheduleInitialSemantics: _scheduleInitialSemantics,
+      onClearSemantics: _clearSemantics,
+    );
     ui.window.onMetricsChanged = handleMetricsChanged;
     initRenderView();
     initSemantics();
@@ -96,12 +100,12 @@ abstract class RendererBinding extends BindingBase implements SchedulerBinding,
   PipelineOwner _pipelineOwner;
 
   /// The render tree that's attached to the output surface.
-  RenderView get renderView => _pipelineOwner.rootRenderObject;
+  RenderView get renderView => _pipelineOwner.rootNode;
   /// Sets the given [RenderView] object (which must not be null), and its tree, to
   /// be the new render tree to display. The previous tree, if any, is detached.
   set renderView(RenderView value) {
     assert(value != null);
-    _pipelineOwner.rootRenderObject = value;
+    _pipelineOwner.rootNode = value;
   }
 
   /// Called when the system metrics change.
@@ -210,7 +214,7 @@ abstract class RendererBinding extends BindingBase implements SchedulerBinding,
   @override
   void reassembleApplication() {
     super.reassembleApplication();
-    pipelineOwner.reassemble();
+    renderView.reassemble();
     handleBeginFrame(null);
   }
 
@@ -229,6 +233,14 @@ abstract class RendererBinding extends BindingBase implements SchedulerBinding,
     };
     instance?.renderView?.visitChildren(visitor);
   }
+
+  void _scheduleInitialSemantics() {
+    renderView.scheduleInitialSemantics();
+  }
+
+  void _clearSemantics() {
+    renderView.clearSemantics();
+  }
 }
 
 /// Prints a textual representation of the entire render tree.

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

@@ -76,6 +76,9 @@ class AbstractNode {
   ///
   /// Typically called only from the parent's attach(), and to mark the root of
   /// a tree attached.
+  ///
+  /// Subclasses with children should attach all their children to the same
+  /// [owner] whenever this method is called.
   @mustCallSuper
   void attach(Object owner) {
     assert(owner != null);
@@ -87,6 +90,9 @@ class AbstractNode {
   ///
   /// Typically called only from the parent's detach(), and to mark the root of
   /// a tree detached.
+  ///
+  /// Subclasses with children should detach all their children whenever this
+  /// method is called.
   @mustCallSuper
   void detach() {
     assert(_owner != null);

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

@@ -789,7 +789,11 @@ class PipelineOwner {
   /// Typically created by the binding (e.g., [RendererBinding]), but can be
   /// created separately from the binding to drive off-screen render objects
   /// through the rendering pipeline.
-  PipelineOwner({ this.onNeedVisualUpdate });
+  PipelineOwner({
+    this.onNeedVisualUpdate,
+    this.onScheduleInitialSemantics,
+    this.onClearSemantics,
+  });
 
   /// Called when a render object associated with this pipeline owner wishes to
   /// update its visual appearance.
@@ -800,6 +804,21 @@ class PipelineOwner {
   /// duplicate calls quickly.
   final VoidCallback onNeedVisualUpdate;
 
+  /// Called when [addSemanticsListener] is called when there was no
+  /// [SemanticsOwner] present, to request that the
+  /// [RenderObject.scheduleInitialSemantics] method be called on the
+  /// appropriate object(s).
+  ///
+  /// For example, the [RendererBinding] calls it on the [RenderView] object.
+  final VoidCallback onScheduleInitialSemantics;
+
+  /// Called when the last [SemanticsListener] is removed from the
+  /// [SemanticsOwner], to request that the [RenderObject.clearSemantics] method
+  /// be called on the appropriate object(s).
+  ///
+  /// For example, the [RendererBinding] calls it on the [RenderView] object.
+  final VoidCallback onClearSemantics;
+
   /// Calls [onNeedVisualUpdate] if [onNeedVisualUpdate] is not null.
   ///
   /// Used to notify the pipeline owner that an associated render object wishes
@@ -809,15 +828,17 @@ class PipelineOwner {
       onNeedVisualUpdate();
   }
 
-  /// The unique render object managed by this pipeline that has no parent.
-  RenderObject get rootRenderObject => _rootRenderObject;
-  RenderObject _rootRenderObject;
-  set rootRenderObject(RenderObject value) {
-    if (_rootRenderObject == value)
+  /// The unique object managed by this pipeline that has no parent.
+  ///
+  /// This object does not have to be a [RenderObject].
+  AbstractNode get rootNode => _rootNode;
+  AbstractNode _rootNode;
+  set rootNode(AbstractNode value) {
+    if (_rootNode == value)
       return;
-    _rootRenderObject?.detach();
-    _rootRenderObject = value;
-    _rootRenderObject?.attach(this);
+    _rootNode?.detach();
+    _rootNode = value;
+    _rootNode?.attach(this);
   }
 
   /// Calls the given listener whenever the semantics of the render tree change.
@@ -829,7 +850,8 @@ class PipelineOwner {
         initialListener: listener,
         onLastListenerRemoved: _handleLastSemanticsListenerRemoved
       );
-      _rootRenderObject.scheduleInitialSemantics();
+      if (onScheduleInitialSemantics != null)
+        onScheduleInitialSemantics();
     } else {
       _semanticsOwner.addListener(listener);
     }
@@ -839,7 +861,8 @@ class PipelineOwner {
 
   void _handleLastSemanticsListenerRemoved() {
     assert(!_debugDoingSemantics);
-    rootRenderObject._clearSemantics();
+    if (onClearSemantics != null)
+      onClearSemantics();
     _semanticsOwner.dispose();
     _semanticsOwner = null;
   }
@@ -993,16 +1016,6 @@ class PipelineOwner {
     }
     _semanticsOwner.sendSemanticsTree();
   }
-
-  /// Cause the entire render tree rooted at [rootRenderObject] to be entirely
-  /// reprocessed. This is used by development tools when the application code
-  /// has changed, to cause the rendering tree to pick up any changed
-  /// implementations.
-  ///
-  /// This is expensive and should not be called except during development.
-  void reassemble() {
-    _rootRenderObject?._reassemble();
-  }
 }
 
 // See _performLayout.
@@ -1038,14 +1051,25 @@ abstract class RenderObject extends AbstractNode implements HitTestTarget {
     _performLayout = performLayout;
   }
 
-  void _reassemble() {
+  /// Cause the entire subtree rooted at the given [RenderObject] to be marked
+  /// dirty for layout, paint, etc. This is called by the [RendererBinding] in
+  /// response to the `ext.flutter.reassemble` hook, which is used by
+  /// development tools when the application code has changed, to cause the
+  /// widget tree to pick up any changed implementations.
+  ///
+  /// This is expensive and should not be called except during development.
+  ///
+  /// See also:
+  ///
+  /// * [BindingBase.reassembleApplication].
+  void reassemble() {
     _performLayout = performLayout;
     markNeedsLayout();
     markNeedsCompositingBitsUpdate();
     markNeedsPaint();
     markNeedsSemanticsUpdate();
     visitChildren((RenderObject child) {
-      child._reassemble();
+      child.reassemble();
     });
   }
 
@@ -1490,12 +1514,13 @@ abstract class RenderObject extends AbstractNode implements HitTestTarget {
     ));
     assert(!_debugDoingThisResize);
     assert(!_debugDoingThisLayout);
-    final RenderObject parent = this.parent;
     RenderObject relayoutBoundary;
-    if (!parentUsesSize || sizedByParent || constraints.isTight || parent is! RenderObject)
+    if (!parentUsesSize || sizedByParent || constraints.isTight || parent is! RenderObject) {
       relayoutBoundary = this;
-    else
+    } else {
+      final RenderObject parent = this.parent;
       relayoutBoundary = parent._relayoutBoundary;
+    }
     assert(parent == this.parent);
     assert(() {
       _debugCanParentUseSize = parentUsesSize;
@@ -1992,12 +2017,17 @@ abstract class RenderObject extends AbstractNode implements HitTestTarget {
   }
 
   /// Removes all semantics from this render object and its descendants.
-  void _clearSemantics() {
+  ///
+  /// Should only be called in response to the [PipelineOwner] calling its
+  /// [PipelineOwner.onClearSemantics] callback.
+  ///
+  /// Should only be called on objects whose [parent] is not a [RenderObject].
+  void clearSemantics() {
     _needsSemanticsUpdate = true;
     _needsSemanticsGeometryUpdate = true;
     _semantics = null;
     visitChildren((RenderObject child) {
-      child._clearSemantics();
+      child.clearSemantics();
     });
   }
 

packages/flutter/test/rendering/non_render_object_root_test.dart

@@ -0,0 +1,62 @@
+// 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/material.dart';
+import 'package:flutter/rendering.dart';
+import 'package:test/test.dart';
+
+import 'rendering_tester.dart';
+
+class RealRoot extends AbstractNode {
+  RealRoot(this.child) {
+    if (child != null)
+      adoptChild(child);
+  }
+
+  final RenderObject child;
+
+  @override
+  void redepthChildren() {
+    if (child != null)
+      redepthChild(child);
+  }
+
+  @override
+  void attach(Object owner) {
+    super.attach(owner);
+    child?.attach(owner);
+  }
+
+  @override
+  void detach() {
+    super.detach();
+    child?.detach();
+  }
+
+  @override
+  PipelineOwner get owner => super.owner;
+
+  void layout() {
+    child?.layout(new BoxConstraints.tight(const Size(500.0, 500.0)));
+  }
+}
+
+void main() {
+  test("non-RenderObject roots", () {
+    RenderPositionedBox child;
+    RealRoot root = new RealRoot(
+      child = new RenderPositionedBox(
+        alignment: FractionalOffset.center,
+        child: new RenderSizedBox(new Size(100.0, 100.0))
+      )
+    );
+    root.attach(new PipelineOwner());
+
+    child.scheduleInitialLayout();
+    root.layout();
+
+    child.markNeedsLayout();
+    root.layout();
+  });
+}