fix($animate): avoid memory leak with `$animate.enabled(element, enab…

…led)`

When disabling/enabling animations on a specific element (via
`$animate.enabled(element, enabled)`), the element is added in a map to
track its state. Previously, the element was never removed from the map,
causing AngularJS to hold on to the element even after it is removed
from the DOM, thus preventing it from being garbage collected.

This commit fixes it by removing the element from the map on `$destroy`.

Fixes #16637.

src/apis.js

@@ -45,11 +45,10 @@ function NgMapShim() {
 }
 NgMapShim.prototype = {
   _idx: function(key) {
-    if (key === this._lastKey) {
-      return this._lastIndex;
+    if (key !== this._lastKey) {
+      this._lastKey = key;
+      this._lastIndex = this._keys.indexOf(key);
     }
-    this._lastKey = key;
-    this._lastIndex = this._keys.indexOf(key);
     return this._lastIndex;
   },
   _transformKey: function(key) {
@@ -62,6 +61,11 @@ NgMapShim.prototype = {
       return this._values[idx];
     }
   },
+  has: function(key) {
+    key = this._transformKey(key);
+    var idx = this._idx(key);
+    return idx !== -1;
+  },
   set: function(key, value) {
     key = this._transformKey(key);
     var idx = this._idx(key);

src/ngAnimate/animateQueue.js

@@ -120,6 +120,10 @@ var $$AnimateQueueProvider = ['$animateProvider', /** @this */ function($animate
     var disabledElementsLookup = new $$Map();
     var animationsEnabled = null;
 
+    function removeFromDisabledElementsLookup(evt) {
+      disabledElementsLookup.delete(evt.target);
+    }
+
     function postDigestTaskFactory() {
       var postDigestCalled = false;
       return function(fn) {
@@ -303,6 +307,11 @@ var $$AnimateQueueProvider = ['$animateProvider', /** @this */ function($animate
               bool = !disabledElementsLookup.get(node);
             } else {
               // (element, bool) - Element setter
+              if (!disabledElementsLookup.has(node)) {
+                // The element is added to the map for the first time.
+                // Create a listener to remove it on `$destroy` (to avoid memory leak).
+                $$jqLite(element).on('$destroy', removeFromDisabledElementsLookup);
+              }
               disabledElementsLookup.set(node, !bool);
             }
           }

test/ApiSpecs.js

@@ -89,6 +89,34 @@ describe('api', function() {
       expect(map.get(keys[2])).toBe(values[2]);
     });
 
+    it('should return if a key exists or not', function() {
+      var map = new NgMapShim();
+      var keys = ['foo', {}];
+
+      expect(map.has(keys[0])).toBe(false);
+      expect(map.has(keys[1])).toBe(false);
+
+      map.set(keys[0], 'bar');
+      expect(map.has(keys[0])).toBe(true);
+      expect(map.has(keys[1])).toBe(false);
+
+      map.set(keys[1], 'baz');
+      expect(map.has(keys[0])).toBe(true);
+      expect(map.has(keys[1])).toBe(true);
+
+      map.delete(keys[0]);
+      expect(map.has(keys[0])).toBe(false);
+      expect(map.has(keys[1])).toBe(true);
+
+      map.delete(keys[1]);
+      expect(map.has(keys[0])).toBe(false);
+      expect(map.has(keys[1])).toBe(false);
+
+      map.set(keys[1], 'qux');
+      expect(map.has(keys[0])).toBe(false);
+      expect(map.has(keys[1])).toBe(true);
+    });
+
     it('should be able to deal with `NaN` keys', function() {
       var map = new NgMapShim();
 

test/ngAnimate/animateSpec.js

@@ -577,6 +577,41 @@ describe('animations', function() {
         $rootScope.$digest();
         expect(capturedAnimation).toBeTruthy();
       }));
+
+      it('should remove the element from the `disabledElementsLookup` map on `$destroy`',
+        inject(function($$Map, $animate, $rootScope) {
+
+        var setSpy = spyOn($$Map.prototype, 'set').and.callThrough();
+        var deleteSpy = spyOn($$Map.prototype, 'delete').and.callThrough();
+
+        parent.append(element);
+
+        $animate.enabled(element, false);
+        $animate.enabled(element, true);
+        $animate.enabled(element, false);
+        expect(setSpy).toHaveBeenCalledWith(element[0], jasmine.any(Boolean));
+        expect(deleteSpy).not.toHaveBeenCalledWith(element[0]);
+        expect($animate.enabled(element)).toBe(false);
+
+        // No clean-up on `detach` (no `$destroy` event).
+        element.detach();
+        expect(deleteSpy).not.toHaveBeenCalledWith(element[0]);
+        expect($animate.enabled(element)).toBe(false);
+
+        // Clean-up on `remove` (causes `$destroy` event).
+        element.remove();
+        expect(deleteSpy).toHaveBeenCalledOnceWith(element[0]);
+        expect($animate.enabled(element)).toBe(true);
+
+        deleteSpy.calls.reset();
+
+        element.triggerHandler('$destroy');
+        expect(deleteSpy).not.toHaveBeenCalledWith(element[0]);
+
+        $animate.enabled(element, true);
+        element.triggerHandler('$destroy');
+        expect(deleteSpy).toHaveBeenCalledOnceWith(element[0]);
+      }));
     });
 
     it('should strip all comment nodes from the animation and not issue an animation if not real elements are found',