From ca5e1f3f23f68aa3d4e2f3e936ccf8df2ced8cdc Mon Sep 17 00:00:00 2001 From: Jason Simmons Date: Thu, 10 Nov 2016 12:55:56 -0800 Subject: [PATCH] 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 --- examples/flutter_gallery/test/smoke_test.dart | 92 ++++++++++--------- .../flutter/lib/src/rendering/binding.dart | 6 +- .../flutter/lib/src/rendering/semantics.dart | 9 +- 3 files changed, 62 insertions(+), 45 deletions(-) diff --git a/examples/flutter_gallery/test/smoke_test.dart b/examples/flutter_gallery/test/smoke_test.dart index 64f1e86c677..8b544e8f84b 100644 --- a/examples/flutter_gallery/test/smoke_test.dart +++ b/examples/flutter_gallery/test/smoke_test.dart @@ -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 smokeDemo(WidgetTester tester, String routeName) async { return null; } +Future 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 scrollDeltas = new List(); + 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 scrollDeltas = new List(); - 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); }); } diff --git a/packages/flutter/lib/src/rendering/binding.dart b/packages/flutter/lib/src/rendering/binding.dart index 2ebb7c68866..456aed5c94b 100644 --- a/packages/flutter/lib/src/rendering/binding.dart +++ b/packages/flutter/lib/src/rendering/binding.dart @@ -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(); diff --git a/packages/flutter/lib/src/rendering/semantics.dart b/packages/flutter/lib/src/rendering/semantics.dart index 5ec395d9309..daa0077e91d 100644 --- a/packages/flutter/lib/src/rendering/semantics.dart +++ b/packages/flutter/lib/src/rendering/semantics.dart @@ -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(); + } } }