fix(scope): correctly handle canceled listeners bookkeeping

dart-archive · Feb 22, 2014 · 259ac5b · 259ac5b
1 parent 2e40350
commit 259ac5b
Show file tree

Hide file tree

Showing 2 changed files with 94 additions and 9 deletions.
diff --git 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
@@ -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'));
+          }
+        }
+      }));
     });