Commit ca5e1f3f authored by Jason Simmons's avatar Jason Simmons Committed by GitHub

In the semantics tree, do not detach a child if it has already been assigned a new parent (#6773)

Fixes https://github.com/flutter/flutter/issues/6690

Also add a version of the Gallery smoke test that enables semantics
parent 546cef52
......@@ -5,9 +5,10 @@
import 'dart:collection' show LinkedHashSet;
import 'package:flutter/material.dart';
import 'package:flutter/rendering.dart';
import 'package:flutter_test/flutter_test.dart';
import 'package:flutter_gallery/gallery/item.dart' show GalleryItem, kAllGalleryItems;
import 'package:flutter_gallery/main.dart' as flutter_gallery_main;
import 'package:flutter_gallery/gallery/app.dart' show GalleryApp;
const String kCaption = 'Flutter Gallery';
......@@ -47,48 +48,55 @@ Future<Null> smokeDemo(WidgetTester tester, String routeName) async {
return null;
}
Future<Null> runSmokeTest(WidgetTester tester) async {
await tester.pumpWidget(new GalleryApp());
await tester.pump(); // see https://github.com/flutter/flutter/issues/1865
await tester.pump(); // triggers a frame
expect(find.text(kCaption), findsOneWidget);
final List<double> scrollDeltas = new List<double>();
double previousY = tester.getTopRight(find.text(demoCategories[0])).y;
for (String routeName in routeNames) {
final double y = tester.getTopRight(findGalleryItemByRouteName(tester, routeName)).y;
scrollDeltas.add(previousY - y);
previousY = y;
}
// Launch each demo and then scroll that item out of the way.
for (int i = 0; i < routeNames.length; i += 1) {
final String routeName = routeNames[i];
await smokeDemo(tester, routeName);
await tester.scroll(findGalleryItemByRouteName(tester, routeName), new Offset(0.0, scrollDeltas[i]));
await tester.pump(); // start the scroll
await tester.pump(const Duration(milliseconds: 500)); // wait for overscroll to timeout, if necessary
await tester.pump(const Duration(seconds: 3)); // wait for overscroll to fade away, if necessary
tester.binding.debugAssertNoTransientCallbacks('A transient callback was still active after leaving route $routeName');
}
Finder navigationMenuButton = find.byTooltip('Open navigation menu');
expect(navigationMenuButton, findsOneWidget);
await tester.tap(navigationMenuButton);
await tester.pump(); // Start opening drawer.
await tester.pump(const Duration(seconds: 1)); // Wait until it's really opened.
// switch theme
await tester.tap(find.text('Dark'));
await tester.pump();
await tester.pump(const Duration(seconds: 1)); // Wait until it's changed.
// switch theme
await tester.tap(find.text('Light'));
await tester.pump();
await tester.pump(const Duration(seconds: 1)); // Wait until it's changed.
}
void main() {
testWidgets('Flutter Gallery app smoke test', runSmokeTest);
testWidgets('Flutter Gallery app smoke test', (WidgetTester tester) async {
flutter_gallery_main
.main(); // builds the app and schedules a frame but doesn't trigger one
await tester.pump(); // see https://github.com/flutter/flutter/issues/1865
await tester.pump(); // triggers a frame
expect(find.text(kCaption), findsOneWidget);
final List<double> scrollDeltas = new List<double>();
double previousY = tester.getTopRight(find.text(demoCategories[0])).y;
for (String routeName in routeNames) {
final double y = tester.getTopRight(findGalleryItemByRouteName(tester, routeName)).y;
scrollDeltas.add(previousY - y);
previousY = y;
}
// Launch each demo and then scroll that item out of the way.
for (int i = 0; i < routeNames.length; i += 1) {
final String routeName = routeNames[i];
await smokeDemo(tester, routeName);
await tester.scroll(findGalleryItemByRouteName(tester, routeName), new Offset(0.0, scrollDeltas[i]));
await tester.pump(); // start the scroll
await tester.pump(const Duration(milliseconds: 500)); // wait for overscroll to timeout, if necessary
await tester.pump(const Duration(seconds: 3)); // wait for overscroll to fade away, if necessary
tester.binding.debugAssertNoTransientCallbacks('A transient callback was still active after leaving route $routeName');
}
Finder navigationMenuButton = find.byTooltip('Open navigation menu');
expect(navigationMenuButton, findsOneWidget);
await tester.tap(navigationMenuButton);
await tester.pump(); // Start opening drawer.
await tester.pump(const Duration(seconds: 1)); // Wait until it's really opened.
// switch theme
await tester.tap(find.text('Dark'));
await tester.pump();
await tester.pump(const Duration(seconds: 1)); // Wait until it's changed.
// switch theme
await tester.tap(find.text('Light'));
await tester.pump();
await tester.pump(const Duration(seconds: 1)); // Wait until it's changed.
RendererBinding.instance.setSemanticsEnabled(true);
await runSmokeTest(tester);
RendererBinding.instance.setSemanticsEnabled(false);
});
}
......@@ -138,7 +138,11 @@ abstract class RendererBinding extends BindingBase implements SchedulerBinding,
SemanticsHandle _semanticsHandle;
void _handleSemanticsEnabledChanged() {
if (ui.window.semanticsEnabled) {
setSemanticsEnabled(ui.window.semanticsEnabled);
}
void setSemanticsEnabled(bool enabled) {
if (enabled) {
_semanticsHandle ??= _pipelineOwner.ensureSemantics();
} else {
_semanticsHandle?.dispose();
......
......@@ -372,6 +372,7 @@ class SemanticsNode extends AbstractNode {
/// child list for the given frame at once instead of needing to process the
/// changes incrementally as new children are compiled.
void finalizeChildren() {
// The goal of this function is updating sawChange.
if (_children != null) {
for (SemanticsNode child in _children)
child._dead = true;
......@@ -473,8 +474,12 @@ class SemanticsNode extends AbstractNode {
owner._detachedNodes.add(this);
super.detach();
if (_children != null) {
for (SemanticsNode child in _children)
child.detach();
for (SemanticsNode child in _children) {
// The list of children may be stale and may contain nodes that have
// been assigned to a different parent.
if (child.parent == this)
child.detach();
}
}
}
......
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