Do not crash when toggling Semantics on, off, on (#13999)

* Do not crash when toggeling Semantics on, off, on

* review comments

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

@@ -627,6 +627,13 @@ class RenderCustomPaint extends RenderProxyBox {
     super.assembleSemanticsNode(node, config, finalChildren);
   }
 
+  @override
+  void clearSemantics() {
+    super.clearSemantics();
+    _backgroundSemanticsNodes = null;
+    _foregroundSemanticsNodes = null;
+  }
+
   /// Updates the nodes of `oldSemantics` using data in `newChildSemantics`, and
   /// returns a new list containing child nodes sorted according to the order
   /// specified by `newChildSemantics`.

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

@@ -2220,6 +2220,10 @@ abstract class RenderObject extends AbstractNode with DiagnosticableTreeMixin im
   /// Removes all semantics from this render object and its descendants.
   ///
   /// Should only be called on objects whose [parent] is not a [RenderObject].
+  ///
+  /// Override this method if you instantiate new [SemanticsNode]s in an
+  /// overridden [assembleSemanticsNode] method, to dispose of those nodes.
+  @mustCallSuper
   void clearSemantics() {
     _needsSemanticsUpdate = true;
     _semantics = null;
@@ -2399,7 +2403,8 @@ abstract class RenderObject extends AbstractNode with DiagnosticableTreeMixin im
   /// `children` to it.
   ///
   /// Subclasses can override this method to add additional [SemanticsNode]s
-  /// to the tree.
+  /// to the tree. If new [SemanticsNode]s are instantiated in this method
+  /// they must be disposed in [clearSemantics].
   void assembleSemanticsNode(
       SemanticsNode node,
       SemanticsConfiguration config,

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

@@ -581,6 +581,13 @@ class _RenderExcludableScrollSemantics extends RenderProxyBox {
     _innerNode.updateWith(config: config, childrenInInversePaintOrder: included);
   }
 
+  @override
+  void clearSemantics() {
+    super.clearSemantics();
+    _innerNode = null;
+    _annotatedNode = null;
+  }
+
   /// Sends a [SemanticsEvent] in the context of the [SemanticsNode] that is
   /// annotated with this object's semantics information.
   void sendSemanticsEvent(SemanticsEvent event) {

packages/flutter/test/widgets/custom_painter_test.dart

@@ -274,6 +274,48 @@ void _defineTests() {
     semanticsTester.dispose();
   });
 
+  testWidgets('Can toggle semantics on, off, on without crash',  (WidgetTester tester) async {
+    await tester.pumpWidget(new CustomPaint(
+      painter: new _PainterWithSemantics(
+        semantics: new CustomPainterSemantics(
+          key: const ValueKey<int>(1),
+          rect: new Rect.fromLTRB(1.0, 2.0, 3.0, 4.0),
+          properties: const SemanticsProperties(
+            checked: false,
+            selected: false,
+            button: false,
+            label: 'label-before',
+            value: 'value-before',
+            increasedValue: 'increase-before',
+            decreasedValue: 'decrease-before',
+            hint: 'hint-before',
+            textDirection: TextDirection.rtl,
+          ),
+        ),
+      ),
+    ));
+
+    // Start with semantics off.
+    expect(tester.binding.pipelineOwner.semanticsOwner, isNull);
+
+    // Semantics on
+    SemanticsTester semantics = new SemanticsTester(tester);
+    await tester.pumpAndSettle();
+    expect(tester.binding.pipelineOwner.semanticsOwner, isNotNull);
+
+    // Semantics off
+    semantics.dispose();
+    await tester.pumpAndSettle();
+    expect(tester.binding.pipelineOwner.semanticsOwner, isNull);
+
+    // Semantics on
+    semantics = new SemanticsTester(tester);
+    await tester.pumpAndSettle();
+    expect(tester.binding.pipelineOwner.semanticsOwner, isNotNull);
+
+    semantics.dispose();
+  });
+
   group('diffing', () {
     testWidgets('complains about duplicate keys', (WidgetTester tester) async {
       final SemanticsTester semanticsTester = new SemanticsTester(tester);

packages/flutter/test/widgets/scrollable_semantics_test.dart

@@ -11,8 +11,15 @@ import 'package:flutter/widgets.dart';
 import 'semantics_tester.dart';
 
 void main() {
+  SemanticsTester semantics;
+
+  tearDown(() {
+    semantics?.dispose();
+    semantics = null;
+  });
+
   testWidgets('scrollable exposes the correct semantic actions', (WidgetTester tester) async {
-    final SemanticsTester semantics = new SemanticsTester(tester);
+    semantics = new SemanticsTester(tester);
 
     final List<Widget> textWidgets = <Widget>[];
     for (int i = 0; i < 80; i++)
@@ -40,7 +47,7 @@ void main() {
   });
 
   testWidgets('showOnScreen works in scrollable', (WidgetTester tester) async {
-    new SemanticsTester(tester); // enables semantics tree generation
+    semantics = new SemanticsTester(tester); // enables semantics tree generation
 
     const double kItemHeight = 40.0;
 
@@ -76,7 +83,7 @@ void main() {
   });
 
   testWidgets('showOnScreen works with pinned app bar and sliver list', (WidgetTester tester) async {
-    new SemanticsTester(tester); // enables semantics tree generation
+    semantics = new SemanticsTester(tester); // enables semantics tree generation
 
     const double kItemHeight = 100.0;
     const double kExpandedAppBarHeight = 56.0;
@@ -134,13 +141,13 @@ void main() {
   });
 
   testWidgets('showOnScreen works with pinned app bar and individual slivers', (WidgetTester tester) async {
-    new SemanticsTester(tester); // enables semantics tree generation
+    semantics = new SemanticsTester(tester); // enables semantics tree generation
 
     const double kItemHeight = 100.0;
     const double kExpandedAppBarHeight = 256.0;
 
 
-    final List<Widget> semantics = <Widget>[];
+    final List<Widget> children = <Widget>[];
     final List<Widget> slivers = new List<Widget>.generate(30, (int i) {
       final Widget child = new MergeSemantics(
         child: new Container(
@@ -148,7 +155,7 @@ void main() {
           height: 72.0,
         ),
       );
-      semantics.add(child);
+      children.add(child);
       return new SliverToBoxAdapter(
         child: child,
       );
@@ -184,17 +191,17 @@ void main() {
 
     expect(scrollController.offset, kItemHeight / 2);
 
-    final int id0 = tester.renderObject(find.byWidget(semantics[0])).debugSemantics.id;
+    final int id0 = tester.renderObject(find.byWidget(children[0])).debugSemantics.id;
     tester.binding.pipelineOwner.semanticsOwner.performAction(id0, SemanticsAction.showOnScreen);
     await tester.pump();
     await tester.pump(const Duration(seconds: 5));
-    expect(tester.getTopLeft(find.byWidget(semantics[0])).dy, kToolbarHeight);
+    expect(tester.getTopLeft(find.byWidget(children[0])).dy, kToolbarHeight);
 
-    final int id1 = tester.renderObject(find.byWidget(semantics[1])).debugSemantics.id;
+    final int id1 = tester.renderObject(find.byWidget(children[1])).debugSemantics.id;
     tester.binding.pipelineOwner.semanticsOwner.performAction(id1, SemanticsAction.showOnScreen);
     await tester.pump();
     await tester.pump(const Duration(seconds: 5));
-    expect(tester.getTopLeft(find.byWidget(semantics[1])).dy, kToolbarHeight);
+    expect(tester.getTopLeft(find.byWidget(children[1])).dy, kToolbarHeight);
   });
 
   testWidgets('vertical scrolling sends ScrollCompletedSemanticsEvent', (WidgetTester tester) async {
@@ -203,7 +210,7 @@ void main() {
       messages.add(message);
     });
 
-    final SemanticsTester semantics = new SemanticsTester(tester);
+    semantics = new SemanticsTester(tester);
 
     final List<Widget> textWidgets = <Widget>[];
     for (int i = 0; i < 80; i++)
@@ -235,8 +242,6 @@ void main() {
     expect(message['pixels'], isNonNegative);
     expect(message['minScrollExtent'], 0.0);
     expect(message['maxScrollExtent'], 520.0);
-
-    semantics.dispose();
   });
 
   testWidgets('horizontal scrolling sends ScrollCompletedSemanticsEvent', (WidgetTester tester) async {
@@ -245,7 +250,7 @@ void main() {
       messages.add(message);
     });
 
-    final SemanticsTester semantics = new SemanticsTester(tester);
+    semantics = new SemanticsTester(tester);
 
     final List<Widget> children = <Widget>[];
     for (int i = 0; i < 80; i++)
@@ -283,12 +288,10 @@ void main() {
     expect(message['pixels'], isNonNegative);
     expect(message['minScrollExtent'], 0.0);
     expect(message['maxScrollExtent'], 7200.0);
-
-    semantics.dispose();
   });
 
   testWidgets('Semantics tree is populated mid-scroll', (WidgetTester tester) async {
-    final SemanticsTester semantics = new SemanticsTester(tester);
+    semantics = new SemanticsTester(tester);
 
     final List<Widget> children = <Widget>[];
     for (int i = 0; i < 80; i++)
@@ -310,8 +313,40 @@ void main() {
     expect(semantics, includesNodeWith(label: 'Item 1'));
     expect(semantics, includesNodeWith(label: 'Item 2'));
     expect(semantics, includesNodeWith(label: 'Item 3'));
+  });
 
+  testWidgets('Can toggle semantics on, off, on without crash',  (WidgetTester tester) async {
+    await tester.pumpWidget(
+      new Directionality(
+        textDirection: TextDirection.ltr,
+        child: new ListView(
+          children: new List<Widget>.generate(40, (int i) {
+            return new Container(
+              child: new Text('item $i'),
+              height: 40.0,
+            );
+          }),
+        ),
+      ),
+    );
+
+    // Start with semantics off.
+    expect(tester.binding.pipelineOwner.semanticsOwner, isNull);
+
+    // Semantics on
+    semantics = new SemanticsTester(tester);
+    await tester.pumpAndSettle();
+    expect(tester.binding.pipelineOwner.semanticsOwner, isNotNull);
+
+    // Semantics off
     semantics.dispose();
+    await tester.pumpAndSettle();
+    expect(tester.binding.pipelineOwner.semanticsOwner, isNull);
+
+    // Semantics on
+    semantics = new SemanticsTester(tester);
+    await tester.pumpAndSettle();
+    expect(tester.binding.pipelineOwner.semanticsOwner, isNotNull);
   });
 }