revert: fix(ngMock): prevent memory leak due to data attached to $rootElement

Matias Niemela · Matias Niemela · commit 1eef631ab504 · 2016-03-14T17:27:49.000-04:00
This reverts commit 571e323.
diff --git a/src/jqLite.js b/src/jqLite.js
@@ -196,12 +196,6 @@ function jqLiteHasData(node) {
   return false;
 }
 
-function jqLiteCleanData(nodes) {
-  for (var i = 0, ii = nodes.length; i < ii; i++) {
-    jqLiteRemoveData(nodes[i]);
-  }
-}
-
 function jqLiteBuildFragment(html, context) {
   var tmp, tag, wrap,
       fragment = context.createDocumentFragment(),
@@ -600,8 +594,7 @@ function getAliasedAttrName(name) {
 forEach({
   data: jqLiteData,
   removeData: jqLiteRemoveData,
-  hasData: jqLiteHasData,
-  cleanData: jqLiteCleanData
+  hasData: jqLiteHasData
 }, function(fn, name) {
   JQLite[name] = fn;
 });
diff --git a/src/ngMock/angular-mocks.js b/src/ngMock/angular-mocks.js
@@ -127,12 +127,12 @@ angular.mock.$Browser = function() {
 };
 angular.mock.$Browser.prototype = {
 
-  /**
-   * @name $browser#poll
-   *
-   * @description
-   * run all fns in pollFns
-   */
+/**
+  * @name $browser#poll
+  *
+  * @description
+  * run all fns in pollFns
+  */
   poll: function poll() {
     angular.forEach(this.pollFns, function(pollFn) {
       pollFn();
@@ -1879,12 +1879,10 @@ angular.mock.$RAFDecorator = ['$delegate', function($delegate) {
 /**
  *
  */
-var originalRootElement;
 angular.mock.$RootElementProvider = function() {
-  this.$get = ['$injector', function($injector) {
-    originalRootElement = angular.element('<div ng-app></div>').data('$injector', $injector);
-    return originalRootElement;
-  }];
+  this.$get = function() {
+    return angular.element('<div ng-app></div>');
+  };
 };
 
 /**
@@ -2298,7 +2296,6 @@ if (window.jasmine || window.mocha) {
 
 
   (window.beforeEach || window.setup)(function() {
-    originalRootElement = null;
     annotatedFunctions = [];
     currentSpec = this;
   });
@@ -2321,15 +2318,7 @@ if (window.jasmine || window.mocha) {
     currentSpec = null;
 
     if (injector) {
-      // Ensure `$rootElement` is instantiated, before checking `originalRootElement`
-      var $rootElement = injector.get('$rootElement');
-      var rootNode = $rootElement && $rootElement[0];
-      var cleanUpNodes = !originalRootElement ? [] : [originalRootElement[0]];
-      if (rootNode && (!originalRootElement || rootNode !== originalRootElement[0])) {
-        cleanUpNodes.push(rootNode);
-      }
-      angular.element.cleanData(cleanUpNodes);
-
+      injector.get('$rootElement').off();
     }
 
     // clean up jquery's fragment cache
diff --git a/test/helpers/testabilityPatch.js b/test/helpers/testabilityPatch.js
@@ -117,8 +117,12 @@ function dealoc(obj) {
   }
 
   function cleanup(element) {
-    angular.element.cleanData(element);
-
+    element.off().removeData();
+    if (window.jQuery) {
+      // jQuery 2.x doesn't expose the cache storage; ensure all element data
+      // is removed during its cleanup.
+      jQuery.cleanData([element]);
+    }
     // Note:  We aren't using element.contents() here.  Under jQuery, element.contents() can fail
     // for IFRAME elements.  jQuery explicitly uses (element.contentDocument ||
     // element.contentWindow.document) and both properties are null for IFRAMES that aren't attached
diff --git a/test/ngMock/angular-mocksSpec.js b/test/ngMock/angular-mocksSpec.js
@@ -1596,10 +1596,6 @@ describe('ngMock', function() {
     it('should create mock application root', inject(function($rootElement) {
       expect($rootElement.text()).toEqual('');
     }));
-
-    it('should attach the `$injector` to `$rootElement`', inject(function($injector, $rootElement) {
-      expect($rootElement.injector()).toBe($injector);
-    }));
   });
 
 
@@ -2117,122 +2113,9 @@ describe('ngMockE2E', function() {
   });
 });
 
-
 describe('make sure that we can create an injector outside of tests', function() {
   //since some libraries create custom injectors outside of tests,
   //we want to make sure that this is not breaking the internals of
   //how we manage annotated function cleanup during tests. See #10967
   angular.injector([function($injector) {}]);
 });
-
-
-describe('`afterEach` clean-up', function() {
-  describe('undecorated `$rootElement`', function() {
-    var prevRootElement;
-    var prevCleanDataSpy;
-
-
-    it('should set up spies so the next test can verify `$rootElement` was cleaned up', function() {
-      module(function($provide) {
-        $provide.decorator('$rootElement', function($delegate) {
-          prevRootElement = $delegate;
-
-          // Spy on `angular.element.cleanData()`, so the next test can verify
-          // that it has been called as necessary
-          prevCleanDataSpy = spyOn(angular.element, 'cleanData').andCallThrough();
-
-          return $delegate;
-        });
-      });
-
-      // Inject the `$rootElement` to ensure it has been created
-      inject(function($rootElement) {
-        expect($rootElement.injector()).toBeDefined();
-      });
-    });
-
-
-    it('should clean up `$rootElement` after each test', function() {
-      // One call is made by `testabilityPatch`'s `dealoc()`
-      // We want to verify the subsequent call, made by `angular-mocks`
-      expect(prevCleanDataSpy.callCount).toBe(2);
-
-      var cleanUpNodes = prevCleanDataSpy.calls[1].args[0];
-      expect(cleanUpNodes.length).toBe(1);
-      expect(cleanUpNodes[0]).toBe(prevRootElement[0]);
-    });
-  });
-
-
-  describe('decorated `$rootElement`', function() {
-    var prevOriginalRootElement;
-    var prevRootElement;
-    var prevCleanDataSpy;
-
-
-    it('should set up spies so the next text can verify `$rootElement` was cleaned up', function() {
-      module(function($provide) {
-        $provide.decorator('$rootElement', function($delegate) {
-          prevOriginalRootElement = $delegate;
-
-          // Mock `$rootElement` to be able to verify that the correct object is cleaned up
-          prevRootElement = angular.element('<div></div>');
-
-          // Spy on `angular.element.cleanData()`, so the next test can verify
-          // that it has been called as necessary
-          prevCleanDataSpy = spyOn(angular.element, 'cleanData').andCallThrough();
-
-          return prevRootElement;
-        });
-      });
-
-      // Inject the `$rootElement` to ensure it has been created
-      inject(function($rootElement) {
-        expect($rootElement).toBe(prevRootElement);
-        expect(prevOriginalRootElement.injector()).toBeDefined();
-        expect(prevRootElement.injector()).toBeUndefined();
-
-        // If we don't clean up `prevOriginalRootElement`-related data now, `testabilityPatch` will
-        // complain about a memory leak, because it doesn't clean up after the original
-        // `$rootElement`
-        // This is a false alarm, because `angular-mocks` would have cleaned up in a subsequent
-        // `afterEach` block
-        prevOriginalRootElement.removeData();
-      });
-    });
-
-
-    it('should clean up `$rootElement` (both original and decorated) after each test', function() {
-      // One call is made by `testabilityPatch`'s `dealoc()`
-      // We want to verify the subsequent call, made by `angular-mocks`
-      expect(prevCleanDataSpy.callCount).toBe(2);
-
-      var cleanUpNodes = prevCleanDataSpy.calls[1].args[0];
-      expect(cleanUpNodes.length).toBe(2);
-      expect(cleanUpNodes[0]).toBe(prevOriginalRootElement[0]);
-      expect(cleanUpNodes[1]).toBe(prevRootElement[0]);
-    });
-  });
-
-
-  describe('uninstantiated or falsy `$rootElement`', function() {
-    it('should not break if `$rootElement` was never instantiated', function() {
-      // Just an empty test to verify that `angular-mocks` doesn't break,
-      // when trying to clean up `$rootElement`, if `$rootElement` was never injected in the test
-      // (and thus never instantiated/created)
-
-      // Ensure the `$injector` is created - if there is no `$injector`, no clean-up takes places
-      inject(function() {});
-    });
-
-
-    it('should not break if the decorated `$rootElement` is falsy (e.g. `null`)', function() {
-      module(function($provide) {
-        $provide.value('$rootElement', null);
-      });
-
-      // Ensure the `$injector` is created - if there is no `$injector`, no clean-up takes places
-      inject(function() {});
-    });
-  });
-});
