From 259ac5b147652522a92b40a12298891dd491c9a7 Mon Sep 17 00:00:00 2001 From: Misko Hevery Date: Fri, 21 Feb 2014 22:05:50 -0800 Subject: [PATCH] fix(scope): correctly handle canceled listeners bookkeeping --- lib/core/scope.dart | 16 ++++--- test/core/scope_spec.dart | 87 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 94 insertions(+), 9 deletions(-) diff --git a/lib/core/scope.dart b/lib/core/scope.dart index 566695daa..bfd0b126a 100644 --- a/lib/core/scope.dart +++ b/lib/core/scope.dart @@ -228,7 +228,6 @@ class Scope { dynamic eval(expression, [Map locals]) { assert(isAttached); - _assertInternalStateConsistency(); assert(expression == null || expression is String || expression is Function); @@ -246,6 +245,7 @@ class Scope { rootScope._zone.run(() => apply(expression, locals)); dynamic apply([expression, Map locals]) { + _assertInternalStateConsistency(); rootScope._transitionState(null, RootScope.STATE_APPLY); try { return eval(expression, locals); @@ -261,23 +261,19 @@ class Scope { ScopeEvent emit(String name, [data]) { assert(isAttached); - _assertInternalStateConsistency(); return _Streams.emit(this, name, data); } ScopeEvent broadcast(String name, [data]) { assert(isAttached); - _assertInternalStateConsistency(); return _Streams.broadcast(this, name, data); } ScopeStream on(String name) { assert(isAttached); - _assertInternalStateConsistency(); return _Streams.on(this, rootScope._exceptionHandler, name); } Scope createChild(Object childContext) { assert(isAttached); - _assertInternalStateConsistency(); var child = new Scope(childContext, rootScope, this, _readWriteGroup.newGroup(childContext), _readOnlyGroup.newGroup(childContext)); @@ -292,7 +288,6 @@ class Scope { void destroy() { assert(isAttached); - _assertInternalStateConsistency(); broadcast(ScopeEvent.DESTROY); _Streams.destroy(this); @@ -328,11 +323,14 @@ class Scope { assert(_parentScope == parentScope); var counts = {}; var typeCounts = _streams == null ? {} : _streams._typeCounts; - log..add(prefix)..add(hashCode)..add(' ')..add(typeCounts)..add('\n'); + var connection = _streams != null && _streams._scope == this ? '=' : '-'; + log..add(prefix)..add(hashCode)..add(connection)..add(typeCounts)..add('\n'); if (_streams == null) { } else if (_streams._scope == this) { - _streams._streams.keys.forEach((k){ - counts[k] = 1 + (counts.containsKey(k) ? counts[k] : 0); + _streams._streams.forEach((k, ScopeStream stream){ + if (stream.subscriptions.isNotEmpty) { + counts[k] = 1 + (counts.containsKey(k) ? counts[k] : 0); + } }); } var childScope = _childHead; diff --git a/test/core/scope_spec.dart b/test/core/scope_spec.dart index cf1974adc..88921677e 100644 --- a/test/core/scope_spec.dart +++ b/test/core/scope_spec.dart @@ -3,6 +3,7 @@ library scope2_spec; import '../_specs.dart'; import 'package:angular/change_detection/change_detection.dart' hide ExceptionHandler; import 'package:angular/change_detection/dirty_checking_change_detector.dart'; +import 'dart:math'; main() => describe('scope', () { beforeEach(module((Module module) { @@ -320,6 +321,92 @@ main() => describe('scope', () { expect(getStreamState()).toEqual([true, true, true, true, true, true, true]); expect(root.apply).not.toThrow(); })); + + + it('should not properly merge streams', inject((RootScope root) { + var a = root.createChild({}); + var a2 = root.createChild({}); + var b = a.createChild({}); + var c = b.createChild({}); + var d = c.createChild({}); + var e = d.createChild({}); + + getStreamState() => [root.hasOwnStreams, a.hasOwnStreams, a2.hasOwnStreams, + b.hasOwnStreams, c.hasOwnStreams, d.hasOwnStreams, + e.hasOwnStreams]; + + expect(getStreamState()).toEqual([false, false, false, false, false, false, false]); + expect(root.apply).not.toThrow(); + + a2.on(ScopeEvent.DESTROY).listen((_) => null); + expect(getStreamState()).toEqual([false, false, true, false, false, false, false]); + expect(root.apply).not.toThrow(); + + e.on(ScopeEvent.DESTROY).listen((_) => null); + expect(getStreamState()).toEqual([true, false, true, false, false, false, true]); + expect(root.apply).not.toThrow(); + })); + + + it('should clean up on cancel', inject((RootScope root) { + var child = root.createChild(null); + var cl = child.on("E").listen((e) => null); + var rl = root.on("E").listen((e) => null); + rl.cancel(); + expect(root.apply).not.toThrow(); + })); + + + it('should find random bugs', inject((RootScope root) { + List scopes; + List listeners; + List steps; + var random = new Random(); + for(var i = 0; i < 1000; i++) { + if (i % 10 == 0) { + scopes = [root.createChild(null)]; + listeners = []; + steps = []; + } + switch(random.nextInt(4)) { + case 0: + if (scopes.length > 10) break; + var index = random.nextInt(scopes.length); + Scope scope = scopes[index]; + var child = scope.createChild(null); + scopes.add(child); + steps.add('scopes[$index].createChild(null)'); + break; + case 1: + var index = random.nextInt(scopes.length); + Scope scope = scopes[index]; + listeners.add(scope.on('E').listen((e) => null)); + steps.add('scopes[$index].on("E").listen((e)=>null)'); + break; + case 2: + if (scopes.length < 3) break; + var index = random.nextInt(scopes.length - 1) + 1; + Scope scope = scopes[index]; + scope.destroy(); + scopes = scopes.where((Scope s) => s.isAttached).toList(); + steps.add('scopes[$index].destroy()'); + break; + case 3: + if (listeners.length == 0) break; + var index = random.nextInt(listeners.length); + var l = listeners[index]; + l.cancel(); + listeners.remove(l); + steps.add('listeners[$index].cancel()'); + break; + } + try { + root.apply(); + } catch (e) { + expect('').toEqual(steps.join(';\n')); + } + } + })); });