Unverified Commit 4e13cd07 authored by Michael Goderbauer's avatar Michael Goderbauer Committed by GitHub

Fixes crash caused by invisible semantics children (#13740)

**tl;dr:** A `RenderObject` can only be an effective semantics boundary if it actually owned a `SemanticsNode` in the previous tree generation.

When using the `BlockSemantics` widget it is possible to introduce `RenderObject`s that are configured to be a semantics boundary, but because their semantics are blocked by `BlockSemantics` they will not (immediately) end up owning a `SemanticsNode`. When now a descendant of such a node-less semantics boundary marks itself as needing a semantics update we walk up the tree until we find the closest semantics boundary (which is our node-less `RenderObject`). We now incorrectly assume that this semantics boundary has a valid `SemanticsNode` and only regenerate the semantics subtree below this node. However, because the identified semantics boundary doesn't actually own a valid `SemanticsNode` asserts are throwing (e.g. `Child with id xx is invisible and should not be added to tree.`).

To fix this problem, we can just abort the walk if we reach a semantics boundary without a semantics node because (for now) we know that the semantics information of this branch will not make it into the final semantics tree.
If the semantics block is ever removed, the current algorithm re-generates the semantics for the entire branch and the semantics will be up-to-date then despite the abort. I've added a test to verify this to make sure it continues to work even when we change the algorithm.

Fixes https://github.com/flutter/flutter/issues/13326.
/cc @gavindoughtie FYI
parent 56061759
......@@ -2233,6 +2233,12 @@ abstract class RenderObject extends AbstractNode with DiagnosticableTreeMixin im
node = node.parent;
node._cachedSemanticsConfiguration = null;
isEffectiveSemanticsBoundary = node._semanticsConfiguration.isSemanticBoundary;
if (isEffectiveSemanticsBoundary && node._semantics == null) {
// We have reached a semantics boundary that doesn't own a semantics node.
// That means the semantics of this branch are currently blocked and will
// not appear in the semantics tree. We can abort the walk here.
return;
}
}
if (node != this && _semantics != null && _needsSemanticsUpdate) {
// If `this` node has already been added to [owner._nodesNeedingSemantics]
......
......@@ -3315,12 +3315,30 @@ class RenderSemanticsAnnotations extends RenderProxyBox {
class RenderBlockSemantics extends RenderProxyBox {
/// Create a render object that blocks semantics for nodes below it in paint
/// order.
RenderBlockSemantics({ RenderBox child }) : super(child);
RenderBlockSemantics({ RenderBox child, bool blocking: true, }) : _blocking = blocking, super(child);
/// Whether this render object is blocking semantics of previously painted
/// [RenderObject]s below a common semantics boundary from the semantic tree.
bool get blocking => _blocking;
bool _blocking;
set blocking(bool value) {
assert(value != null);
if (value == _blocking)
return;
_blocking = value;
markNeedsSemanticsUpdate();
}
@override
void describeSemanticsConfiguration(SemanticsConfiguration config) {
super.describeSemanticsConfiguration(config);
config.isBlockingSemanticsOfPreviouslyPaintedNodes = true;
config.isBlockingSemanticsOfPreviouslyPaintedNodes = blocking;
}
@override
void debugFillProperties(DiagnosticPropertiesBuilder description) {
super.debugFillProperties(description);
description.add(new DiagnosticsProperty<bool>('blocking', blocking));
}
}
......
......@@ -4966,10 +4966,25 @@ class MergeSemantics extends SingleChildRenderObjectWidget {
class BlockSemantics extends SingleChildRenderObjectWidget {
/// Creates a widget that excludes the semantics of all widgets painted before
/// it in the same semantic container.
const BlockSemantics({ Key key, Widget child }) : super(key: key, child: child);
const BlockSemantics({ Key key, this.blocking: true, Widget child }) : super(key: key, child: child);
/// Whether this widget is blocking semantics of all widget that were painted
/// before it in the same semantic container.
final bool blocking;
@override
RenderBlockSemantics createRenderObject(BuildContext context) => new RenderBlockSemantics(blocking: blocking);
@override
RenderBlockSemantics createRenderObject(BuildContext context) => new RenderBlockSemantics();
void updateRenderObject(BuildContext context, RenderBlockSemantics renderObject) {
renderObject.blocking = blocking;
}
@override
void debugFillProperties(DiagnosticPropertiesBuilder description) {
super.debugFillProperties(description);
description.add(new DiagnosticsProperty<bool>('blocking', blocking));
}
}
/// A widget that drops all the semantics of its descendants.
......
// 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 'package:meta/meta.dart';
import 'semantics_tester.dart';
void main() {
testWidgets('can change semantics in a branch blocked by BlockSemantics', (WidgetTester tester) async {
final SemanticsTester semantics = new SemanticsTester(tester);
final TestSemantics expectedSemantics = new TestSemantics.root(
children: <TestSemantics>[
new TestSemantics.rootChild(
id: 1,
label: 'hello',
textDirection: TextDirection.ltr,
rect: TestSemantics.fullScreen,
)
],
);
await tester.pumpWidget(buildWidget(
blockedText: 'one',
));
expect(semantics, hasSemantics(expectedSemantics));
// The purpose of the test is to ensure that this change does not throw.
await tester.pumpWidget(buildWidget(
blockedText: 'two',
));
expect(semantics, hasSemantics(expectedSemantics));
// Ensure that the previously blocked semantics end up in the tree correctly when unblocked.
await tester.pumpWidget(buildWidget(
blockedText: 'two',
blocking: false,
));
expect(semantics, includesNodeWith(label: 'two', textDirection: TextDirection.ltr));
semantics.dispose();
});
}
Widget buildWidget({ @required String blockedText, bool blocking: true }) {
assert(blockedText != null);
return new Directionality(
textDirection: TextDirection.ltr,
child: new Stack(
fit: StackFit.expand,
children: <Widget>[
new Semantics(
container: true,
child: new ListView(
children: <Widget>[
new Text(blockedText),
],
),
),
new BlockSemantics(
blocking: blocking,
child: new Semantics(
label: 'hello',
container: true,
),
),
]
),
);
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment