diff --git a/src/ng/rootScope.js b/src/ng/rootScope.js index e293b7f4e483..4839af0c5e5f 100644 --- a/src/ng/rootScope.js +++ b/src/ng/rootScope.js @@ -1167,7 +1167,7 @@ function $RootScopeProvider() { $on: function(name, listener) { var namedListeners = this.$$listeners[name]; if (!namedListeners) { - this.$$listeners[name] = namedListeners = []; + this.$$listeners[name] = namedListeners = extend([], {$$index: -1}); } namedListeners.push(listener); @@ -1181,10 +1181,12 @@ function $RootScopeProvider() { var self = this; return function() { - var indexOfListener = namedListeners.indexOf(listener); - if (indexOfListener !== -1) { - namedListeners[indexOfListener] = null; + var index = arrayRemove(namedListeners, listener); + if (index >= 0) { decrementListenerCount(self, 1, name); + if (index <= namedListeners.$$index) { + namedListeners.$$index--; + } } }; }, @@ -1213,9 +1215,7 @@ function $RootScopeProvider() { * @return {Object} Event object (see {@link ng.$rootScope.Scope#$on}). */ $emit: function(name, args) { - var empty = [], - namedListeners, - scope = this, + var scope = this, stopPropagation = false, event = { name: name, @@ -1226,36 +1226,14 @@ function $RootScopeProvider() { }, defaultPrevented: false }, - listenerArgs = concat([event], arguments, 1), - i, length; + listenerArgs = concat([event], arguments, 1); do { - namedListeners = scope.$$listeners[name] || empty; - event.currentScope = scope; - for (i = 0, length = namedListeners.length; i < length; i++) { - - // if listeners were deregistered, defragment the array - if (!namedListeners[i]) { - namedListeners.splice(i, 1); - i--; - length--; - continue; - } - try { - //allow all listeners attached to the current scope to run - namedListeners[i].apply(null, listenerArgs); - } catch (e) { - $exceptionHandler(e); - } - } - //if any listener on the current scope stops propagation, prevent bubbling - if (stopPropagation) { - event.currentScope = null; - return event; - } + invokeListeners(scope, event, listenerArgs, name); + //traverse upwards - scope = scope.$parent; - } while (scope); + //if any listener on the current scope stops propagation, prevent bubbling + } while (!stopPropagation && (scope = scope.$parent)); event.currentScope = null; @@ -1299,28 +1277,11 @@ function $RootScopeProvider() { if (!target.$$listenerCount[name]) return event; - var listenerArgs = concat([event], arguments, 1), - listeners, i, length; + var listenerArgs = concat([event], arguments, 1); //down while you can, then up and next sibling or up and next sibling until back at root while ((current = next)) { - event.currentScope = current; - listeners = current.$$listeners[name] || []; - for (i = 0, length = listeners.length; i < length; i++) { - // if listeners were deregistered, defragment the array - if (!listeners[i]) { - listeners.splice(i, 1); - i--; - length--; - continue; - } - - try { - listeners[i].apply(null, listenerArgs); - } catch (e) { - $exceptionHandler(e); - } - } + invokeListeners(current, event, listenerArgs, name); // Insanity Warning: scope depth-first traversal // yes, this code is a bit crazy, but it works and we have tests to prove it! @@ -1350,6 +1311,24 @@ function $RootScopeProvider() { return $rootScope; + function invokeListeners(scope, event, listenerArgs, name) { + var listeners = scope.$$listeners[name]; + if (listeners) { + if (listeners.$$index !== -1) { + throw $rootScopeMinErr('inevt', '{0} already $emit/$broadcast-ing', name); + } + event.currentScope = scope; + for (listeners.$$index = 0; listeners.$$index < listeners.length; listeners.$$index++) { + try { + //allow all listeners attached to the current scope to run + listeners[listeners.$$index].apply(null, listenerArgs); + } catch (e) { + $exceptionHandler(e); + } + } + listeners.$$index = -1; + } + } function beginPhase(phase) { if ($rootScope.$$phase) { diff --git a/test/ng/rootScopeSpec.js b/test/ng/rootScopeSpec.js index bd157467b092..65e08ce05160 100644 --- a/test/ng/rootScopeSpec.js +++ b/test/ng/rootScopeSpec.js @@ -2316,6 +2316,19 @@ describe('Scope', function() { })); + // See issue https://github.com/angular/angular.js/issues/16135 + it('should deallocate the listener array entry', inject(function($rootScope) { + var remove1 = $rootScope.$on('abc', noop); + $rootScope.$on('abc', noop); + + expect($rootScope.$$listeners['abc'].length).toBe(2); + + remove1(); + + expect($rootScope.$$listeners['abc'].length).toBe(1); + })); + + it('should call next listener when removing current', inject(function($rootScope) { var listener1 = jasmine.createSpy().and.callFake(function() { remove1(); }); var remove1 = $rootScope.$on('abc', listener1); @@ -2442,6 +2455,41 @@ describe('Scope', function() { expect($rootScope.$$listenerCount).toEqual({abc: 1}); expect(child.$$listenerCount).toEqual({abc: 1}); })); + + + it('should throw on recursive $broadcast', inject(function($rootScope) { + $rootScope.$on('e', function() { $rootScope.$broadcast('e'); }); + + expect(function() { $rootScope.$emit('e', 5); }).toThrowMinErr('$rootScope', 'inevt', 'e already $emit/$broadcast-ing'); + expect(function() { $rootScope.$broadcast('e', 5); }).toThrowMinErr('$rootScope', 'inevt', 'e already $emit/$broadcast-ing'); + })); + + + it('should throw on recursive $emit', inject(function($rootScope) { + $rootScope.$on('e', function() { $rootScope.$emit('e'); }); + + expect(function() { $rootScope.$emit('e', 5); }).toThrowMinErr('$rootScope', 'inevt', 'e already $emit/$broadcast-ing'); + expect(function() { $rootScope.$broadcast('e', 5); }).toThrowMinErr('$rootScope', 'inevt', 'e already $emit/$broadcast-ing'); + })); + + + it('should throw on recursive $broadcast on child listener', inject(function($rootScope) { + var child = $rootScope.$new(); + child.$on('e', function() { $rootScope.$broadcast('e'); }); + + expect(function() { child.$emit('e', 5); }).toThrowMinErr('$rootScope', 'inevt', 'e already $emit/$broadcast-ing'); + expect(function() { child.$broadcast('e', 5); }).toThrowMinErr('$rootScope', 'inevt', 'e already $emit/$broadcast-ing'); + })); + + + it('should throw on recursive $emit parent listener', inject(function($rootScope) { + var child = $rootScope.$new(); + $rootScope.$on('e', function() { child.$emit('e'); }); + + expect(function() { child.$emit('e', 5); }).toThrowMinErr('$rootScope', 'inevt', 'e already $emit/$broadcast-ing'); + expect(function() { $rootScope.$emit('e', 5); }).toThrowMinErr('$rootScope', 'inevt', 'e already $emit/$broadcast-ing'); + expect(function() { $rootScope.$broadcast('e', 5); }).toThrowMinErr('$rootScope', 'inevt', 'e already $emit/$broadcast-ing'); + })); }); }); @@ -2531,7 +2579,7 @@ describe('Scope', function() { expect(spy1).toHaveBeenCalledOnce(); expect(spy2).toHaveBeenCalledOnce(); expect(spy3).toHaveBeenCalledOnce(); - expect(child.$$listeners['evt'].length).toBe(3); // cleanup will happen on next $emit + expect(child.$$listeners['evt'].length).toBe(2); spy1.calls.reset(); spy2.calls.reset(); @@ -2565,7 +2613,7 @@ describe('Scope', function() { expect(spy1).toHaveBeenCalledOnce(); expect(spy2).toHaveBeenCalledOnce(); expect(spy3).toHaveBeenCalledOnce(); - expect(child.$$listeners['evt'].length).toBe(3); //cleanup will happen on next $broadcast + expect(child.$$listeners['evt'].length).toBe(2); spy1.calls.reset(); spy2.calls.reset();