fix(jqLite): .data()/.bind() memory leak

Since angular attaches scope/injector/controller
into DOM it should clean up after itself. No need
to complain about memory leaks, since they can
only happened on detached DOM. Detached DOM would
only be in tests, since in production the DOM
would be attached to render tree and removal
would automatically clear memory.

5 files changed, 135 insertions, 51 deletions

diff --git a/src/jqLite.js b/src/jqLite.js
index 02291932..516438ff 100644
--- a/src/jqLite.js
+++ b/src/jqLite.js
@@ -74,7 +74,7 @@
  * @returns {Object} jQuery object.
  */
 
-var jqCache = {},
+var jqCache = JQLite.cache = {},
     jqName = JQLite.expando = 'ng-' + new Date().getTime(),
     jqId = 1,
     addEventListenerFn = (window.document.addEventListener
@@ -122,15 +122,15 @@ function JQLitePatchJQueryRemove(name, dispatchThis) {
         fireEvent = dispatchThis,
         set, setIndex, setLength,
         element, childIndex, childLength, children,
-        fns, data;
+        fns, events;
 
     while(list.length) {
       set = list.shift();
       for(setIndex = 0, setLength = set.length; setIndex < setLength; setIndex++) {
         element = jqLite(set[setIndex]);
         if (fireEvent) {
-          data = element.data('events');
-          if ( (fns = data && data.$destroy) ) {
+          events = element.data('events');
+          if ( (fns = events && events.$destroy) ) {
             forEach(fns, function(fn){
               fn.handler();
             });
@@ -185,19 +185,35 @@ function JQLiteDealoc(element){
   }
 }
 
+function JQLiteUnbind(element, type, fn) {
+  var events = JQLiteData(element, 'events'),
+      handle = JQLiteData(element, 'handle');
+
+  if (!handle) return; //no listeners registered
+
+  if (isUndefined(type)) {
+    forEach(events, function(eventHandler, type) {
+      removeEventListenerFn(element, type, eventHandler);
+      delete events[type];
+    });
+  } else {
+    if (isUndefined(fn)) {
+      removeEventListenerFn(element, type, events[type]);
+      delete events[type];
+    } else {
+      arrayRemove(events[type], fn);
+    }
+  }
+}
+
 function JQLiteRemoveData(element) {
   var cacheId = element[jqName],
       cache = jqCache[cacheId];
 
   if (cache) {
-    if (cache.bind) {
-      forEach(cache.bind, function(fn, type){
-        if (type == '$destroy') {
-          fn({});
-        } else {
-          removeEventListenerFn(element, type, fn);
-        }
-      });
+    if (cache.handle) {
+      cache.events.$destroy && cache.handle({}, '$destroy');
+      JQLiteUnbind(element);
     }
     delete jqCache[cacheId];
     element[jqName] = undefined; // ie does not allow deletion of attributes on elements.
@@ -499,8 +515,8 @@ forEach({
   };
 });
 
-function createEventHandler(element) {
-  var eventHandler = function (event) {
+function createEventHandler(element, events) {
+  var eventHandler = function (event, type) {
     if (!event.preventDefault) {
       event.preventDefault = function() {
         event.returnValue = false; //ie
@@ -530,8 +546,12 @@ function createEventHandler(element) {
       return event.defaultPrevented;
     };
 
-    forEach(eventHandler.fns, function(fn){
-      fn.call(element, event);
+    forEach(events[type || event.type], function(fn) {
+      try {
+        fn.call(element, event);
+      } catch (e) {
+        // Not much to do here since jQuery ignores these anyway
+      }
     });
 
     // Remove monkey-patched methods (IE),
@@ -548,7 +568,7 @@ function createEventHandler(element) {
       delete event.isDefaultPrevented;
     }
   };
-  eventHandler.fns = [];
+  eventHandler.elem = element;
   return eventHandler;
 }
 
@@ -563,61 +583,45 @@ forEach({
   dealoc: JQLiteDealoc,
 
   bind: function bindFn(element, type, fn){
-    var bind = JQLiteData(element, 'bind');
+    var events = JQLiteData(element, 'events'),
+        handle = JQLiteData(element, 'handle');
 
+    if (!events) JQLiteData(element, 'events', events = {});
+    if (!handle) JQLiteData(element, 'handle', handle = createEventHandler(element, events));
 
-    if (!bind) JQLiteData(element, 'bind', bind = {});
     forEach(type.split(' '), function(type){
-      var eventHandler = bind[type];
-
+      var eventFns = events[type];
 
-      if (!eventHandler) {
+      if (!eventFns) {
         if (type == 'mouseenter' || type == 'mouseleave') {
-          var mouseenter = bind.mouseenter = createEventHandler(element);
-          var mouseleave = bind.mouseleave = createEventHandler(element);
           var counter = 0;
 
+          events.mouseenter = [];
+          events.mouseleave = [];
 
           bindFn(element, 'mouseover', function(event) {
             counter++;
             if (counter == 1) {
-              mouseenter(event);
+              handle(event, 'mouseenter');
             }
           });
           bindFn(element, 'mouseout', function(event) {
             counter --;
             if (counter == 0) {
-              mouseleave(event);
+              handle(event, 'mouseleave');
             }
           });
-          eventHandler = bind[type];
         } else {
-          eventHandler = bind[type] = createEventHandler(element);
-          addEventListenerFn(element, type, eventHandler);
+          addEventListenerFn(element, type, handle);
+          events[type] = [];
         }
+        eventFns = events[type]
       }
-      eventHandler.fns.push(fn);
+      eventFns.push(fn);
     });
   },
 
-  unbind: function(element, type, fn) {
-    var bind = JQLiteData(element, 'bind');
-    if (!bind) return; //no listeners registered
-
-    if (isUndefined(type)) {
-      forEach(bind, function(eventHandler, type) {
-        removeEventListenerFn(element, type, eventHandler);
-        delete bind[type];
-      });
-    } else {
-      if (isUndefined(fn)) {
-        removeEventListenerFn(element, type, bind[type]);
-        delete bind[type];
-      } else {
-        arrayRemove(bind[type].fns, fn);
-      }
-    }
-  },
+  unbind: JQLiteUnbind,
 
   replaceWith: function(element, replaceNode) {
     var index, parent = element.parentNode;
diff --git a/src/ngMock/angular-mocks.js b/src/ngMock/angular-mocks.js
index b0f6383a..8b5d100a 100644
--- a/src/ngMock/angular-mocks.js
+++ b/src/ngMock/angular-mocks.js
@@ -1526,6 +1526,20 @@ angular.mock.e2e = {};
 angular.mock.e2e.$httpBackendDecorator = ['$delegate', '$browser', createHttpBackendMock];
 
 
+angular.mock.clearDataCache = function() {
+  var key,
+      cache = angular.element.cache;
+
+  for(key in cache) {
+    if (cache.hasOwnProperty(key)) {
+      var handle = cache[key].handle;
+
+      handle && angular.element(handle.elem).unbind();
+      delete cache[key];
+    }
+  }
+};
+
 
 window.jstestdriver && (function(window) {
   /**
@@ -1550,6 +1564,7 @@ window.jasmine && (function(window) {
     var spec = getCurrentSpec();
     spec.$injector = null;
     spec.$modules = null;
+    angular.mock.clearDataCache();
   });
 
   function getCurrentSpec() {
diff --git a/test/jqLiteSpec.js b/test/jqLiteSpec.js
index f159e08f..406b7a5a 100644
--- a/test/jqLiteSpec.js
+++ b/test/jqLiteSpec.js
@@ -291,11 +291,37 @@ describe('jqLite', function() {
       expect(element.data()).toEqual({meLike: 'turtles', youLike: 'carrots', existing: 'val'});
       expect(element.data()).toBe(oldData); // merge into the old object
     });
+
+    describe('data cleanup', function() {
+      it('should remove data on element removal', function() {
+        var div = jqLite('<div><span>text</span></div>'),
+            span = div.find('span');
+
+        span.data('name', 'angular');
+        span.remove();
+        expect(span.data('name')).toBeUndefined();
+      });
+
+      it('should remove event listeners on element removal', function() {
+        var div = jqLite('<div><span>text</span></div>'),
+            span = div.find('span'),
+            log = '';
+
+        span.bind('click', function() { log+= 'click;'});
+        browserTrigger(span);
+        expect(log).toEqual('click;');
+
+        span.remove();
+
+        browserTrigger(span);
+        expect(log).toEqual('click;');
+      });
+    });
   });
 
 
   describe('attr', function() {
-    it('shoul read write and remove attr', function() {
+    it('should read write and remove attr', function() {
       var selector = jqLite([a, b]);
 
       expect(selector.attr('prop', 'value')).toEqual(selector);
@@ -667,7 +693,7 @@ describe('jqLite', function() {
       var jWindow = jqLite(window).bind('hashchange', function() {
         log = 'works!';
       });
-      eventFn({});
+      eventFn({type: 'hashchange'});
       expect(log).toEqual('works!');
       dealoc(jWindow);
     });
diff --git a/test/ngMock/angular-mocksSpec.js b/test/ngMock/angular-mocksSpec.js
index 22c91a4d..88946ab9 100644
--- a/test/ngMock/angular-mocksSpec.js
+++ b/test/ngMock/angular-mocksSpec.js
@@ -148,6 +148,7 @@ describe('ngMock', function() {
     });
   });
 
+
   describe('$log', function() {
     var $log;
     beforeEach(inject(['$log', function(log) {
@@ -229,6 +230,7 @@ describe('ngMock', function() {
     });
   });
 
+
   describe('defer', function() {
     var browser, log;
     beforeEach(inject(function($browser) {
@@ -341,6 +343,44 @@ describe('ngMock', function() {
     });
   });
 
+
+  describe('angular.mock.clearDataCache', function() {
+    function keys(obj) {
+      var keys = [];
+      for(var key in obj) {
+        if (obj.hasOwnProperty(key)) keys.push(key);
+      }
+      return keys.sort();
+    }
+
+    it('should remove data', function() {
+      expect(angular.element.cache).toEqual({});
+      var div = angular.element('<div></div>');
+      div.data('name', 'angular');
+      expect(keys(angular.element.cache)).not.toEqual([]);
+      angular.mock.clearDataCache();
+      expect(keys(angular.element.cache)).toEqual([]);
+    });
+
+    it('should deregister event handlers', function() {
+      expect(keys(angular.element.cache)).toEqual([]);
+
+      var div = angular.element('<div></div>');
+
+      div.bind('click', angular.noop);
+      div.bind('mousemove', angular.noop);
+      div.data('some', 'data');
+      expect(keys(angular.element.cache).length).toBe(1);
+
+      angular.mock.clearDataCache();
+      expect(keys(angular.element.cache)).toEqual([]);
+      expect(div.data('some')).toBeUndefined();
+
+      div.remove();
+    });
+  });
+
+
   describe('jasmine module and inject', function(){
     var log;
 
diff --git a/test/testabilityPatch.js b/test/testabilityPatch.js
index 12724192..f033dda2 100644
--- a/test/testabilityPatch.js
+++ b/test/testabilityPatch.js
@@ -42,7 +42,6 @@ afterEach(function() {
   var count = 0;
   forEachSorted(jqCache, function(value, key){
     count ++;
-    delete jqCache[key];
     forEach(value, function(value, key){
       if (value.$element) {
         dump('LEAK', key, value.$id, sortedHtml(value.$element));