From 839def55baacfde8aa0dc6a3e5fd1e63d446cedd Mon Sep 17 00:00:00 2001 From: Adam Barth Date: Fri, 1 Jul 2016 17:57:10 -0700 Subject: [PATCH] Teach the system to shutdown semantics (#4811) We now stop updating semantics when there are no remaining clients. --- .../flutter/lib/src/rendering/binding.dart | 65 ++++++++++----- .../flutter/lib/src/rendering/object.dart | 67 ++++++++++++--- .../flutter/lib/src/rendering/semantics.dart | 81 ++++++++++++------- .../lib/src/widgets/semantics_debugger.dart | 27 ++++--- .../flutter/test/material/tooltip_test.dart | 11 ++- .../flutter/test/rendering/reattach_test.dart | 24 ++---- .../test/rendering/test_semantics_client.dart | 25 ++++++ .../flutter/test/widget/buttons_test.dart | 8 +- .../flutter/test/widget/semantics_1_test.dart | 20 ++--- .../flutter/test/widget/semantics_2_test.dart | 14 ++-- .../flutter/test/widget/semantics_3_test.dart | 18 ++--- .../flutter/test/widget/semantics_4_test.dart | 15 ++-- .../flutter/test/widget/semantics_5_test.dart | 9 +-- .../flutter/test/widget/semantics_7_test.dart | 14 ++-- .../flutter/test/widget/semantics_8_test.dart | 12 ++- .../flutter/test/widget/semantics_test.dart | 53 ++++++++++++ .../flutter/test/widget/test_semantics.dart | 22 ----- 17 files changed, 304 insertions(+), 181 deletions(-) create mode 100644 packages/flutter/test/rendering/test_semantics_client.dart create mode 100644 packages/flutter/test/widget/semantics_test.dart delete mode 100644 packages/flutter/test/widget/test_semantics.dart diff --git a/packages/flutter/lib/src/rendering/binding.dart b/packages/flutter/lib/src/rendering/binding.dart index b3e00b9a87e..7b9838d5830 100644 --- a/packages/flutter/lib/src/rendering/binding.dart +++ b/packages/flutter/lib/src/rendering/binding.dart @@ -90,18 +90,12 @@ abstract class RendererBinding extends BindingBase implements SchedulerBinding, PipelineOwner _pipelineOwner; /// The render tree that's attached to the output surface. - RenderView get renderView => _renderView; - RenderView _renderView; + RenderView get renderView => _pipelineOwner.rootRenderObject; /// Sets the given [RenderView] object (which must not be null), and its tree, to /// be the new render tree to display. The previous tree, if any, is detached. set renderView(RenderView value) { assert(value != null); - if (_renderView == value) - return; - if (_renderView != null) - _renderView.detach(); - _renderView = value; - _renderView.attach(pipelineOwner); + _pipelineOwner.rootRenderObject = value; } /// Called when the system metrics change. @@ -134,18 +128,15 @@ abstract class RendererBinding extends BindingBase implements SchedulerBinding, /// Called automatically when the binding is created. void initSemantics() { shell.provideService(mojom.SemanticsServer.serviceName, (core.MojoMessagePipeEndpoint endpoint) { - ensureSemantics(); - mojom.SemanticsServerStub server = new mojom.SemanticsServerStub.fromEndpoint(endpoint); - server.impl = new SemanticsServer(semanticsOwner: pipelineOwner.semanticsOwner); + mojom.SemanticsServerStub stub = new mojom.SemanticsServerStub.fromEndpoint(endpoint); + SemanticsServer server = new SemanticsServer(pipelineOwner); + stub.impl = server; + stub.ctrl.onError = (_) { + server.dispose(); + }; }); } - void ensureSemantics() { - if (pipelineOwner.semanticsOwner == null) - renderView.scheduleInitialSemantics(); - assert(pipelineOwner.semanticsOwner != null); - } - void _handlePersistentFrameCallback(Duration timeStamp) { beginFrame(); } @@ -165,7 +156,7 @@ abstract class RendererBinding extends BindingBase implements SchedulerBinding, @override void reassembleApplication() { super.reassembleApplication(); - pipelineOwner.reassemble(renderView); + pipelineOwner.reassemble(); } @override @@ -202,6 +193,44 @@ void debugDumpSemanticsTree() { debugPrint(RendererBinding.instance?.renderView?.debugSemantics?.toStringDeep() ?? 'Semantics not collected.'); } +/// Exposes the [SemanticsNode] tree to the underlying platform. +class SemanticsServer extends mojom.SemanticsServer { + /// Creates a semantics server that listens to semantic informationa about the + /// given [PipelineOwner]. + /// + /// Call [dispose] to stop listening for semantic updates. + SemanticsServer(PipelineOwner pipelineOwner) { + _semanticsOwner = pipelineOwner.addSemanticsListener(_updateSemanticsTree); + } + + SemanticsOwner _semanticsOwner; + final List _listeners = []; + + /// Stops listening for semantic updates and closes all outstanding listeners. + void dispose() { + for (mojom.SemanticsListenerProxy listener in _listeners) + listener.close(); + _listeners.clear(); + _semanticsOwner.removeListener(_updateSemanticsTree); + _semanticsOwner = null; + } + + void _updateSemanticsTree(List nodes) { + for (mojom.SemanticsListenerProxy listener in _listeners) + listener.updateSemanticsTree(nodes); + } + + @override + void addSemanticsListener(mojom.SemanticsListenerProxy listener) { + _listeners.add(listener); + } + + @override + void performAction(int id, mojom.SemanticAction encodedAction) { + _semanticsOwner.performAction(id, SemanticAction.values[encodedAction.mojoEnumValue]); + } +} + /// A concrete binding for applications that use the Rendering framework /// directly. This is the glue that binds the framework to the Flutter engine. /// diff --git a/packages/flutter/lib/src/rendering/object.dart b/packages/flutter/lib/src/rendering/object.dart index 26c1298de23..089780a740c 100644 --- a/packages/flutter/lib/src/rendering/object.dart +++ b/packages/flutter/lib/src/rendering/object.dart @@ -793,6 +793,41 @@ class PipelineOwner { onNeedVisualUpdate(); } + /// The unique render object managed by this pipeline that has no parent. + RenderObject get rootRenderObject => _rootRenderObject; + RenderObject _rootRenderObject; + set rootRenderObject(RenderObject value) { + if (_rootRenderObject == value) + return; + _rootRenderObject?.detach(); + _rootRenderObject = value; + _rootRenderObject?.attach(this); + } + + /// Calls the given listener whenever the semantics of the render tree change. + /// + /// Creates [semanticsOwner] if necessary. + SemanticsOwner addSemanticsListener(SemanticsListener listener) { + if (_semanticsOwner == null) { + _semanticsOwner = new SemanticsOwner( + initialListener: listener, + onLastListenerRemoved: _handleLastSemanticsListenerRemoved + ); + _rootRenderObject.scheduleInitialSemantics(); + } else { + _semanticsOwner.addListener(listener); + } + assert(_semanticsOwner != null); + return _semanticsOwner; + } + + void _handleLastSemanticsListenerRemoved() { + assert(!_debugDoingSemantics); + rootRenderObject._clearSemantics(); + _semanticsOwner.dispose(); + _semanticsOwner = null; + } + List _nodesNeedingLayout = []; /// Whether this pipeline is currently in the layout phase. @@ -883,7 +918,7 @@ class PipelineOwner { SemanticsOwner get semanticsOwner => _semanticsOwner; SemanticsOwner _semanticsOwner; bool _debugDoingSemantics = false; - List _nodesNeedingSemantics = []; + final List _nodesNeedingSemantics = []; /// Update the semantics for all render objects. /// @@ -912,17 +947,14 @@ class PipelineOwner { _semanticsOwner.sendSemanticsTree(); } - /// Cause the entire subtree rooted at the given [RenderObject] to - /// be entirely reprocessed. This is used by development tools when - /// the application code has changed, to cause the rendering tree to - /// pick up any changed implementations. + /// Cause the entire render tree rooted at [rootRenderObject] to be entirely + /// reprocessed. This is used by development tools when the application code + /// has changed, to cause the rendering tree to pick up any changed + /// implementations. /// - /// This is expensive and should not be called except during - /// development. - void reassemble(RenderObject root) { - assert(root.parent is! RenderObject); - assert(root.owner == this); - root._reassemble(); + /// This is expensive and should not be called except during development. + void reassemble() { + _rootRenderObject?._reassemble(); } } @@ -1862,8 +1894,7 @@ abstract class RenderObject extends AbstractNode implements HitTestTarget { assert(!owner._debugDoingSemantics); assert(_semantics == null); assert(_needsSemanticsUpdate); - assert(owner._semanticsOwner == null); - owner._semanticsOwner = new SemanticsOwner(); + assert(owner._semanticsOwner != null); owner._nodesNeedingSemantics.add(this); owner.requestVisualUpdate(); } @@ -1896,6 +1927,16 @@ abstract class RenderObject extends AbstractNode implements HitTestTarget { return result; } + /// Removes all semantics from this render object and its descendants. + void _clearSemantics() { + _needsSemanticsUpdate = true; + _needsSemanticsGeometryUpdate = true; + _semantics = null; + visitChildren((RenderObject child) { + child._clearSemantics(); + }); + } + /// Mark this node as needing an update to its semantics /// description. /// diff --git a/packages/flutter/lib/src/rendering/semantics.dart b/packages/flutter/lib/src/rendering/semantics.dart index 75cfde68223..cf5e66f6290 100644 --- a/packages/flutter/lib/src/rendering/semantics.dart +++ b/packages/flutter/lib/src/rendering/semantics.dart @@ -455,32 +455,68 @@ class SemanticsNode extends AbstractNode { } } +/// Signature for functions that receive updates about render tree semantics. +typedef void SemanticsListener(List nodes); + +/// Owns [SemanticsNode] objects and notifies listeners of changes to the +/// render tree semantics. +/// +/// To listen for semantic updates, call [PipelineOwner.addSemanticsListener], +/// which will create a [SemanticsOwner] if necessary. class SemanticsOwner { + /// Creates a [SemanticsOwner]. + /// + /// The `onLastListenerRemoved` argument must not be null and will be called + /// when the last listener is removed from this object. + SemanticsOwner({ + @required SemanticsListener initialListener, + @required VoidCallback onLastListenerRemoved + }) : _onLastListenerRemoved = onLastListenerRemoved { + assert(_onLastListenerRemoved != null); + addListener(initialListener); + } + + final VoidCallback _onLastListenerRemoved; + final List _dirtyNodes = []; final Map _nodes = {}; final Set _detachedNodes = new Set(); - List _listeners; + final List _listeners = []; - /// Whether there are currently any consumers of semantic data. + /// Releases any resources retained by this object. /// - /// If there are no consumers of semantic data, there is no need to compile - /// semantic data into a [SemanticsNode] tree. - bool get hasListeners => _listeners != null && _listeners.length > 0; + /// Requires that there are no listeners registered with [addListener]. + void dispose() { + assert(_listeners.isEmpty); + _dirtyNodes.clear(); + _nodes.clear(); + _detachedNodes.clear(); + } /// Add a consumer of semantic data. /// /// After the [PipelineOwner] updates the semantic data for a given frame, it /// calls [sendSemanticsTree], which uploads the data to each listener /// registered with this function. - void addListener(mojom.SemanticsListener listener) { - _listeners ??= []; + /// + /// Listeners can be removed with [removeListener]. + void addListener(SemanticsListener listener) { _listeners.add(listener); } + /// Removes a consumer of semantic data. + /// + /// Listeners can be added with [addListener]. + void removeListener(SemanticsListener listener) { + _listeners.remove(listener); + if (_listeners.isEmpty) + _onLastListenerRemoved(); + } + /// Uploads the semantics tree to the listeners registered with [addListener]. void sendSemanticsTree() { - assert(hasListeners); + assert(_listeners.isNotEmpty); for (SemanticsNode oldNode in _detachedNodes) { // The other side will have forgotten this node if we even send // it again, so make sure to mark it dirty so that it'll get @@ -540,8 +576,8 @@ class SemanticsOwner { if (node._dirty && node.attached) updatedNodes.add(node._serialize()); } - for (mojom.SemanticsListener listener in _listeners) - listener.updateSemanticsTree(updatedNodes); + for (SemanticsListener listener in new List.from(_listeners)) + listener(updatedNodes); _dirtyNodes.clear(); } @@ -562,29 +598,12 @@ class SemanticsOwner { return result._actionHandler; } + /// Asks the [SemanticsNode] with the given id to perform the given action. + /// + /// If the [SemanticsNode] has not indicated that it can perform the action, + /// this function does nothing. void performAction(int id, SemanticAction action) { SemanticActionHandler handler = _getSemanticActionHandlerForId(id, action: action); handler?.performAction(action); } } - -/// Exposes the [SemanticsNode] tree to the underlying platform. -class SemanticsServer extends mojom.SemanticsServer { - SemanticsServer({ @required this.semanticsOwner }) { - assert(semanticsOwner != null); - } - - final SemanticsOwner semanticsOwner; - - @override - void addSemanticsListener(mojom.SemanticsListenerProxy listener) { - // TODO(abarth): We should remove the listener when this pipe closes. - // See . - semanticsOwner.addListener(listener); - } - - @override - void performAction(int id, mojom.SemanticAction encodedAction) { - semanticsOwner.performAction(id, SemanticAction.values[encodedAction.mojoEnumValue]); - } -} diff --git a/packages/flutter/lib/src/widgets/semantics_debugger.dart b/packages/flutter/lib/src/widgets/semantics_debugger.dart index 5a73b1b5813..c3bd2f5bf42 100644 --- a/packages/flutter/lib/src/widgets/semantics_debugger.dart +++ b/packages/flutter/lib/src/widgets/semantics_debugger.dart @@ -40,14 +40,15 @@ class _SemanticsDebuggerState extends State { // static here because we might not be in a tree that's attached to that // binding. Instead, we should find a way to get to the PipelineOwner from // the BuildContext. - WidgetsBinding.instance.ensureSemantics(); - _client = new _SemanticsClient(WidgetsBinding.instance.pipelineOwner.semanticsOwner) + _client = new _SemanticsClient(WidgetsBinding.instance.pipelineOwner) ..addListener(_update); } @override void dispose() { - _client.removeListener(_update); + _client + ..removeListener(_update) + ..dispose(); super.dispose(); } @@ -310,12 +311,19 @@ class _SemanticsDebuggerEntry { } } -class _SemanticsClient extends ChangeNotifier implements mojom.SemanticsListener { - _SemanticsClient(this.semanticsOwner) { - semanticsOwner.addListener(this); +class _SemanticsClient extends ChangeNotifier { + _SemanticsClient(PipelineOwner pipelineOwner) { + _semanticsOwner = pipelineOwner.addSemanticsListener(_updateSemanticsTree); } - final SemanticsOwner semanticsOwner; + SemanticsOwner _semanticsOwner; + + @override + void dispose() { + _semanticsOwner.removeListener(_updateSemanticsTree); + _semanticsOwner = null; + super.dispose(); + } _SemanticsDebuggerEntry get rootNode => _nodes[0]; final Map _nodes = {}; @@ -353,8 +361,7 @@ class _SemanticsClient extends ChangeNotifier implements mojom.SemanticsListener int generation = 0; - @override - void updateSemanticsTree(List nodes) { + void _updateSemanticsTree(List nodes) { generation += 1; for (mojom.SemanticsNode node in nodes) _updateNode(node); @@ -368,7 +375,7 @@ class _SemanticsClient extends ChangeNotifier implements mojom.SemanticsListener void _performAction(Point position, SemanticAction action) { _SemanticsDebuggerEntry entry = _hitTest(position, (_SemanticsDebuggerEntry entry) => entry.actions.contains(action)); - semanticsOwner.performAction(entry?.id ?? 0, action); + _semanticsOwner.performAction(entry?.id ?? 0, action); } void handlePanEnd(Point position, Velocity velocity) { diff --git a/packages/flutter/test/material/tooltip_test.dart b/packages/flutter/test/material/tooltip_test.dart index 8e231e88d14..0155f4de566 100644 --- a/packages/flutter/test/material/tooltip_test.dart +++ b/packages/flutter/test/material/tooltip_test.dart @@ -7,7 +7,7 @@ import 'package:flutter/rendering.dart'; import 'package:flutter/widgets.dart'; import 'package:flutter_test/flutter_test.dart'; -import '../widget/test_semantics.dart'; +import '../rendering/test_semantics_client.dart'; // This file uses "as dynamic" in a few places to defeat the static // analysis. In general you want to avoid using this style in your @@ -394,7 +394,7 @@ void main() { }); testWidgets('Does tooltip contribute semantics', (WidgetTester tester) async { - TestSemanticsListener client = new TestSemanticsListener(tester); + TestSemanticsClient client = new TestSemanticsClient(tester.binding.pipelineOwner); GlobalKey key = new GlobalKey(); await tester.pumpWidget( new Overlay( @@ -419,7 +419,7 @@ void main() { ] ) ); - expect(client.updates.length, equals(2)); + expect(client.updates.length, equals(1)); expect(client.updates[0].id, equals(0)); expect(client.updates[0].actions, isEmpty); expect(client.updates[0].flags.hasCheckedState, isFalse); @@ -431,14 +431,13 @@ void main() { 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(); // before using "as dynamic" in your code, see note top of file (key.currentState as dynamic).ensureTooltipVisible(); // this triggers a rebuild of the semantics because the tree changes await tester.pump(const Duration(seconds: 2)); // faded in, show timer started (and at 0.0) - expect(client.updates.length, equals(2)); + expect(client.updates.length, equals(1)); expect(client.updates[0].id, equals(0)); expect(client.updates[0].actions, isEmpty); expect(client.updates[0].flags.hasCheckedState, isFalse); @@ -450,7 +449,7 @@ void main() { 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(); + client.dispose(); }); } diff --git a/packages/flutter/test/rendering/reattach_test.dart b/packages/flutter/test/rendering/reattach_test.dart index c264bf6433b..216d91e6380 100644 --- a/packages/flutter/test/rendering/reattach_test.dart +++ b/packages/flutter/test/rendering/reattach_test.dart @@ -4,10 +4,10 @@ import 'package:flutter/material.dart'; import 'package:flutter/rendering.dart'; -import 'package:sky_services/semantics/semantics.mojom.dart' as mojom; import 'package:test/test.dart'; import 'rendering_tester.dart'; +import 'test_semantics_client.dart'; class TestTree { TestTree() { @@ -77,15 +77,6 @@ class TestCompositingBitsTree { bool painted = false; } -class TestSemanticsListener implements mojom.SemanticsListener { - final List updates = []; - - @override - void updateSemanticsTree(List nodes) { - updates.addAll(nodes); - } -} - void main() { test('objects can be detached and re-attached: layout', () { TestTree testTree = new TestTree(); @@ -135,21 +126,20 @@ void main() { }); test('objects can be detached and re-attached: semantics', () { TestTree testTree = new TestTree(); - TestSemanticsListener listener = new TestSemanticsListener(); - renderer.ensureSemantics(); - renderer.pipelineOwner.semanticsOwner.addListener(listener); + TestSemanticsClient client = new TestSemanticsClient(renderer.pipelineOwner); // Lay out, composite, paint, and update semantics layout(testTree.root, phase: EnginePhase.flushSemantics); - expect(listener.updates.length, equals(1)); + expect(client.updates.length, equals(1)); // Remove testTree from the custom render view renderer.renderView.child = null; expect(testTree.child.owner, isNull); // Dirty one of the elements - listener.updates.clear(); + client.updates.clear(); testTree.child.markNeedsSemanticsUpdate(); - expect(listener.updates.length, equals(0)); + expect(client.updates.length, equals(0)); // Lay out, composite, paint, and update semantics again layout(testTree.root, phase: EnginePhase.flushSemantics); - expect(listener.updates.length, equals(1)); + expect(client.updates.length, equals(1)); + client.dispose(); }); } diff --git a/packages/flutter/test/rendering/test_semantics_client.dart b/packages/flutter/test/rendering/test_semantics_client.dart new file mode 100644 index 00000000000..19599dd402f --- /dev/null +++ b/packages/flutter/test/rendering/test_semantics_client.dart @@ -0,0 +1,25 @@ +// 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/rendering.dart'; +import 'package:sky_services/semantics/semantics.mojom.dart' as mojom; + +class TestSemanticsClient { + TestSemanticsClient(PipelineOwner pipelineOwner) { + _semanticsOwner = pipelineOwner.addSemanticsListener(_updateSemanticsTree); + } + + SemanticsOwner _semanticsOwner; + + void dispose() { + _semanticsOwner.removeListener(_updateSemanticsTree); + _semanticsOwner = null; + } + + final List updates = []; + + void _updateSemanticsTree(List nodes) { + updates.addAll(nodes); + } +} diff --git a/packages/flutter/test/widget/buttons_test.dart b/packages/flutter/test/widget/buttons_test.dart index 863aa14e8d5..a8460f7315e 100644 --- a/packages/flutter/test/widget/buttons_test.dart +++ b/packages/flutter/test/widget/buttons_test.dart @@ -7,11 +7,11 @@ import 'package:flutter/widgets.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:sky_services/semantics/semantics.mojom.dart' as mojom; -import 'test_semantics.dart'; +import '../rendering/test_semantics_client.dart'; void main() { testWidgets('Does FlatButton contribute semantics', (WidgetTester tester) async { - TestSemanticsListener client = new TestSemanticsListener(tester); + TestSemanticsClient client = new TestSemanticsClient(tester.binding.pipelineOwner); await tester.pumpWidget( new Material( child: new Center( @@ -22,7 +22,7 @@ void main() { ) ) ); - expect(client.updates.length, equals(2)); + expect(client.updates.length, equals(1)); expect(client.updates[0].id, equals(0)); expect(client.updates[0].actions, isEmpty); expect(client.updates[0].flags.hasCheckedState, isFalse); @@ -40,7 +40,7 @@ void main() { expect(client.updates[0].children[0].flags.isChecked, isFalse); expect(client.updates[0].children[0].strings.label, equals('Hello')); expect(client.updates[0].children[0].children.length, equals(0)); - expect(client.updates[1], isNull); client.updates.clear(); + client.dispose(); }); } diff --git a/packages/flutter/test/widget/semantics_1_test.dart b/packages/flutter/test/widget/semantics_1_test.dart index d58f71c9a47..96ba3a720af 100644 --- a/packages/flutter/test/widget/semantics_1_test.dart +++ b/packages/flutter/test/widget/semantics_1_test.dart @@ -7,11 +7,11 @@ import 'package:flutter/rendering.dart'; import 'package:flutter/widgets.dart'; import 'package:flutter_test/flutter_test.dart'; -import 'test_semantics.dart'; +import '../rendering/test_semantics_client.dart'; void main() { testWidgets('Semantics 1', (WidgetTester tester) async { - TestSemanticsListener client = new TestSemanticsListener(tester); + TestSemanticsClient client = new TestSemanticsClient(tester.binding.pipelineOwner); // smoketest await tester.pumpWidget( @@ -22,7 +22,7 @@ void main() { ) ) ); - expect(client.updates.length, equals(2)); + expect(client.updates.length, equals(1)); expect(client.updates[0].id, equals(0)); expect(client.updates[0].actions, isEmpty); expect(client.updates[0].flags.hasCheckedState, isFalse); @@ -34,7 +34,6 @@ void main() { 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(); // control for forking @@ -56,7 +55,7 @@ void main() { crossAxisAlignment: CrossAxisAlignment.stretch ) ); - expect(client.updates.length, equals(2)); + expect(client.updates.length, equals(1)); expect(client.updates[0].id, equals(0)); expect(client.updates[0].actions, isEmpty); expect(client.updates[0].flags.hasCheckedState, isFalse); @@ -68,7 +67,6 @@ void main() { 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(); // forking semantics @@ -90,7 +88,7 @@ void main() { crossAxisAlignment: CrossAxisAlignment.stretch ) ); - expect(client.updates.length, equals(2)); + expect(client.updates.length, equals(1)); expect(client.updates[0].id, equals(0)); expect(client.updates[0].actions, isEmpty); expect(client.updates[0].flags.hasCheckedState, isFalse); @@ -124,7 +122,6 @@ void main() { expect(client.updates[0].children[1].geometry.width, equals(800.0)); expect(client.updates[0].children[1].geometry.height, equals(10.0)); expect(client.updates[0].children[1].children.length, equals(0)); - expect(client.updates[1], isNull); client.updates.clear(); // toggle a branch off @@ -146,7 +143,7 @@ void main() { crossAxisAlignment: CrossAxisAlignment.stretch ) ); - expect(client.updates.length, equals(2)); + expect(client.updates.length, equals(1)); expect(client.updates[0].id, equals(0)); expect(client.updates[0].actions, isEmpty); expect(client.updates[0].flags.hasCheckedState, isFalse); @@ -158,7 +155,6 @@ void main() { 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(); // toggle a branch back on @@ -180,7 +176,7 @@ void main() { crossAxisAlignment: CrossAxisAlignment.stretch ) ); - expect(client.updates.length, equals(2)); + expect(client.updates.length, equals(1)); expect(client.updates[0].id, equals(0)); expect(client.updates[0].actions, isEmpty); expect(client.updates[0].flags.hasCheckedState, isFalse); @@ -214,7 +210,7 @@ void main() { expect(client.updates[0].children[1].geometry.width, equals(800.0)); expect(client.updates[0].children[1].geometry.height, equals(10.0)); expect(client.updates[0].children[1].children.length, equals(0)); - expect(client.updates[1], isNull); client.updates.clear(); + client.dispose(); }); } diff --git a/packages/flutter/test/widget/semantics_2_test.dart b/packages/flutter/test/widget/semantics_2_test.dart index 954c8657179..9e666852e78 100644 --- a/packages/flutter/test/widget/semantics_2_test.dart +++ b/packages/flutter/test/widget/semantics_2_test.dart @@ -7,11 +7,11 @@ import 'package:flutter/rendering.dart'; import 'package:flutter/widgets.dart'; import 'package:flutter_test/flutter_test.dart'; -import 'test_semantics.dart'; +import '../rendering/test_semantics_client.dart'; void main() { testWidgets('Semantics 2', (WidgetTester tester) async { - TestSemanticsListener client = new TestSemanticsListener(tester); + TestSemanticsClient client = new TestSemanticsClient(tester.binding.pipelineOwner); // this test is the same as the test in Semantics 1, but // starting with the second branch being ignored and then @@ -36,7 +36,7 @@ void main() { crossAxisAlignment: CrossAxisAlignment.stretch ) ); - expect(client.updates.length, equals(2)); + expect(client.updates.length, equals(1)); expect(client.updates[0].id, equals(0)); expect(client.updates[0].actions, isEmpty); expect(client.updates[0].flags.hasCheckedState, isFalse); @@ -70,7 +70,6 @@ void main() { expect(client.updates[0].children[1].geometry.width, equals(800.0)); expect(client.updates[0].children[1].geometry.height, equals(10.0)); expect(client.updates[0].children[1].children.length, equals(0)); - expect(client.updates[1], isNull); client.updates.clear(); // toggle a branch off @@ -92,7 +91,7 @@ void main() { crossAxisAlignment: CrossAxisAlignment.stretch ) ); - expect(client.updates.length, equals(2)); + expect(client.updates.length, equals(1)); expect(client.updates[0].id, equals(0)); expect(client.updates[0].actions, isEmpty); expect(client.updates[0].flags.hasCheckedState, isFalse); @@ -104,7 +103,6 @@ void main() { 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(); // toggle a branch back on @@ -126,7 +124,7 @@ void main() { crossAxisAlignment: CrossAxisAlignment.stretch ) ); - expect(client.updates.length, equals(2)); + expect(client.updates.length, equals(1)); expect(client.updates[0].id, equals(0)); expect(client.updates[0].actions, isEmpty); expect(client.updates[0].flags.hasCheckedState, isFalse); @@ -160,7 +158,7 @@ void main() { expect(client.updates[0].children[1].geometry.width, equals(800.0)); expect(client.updates[0].children[1].geometry.height, equals(10.0)); expect(client.updates[0].children[1].children.length, equals(0)); - expect(client.updates[1], isNull); client.updates.clear(); + client.dispose(); }); } diff --git a/packages/flutter/test/widget/semantics_3_test.dart b/packages/flutter/test/widget/semantics_3_test.dart index 92ca2fc6a65..3302d0d6a0d 100644 --- a/packages/flutter/test/widget/semantics_3_test.dart +++ b/packages/flutter/test/widget/semantics_3_test.dart @@ -6,11 +6,11 @@ import 'package:flutter/material.dart'; import 'package:flutter/widgets.dart'; import 'package:flutter_test/flutter_test.dart'; -import 'test_semantics.dart'; +import '../rendering/test_semantics_client.dart'; void main() { testWidgets('Semantics 3', (WidgetTester tester) async { - TestSemanticsListener client = new TestSemanticsListener(tester); + TestSemanticsClient client = new TestSemanticsClient(tester.binding.pipelineOwner); // implicit annotators await tester.pumpWidget( @@ -25,7 +25,7 @@ void main() { ) ) ); - expect(client.updates.length, equals(2)); + expect(client.updates.length, equals(1)); expect(client.updates[0].id, equals(0)); expect(client.updates[0].actions, isEmpty); expect(client.updates[0].flags.hasCheckedState, isTrue); @@ -37,7 +37,6 @@ void main() { 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(); // remove one @@ -50,7 +49,7 @@ void main() { ) ) ); - expect(client.updates.length, equals(2)); + expect(client.updates.length, equals(1)); expect(client.updates[0].id, equals(0)); expect(client.updates[0].actions, isEmpty); expect(client.updates[0].flags.hasCheckedState, isTrue); @@ -62,7 +61,6 @@ void main() { 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(); // change what it says @@ -75,7 +73,7 @@ void main() { ) ) ); - expect(client.updates.length, equals(2)); + expect(client.updates.length, equals(1)); expect(client.updates[0].id, equals(0)); expect(client.updates[0].actions, isEmpty); expect(client.updates[0].flags.hasCheckedState, isFalse); @@ -87,7 +85,6 @@ void main() { 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(); // add a node @@ -103,7 +100,7 @@ void main() { ) ) ); - expect(client.updates.length, equals(2)); + expect(client.updates.length, equals(1)); expect(client.updates[0].id, equals(0)); expect(client.updates[0].actions, isEmpty); expect(client.updates[0].flags.hasCheckedState, isTrue); @@ -115,7 +112,6 @@ void main() { 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(); // make no changes @@ -132,6 +128,6 @@ void main() { ) ); expect(client.updates.length, equals(0)); - + client.dispose(); }); } diff --git a/packages/flutter/test/widget/semantics_4_test.dart b/packages/flutter/test/widget/semantics_4_test.dart index f677312c773..b99d7284872 100644 --- a/packages/flutter/test/widget/semantics_4_test.dart +++ b/packages/flutter/test/widget/semantics_4_test.dart @@ -6,11 +6,11 @@ import 'package:flutter/material.dart'; import 'package:flutter/widgets.dart'; import 'package:flutter_test/flutter_test.dart'; -import 'test_semantics.dart'; +import '../rendering/test_semantics_client.dart'; void main() { testWidgets('Semantics 4', (WidgetTester tester) async { - TestSemanticsListener client = new TestSemanticsListener(tester); + TestSemanticsClient client = new TestSemanticsClient(tester.binding.pipelineOwner); // O // / \ O=root @@ -40,7 +40,7 @@ void main() { ] ) ); - expect(client.updates.length, equals(2)); + expect(client.updates.length, equals(1)); expect(client.updates[0].id, equals(0)); expect(client.updates[0].children.length, equals(2)); expect(client.updates[0].children[0].id, equals(1)); @@ -51,7 +51,6 @@ void main() { expect(client.updates[0].children[1].children[0].children.length, equals(0)); expect(client.updates[0].children[1].children[1].id, equals(4)); expect(client.updates[0].children[1].children[1].children.length, equals(0)); - expect(client.updates[1], isNull); client.updates.clear(); // O O=root @@ -79,10 +78,9 @@ void main() { ] ) ); - expect(client.updates.length, equals(2)); + expect(client.updates.length, equals(1)); expect(client.updates[0].id, equals(2)); expect(client.updates[0].children.length, equals(0)); - expect(client.updates[1], isNull); client.updates.clear(); // O=root @@ -107,11 +105,10 @@ void main() { ] ) ); - expect(client.updates.length, equals(2)); + expect(client.updates.length, equals(1)); expect(client.updates[0].id, equals(0)); expect(client.updates[0].children.length, equals(0)); - expect(client.updates[1], isNull); client.updates.clear(); - + client.dispose(); }); } diff --git a/packages/flutter/test/widget/semantics_5_test.dart b/packages/flutter/test/widget/semantics_5_test.dart index 7399040651c..b0b2f8be8fa 100644 --- a/packages/flutter/test/widget/semantics_5_test.dart +++ b/packages/flutter/test/widget/semantics_5_test.dart @@ -6,11 +6,11 @@ import 'package:flutter/material.dart'; import 'package:flutter/widgets.dart'; import 'package:flutter_test/flutter_test.dart'; -import 'test_semantics.dart'; +import '../rendering/test_semantics_client.dart'; void main() { testWidgets('Semantics 5', (WidgetTester tester) async { - TestSemanticsListener client = new TestSemanticsListener(tester); + TestSemanticsClient client = new TestSemanticsClient(tester.binding.pipelineOwner); await tester.pumpWidget( new Stack( @@ -28,7 +28,7 @@ void main() { ] ) ); - expect(client.updates.length, equals(2)); + expect(client.updates.length, equals(1)); expect(client.updates[0].id, equals(0)); expect(client.updates[0].flags.hasCheckedState, isFalse); expect(client.updates[0].strings.label, equals('')); @@ -39,8 +39,7 @@ void main() { expect(client.updates[0].children[1].id, equals(2)); expect(client.updates[0].children[1].flags.hasCheckedState, isFalse); expect(client.updates[0].children[1].strings.label, equals('label')); - expect(client.updates[1], isNull); client.updates.clear(); - + client.dispose(); }); } diff --git a/packages/flutter/test/widget/semantics_7_test.dart b/packages/flutter/test/widget/semantics_7_test.dart index 2e38118efd6..d0accda6204 100644 --- a/packages/flutter/test/widget/semantics_7_test.dart +++ b/packages/flutter/test/widget/semantics_7_test.dart @@ -5,13 +5,13 @@ import 'package:flutter/material.dart'; import 'package:flutter/widgets.dart'; import 'package:flutter_test/flutter_test.dart'; - -import 'test_semantics.dart'; import 'package:sky_services/semantics/semantics.mojom.dart' as mojom; +import '../rendering/test_semantics_client.dart'; + void main() { testWidgets('Semantics 7 - Merging', (WidgetTester tester) async { - TestSemanticsListener client = new TestSemanticsListener(tester); + TestSemanticsClient client = new TestSemanticsClient(tester.binding.pipelineOwner); String label; @@ -44,7 +44,7 @@ void main() { ] ) ); - expect(client.updates.length, equals(2)); + expect(client.updates.length, equals(1)); expect(client.updates[0].id, equals(0)); expect(client.updates[0].actions, isEmpty); expect(client.updates[0].flags.hasCheckedState, isFalse); @@ -80,7 +80,6 @@ void main() { 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'; @@ -112,8 +111,7 @@ void main() { ] ) ); - expect(client.updates.length, equals(3)); - expect(client.updates[2], isNull); + expect(client.updates.length, equals(2)); // The order of the nodes is undefined, so allow both orders. mojom.SemanticsNode a, b; @@ -150,6 +148,6 @@ void main() { expect(b.children.length, equals(0)); client.updates.clear(); - + client.dispose(); }); } diff --git a/packages/flutter/test/widget/semantics_8_test.dart b/packages/flutter/test/widget/semantics_8_test.dart index d9244046c7b..8cc882d02bb 100644 --- a/packages/flutter/test/widget/semantics_8_test.dart +++ b/packages/flutter/test/widget/semantics_8_test.dart @@ -6,11 +6,11 @@ import 'package:flutter/material.dart'; import 'package:flutter/widgets.dart'; import 'package:flutter_test/flutter_test.dart'; -import 'test_semantics.dart'; +import '../rendering/test_semantics_client.dart'; void main() { testWidgets('Semantics 8 - Merging with reset', (WidgetTester tester) async { - TestSemanticsListener client = new TestSemanticsListener(tester); + TestSemanticsClient client = new TestSemanticsClient(tester.binding.pipelineOwner); await tester.pumpWidget( new MergeSemantics( @@ -32,7 +32,7 @@ void main() { ) ) ); - expect(client.updates.length, equals(2)); + expect(client.updates.length, equals(1)); expect(client.updates[0].id, equals(0)); expect(client.updates[0].actions, isEmpty); expect(client.updates[0].flags.hasCheckedState, isTrue); @@ -44,7 +44,6 @@ void main() { 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 @@ -68,7 +67,7 @@ void main() { ) ) ); - expect(client.updates.length, equals(2)); + expect(client.updates.length, equals(1)); expect(client.updates[0].id, equals(0)); expect(client.updates[0].actions, isEmpty); expect(client.updates[0].flags.hasCheckedState, isTrue); @@ -80,8 +79,7 @@ void main() { 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(); - + client.dispose(); }); } diff --git a/packages/flutter/test/widget/semantics_test.dart b/packages/flutter/test/widget/semantics_test.dart new file mode 100644 index 00000000000..c0165da9d8b --- /dev/null +++ b/packages/flutter/test/widget/semantics_test.dart @@ -0,0 +1,53 @@ +// 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:sky_services/semantics/semantics.mojom.dart' as mojom; + +import '../rendering/test_semantics_client.dart'; + +void main() { + testWidgets('Semantics shutdown and restart', (WidgetTester tester) async { + TestSemanticsClient client = new TestSemanticsClient(tester.binding.pipelineOwner); + + await tester.pumpWidget( + new Container( + child: new Semantics( + label: 'test1', + child: new Container() + ) + ) + ); + + void checkUpdates(List updates) { + expect(updates.length, equals(1)); + expect(updates[0].id, equals(0)); + expect(updates[0].actions, isEmpty); + expect(updates[0].flags.hasCheckedState, isFalse); + expect(updates[0].flags.isChecked, isFalse); + expect(updates[0].strings.label, equals('test1')); + expect(updates[0].geometry.transform, isNull); + expect(updates[0].geometry.left, equals(0.0)); + expect(updates[0].geometry.top, equals(0.0)); + expect(updates[0].geometry.width, equals(800.0)); + expect(updates[0].geometry.height, equals(600.0)); + expect(updates[0].children.length, equals(0)); + } + + checkUpdates(client.updates); + client.updates.clear(); + client.dispose(); + + expect(tester.binding.hasScheduledFrame, isFalse); + client = new TestSemanticsClient(tester.binding.pipelineOwner); + expect(tester.binding.hasScheduledFrame, isTrue); + await tester.pump(); + + checkUpdates(client.updates); + client.updates.clear(); + client.dispose(); + }); +} diff --git a/packages/flutter/test/widget/test_semantics.dart b/packages/flutter/test/widget/test_semantics.dart deleted file mode 100644 index 294d1d3e444..00000000000 --- a/packages/flutter/test/widget/test_semantics.dart +++ /dev/null @@ -1,22 +0,0 @@ -// 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_test/flutter_test.dart'; -import 'package:sky_services/semantics/semantics.mojom.dart' as mojom; - -class TestSemanticsListener implements mojom.SemanticsListener { - TestSemanticsListener(WidgetTester tester) { - tester.binding.ensureSemantics(); - tester.binding.pipelineOwner.semanticsOwner.addListener(this); - } - - final List updates = []; - - @override - void updateSemanticsTree(List nodes) { - assert(!nodes.any((mojom.SemanticsNode node) => node == null)); - updates.addAll(nodes); - updates.add(null); - } -}