diff --git a/src/ng/compile.js b/src/ng/compile.js index 63eb52fcb77f..a2e5a718ed72 100644 --- a/src/ng/compile.js +++ b/src/ng/compile.js @@ -976,11 +976,11 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { function createBoundTranscludeFn(scope, transcludeFn, previousBoundTranscludeFn) { - var boundTranscludeFn = function(transcludedScope, cloneFn, controllers) { + var boundTranscludeFn = function(transcludedScope, cloneFn, controllers, containingScope) { var scopeCreated = false; if (!transcludedScope) { - transcludedScope = scope.$new(); + transcludedScope = scope.$new(false, containingScope); transcludedScope.$$transcluded = true; scopeCreated = true; } @@ -1592,7 +1592,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { transcludeControllers = elementControllers; } - return boundTranscludeFn(scope, cloneAttachFn, transcludeControllers); + return boundTranscludeFn(scope, cloneAttachFn, transcludeControllers, scopeToChild); } } } @@ -1754,6 +1754,8 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { boundTranscludeFn = linkQueue.shift(), linkNode = $compileNode[0]; + if (scope.$$destroyed) continue; + if (beforeTemplateLinkNode !== beforeTemplateCompileNode) { var oldClasses = beforeTemplateLinkNode.className; @@ -1784,6 +1786,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { return function delayedNodeLinkFn(ignoreChildLinkFn, scope, node, rootElement, boundTranscludeFn) { var childBoundTranscludeFn = boundTranscludeFn; + if (scope.$$destroyed) return; if (linkQueue) { linkQueue.push(scope); linkQueue.push(node); diff --git a/src/ng/rootScope.js b/src/ng/rootScope.js index 29295f76a3e6..24b49443d7ee 100644 --- a/src/ng/rootScope.js +++ b/src/ng/rootScope.js @@ -182,12 +182,17 @@ function $RootScopeProvider(){ * When creating widgets, it is useful for the widget to not accidentally read parent * state. * + * @param {Scope} [parent=this] The {@link ng.$rootScope.Scope `Scope`} that will contain this + * the newly created scope. Defaults to `this` scope if not provided. + * This is used to ensure that $destroy events are handled correctly. + * * @returns {Object} The newly created child scope. * */ - $new: function(isolate) { - var ChildScope, - child; + $new: function(isolate, parent) { + var child; + + parent = parent || this; if (isolate) { child = new Scope(); @@ -220,6 +225,19 @@ function $RootScopeProvider(){ } else { this.$$childHead = this.$$childTail = child; } + + // When the new scope is not isolated or we inherit from `this`, and + // the parent scope is destroyed, the property `$$destroyed` is inherited + // prototypically. In all other cases, this property needs to be set + // when the parent scope is destroyed. + // The listener needs to be added after the parent is set + if (isolate || parent != this) child.$on('$destroy', destroyChild); + + + function destroyChild() { + child.$$destroyed = true; + } + return child; }, diff --git a/test/.jshintrc b/test/.jshintrc index 0b19aa1974bc..fc6add91b960 100644 --- a/test/.jshintrc +++ b/test/.jshintrc @@ -162,6 +162,7 @@ "provideLog": false, "spyOnlyCallsWithArgs": false, "createMockStyleSheet": false, - "browserTrigger": false + "browserTrigger": false, + "jqLiteCacheSize": false } } diff --git a/test/helpers/testabilityPatch.js b/test/helpers/testabilityPatch.js index 6bdd5a5c0e22..83640362e3d2 100644 --- a/test/helpers/testabilityPatch.js +++ b/test/helpers/testabilityPatch.js @@ -118,6 +118,15 @@ function dealoc(obj) { } } + +function jqLiteCacheSize() { + var size = 0; + forEach(jqLite.cache, function() { size++; }); + return size - jqLiteCacheSize.initSize; +} +jqLiteCacheSize.initSize = 0; + + /** * @param {DOMElement} element * @param {boolean=} showNgClass diff --git a/test/ng/compileSpec.js b/test/ng/compileSpec.js index 992d7edc51e1..77db97ad78b8 100755 --- a/test/ng/compileSpec.js +++ b/test/ng/compileSpec.js @@ -3740,6 +3740,119 @@ describe('$compile', function() { }); }); + it('should not leak when continuing the compilation of elements on a scope that was destroyed', function() { + if (jQuery) { + // jQuery 2.x doesn't expose the cache storage. + return; + } + + var linkFn = jasmine.createSpy('linkFn'); + + module(function($controllerProvider, $compileProvider) { + $controllerProvider.register('Leak', function ($scope, $timeout) { + $scope.code = 'red'; + $timeout(function () { + $scope.code = 'blue'; + }); + }); + $compileProvider.directive('isolateRed', function() { + return { + restrict: 'A', + scope: {}, + template: '
' + }; + }); + $compileProvider.directive('red', function() { + return { + restrict: 'A', + templateUrl: 'red.html', + scope: {}, + link: linkFn + }; + }); + }); + + inject(function($compile, $rootScope, $httpBackend, $timeout, $templateCache) { + $httpBackend.whenGET('red.html').respond('

red.html

'); + var template = $compile( + '
' + + '
' + + '
' + + '
' + + '
' + + '
' + + '
'); + element = template($rootScope); + $rootScope.$digest(); + $timeout.flush(); + $httpBackend.flush(); + expect(linkFn).not.toHaveBeenCalled(); + expect(jqLiteCacheSize()).toEqual(2); + + $templateCache.removeAll(); + var destroyedScope = $rootScope.$new(); + destroyedScope.$destroy(); + var clone = template(destroyedScope); + $rootScope.$digest(); + $timeout.flush(); + expect(linkFn).not.toHaveBeenCalled(); + }); + }); + + if (jQuery) { + describe('cleaning up after a replaced element', function () { + var $compile, xs; + beforeEach(inject(function (_$compile_) { + $compile = _$compile_; + xs = [0, 1]; + })); + + function testCleanup() { + var privateData, firstRepeatedElem; + + element = $compile('
{{x}}
')($rootScope); + + $rootScope.$apply('xs = [' + xs + ']'); + firstRepeatedElem = element.children('.ng-scope').eq(0); + + expect(firstRepeatedElem.data('$scope')).toBeDefined(); + privateData = jQuery._data(firstRepeatedElem[0]); + expect(privateData.events).toBeDefined(); + expect(privateData.events.click).toBeDefined(); + expect(privateData.events.click[0]).toBeDefined(); + + $rootScope.$apply('xs = null'); + + expect(firstRepeatedElem.data('$scope')).not.toBeDefined(); + privateData = jQuery._data(firstRepeatedElem[0]); + expect(privateData && privateData.events).not.toBeDefined(); + } + + it('should work without external libraries (except jQuery)', testCleanup); + + it('should work with another library patching jQuery.cleanData after Angular', function () { + var cleanedCount = 0; + var currentCleanData = jQuery.cleanData; + jQuery.cleanData = function (elems) { + cleanedCount += elems.length; + // Don't return the output and expicitly pass only the first parameter + // so that we're sure we're not relying on either of them. jQuery UI patch + // behaves in this way. + currentCleanData(elems); + }; + + testCleanup(); + + // The initial ng-repeat div is dumped after parsing hence we expect cleanData + // count to be one larger than size of the iterated array. + expect(cleanedCount).toBe(xs.length + 1); + + // Restore the previous jQuery.cleanData. + jQuery.cleanData = currentCleanData; + }); + }); + } + it('should add a $$transcluded property onto the transcluded scope', function() { module(function() { @@ -4097,6 +4210,179 @@ describe('$compile', function() { }); + // see issue https://github.com/angular/angular.js/issues/9095 + describe('removing a transcluded element', function() { + + function countScopes($rootScope) { + return [$rootScope].concat( + getChildScopes($rootScope) + ).length; + } + + function getChildScopes(scope) { + var children = []; + if (!scope.$$childHead) { return children; } + var childScope = scope.$$childHead; + do { + children.push(childScope); + children = children.concat(getChildScopes(childScope)); + } while ((childScope = childScope.$$nextSibling)); + return children; + } + + beforeEach(module(function() { + directive('toggle', function() { + return { + transclude: true, + template: '
' + }; + }); + })); + + + it('should not leak the transclude scope when the transcluded content is an element transclusion directive', + inject(function($compile, $rootScope) { + + element = $compile( + '
' + + '
{{ msg }}
' + + '
' + )($rootScope); + + $rootScope.$apply('t = true'); + expect(element.text()).toContain('msg-1'); + // Expected scopes: $rootScope, ngIf, transclusion, ngRepeat + expect(countScopes($rootScope)).toEqual(4); + + $rootScope.$apply('t = false'); + expect(element.text()).not.toContain('msg-1'); + // Expected scopes: $rootScope + expect(countScopes($rootScope)).toEqual(1); + + $rootScope.$apply('t = true'); + expect(element.text()).toContain('msg-1'); + // Expected scopes: $rootScope, ngIf, transclusion, ngRepeat + expect(countScopes($rootScope)).toEqual(4); + + $rootScope.$apply('t = false'); + expect(element.text()).not.toContain('msg-1'); + // Expected scopes: $rootScope + expect(countScopes($rootScope)).toEqual(1); + })); + + + it('should not leak the transclude scope when the transcluded content is an multi-element transclusion directive', + inject(function($compile, $rootScope) { + + element = $compile( + '
' + + '
{{ msg }}
' + + '
{{ msg }}
' + + '
' + )($rootScope); + + $rootScope.$apply('t = true'); + expect(element.text()).toContain('msg-1msg-1'); + // Expected scopes: $rootScope, ngIf, transclusion, ngRepeat + expect(countScopes($rootScope)).toEqual(4); + + $rootScope.$apply('t = false'); + expect(element.text()).not.toContain('msg-1msg-1'); + // Expected scopes: $rootScope + expect(countScopes($rootScope)).toEqual(1); + + $rootScope.$apply('t = true'); + expect(element.text()).toContain('msg-1msg-1'); + // Expected scopes: $rootScope, ngIf, transclusion, ngRepeat + expect(countScopes($rootScope)).toEqual(4); + + $rootScope.$apply('t = false'); + expect(element.text()).not.toContain('msg-1msg-1'); + // Expected scopes: $rootScope + expect(countScopes($rootScope)).toEqual(1); + })); + + + it('should not leak the transclude scope if the transcluded contains only comments', + inject(function($compile, $rootScope) { + + element = $compile( + '
' + + '' + + '
' + )($rootScope); + + $rootScope.$apply('t = true'); + expect(element.html()).toContain('some comment'); + // Expected scopes: $rootScope, ngIf, transclusion + expect(countScopes($rootScope)).toEqual(3); + + $rootScope.$apply('t = false'); + expect(element.html()).not.toContain('some comment'); + // Expected scopes: $rootScope + expect(countScopes($rootScope)).toEqual(1); + + $rootScope.$apply('t = true'); + expect(element.html()).toContain('some comment'); + // Expected scopes: $rootScope, ngIf, transclusion + expect(countScopes($rootScope)).toEqual(3); + + $rootScope.$apply('t = false'); + expect(element.html()).not.toContain('some comment'); + // Expected scopes: $rootScope + expect(countScopes($rootScope)).toEqual(1); + })); + + it('should not leak the transclude scope if the transcluded contains only text nodes', + inject(function($compile, $rootScope) { + + element = $compile( + '
' + + 'some text' + + '
' + )($rootScope); + + $rootScope.$apply('t = true'); + expect(element.html()).toContain('some text'); + // Expected scopes: $rootScope, ngIf, transclusion + expect(countScopes($rootScope)).toEqual(3); + + $rootScope.$apply('t = false'); + expect(element.html()).not.toContain('some text'); + // Expected scopes: $rootScope + expect(countScopes($rootScope)).toEqual(1); + + $rootScope.$apply('t = true'); + expect(element.html()).toContain('some text'); + // Expected scopes: $rootScope, ngIf, transclusion + expect(countScopes($rootScope)).toEqual(3); + + $rootScope.$apply('t = false'); + expect(element.html()).not.toContain('some text'); + // Expected scopes: $rootScope + expect(countScopes($rootScope)).toEqual(1); + })); + + it('should mark as destroyed all sub scopes of the scope being destroyed', + inject(function($compile, $rootScope) { + + element = $compile( + '
' + + '
{{ msg }}
' + + '
' + )($rootScope); + + $rootScope.$apply('t = true'); + var childScopes = getChildScopes($rootScope); + + $rootScope.$apply('t = false'); + for (var i = 0; i < childScopes.length; ++i) { + expect(childScopes[i].$$destroyed).toBe(true); + } + })); + }); + + describe('nested transcludes', function() { beforeEach(module(function($compileProvider) { @@ -4165,6 +4451,29 @@ describe('$compile', function() { $rootScope.$digest(); expect(element.text()).toEqual('transcluded content'); })); + + + it('should not leak memory with nested transclusion', function() { + inject(function($compile, $rootScope) { + var size; + + expect(jqLiteCacheSize()).toEqual(0); + + element = jqLite('
'); + $compile(element)($rootScope.$new()); + + $rootScope.nums = [0,1,2]; + $rootScope.$apply(); + size = jqLiteCacheSize(); + + $rootScope.nums = [3,4,5]; + $rootScope.$apply(); + expect(jqLiteCacheSize()).toEqual(size); + + element.remove(); + expect(jqLiteCacheSize()).toEqual(0); + }); + }); });