fix($rootScope): fix potential memory leak when removing scope listeners

Previously the array entry for listeners was set to null but the array size was not trimmed
until the event was broadcasted again (see angular@e6966e0).

By keeping track of the listener iteration index globally it can be adjusted if a listener
removal effects the index.

Fixes angular#16135

Author: jbedard

src/ng/rootScope.js

@@ -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.$$listenerIndex) {
+              namedListeners.$$listenerIndex--;
+            }
           }
         };
       },
@@ -1226,24 +1228,15 @@ 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;
-            }
+          for (namedListeners.$$listenerIndex = 0; namedListeners.$$listenerIndex < namedListeners.length; namedListeners.$$listenerIndex++) {
             try {
               //allow all listeners attached to the current scope to run
-              namedListeners[i].apply(null, listenerArgs);
+              namedListeners[namedListeners.$$listenerIndex].apply(null, listenerArgs);
             } catch (e) {
               $exceptionHandler(e);
             }
@@ -1300,23 +1293,15 @@ function $RootScopeProvider() {
         if (!target.$$listenerCount[name]) return event;
 
         var listenerArgs = concat([event], arguments, 1),
-            listeners, i, length;
+            listeners;
 
         //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;
-            }
-
+          for (listeners.$$listenerIndex = 0; listeners.$$listenerIndex < listeners.length; listeners.$$listenerIndex++) {
             try {
-              listeners[i].apply(null, listenerArgs);
+              listeners[listeners.$$listenerIndex].apply(null, listenerArgs);
             } catch (e) {
               $exceptionHandler(e);
             }

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);
@@ -2531,7 +2544,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 +2578,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();