Make "mergeIntoParent" information available during compile of semantics tree (#12332)

* ++

* Allow unmerging of SemanticsNodes

* test passing

* ++

* remove prints

* doc comments

* rectify comment

* review comments

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

@@ -710,7 +710,12 @@ abstract class _InterestingSemanticsFragment extends _SemanticsFragment {
     SemanticsAnnotator annotator,
     Iterable<_SemanticsFragment> children,
     bool dropSemanticsOfPreviousSiblings,
-  }) : super(renderObjectOwner: renderObjectOwner, annotator: annotator, children: children, dropSemanticsOfPreviousSiblings: dropSemanticsOfPreviousSiblings);
+  }) : super(
+         renderObjectOwner: renderObjectOwner,
+         annotator: annotator,
+         children: children,
+         dropSemanticsOfPreviousSiblings: dropSemanticsOfPreviousSiblings,
+       );
 
   @override
   Iterable<SemanticsNode> compile({ _SemanticsGeometry geometry, SemanticsNode currentSemantics, SemanticsNode parentSemantics }) sync* {
@@ -743,7 +748,12 @@ class _RootSemanticsFragment extends _InterestingSemanticsFragment {
     SemanticsAnnotator annotator,
     Iterable<_SemanticsFragment> children,
     bool dropSemanticsOfPreviousSiblings,
-  }) : super(renderObjectOwner: renderObjectOwner, annotator: annotator, children: children, dropSemanticsOfPreviousSiblings: dropSemanticsOfPreviousSiblings);
+  }) : super(
+         renderObjectOwner: renderObjectOwner,
+         annotator: annotator,
+         children: children,
+         dropSemanticsOfPreviousSiblings: dropSemanticsOfPreviousSiblings,
+       );
 
   @override
   SemanticsNode establishSemanticsNode(_SemanticsGeometry geometry, SemanticsNode currentSemantics, SemanticsNode parentSemantics) {
@@ -787,7 +797,19 @@ class _ConcreteSemanticsFragment extends _InterestingSemanticsFragment {
     SemanticsAnnotator annotator,
     Iterable<_SemanticsFragment> children,
     bool dropSemanticsOfPreviousSiblings,
-  }) : super(renderObjectOwner: renderObjectOwner, annotator: annotator, children: children, dropSemanticsOfPreviousSiblings: dropSemanticsOfPreviousSiblings);
+    bool mergeIntoParent,
+    bool mergesAllDescendants,
+  }) : _mergeIntoParent = mergeIntoParent,
+       _mergesAllDescendants = mergesAllDescendants,
+       super(
+         renderObjectOwner: renderObjectOwner,
+         annotator: annotator,
+         children: children,
+         dropSemanticsOfPreviousSiblings: dropSemanticsOfPreviousSiblings,
+       );
+
+  final bool _mergeIntoParent;
+  final bool _mergesAllDescendants;
 
   @override
   SemanticsNode establishSemanticsNode(_SemanticsGeometry geometry, SemanticsNode currentSemantics, SemanticsNode parentSemantics) {
@@ -796,6 +818,8 @@ class _ConcreteSemanticsFragment extends _InterestingSemanticsFragment {
       showOnScreen: renderObjectOwner.showOnScreen,
     );
     final SemanticsNode node = renderObjectOwner._semantics;
+    node.isMergedIntoParent = _mergeIntoParent;
+    node.mergeAllDescendantsIntoThisNode = _mergesAllDescendants;
     if (geometry != null) {
       geometry.applyAncestorChain(_ancestorChain);
       geometry.updateSemanticsNode(rendering: renderObjectOwner, semantics: node, parentSemantics: parentSemantics);
@@ -830,27 +854,43 @@ class _ImplicitSemanticsFragment extends _InterestingSemanticsFragment {
     SemanticsAnnotator annotator,
     Iterable<_SemanticsFragment> children,
     bool dropSemanticsOfPreviousSiblings,
-  }) : super(renderObjectOwner: renderObjectOwner, annotator: annotator, children: children, dropSemanticsOfPreviousSiblings: dropSemanticsOfPreviousSiblings);
+    bool mergeIntoParent,
+    bool mergesAllDescendants,
+  }) : _mergeIntoParent = mergeIntoParent,
+       _mergesAllDescendants = mergesAllDescendants,
+       super(
+         renderObjectOwner: renderObjectOwner,
+         annotator: annotator,
+         children: children,
+         dropSemanticsOfPreviousSiblings: dropSemanticsOfPreviousSiblings,
+       );
 
   // If true, this fragment will introduce its own node into the Semantics Tree.
   // If false, a borrowed semantics node from an ancestor is used.
   bool _introducesOwnNode;
 
