Fix drawer in accessibility mode

The "leaf merge" feature was getting confused when we reset the
SemanticsNode. We now separately track whether the node itself is marked
as being merged vs whether we inherited that state, and we don't reset
the inherited state until you're reattached or reserialised. In the
latter case, we do a "just in time" clearing of the flag just like we
previously did a "just in time" setting of the flag, except now the flag
we're setting or clearing is the inherited flag not the actual flag.

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

@@ -40,6 +40,7 @@ abstract class SemanticActionHandler {
 
 enum _SemanticFlags {
   mergeAllDescendantsIntoThisNode,
+  inheritedMergeAllDescendantsIntoThisNode, // whether an ancestor had mergeAllDescendantsIntoThisNode set
   canBeTapped,
   canBeLongPressed,
   canBeScrolledHorizontally,
@@ -119,6 +120,11 @@ class SemanticsNode extends AbstractNode {
   bool get mergeAllDescendantsIntoThisNode => _flags[_SemanticFlags.mergeAllDescendantsIntoThisNode];
   void set mergeAllDescendantsIntoThisNode(bool value) => _setFlag(_SemanticFlags.mergeAllDescendantsIntoThisNode, value);
 
+  bool get _inheritedMergeAllDescendantsIntoThisNode => _flags[_SemanticFlags.inheritedMergeAllDescendantsIntoThisNode];
+  void set _inheritedMergeAllDescendantsIntoThisNode(bool value) => _setFlag(_SemanticFlags.inheritedMergeAllDescendantsIntoThisNode, value);
+
+  bool get _shouldMergeAllDescendantsIntoThisNode => mergeAllDescendantsIntoThisNode || _inheritedMergeAllDescendantsIntoThisNode;
+
   bool get canBeTapped => _flags[_SemanticFlags.canBeTapped];
   void set canBeTapped(bool value) => _setFlag(_SemanticFlags.canBeTapped, value, needsHandler: true);
 
@@ -148,7 +154,10 @@ class SemanticsNode extends AbstractNode {
   }
 
   void reset() {
+    bool hadInheritedMergeAllDescendantsIntoThisNode = _inheritedMergeAllDescendantsIntoThisNode;
     _flags.reset();
+    if (hadInheritedMergeAllDescendantsIntoThisNode)
+      _inheritedMergeAllDescendantsIntoThisNode = true;
     _label = '';
     _markDirty();
   }
@@ -257,6 +266,8 @@ class SemanticsNode extends AbstractNode {
     assert(!_nodes.containsKey(_id));
     _nodes[_id] = this;
     _detachedNodes.remove(this);
+    if (parent != null)
+      _inheritedMergeAllDescendantsIntoThisNode = parent._shouldMergeAllDescendantsIntoThisNode;
     if (_children != null) {
       for (SemanticsNode child in _children)
         child.attach();
@@ -308,7 +319,7 @@ class SemanticsNode extends AbstractNode {
       result.strings = new mojom.SemanticStrings();
       result.strings.label = label;
       List<mojom.SemanticsNode> children = <mojom.SemanticsNode>[];
-      if (mergeAllDescendantsIntoThisNode) {
+      if (_shouldMergeAllDescendantsIntoThisNode) {
         _visitDescendants((SemanticsNode node) {
           result.flags.canBeTapped = result.flags.canBeTapped || node.canBeTapped;
           result.flags.canBeLongPressed = result.flags.canBeLongPressed || node.canBeLongPressed;
@@ -318,6 +329,7 @@ class SemanticsNode extends AbstractNode {
           result.flags.isChecked = result.flags.isChecked || node.isChecked;
           if (node.label != '')
             result.strings.label = result.strings.label.isNotEmpty ? '${result.strings.label}\n${node.label}' : node.label;
+          node._dirty = false;
           return true; // continue walk
         });
         // and we pretend to have no children
@@ -359,16 +371,32 @@ class SemanticsNode extends AbstractNode {
       // we mutate the list as we walk it here, which is why we use an index instead of an iterator
       SemanticsNode node = _dirtyNodes[index];
       assert(node._dirty);
-      assert(node.parent == null || !node.parent.mergeAllDescendantsIntoThisNode || node.mergeAllDescendantsIntoThisNode);
-      if (node.mergeAllDescendantsIntoThisNode) {
-        // if we're merged into our parent, make sure our parent is added to the list
-        if (node.parent != null && node.parent.mergeAllDescendantsIntoThisNode)
-          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.mergeAllDescendantsIntoThisNode = true; // this can add the node to the dirty list
-      }
+      assert(node.parent == null || !node.parent._shouldMergeAllDescendantsIntoThisNode || node._inheritedMergeAllDescendantsIntoThisNode);
+      if (node._shouldMergeAllDescendantsIntoThisNode) {
+        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
+          }
+        }
+      } 
       assert(_dirtyNodes[index] == node); // make sure nothing went in front of us in the list
     }
     _dirtyNodes.sort((SemanticsNode a, SemanticsNode b) => a.depth - b.depth);
@@ -396,7 +424,7 @@ class SemanticsNode extends AbstractNode {
   static SemanticActionHandler getSemanticActionHandlerForId(int id, { _SemanticFlags neededFlag }) {
     assert(neededFlag != null);
     SemanticsNode result = _nodes[id];
-    if (result != null && result.mergeAllDescendantsIntoThisNode && !result._canHandle(neededFlag)) {
+    if (result != null && result._shouldMergeAllDescendantsIntoThisNode && !result._canHandle(neededFlag)) {
       result._visitDescendants((SemanticsNode node) {
         if (node._actionHandler != null && node._flags[neededFlag]) {
           result = node;
@@ -412,8 +440,8 @@ class SemanticsNode extends AbstractNode {
 
   String toString() {
     return '$runtimeType($_id'
-             '${_dirty ? " (dirty)" : ""}'
-             '${mergeAllDescendantsIntoThisNode ? " (leaf merge)" : ""}'
+             '${_dirty ? " (${ _dirtyNodes.contains(this) ? 'dirty' : 'STALE' })" : ""}'
+             '${_shouldMergeAllDescendantsIntoThisNode ? " (leaf merge)" : ""}'
              '; $rect'
              '${wasAffectedByClip ? " (clipped)" : ""}'
              '${canBeTapped ? "; canBeTapped" : ""}'

packages/flutter/test/widget/semantics_7_test.dart

+// 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/widgets.dart';
+import 'package:flutter_test/flutter_test.dart';
+import 'package:test/test.dart';
+
+import 'test_semantics.dart';
+import 'package:sky_services/semantics/semantics.mojom.dart' as mojom;
+
+void main() {
+  test('Semantics 7 - Merging', () {
+    testWidgets((WidgetTester tester) {
+      TestSemanticsListener client = new TestSemanticsListener();
+
+      String label;
+
+      label = '1';
+      tester.pumpWidget(
+        new Stack(
+          children: <Widget>[
+            new MergeSemantics(
+              child: new Semantics(
+                checked: true,
+                container: true,
+                child: new Semantics(
+                  container: true,
+                  label: label
+                )
+              )
+            ),
+            new MergeSemantics(
+              child: new Stack(
+                children: <Widget>[
+                  new Semantics(
+                    checked: true
+                  ),
+                  new Semantics(
+                    label: label
+                  )
+                ]
+              )
+            ),
+          ]
+        )
+      );
+      expect(client.updates.length, equals(2));
+      expect(client.updates[0].id, equals(0));
+      expect(client.updates[0].flags.canBeTapped, isFalse);
+      expect(client.updates[0].flags.canBeLongPressed, isFalse);
+      expect(client.updates[0].flags.canBeScrolledHorizontally, isFalse);
+      expect(client.updates[0].flags.canBeScrolledVertically, isFalse);
+      expect(client.updates[0].flags.hasCheckedState, isFalse);
+      expect(client.updates[0].flags.isChecked, isFalse);
+      expect(client.updates[0].strings.label, equals(''));
+      expect(client.updates[0].geometry.transform, isNull);
+      expect(client.updates[0].geometry.left, equals(0.0));
+      expect(client.updates[0].geometry.top, equals(0.0));
+      expect(client.updates[0].geometry.width, equals(800.0));
+      expect(client.updates[0].geometry.height, equals(600.0));
+      expect(client.updates[0].children.length, equals(2));
+      expect(client.updates[0].children[0].id, equals(1));
+      expect(client.updates[0].children[0].flags.canBeTapped, isFalse);
+      expect(client.updates[0].children[0].flags.canBeLongPressed, isFalse);
+      expect(client.updates[0].children[0].flags.canBeScrolledHorizontally, isFalse);
+      expect(client.updates[0].children[0].flags.canBeScrolledVertically, isFalse);
+      expect(client.updates[0].children[0].flags.hasCheckedState, isTrue);
+      expect(client.updates[0].children[0].flags.isChecked, isTrue);
+      expect(client.updates[0].children[0].strings.label, equals(label));
+      expect(client.updates[0].children[0].geometry.transform, isNull);
+      expect(client.updates[0].children[0].geometry.left, equals(0.0));
+      expect(client.updates[0].children[0].geometry.top, equals(0.0));
+      expect(client.updates[0].children[0].geometry.width, equals(800.0));
+      expect(client.updates[0].children[0].geometry.height, equals(600.0));
+      expect(client.updates[0].children[0].children.length, equals(0));
+      // IDs 2 and 3 are used up by the nodes that get merged in
+      expect(client.updates[0].children[1].id, equals(4));
+      expect(client.updates[0].children[1].flags.canBeTapped, isFalse);
+      expect(client.updates[0].children[1].flags.canBeLongPressed, isFalse);
+      expect(client.updates[0].children[1].flags.canBeScrolledHorizontally, isFalse);
+      expect(client.updates[0].children[1].flags.canBeScrolledVertically, isFalse);
+      expect(client.updates[0].children[1].flags.hasCheckedState, isTrue);
+      expect(client.updates[0].children[1].flags.isChecked, isTrue);
+      expect(client.updates[0].children[1].strings.label, equals(label));
+      expect(client.updates[0].children[1].geometry.transform, isNull);
+      expect(client.updates[0].children[1].geometry.left, equals(0.0));
+      expect(client.updates[0].children[1].geometry.top, equals(0.0));
+      expect(client.updates[0].children[1].geometry.width, equals(800.0));
+      expect(client.updates[0].children[1].geometry.height, equals(600.0));
+      expect(client.updates[0].children[1].children.length, equals(0));
+      // IDs 5 and 6 are used up by the nodes that get merged in
+      expect(client.updates[1], isNull);
+      client.updates.clear();
+
+      label = '2';
+      tester.pumpWidget(
+        new Stack(
+          children: <Widget>[
+            new MergeSemantics(
+              child: new Semantics(
+                checked: true,
+                container: true,
+                child: new Semantics(
+                  container: true,
+                  label: label
+                )
+              )
+            ),
+            new MergeSemantics(
+              child: new Stack(
+                children: <Widget>[
+                  new Semantics(
+                    checked: true
+                  ),
+                  new Semantics(
+                    label: label
+                  )
+                ]
+              )
+            ),
+          ]
+        )
+      );
+      expect(client.updates.length, equals(3));
+      expect(client.updates[2], isNull);
+
+      // The order of the nodes is undefined, so allow both orders.
+      mojom.SemanticsNode a, b;
+      if (client.updates[0].id == 1) {
+        a = client.updates[0];
+        b = client.updates[1];
+      } else {
+        a = client.updates[1];
+        b = client.updates[0];
+      }
+      
+      expect(a.id, equals(1));
+      expect(a.flags.canBeTapped, isFalse);
+      expect(a.flags.canBeLongPressed, isFalse);
+      expect(a.flags.canBeScrolledHorizontally, isFalse);
+      expect(a.flags.canBeScrolledVertically, isFalse);
+      expect(a.flags.hasCheckedState, isTrue);
+      expect(a.flags.isChecked, isTrue);
+      expect(a.strings.label, equals(label));
+      expect(a.geometry.transform, isNull);
+      expect(a.geometry.left, equals(0.0));
+      expect(a.geometry.top, equals(0.0));
+      expect(a.geometry.width, equals(800.0));
+      expect(a.geometry.height, equals(600.0));
+      expect(a.children.length, equals(0));
+
+      expect(b.id, equals(4));
+      expect(b.flags.canBeTapped, isFalse);
+      expect(b.flags.canBeLongPressed, isFalse);
+      expect(b.flags.canBeScrolledHorizontally, isFalse);
+      expect(b.flags.canBeScrolledVertically, isFalse);
+      expect(b.flags.hasCheckedState, isTrue);
+      expect(b.flags.isChecked, isTrue);
+      expect(b.strings.label, equals(label));
+      expect(b.geometry.transform, isNull);
+      expect(b.geometry.left, equals(0.0));
+      expect(b.geometry.top, equals(0.0));
+      expect(b.geometry.width, equals(800.0));
+      expect(b.geometry.height, equals(600.0));
+      expect(b.children.length, equals(0));
+
+      client.updates.clear();
+
+    });
+  });
+}

packages/flutter/test/widget/semantics_8_test.dart

+// 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/widgets.dart';
+import 'package:flutter_test/flutter_test.dart';
+import 'package:test/test.dart';
+
+import 'test_semantics.dart';
+
+void main() {
+  test('Semantics 8 - Merging with reset', () {
+    testWidgets((WidgetTester tester) {
+      TestSemanticsListener client = new TestSemanticsListener();
+
+      tester.pumpWidget(
+        new MergeSemantics(
+          child: new Semantics(
+            container: true,
+            child: new Semantics(
+              container: true,
+              child: new Stack(
+                children: <Widget>[
+                  new Semantics(
+                    checked: true
+                  ),
+                  new Semantics(
+                    label: 'label'
+                  )
+                ]
+              )
+            )
+          )
+        )
+      );
+      expect(client.updates.length, equals(2));
+      expect(client.updates[0].id, equals(0));
+      expect(client.updates[0].flags.canBeTapped, isFalse);
+      expect(client.updates[0].flags.canBeLongPressed, isFalse);
+      expect(client.updates[0].flags.canBeScrolledHorizontally, isFalse);
+      expect(client.updates[0].flags.canBeScrolledVertically, isFalse);
+      expect(client.updates[0].flags.hasCheckedState, isTrue);
+      expect(client.updates[0].flags.isChecked, isTrue);
+      expect(client.updates[0].strings.label, equals('label'));
+      expect(client.updates[0].geometry.transform, isNull);
+      expect(client.updates[0].geometry.left, equals(0.0));
+      expect(client.updates[0].geometry.top, equals(0.0));
+      expect(client.updates[0].geometry.width, equals(800.0));
+      expect(client.updates[0].geometry.height, equals(600.0));
+      expect(client.updates[0].children.length, equals(0));
+      expect(client.updates[1], isNull);
+      client.updates.clear();
+
+      // switch the order of the inner Semantics node to trigger a reset
+      tester.pumpWidget(
+        new MergeSemantics(
+          child: new Semantics(
+            container: true,
+            child: new Semantics(
+              container: true,
+              child: new Stack(
+                children: <Widget>[
+                  new Semantics(
+                    label: 'label'
+                  ),
+                  new Semantics(
+                    checked: true
+                  )
+                ]
+              )
+            )
+          )
+        )
+      );
+      expect(client.updates.length, equals(2));
+      expect(client.updates[0].id, equals(0));
+      expect(client.updates[0].flags.canBeTapped, isFalse);
+      expect(client.updates[0].flags.canBeLongPressed, isFalse);
+      expect(client.updates[0].flags.canBeScrolledHorizontally, isFalse);
+      expect(client.updates[0].flags.canBeScrolledVertically, isFalse);
+      expect(client.updates[0].flags.hasCheckedState, isTrue);
+      expect(client.updates[0].flags.isChecked, isTrue);
+      expect(client.updates[0].strings.label, equals('label'));
+      expect(client.updates[0].geometry.transform, isNull);
+      expect(client.updates[0].geometry.left, equals(0.0));
+      expect(client.updates[0].geometry.top, equals(0.0));
+      expect(client.updates[0].geometry.width, equals(800.0));
+      expect(client.updates[0].geometry.height, equals(600.0));
+      expect(client.updates[0].children.length, equals(0));
+      expect(client.updates[1], isNull);
+      client.updates.clear();
+
+    });
+  });
+}