Commit d7ba5145 authored by Adam Barth's avatar Adam Barth

Fix global key error while using fitness app

We were making local copies of the listener maps, but we were actually
iterating the underlying sets. Now we make local copies of the sets.

Fixes #803
parent 1883d06a
......@@ -97,10 +97,7 @@ abstract class GlobalKey extends Key {
_syncedKeys.add(this);
}
static bool _notifyingListeners = false;
static void registerSyncListener(GlobalKey key, GlobalKeySyncListener listener) {
assert(!_notifyingListeners);
assert(key != null);
Set<GlobalKeySyncListener> listeners =
_syncListeners.putIfAbsent(key, () => new Set<GlobalKeySyncListener>());
......@@ -109,7 +106,6 @@ abstract class GlobalKey extends Key {
}
static void unregisterSyncListener(GlobalKey key, GlobalKeySyncListener listener) {
assert(!_notifyingListeners);
assert(key != null);
assert(_syncListeners.containsKey(key));
bool removed = _syncListeners[key].remove(listener);
......@@ -119,7 +115,6 @@ abstract class GlobalKey extends Key {
}
static void registerRemoveListener(GlobalKey key, GlobalKeyRemoveListener listener) {
assert(!_notifyingListeners);
assert(key != null);
Set<GlobalKeyRemoveListener> listeners =
_removeListeners.putIfAbsent(key, () => new Set<GlobalKeyRemoveListener>());
......@@ -128,7 +123,6 @@ abstract class GlobalKey extends Key {
}
static void unregisterRemoveListener(GlobalKey key, GlobalKeyRemoveListener listener) {
assert(!_notifyingListeners);
assert(key != null);
assert(_removeListeners.containsKey(key));
bool removed = _removeListeners[key].remove(listener);
......@@ -148,31 +142,27 @@ abstract class GlobalKey extends Key {
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])
if (widget != null && _syncListeners.containsKey(key)) {
Set<GlobalKeySyncListener> localListeners = new Set<GlobalKeySyncListener>.from(_syncListeners[key]);
for (GlobalKeySyncListener listener in localListeners)
listener(key, widget);
}
}
for (GlobalKey key in _removedKeys) {
if (!_registry.containsKey(key) && localRemoveListeners.containsKey(key)) {
for (GlobalKeyRemoveListener listener in localRemoveListeners[key])
if (!_registry.containsKey(key) && _removeListeners.containsKey(key)) {
Set<GlobalKeyRemoveListener> localListeners = new Set<GlobalKeyRemoveListener>.from(_removeListeners[key]);
for (GlobalKeyRemoveListener listener in localListeners)
listener(key);
}
}
} finally {
_removedKeys.clear();
_syncedKeys.clear();
_notifyingListeners = false;
}
}
......
......@@ -106,4 +106,48 @@ void main() {
GlobalKey.unregisterSyncListener(globalKey, syncListener);
GlobalKey.unregisterRemoveListener(globalKey, removeListener);
});
test('Global key mutate during iteration', () {
GlobalKey globalKey = new GlobalKey();
bool syncListenerCalled = false;
bool removeListenerCalled = false;
void syncListener(GlobalKey key, Widget widget) {
GlobalKey.unregisterSyncListener(globalKey, syncListener);
syncListenerCalled = true;
}
void removeListener(GlobalKey key) {
GlobalKey.unregisterRemoveListener(globalKey, removeListener);
removeListenerCalled = true;
}
GlobalKey.registerSyncListener(globalKey, syncListener);
GlobalKey.registerRemoveListener(globalKey, removeListener);
WidgetTester tester = new WidgetTester();
tester.pumpFrame(() {
return new Container(key: globalKey);
});
expect(syncListenerCalled, isTrue);
expect(removeListenerCalled, isFalse);
syncListenerCalled = false;
removeListenerCalled = false;
tester.pumpFrame(() {
return new Container();
});
expect(syncListenerCalled, isFalse);
expect(removeListenerCalled, isTrue);
syncListenerCalled = false;
removeListenerCalled = false;
tester.pumpFrame(() {
return new Container(key: globalKey);
});
expect(syncListenerCalled, isFalse);
expect(removeListenerCalled, isFalse);
});
}
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