+  final bool _mergeIntoParent;
+  final bool _mergesAllDescendants;
+
   @override
   SemanticsNode establishSemanticsNode(_SemanticsGeometry geometry, SemanticsNode currentSemantics, SemanticsNode parentSemantics) {
     SemanticsNode node;
     assert(_introducesOwnNode == null);
-    _introducesOwnNode = currentSemantics == null && annotator != null;
+    assert(annotator != null || _mergesAllDescendants);
+    _introducesOwnNode = currentSemantics == null;
     if (_introducesOwnNode) {
       renderObjectOwner._semantics ??= new SemanticsNode(
         handler: renderObjectOwner is SemanticsActionHandler ? renderObjectOwner as dynamic : null,
         showOnScreen: renderObjectOwner.showOnScreen,
       );
       node = renderObjectOwner._semantics;
+      node.isMergedIntoParent = _mergeIntoParent;
     } else {
       renderObjectOwner._semantics = null;
       node = currentSemantics;
     }
+    if (!node.mergeAllDescendantsIntoThisNode)
+      node.mergeAllDescendantsIntoThisNode = _mergesAllDescendants;
     if (geometry != null) {
       geometry.applyAncestorChain(_ancestorChain);
       if (_introducesOwnNode)
@@ -2513,6 +2553,14 @@ abstract class RenderObject extends AbstractNode with DiagnosticableTreeMixin im
   /// determine if a node is previous to this one.
   bool get isBlockingSemanticsOfPreviouslyPaintedNodes => false;
 
+  /// Whether the semantic information provided by this [RenderObject] and all
+  /// of its descendants should be treated as one logical entity.
+  ///
+  /// If true is returned, the descendants of this [RenderObject]'s
+  /// [SemanticsNode] will merge their semantic information into the
+  /// [SemanticsNode] representing this [RenderObject].
+  bool get isMergingSemanticsOfDescendants => false;
+
   /// The bounding box, in the local coordinate system, of this
   /// object, for accessibility purposes.
   Rect get semanticBounds;
@@ -2662,7 +2710,7 @@ abstract class RenderObject extends AbstractNode with DiagnosticableTreeMixin im
     try {
       assert(_needsSemanticsUpdate);
       assert(_semantics != null || parent is! RenderObject);
-      final _SemanticsFragment fragment = _getSemanticsFragment();
+      final _SemanticsFragment fragment = _getSemanticsFragment(mergeIntoParent: _semantics?.parent?.isPartOfNodeMerging ?? false);
       assert(fragment is _InterestingSemanticsFragment);
       final SemanticsNode node = fragment.compile(parentSemantics: _semantics?.parent).single;
       assert(node != null);
@@ -2678,14 +2726,20 @@ abstract class RenderObject extends AbstractNode with DiagnosticableTreeMixin im
   /// children collecting [_SemanticsFragments] for them, and then returns an
   /// appropriate [_SemanticsFragment] object that describes the RenderObject's
   /// semantics.
-  _SemanticsFragment _getSemanticsFragment() {
+  _SemanticsFragment _getSemanticsFragment({ bool mergeIntoParent: false }) {
     // early-exit if we're not dirty and have our own semantics
     if (!_needsSemanticsUpdate && isSemanticBoundary) {
       assert(_semantics != null);
-      return new _CleanSemanticsFragment(renderObjectOwner: this, dropSemanticsOfPreviousSiblings: isBlockingSemanticsOfPreviouslyPaintedNodes);
+      if (mergeIntoParent == _semantics.isMergedIntoParent) {
+        return new _CleanSemanticsFragment(
+          renderObjectOwner: this,
+          dropSemanticsOfPreviousSiblings: isBlockingSemanticsOfPreviouslyPaintedNodes,
+        );
+      }
     }
     List<_SemanticsFragment> children;
     bool dropSemanticsOfPreviousSiblings = isBlockingSemanticsOfPreviouslyPaintedNodes;
+    final bool childrenMergeIntoParent = mergeIntoParent || isMergingSemanticsOfDescendants;
     visitChildrenForSemantics((RenderObject child) {
       if (_needsSemanticsGeometryUpdate) {
         // If our geometry changed, make sure the child also does a
@@ -2694,7 +2748,7 @@ abstract class RenderObject extends AbstractNode with DiagnosticableTreeMixin im
         child._needsSemanticsUpdate = true;
         child._needsSemanticsGeometryUpdate = true;
       }
-      final _SemanticsFragment fragment = child._getSemanticsFragment();
+      final _SemanticsFragment fragment = child._getSemanticsFragment(mergeIntoParent: childrenMergeIntoParent);
       assert(fragment != null);
       if (fragment.dropSemanticsOfPreviousSiblings) {
         children = null; // throw away all left siblings of [child].
@@ -2714,19 +2768,51 @@ abstract class RenderObject extends AbstractNode with DiagnosticableTreeMixin im
     _needsSemanticsUpdate = false;
     _needsSemanticsGeometryUpdate = false;
     final SemanticsAnnotator annotator = semanticsAnnotator;
-    if (parent is! RenderObject)
-      return new _RootSemanticsFragment(renderObjectOwner: this, annotator: annotator, children: children, dropSemanticsOfPreviousSiblings: dropSemanticsOfPreviousSiblings);
-    if (isSemanticBoundary)
-      return new _ConcreteSemanticsFragment(renderObjectOwner: this, annotator: annotator, children: children, dropSemanticsOfPreviousSiblings: dropSemanticsOfPreviousSiblings);
-    if (annotator != null)
-      return new _ImplicitSemanticsFragment(renderObjectOwner: this, annotator: annotator, children: children, dropSemanticsOfPreviousSiblings: dropSemanticsOfPreviousSiblings);
+    if (parent is! RenderObject) {
+      assert(!mergeIntoParent);
+      assert(!isMergingSemanticsOfDescendants);
+      return new _RootSemanticsFragment(
+        renderObjectOwner: this,
+        annotator: annotator,
+        children: children,
+        dropSemanticsOfPreviousSiblings: dropSemanticsOfPreviousSiblings,
+      );
+    }
+    if (isSemanticBoundary) {
+      return new _ConcreteSemanticsFragment(
+        renderObjectOwner: this,
+        annotator: annotator,
+        children: children,
+        dropSemanticsOfPreviousSiblings: dropSemanticsOfPreviousSiblings,
+        mergeIntoParent: mergeIntoParent,
+        mergesAllDescendants: isMergingSemanticsOfDescendants,
+      );
+    }
+     if (annotator != null || isMergingSemanticsOfDescendants) {
+       return new _ImplicitSemanticsFragment(
+         renderObjectOwner: this,
+         annotator: annotator,
+         children: children,
+         dropSemanticsOfPreviousSiblings: dropSemanticsOfPreviousSiblings,
+         mergeIntoParent: mergeIntoParent,
+         mergesAllDescendants: isMergingSemanticsOfDescendants,
+       );
+     }
     _semantics = null;
     if (children == null) {
       // Introduces no semantics and has no descendants that introduce semantics.
-      return new _EmptySemanticsFragment(renderObjectOwner: this, dropSemanticsOfPreviousSiblings: dropSemanticsOfPreviousSiblings);
+      return new _EmptySemanticsFragment(
+        renderObjectOwner: this,
+        dropSemanticsOfPreviousSiblings: dropSemanticsOfPreviousSiblings,
+      );
+    }
+    if (children.length > 1) {
+      return new _ForkingSemanticsFragment(
+        renderObjectOwner: this,
+        children: children,
+        dropSemanticsOfPreviousSiblings: dropSemanticsOfPreviousSiblings,
+      );
     }
-    if (children.length > 1)
-      return new _ForkingSemanticsFragment(renderObjectOwner: this, children: children, dropSemanticsOfPreviousSiblings: dropSemanticsOfPreviousSiblings);
     assert(children.length == 1);
     return children.single..dropSemanticsOfPreviousSiblings = dropSemanticsOfPreviousSiblings;
   }

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

@@ -3014,6 +3014,7 @@ class RenderSemanticsGestureHandler extends RenderProxyBox implements SemanticsA
     _innerNode ??= new SemanticsNode(handler: this, showOnScreen: showOnScreen);
     _innerNode
       ..wasAffectedByClip = node.wasAffectedByClip
+      ..isMergedIntoParent = node.isPartOfNodeMerging
       ..rect = Offset.zero & node.rect.size;
 
     semanticsAnnotator(_innerNode);
@@ -3270,11 +3271,8 @@ class RenderMergeSemantics extends RenderProxyBox {
   RenderMergeSemantics({ RenderBox child }) : super(child);
 
   @override
-  SemanticsAnnotator get semanticsAnnotator => _annotate;
+  bool get isMergingSemanticsOfDescendants => true;
 
-  void _annotate(SemanticsNode node) {
-    node.mergeAllDescendantsIntoThisNode = true;
-  }
 }
 
 /// Excludes this subtree from the semantic tree.

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

@@ -330,7 +330,7 @@ class SemanticsNode extends AbstractNode with DiagnosticableTreeMixin {
     return _actionHandler != null && (_actions & action.index) != 0;
   }
 
-  /// Whether all this node and all of its descendants should be treated as one logical entity.
+  /// Whether this node and all of its descendants should be treated as one logical entity.
   bool get mergeAllDescendantsIntoThisNode => _mergeAllDescendantsIntoThisNode;
   bool _mergeAllDescendantsIntoThisNode = false;
   set mergeAllDescendantsIntoThisNode(bool value) {
@@ -341,17 +341,26 @@ class SemanticsNode extends AbstractNode with DiagnosticableTreeMixin {
     _markDirty();
   }
 
-  bool get _inheritedMergeAllDescendantsIntoThisNode => _inheritedMergeAllDescendantsIntoThisNodeValue;
-  bool _inheritedMergeAllDescendantsIntoThisNodeValue = false;
-  set _inheritedMergeAllDescendantsIntoThisNode(bool value) {
+  /// Whether this node merges its semantic information into an ancestor node.
+  bool get isMergedIntoParent => _isMergedIntoParent;
+  bool _isMergedIntoParent = false;
+  set isMergedIntoParent(bool value) {
     assert(value != null);
-    if (_inheritedMergeAllDescendantsIntoThisNodeValue == value)
+    if (_isMergedIntoParent == value)
       return;
-    _inheritedMergeAllDescendantsIntoThisNodeValue = value;
+    _isMergedIntoParent = value;
     _markDirty();
   }
 
-  bool get _shouldMergeAllDescendantsIntoThisNode => mergeAllDescendantsIntoThisNode || _inheritedMergeAllDescendantsIntoThisNode;
+  /// Whether this node is taking part in a merge of semantic information.
+  ///
+  /// This returns true if the node is either merged into an ancestor node or if
+  /// decedent nodes are merged into this node.
+  ///
+  /// See also:
+  ///  * [isMergedIntoParent]
+  ///  * [mergeAllDescendantsIntoThisNode]
+  bool get isPartOfNodeMerging => mergeAllDescendantsIntoThisNode || isMergedIntoParent;
 
   int _flags = 0;
   void _setFlag(SemanticsFlags flag, bool value) {
@@ -433,13 +442,11 @@ class SemanticsNode extends AbstractNode with DiagnosticableTreeMixin {
 
   /// Restore this node to its default state.
   void reset() {
-    final bool hadInheritedMergeAllDescendantsIntoThisNode = _inheritedMergeAllDescendantsIntoThisNode;
     _actions = 0;
     _flags = 0;
-    if (hadInheritedMergeAllDescendantsIntoThisNode)
-      _inheritedMergeAllDescendantsIntoThisNodeValue = true;
     _label = '';
     _textDirection = null;
+    _mergeAllDescendantsIntoThisNode = false;
     _tags.clear();
     _markDirty();
   }
@@ -603,8 +610,7 @@ class SemanticsNode extends AbstractNode with DiagnosticableTreeMixin {
       _dirty = false;
       _markDirty();
     }
-    if (parent != null)
-      _inheritedMergeAllDescendantsIntoThisNode = parent._shouldMergeAllDescendantsIntoThisNode;
+    assert(isMergedIntoParent == (parent?.isPartOfNodeMerging ?? false));
     if (_children != null) {
       for (SemanticsNode child in _children)
         child.attach(owner);
@@ -759,7 +765,7 @@ class SemanticsNode extends AbstractNode with DiagnosticableTreeMixin {
       hideOwner = inDirtyNodes;
     }
     properties.add(new DiagnosticsProperty<SemanticsOwner>('owner', owner, level: hideOwner ? DiagnosticLevel.hidden : DiagnosticLevel.info));
-    properties.add(new FlagProperty('shouldMergeAllDescendantsIntoThisNode', value: _shouldMergeAllDescendantsIntoThisNode, ifTrue: 'leaf merge'));
+    properties.add(new FlagProperty('isPartOfNodeMerging', value: isPartOfNodeMerging, ifTrue: 'leaf merge'));
     final Offset offset = transform != null ? MatrixUtils.getAsTranslation(transform) : null;
     if (offset != null) {
       properties.add(new DiagnosticsProperty<Rect>('rect', rect.shift(offset), showName: false));
@@ -885,31 +891,12 @@ class SemanticsOwner extends ChangeNotifier {
       visitedNodes.addAll(localDirtyNodes);
       for (SemanticsNode node in localDirtyNodes) {
         assert(node._dirty);
-        assert(node.parent == null || !node.parent._shouldMergeAllDescendantsIntoThisNode || node._inheritedMergeAllDescendantsIntoThisNode);
-        if (node._shouldMergeAllDescendantsIntoThisNode) {
+        assert(node.parent == null || !node.parent.isPartOfNodeMerging || node.isMergedIntoParent);
+        if (node.isPartOfNodeMerging) {
           assert(node.mergeAllDescendantsIntoThisNode || node.parent != null);
-          if (node.mergeAllDescendantsIntoThisNode ||
-              node.parent != null && node.parent._shouldMergeAllDescendantsIntoThisNode) {
-            // if we're merged into our parent, make sure our parent is added to the list
-            if (node.parent != null && node.parent._shouldMergeAllDescendantsIntoThisNode)
-              node.parent._markDirty(); // this can add the node to the dirty list
-            // make sure all the descendants are also marked, so that if one gets marked dirty later we know to walk up then too
-            if (node._children != null) {
-              for (SemanticsNode child in node._children)
-                child._inheritedMergeAllDescendantsIntoThisNode = true; // this can add the node to the dirty list
-            }
-          } else {
-            // we previously were being merged but aren't any more
-            // update our bits and all our descendants'
-            assert(node._inheritedMergeAllDescendantsIntoThisNode);
-            assert(!node.mergeAllDescendantsIntoThisNode);
-            assert(node.parent == null || !node.parent._shouldMergeAllDescendantsIntoThisNode);
-            node._inheritedMergeAllDescendantsIntoThisNode = false;
-            if (node._children != null) {
-              for (SemanticsNode child in node._children)
-                child._inheritedMergeAllDescendantsIntoThisNode = false; // this can add the node to the dirty list
-            }
-          }
+          // if we're merged into our parent, make sure our parent is added to the dirty list
+          if (node.parent != null && node.parent.isPartOfNodeMerging)
+            node.parent._markDirty(); // this can add the node to the dirty list
         }
       }
     }
@@ -937,7 +924,7 @@ class SemanticsOwner extends ChangeNotifier {
 
   SemanticsActionHandler _getSemanticsActionHandlerForId(int id, SemanticsAction action) {
     SemanticsNode result = _nodes[id];
-    if (result != null && result._shouldMergeAllDescendantsIntoThisNode && !result._canPerformAction(action)) {
+    if (result != null && result.isPartOfNodeMerging && !result._canPerformAction(action)) {
       result._visitDescendants((SemanticsNode node) {
         if (node._canPerformAction(action)) {
           result = node;

packages/flutter/test/rendering/semantics_test.dart

@@ -177,7 +177,7 @@ void main() {
 
     expect(
       minimalProperties.toStringDeep(minLevel: DiagnosticLevel.hidden),
-      'SemanticsNode#16(owner: null, shouldMergeAllDescendantsIntoThisNode: false, Rect.fromLTRB(0.0, 0.0, 0.0, 0.0), wasAffectedByClip: false, actions: [], tags: [], isSelected: false, label: "", textDirection: null)\n',
+      'SemanticsNode#16(owner: null, isPartOfNodeMerging: false, Rect.fromLTRB(0.0, 0.0, 0.0, 0.0), wasAffectedByClip: false, actions: [], tags: [], isSelected: false, label: "", textDirection: null)\n',
     );
 
     final SemanticsNode allProperties = new SemanticsNode()

packages/flutter/test/widgets/semantics_merge_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/material.dart';
+import 'package:flutter/rendering.dart';
+import 'package:flutter/widgets.dart';
+import 'package:flutter_test/flutter_test.dart';
+
+import 'semantics_tester.dart';
+
+void main() {
+  testWidgets('MergeSemantics', (WidgetTester tester) async {
+    final SemanticsTester semantics = new SemanticsTester(tester);
+
+    // not merged
+    await tester.pumpWidget(
+      new Directionality(
+        textDirection: TextDirection.ltr,
+        child: new Row(
+          children: <Widget>[
+            new Semantics(
+                label: 'test1',
+                textDirection: TextDirection.ltr,
+                child: new Container()
+            ),
+            new Semantics(
+                label: 'test2',
+                textDirection: TextDirection.ltr,
+                child: new Container()
+            )
+          ],
+        ),
+      ),
+    );
+
+    expect(semantics, hasSemantics(
+      new TestSemantics.root(
+        children: <TestSemantics>[
+          new TestSemantics.rootChild(id: 1, label: 'test1'),
+          new TestSemantics.rootChild(id: 2, label: 'test2'),
+        ],
+      ),
+      ignoreRect: true,
+      ignoreTransform: true,
+    ));
+
+    // merged
+    await tester.pumpWidget(
+      new Directionality(
+        textDirection: TextDirection.ltr,
+        child: new MergeSemantics(
+          child: new Row(
+            children: <Widget>[
+              new Semantics(
+                  label: 'test1',
+                  textDirection: TextDirection.ltr,
+                  child: new Container()
+              ),
+              new Semantics(
+                  label: 'test2',
+                  textDirection: TextDirection.ltr,
+                  child: new Container()
+              )
+            ],
+          ),
+        ),
+      ),
+    );
+
+    expect(semantics, hasSemantics(
+      new TestSemantics.root(label: 'test1\ntest2'),
+      ignoreRect: true,
+      ignoreTransform: true,
+    ));
+
+    // not merged
+    await tester.pumpWidget(
+      new Directionality(
+        textDirection: TextDirection.ltr,
+        child: new Row(
+          children: <Widget>[
+            new Semantics(
+                label: 'test1',
+                textDirection: TextDirection.ltr,
+                child: new Container()
+            ),
+            new Semantics(
+                label: 'test2',
+                textDirection: TextDirection.ltr,
+                child: new Container()
+            )
+          ],
+        ),
+      ),
+    );
+
+    expect(semantics, hasSemantics(
+      new TestSemantics.root(
+        children: <TestSemantics>[
+          new TestSemantics.rootChild(id: 5, label: 'test1'),
+          new TestSemantics.rootChild(id: 6, label: 'test2'),
+        ],
+      ),
+      ignoreRect: true,
+      ignoreTransform: true,
+    ));
+
+    semantics.dispose();
+  });
+}