fix($compile): do not rebind parent bound transclude functions

Do not rebind parent bound transclude functions passed to the
link function returned from `$compile`. Change parameter
order of ```publicLinkFn``` to allow parent bound transclude
functions to be passed (do not make public yet).

The current `$compile` public API documentation indicates that
a transclude function may be passed as a second parameter, but
it is not clear that this is not the same function that is given
to directive link functions as the `transcludeFn` parameter. This
parameter must be passed in order to implement, for example,
lazy compilation.

In order to pass the `transcludeFn` parameter given to
a directive's link function, it must be passed to the
result of a call to `$compile` (i.e. `publicLinkFn`). Doing so,
however, would cause two bugs. First, the passed transclude
function would get rebound, improperly changing its scope
from its original binding. Second, the `containingScope` would
not be updated and the wrong `$parent` would be assigned to
the `transcludedScope` resulting in a memory leak. This patch
fixes both of these issues.

This patch does not expose a new public API for `publicLinkFn`,
rather, it leaves this to a later change that better considers
how that public API should look.

Thanks to @lgalfaso, @tbosch, and @petebacondarwin.

Related to angular#9413

Author: dlongley

src/ng/compile.js

@@ -1153,7 +1153,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
                            maxPriority, ignoreDirective, previousCompileContext);
       compile.$$addScopeClass($compileNodes);
       var namespace = null;
-      return function publicLinkFn(scope, cloneConnectFn, transcludeControllers, parentBoundTranscludeFn, futureParentElement){
+      return function publicLinkFn(scope, cloneConnectFn, parentBoundTranscludeFn, transcludeControllers, futureParentElement){
         assertArg(scope, 'scope');
         if (!namespace) {
           namespace = detectNamespaceForChildElements(futureParentElement);
@@ -1324,7 +1324,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
           transcludedScope.$$transcluded = true;
         }
 
-        return transcludeFn(transcludedScope, cloneFn, controllers, previousBoundTranscludeFn, futureParentElement);
+        return transcludeFn(transcludedScope, cloneFn, previousBoundTranscludeFn, controllers, futureParentElement);
       };
 
       return boundTranscludeFn;
@@ -1793,7 +1793,11 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
           isolateScope = scope.$new(true);
         }
 
-        transcludeFn = boundTranscludeFn && controllersBoundTransclude;
+        if (boundTranscludeFn) {
+          transcludeFn = controllersBoundTransclude;
+          controllersBoundTransclude.$$boundTransclude = boundTranscludeFn;
+        }
+
         if (controllerDirectives) {
           // TODO: merge `controllers` and `elementControllers` into single object.
           controllers = {};
@@ -1965,7 +1969,13 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
           if (!futureParentElement) {
             futureParentElement = hasElementTranscludeDirective ? $element.parent() : $element;
           }
-          return boundTranscludeFn(scope, cloneAttachFn, transcludeControllers, futureParentElement, scopeToChild);
+
+          // When a transcludeFn is passed to publicLinkFn, it is already
+          // a `controllersBoundTransclude` function, so prevent double-wrapping
+          // by using the previously-wrapped boundTranscludeFn.
+          var unwrappedFn = boundTranscludeFn.$$boundTransclude || boundTranscludeFn;
+          controllersBoundTransclude.$$boundTransclude = unwrappedFn;
+          return unwrappedFn(scope, cloneAttachFn, transcludeControllers, futureParentElement, scopeToChild);
         }
       }
     }

test/ng/compileSpec.js

@@ -5124,6 +5124,119 @@ describe('$compile', function() {
       });
 
 
+      // see issue https://github.com/angular/angular.js/issues/9413
+      describe('passing a parent bound transclude function to the link ' +
+          'function returned from `$compile`', 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('lazyCompile', function($compile) {
+            return {
+              compile: function(tElement, tAttrs) {
+                var content = tElement.contents();
+                tElement.empty();
+                return function(scope, element, attrs, ctrls, transcludeFn) {
+                  element.append(content);
+                  $compile(content)(scope, undefined, transcludeFn);
+                };
+              }
+            };
+          });
+          directive('toggle', function() {
+            return {
+              scope: {t: '=toggle'},
+              transclude: true,
+              template: '<div ng-if="t"><lazy-compile><div ng-transclude></div></lazy-compile></div>'
+            };
+          });
+        }));
+
+        it('should preserve the bound scope', function() {
+
+          inject(function($compile, $rootScope) {
+            element = $compile(
+              '<div>' +
+                '<div ng-init="outer=true"></div>' +
+                '<div toggle="t">' +
+                  '<span ng-if="outer">Success</span><span ng-if="!outer">Error</span>' +
+                '</div>' +
+              '</div>')($rootScope);
+
+            $rootScope.$apply('t = false');
+            expect(countScopes($rootScope)).toBe(2);
+            expect(element.text()).toBe('');
+
+            $rootScope.$apply('t = true');
+            expect(countScopes($rootScope)).toBe(5);
+            expect(element.text()).toBe('Success');
+
+            $rootScope.$apply('t = false');
+            expect(countScopes($rootScope)).toBe(2);
+            expect(element.text()).toBe('');
+
+            $rootScope.$apply('t = true');
+            expect(countScopes($rootScope)).toBe(5);
+            expect(element.text()).toBe('Success');
+          });
+        });
+
+
+        it('should preserve the bound scope when using recursive transclusion', function() {
+
+          directive('recursiveTransclude', function() {
+            return {
+              transclude: true,
+              template: '<div><lazy-compile><div ng-transclude></div></lazy-compile></div>'
+            };
+          });
+
+          inject(function($compile, $rootScope) {
+            element = $compile(
+              '<div>' +
+                '<div ng-init="outer=true"></div>' +
+                '<div toggle="t">' +
+                  '<div recursive-transclude>' +
+                    '<span ng-if="outer">Success</span><span ng-if="!outer">Error</span>' +
+                  '</div>' +
+                '</div>' +
+              '</div>')($rootScope);
+
+            $rootScope.$apply('t = false');
+            expect(countScopes($rootScope)).toBe(2);
+            expect(element.text()).toBe('');
+
+            $rootScope.$apply('t = true');
+            expect(countScopes($rootScope)).toBe(5);
+            expect(element.text()).toBe('Success');
+
+            $rootScope.$apply('t = false');
+            expect(countScopes($rootScope)).toBe(2);
+            expect(element.text()).toBe('');
+
+            $rootScope.$apply('t = true');
+            expect(countScopes($rootScope)).toBe(5);
+            expect(element.text()).toBe('Success');
+          });
+        });
+      });
+
+
       // see issue https://github.com/angular/angular.js/issues/9095
       describe('removing a transcluded element', function() {