Merge pull request #774 from abarth/add_listener

Add GlobalKey.registerAddListener

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;
     }
   }

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<GlobalKey, Widget> _registry = new Map<GlobalKey, Widget>();
   static final Map<GlobalKey, int> _debugDuplicates = new Map<GlobalKey, int>();
-  static final Map<GlobalKey, Set<GlobalKeyRemovalListener>> _removalListeners = new Map<GlobalKey, Set<GlobalKeyRemovalListener>>();
+  static final Map<GlobalKey, Set<GlobalKeySyncListener>> _syncListeners = new Map<GlobalKey, Set<GlobalKeySyncListener>>();
+  static final Map<GlobalKey, Set<GlobalKeyRemoveListener>> _removeListeners = new Map<GlobalKey, Set<GlobalKeyRemoveListener>>();
+  static final Set<GlobalKey> _syncedKeys = new Set<GlobalKey>();
   static final Set<GlobalKey> _removedKeys = new Set<GlobalKey>();
 
   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))
+    Set<GlobalKeySyncListener> listeners =
-      _removalListeners[key] = new Set<GlobalKeyRemovalListener>();
+        _syncListeners.putIfAbsent(key, () => new Set<GlobalKeySyncListener>());
-    bool added = _removalListeners[key].add(listener);
+    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));
+    assert(_syncListeners.containsKey(key));
-    bool removed = _removalListeners[key].remove(listener);
+    bool removed = _syncListeners[key].remove(listener);
-    if (_removalListeners[key].isEmpty)
+    if (_syncListeners[key].isEmpty)
-      _removalListeners.remove(key);
+      _syncListeners.remove(key);
+    assert(removed);
+  }
+
+  static void registerRemoveListener(GlobalKey key, GlobalKeyRemoveListener listener) {
+    assert(!_notifyingListeners);
+    assert(key != null);
+    Set<GlobalKeyRemoveListener> listeners =
+        _removeListeners.putIfAbsent(key, () => new Set<GlobalKeyRemoveListener>());
+    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,17 +146,35 @@ abstract class GlobalKey extends Key {
     assert(!_inBuildDirtyComponents);
     assert(!Widget._notifyingMountStatus);
     assert(_debugDuplicates.isEmpty);
+    if (_syncedKeys.isEmpty && _removedKeys.isEmpty)
+      return;
     _notifyingListeners = true;
+    try {
+      Map<GlobalKey, Set<GlobalKeyRemoveListener>> localRemoveListeners =
+          new Map<GlobalKey, Set<GlobalKeyRemoveListener>>.from(_removeListeners);
+      Map<GlobalKey, Set<GlobalKeySyncListener>> localSyncListeners =
+          new Map<GlobalKey, Set<GlobalKeySyncListener>>.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) && _removalListeners.containsKey(key)) {
+        if (!_registry.containsKey(key) && localRemoveListeners.containsKey(key)) {
-        for (GlobalKeyRemovalListener listener in _removalListeners[key])
+          for (GlobalKeyRemoveListener listener in localRemoveListeners[key])
             listener(key);
-        _removalListeners.remove(key);
         }
       }
+    } finally {
       _removedKeys.clear();
+      _syncedKeys.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) {

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();
+  }
+}

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);
+  });
+}