diff --git a/packages/flutter/lib/widgets/focus.dart b/packages/flutter/lib/widgets/focus.dart index f75fb921a2..44f8aaacfa 100644 --- a/packages/flutter/lib/widgets/focus.dart +++ b/packages/flutter/lib/widgets/focus.dart @@ -104,9 +104,9 @@ class Focus extends StatefulComponent { } } - void _widgetRemoved(GlobalKey key) { + void _handleWidgetRemoved(GlobalKey key) { assert(_focusedWidget == key); - _currentlyRegisteredWidgetRemovalListenerKey = null; + _updateWidgetRemovalListener(null); setState(() { _focusedWidget = null; }); @@ -115,9 +115,9 @@ class Focus extends StatefulComponent { void _updateWidgetRemovalListener(GlobalKey key) { if (_currentlyRegisteredWidgetRemovalListenerKey != key) { if (_currentlyRegisteredWidgetRemovalListenerKey != null) - GlobalKey.unregisterRemovalListener(_currentlyRegisteredWidgetRemovalListenerKey, _widgetRemoved); + GlobalKey.unregisterRemoveListener(_currentlyRegisteredWidgetRemovalListenerKey, _handleWidgetRemoved); if (key != null) - GlobalKey.registerRemovalListener(key, _widgetRemoved); + GlobalKey.registerRemoveListener(key, _handleWidgetRemoved); _currentlyRegisteredWidgetRemovalListenerKey = key; } } @@ -151,9 +151,9 @@ class Focus extends StatefulComponent { void _updateScopeRemovalListener(GlobalKey key) { if (_currentlyRegisteredScopeRemovalListenerKey != key) { if (_currentlyRegisteredScopeRemovalListenerKey != null) - GlobalKey.unregisterRemovalListener(_currentlyRegisteredScopeRemovalListenerKey, _scopeRemoved); + GlobalKey.unregisterRemoveListener(_currentlyRegisteredScopeRemovalListenerKey, _scopeRemoved); if (key != null) - GlobalKey.registerRemovalListener(key, _scopeRemoved); + GlobalKey.registerRemoveListener(key, _scopeRemoved); _currentlyRegisteredScopeRemovalListenerKey = key; } } diff --git a/packages/flutter/lib/widgets/framework.dart b/packages/flutter/lib/widgets/framework.dart index a82b3104fe..ec93c0c9b6 100644 --- a/packages/flutter/lib/widgets/framework.dart +++ b/packages/flutter/lib/widgets/framework.dart @@ -47,7 +47,8 @@ class ObjectKey extends Key { int get hashCode => identityHashCode(value); } -typedef void GlobalKeyRemovalListener(GlobalKey key); +typedef void GlobalKeySyncListener(GlobalKey key, Widget widget); +typedef void GlobalKeyRemoveListener(GlobalKey key); abstract class GlobalKey extends Key { const GlobalKey.constructor() : super.constructor(); // so that subclasses can call us, since the Key() factory constructor shadows the implicit constructor @@ -56,7 +57,9 @@ abstract class GlobalKey extends Key { static final Map _registry = new Map(); static final Map _debugDuplicates = new Map(); - static final Map> _removalListeners = new Map>(); + static final Map> _syncListeners = new Map>(); + static final Map> _removeListeners = new Map>(); + static final Set _syncedKeys = new Set(); static final Set _removedKeys = new Set(); void _register(Widget widget) { @@ -90,24 +93,47 @@ abstract class GlobalKey extends Key { } } + void _didSync() { + _syncedKeys.add(this); + } + static bool _notifyingListeners = false; - static void registerRemovalListener(GlobalKey key, GlobalKeyRemovalListener listener) { + static void registerSyncListener(GlobalKey key, GlobalKeySyncListener listener) { assert(!_notifyingListeners); assert(key != null); - if (!_removalListeners.containsKey(key)) - _removalListeners[key] = new Set(); - bool added = _removalListeners[key].add(listener); + Set listeners = + _syncListeners.putIfAbsent(key, () => new Set()); + bool added = listeners.add(listener); assert(added); } - static void unregisterRemovalListener(GlobalKey key, GlobalKeyRemovalListener listener) { + static void unregisterSyncListener(GlobalKey key, GlobalKeySyncListener listener) { assert(!_notifyingListeners); assert(key != null); - assert(_removalListeners.containsKey(key)); - bool removed = _removalListeners[key].remove(listener); - if (_removalListeners[key].isEmpty) - _removalListeners.remove(key); + assert(_syncListeners.containsKey(key)); + bool removed = _syncListeners[key].remove(listener); + if (_syncListeners[key].isEmpty) + _syncListeners.remove(key); + assert(removed); + } + + static void registerRemoveListener(GlobalKey key, GlobalKeyRemoveListener listener) { + assert(!_notifyingListeners); + assert(key != null); + Set listeners = + _removeListeners.putIfAbsent(key, () => new Set()); + bool added = listeners.add(listener); + assert(added); + } + + static void unregisterRemoveListener(GlobalKey key, GlobalKeyRemoveListener listener) { + assert(!_notifyingListeners); + assert(key != null); + assert(_removeListeners.containsKey(key)); + bool removed = _removeListeners[key].remove(listener); + if (_removeListeners[key].isEmpty) + _removeListeners.remove(key); assert(removed); } @@ -120,16 +146,34 @@ abstract class GlobalKey extends Key { assert(!_inBuildDirtyComponents); assert(!Widget._notifyingMountStatus); assert(_debugDuplicates.isEmpty); + if (_syncedKeys.isEmpty && _removedKeys.isEmpty) + return; _notifyingListeners = true; - for (GlobalKey key in _removedKeys) { - if (!_registry.containsKey(key) && _removalListeners.containsKey(key)) { - for (GlobalKeyRemovalListener listener in _removalListeners[key]) - listener(key); - _removalListeners.remove(key); + try { + Map> localRemoveListeners = + new Map>.from(_removeListeners); + Map> localSyncListeners = + new Map>.from(_syncListeners); + + for (GlobalKey key in _syncedKeys) { + Widget widget = _registry[key]; + if (widget != null && localSyncListeners.containsKey(key)) { + for (GlobalKeySyncListener listener in localSyncListeners[key]) + listener(key, widget); + } } + + for (GlobalKey key in _removedKeys) { + if (!_registry.containsKey(key) && localRemoveListeners.containsKey(key)) { + for (GlobalKeyRemoveListener listener in localRemoveListeners[key]) + listener(key); + } + } + } finally { + _removedKeys.clear(); + _syncedKeys.clear(); + _notifyingListeners = false; } - _removedKeys.clear(); - _notifyingListeners = false; } } @@ -260,7 +304,10 @@ abstract class Widget { // Component.retainStatefulNodeIfPossible() calls syncConstructorArguments(). bool retainStatefulNodeIfPossible(Widget newNode) => false; - void _sync(Widget old, dynamic slot); + void _sync(Widget old, dynamic slot) { + if (key is GlobalKey) + (key as GlobalKey)._didSync(); // TODO(ianh): Remove the cast once the analyzer is cleverer. + } void updateSlot(dynamic newSlot); // 'slot' is the identifier that the ancestor RenderObjectWrapper uses to know // where to put this descendant. If you just defer to a child, then make sure @@ -433,6 +480,7 @@ abstract class TagNode extends Widget { } else { _renderObject = null; } + super._sync(old, slot); } void updateSlot(dynamic newSlot) { @@ -669,6 +717,7 @@ abstract class Component extends Widget { _renderObject = _child.renderObject; assert(_renderObject == renderObject); // in case a subclass reintroduces it assert(renderObject != null); + super._sync(old, slot); } void _buildIfDirty() { @@ -911,6 +960,7 @@ abstract class RenderObjectWrapper extends Widget { assert(mounted); _nodeMap[renderObject] = this; syncRenderObject(old); + super._sync(old, slot); } void updateSlot(dynamic newSlot) { diff --git a/packages/unit/test/widget/build_utils.dart b/packages/unit/test/widget/build_utils.dart new file mode 100644 index 0000000000..3ef1bdb532 --- /dev/null +++ b/packages/unit/test/widget/build_utils.dart @@ -0,0 +1,48 @@ +import 'package:sky/rendering.dart'; +import 'package:sky/widgets.dart'; + +const Size _kTestViewSize = const Size(800.0, 600.0); + +class TestRenderView extends RenderView { + TestRenderView({ RenderBox child }) : super(child: child) { + attach(); + rootConstraints = new ViewConstraints(size: _kTestViewSize); + scheduleInitialLayout(); + } +} + +typedef Widget WidgetBuilder(); + +class TestApp extends App { + TestApp(); + + WidgetBuilder _builder; + void set builder (WidgetBuilder value) { + setState(() { + _builder = value; + }); + } + + Widget build() { + if (_builder != null) + return _builder(); + return new Container(); + } +} + +class WidgetTester { + WidgetTester() { + _app = new TestApp(); + _renderView = new TestRenderView(); + runApp(_app, renderViewOverride: _renderView); + } + + TestApp _app; + RenderView _renderView; + + void pumpFrame(WidgetBuilder builder) { + _app.builder = builder; + Component.flushBuild(); + RenderObject.flushLayout(); + } +} diff --git a/packages/unit/test/widget/global_key_test.dart b/packages/unit/test/widget/global_key_test.dart new file mode 100644 index 0000000000..18e1406aff --- /dev/null +++ b/packages/unit/test/widget/global_key_test.dart @@ -0,0 +1,109 @@ +import 'package:sky/widgets.dart'; +import 'package:test/test.dart'; + +import 'build_utils.dart'; + +void main() { + test('Global keys notify add and remove', () { + GlobalKey globalKey = new GlobalKey(); + Container container; + + bool syncListenerCalled = false; + bool removeListenerCalled = false; + + void syncListener(GlobalKey key, Widget widget) { + syncListenerCalled = true; + expect(key, equals(globalKey)); + expect(container, isNotNull); + expect(widget, equals(container)); + } + + void removeListener(GlobalKey key) { + removeListenerCalled = true; + expect(key, equals(globalKey)); + } + + WidgetTester tester = new WidgetTester(); + + GlobalKey.registerSyncListener(globalKey, syncListener); + GlobalKey.registerRemoveListener(globalKey, removeListener); + tester.pumpFrame(() { + container = new Container(key: globalKey); + return container; + }); + expect(syncListenerCalled, isTrue); + expect(removeListenerCalled, isFalse); + + syncListenerCalled = false; + removeListenerCalled = false; + tester.pumpFrame(() => new Container()); + expect(syncListenerCalled, isFalse); + expect(removeListenerCalled, isTrue); + + syncListenerCalled = false; + removeListenerCalled = false; + GlobalKey.unregisterSyncListener(globalKey, syncListener); + GlobalKey.unregisterRemoveListener(globalKey, removeListener); + tester.pumpFrame(() { + container = new Container(key: globalKey); + return container; + }); + expect(syncListenerCalled, isFalse); + expect(removeListenerCalled, isFalse); + + tester.pumpFrame(() => new Container()); + expect(syncListenerCalled, isFalse); + expect(removeListenerCalled, isFalse); + }); + + test('Global key reparenting', () { + GlobalKey globalKey = new GlobalKey(); + + bool syncListenerCalled = false; + bool removeListenerCalled = false; + + void syncListener(GlobalKey key, Widget widget) { + syncListenerCalled = true; + } + + void removeListener(GlobalKey key) { + removeListenerCalled = true; + } + + GlobalKey.registerSyncListener(globalKey, syncListener); + GlobalKey.registerRemoveListener(globalKey, removeListener); + WidgetTester tester = new WidgetTester(); + + tester.pumpFrame(() { + return new Container( + child: new Container( + key: globalKey + ) + ); + }); + expect(syncListenerCalled, isTrue); + expect(removeListenerCalled, isFalse); + + tester.pumpFrame(() { + return new Container( + key: globalKey, + child: new Container() + ); + }); + expect(syncListenerCalled, isTrue); + expect(removeListenerCalled, isFalse); + + tester.pumpFrame(() { + return new Container( + child: new Container( + key: globalKey + ) + ); + }); + expect(syncListenerCalled, isTrue); + expect(removeListenerCalled, isFalse); + + GlobalKey.unregisterSyncListener(globalKey, syncListener); + GlobalKey.unregisterRemoveListener(globalKey, removeListener); + }); +}