From e2b1e1577d37e443452b0f16386b6f1279dda04d Mon Sep 17 00:00:00 2001
From: Tobias Bosch <tbosch1009@gmail.com>
Date: Wed, 4 May 2016 11:53:12 -0700
Subject: [PATCH] =?UTF-8?q?fix(core):=20don=E2=80=99t=20detach=20nested=20?=
 =?UTF-8?q?view=20containers=20when=20destroying=20a=20view?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

When a view is destroyed, we destroy all
views in view containers and should not detach them. However, previously, we also detached them which lead to problems during the iteration loop.

Closes #8458
Closes #8471

Introduced by 0c600cf6e31b7f69f5a36f7dea959e4884217a4d
---
 modules/@angular/core/src/linker/view.ts      |  8 +----
 .../change_detection_integration_spec.ts      |  8 +++--
 .../core/test/linker/integration_spec.ts      | 31 ++++++++++++++++++-
 3 files changed, 37 insertions(+), 10 deletions(-)

diff --git a/modules/@angular/core/src/linker/view.ts b/modules/@angular/core/src/linker/view.ts
index 77d769574c..929c611934 100644
--- a/modules/@angular/core/src/linker/view.ts
+++ b/modules/@angular/core/src/linker/view.ts
@@ -192,13 +192,7 @@ export abstract class AppView<T> {
       ObservableWrapper.dispose(this.subscriptions[i]);
     }
     this.destroyInternal();
-    if (this._hasExternalHostElement) {
-      this.renderer.detachView(this.flatRootNodes);
-    } else if (isPresent(this.viewContainerElement)) {
-      this.viewContainerElement.detachView(this.viewContainerElement.nestedViews.indexOf(this));
-    } else {
-      this.dirtyParentQueriesInternal();
-    }
+    this.dirtyParentQueriesInternal();
     this.renderer.destroyView(hostElement, this.allNodes);
   }
 
diff --git a/modules/@angular/core/test/linker/change_detection_integration_spec.ts b/modules/@angular/core/test/linker/change_detection_integration_spec.ts
index fd4c7aef01..ccd6264be4 100644
--- a/modules/@angular/core/test/linker/change_detection_integration_spec.ts
+++ b/modules/@angular/core/test/linker/change_detection_integration_spec.ts
@@ -1005,14 +1005,18 @@ export function main() {
            }));
 
         it('should be called after processing the content and view children', fakeAsync(() => {
-             var ctx = createCompWithContentAndViewChild();
+            var ctx = createCompFixture(
+                '<div testDirective="parent"><div *ngFor="let x of [0,1]" testDirective="contentChild{{x}}"></div>' +
+                '<other-cmp></other-cmp></div>',
+                TestComponent,
+                tcb.overrideTemplate(AnotherComponent, '<div testDirective="viewChild"></div>'));
 
              ctx.detectChanges(false);
              ctx.destroy();
 
              expect(directiveLog.filter(['ngOnDestroy']))
                  .toEqual(
-                     ['contentChild.ngOnDestroy', 'viewChild.ngOnDestroy', 'parent.ngOnDestroy']);
+                     ['contentChild0.ngOnDestroy', 'contentChild1.ngOnDestroy', 'viewChild.ngOnDestroy', 'parent.ngOnDestroy']);
            }));
 
         it('should be called in reverse order so the child is always notified before the parent',
diff --git a/modules/@angular/core/test/linker/integration_spec.ts b/modules/@angular/core/test/linker/integration_spec.ts
index 8f4d57902a..11a28a3158 100644
--- a/modules/@angular/core/test/linker/integration_spec.ts
+++ b/modules/@angular/core/test/linker/integration_spec.ts
@@ -483,6 +483,35 @@ function declareTests(isJit: boolean) {
                });
          }));
 
+      it('should not detach views in ViewContainers when the parent view is destroyed.',
+         inject([TestComponentBuilder, AsyncTestCompleter], (tcb: TestComponentBuilder, async) => {
+           tcb.overrideView(
+                  MyComp, new ViewMetadata({
+                    template:
+                        '<div *ngIf="ctxBoolProp"><template some-viewport let-greeting="someTmpl"><span>{{greeting}}</span></template></div>',
+                    directives: [SomeViewport]
+                  }))
+
+               .createAsync(MyComp)
+               .then((fixture) => {
+                 fixture.debugElement.componentInstance.ctxBoolProp = true;
+                 fixture.detectChanges();
+
+                 var ngIfEl = fixture.debugElement.children[0];
+                 var someViewport:SomeViewport = ngIfEl.childNodes[0].inject(SomeViewport);
+                 expect(someViewport.container.length).toBe(2);
+                 expect(ngIfEl.children.length).toBe(2);
+
+                 fixture.debugElement.componentInstance.ctxBoolProp = false;
+                 fixture.detectChanges();
+
+                 expect(someViewport.container.length).toBe(2);
+                 expect(fixture.debugElement.children.length).toBe(0);
+
+                 async.done();
+               });
+         }));
+
       it('should use a comment while stamping out `<template>` elements.',
          inject([TestComponentBuilder, AsyncTestCompleter], (tcb: TestComponentBuilder, async) => {
            tcb.overrideView(MyComp, new ViewMetadata({template: '<template></template>'}))
@@ -2165,7 +2194,7 @@ class SomeViewportContext {
 @Directive({selector: '[some-viewport]'})
 @Injectable()
 class SomeViewport {
-  constructor(container: ViewContainerRef, templateRef: TemplateRef<SomeViewportContext>) {
+  constructor(public container: ViewContainerRef, templateRef: TemplateRef<SomeViewportContext>) {
     container.createEmbeddedView(templateRef, new SomeViewportContext('hello'));
     container.createEmbeddedView(templateRef, new SomeViewportContext('again'));
   }