From b342a69bbcff77cf478bcae1cd45361367dc4e5c Mon Sep 17 00:00:00 2001
From: Andrew Kushnir <akushnir@google.com>
Date: Wed, 4 Dec 2019 14:10:50 -0800
Subject: [PATCH] fix(ivy): do not invoke change detection for destroyed views
 (#34241)

Prior to this commit, calling change detection for destroyed views resulted in errors being thrown in some cases. This commit adds a check to make sure change detection is invoked for non-destroyed views only.

PR Close #34241
---
 .../core/src/render3/instructions/shared.ts   |  3 +-
 .../core/test/acceptance/lifecycle_spec.ts    | 55 ++++++++++++++++++-
 2 files changed, 56 insertions(+), 2 deletions(-)

diff --git a/packages/core/src/render3/instructions/shared.ts b/packages/core/src/render3/instructions/shared.ts
index bc2427fd8a..c61daf9d48 100644
--- a/packages/core/src/render3/instructions/shared.ts
+++ b/packages/core/src/render3/instructions/shared.ts
@@ -380,8 +380,9 @@ export function renderView<T>(lView: LView, tView: TView, context: T): void {
 export function refreshView<T>(
     lView: LView, tView: TView, templateFn: ComponentTemplate<{}>| null, context: T) {
   ngDevMode && assertEqual(isCreationMode(lView), false, 'Should be run in update mode');
-  enterView(lView, lView[T_HOST]);
   const flags = lView[FLAGS];
+  if ((flags & LViewFlags.Destroyed) === LViewFlags.Destroyed) return;
+  enterView(lView, lView[T_HOST]);
   try {
     resetPreOrderHookFlags(lView);
 
diff --git a/packages/core/test/acceptance/lifecycle_spec.ts b/packages/core/test/acceptance/lifecycle_spec.ts
index 359be58390..ebfd9aa7f8 100644
--- a/packages/core/test/acceptance/lifecycle_spec.ts
+++ b/packages/core/test/acceptance/lifecycle_spec.ts
@@ -7,7 +7,7 @@
  */
 
 import {CommonModule} from '@angular/common';
-import {Component, ComponentFactoryResolver, Directive, Input, NgModule, OnChanges, SimpleChanges, ViewChild, ViewContainerRef} from '@angular/core';
+import {ChangeDetectorRef, Component, ComponentFactoryResolver, ContentChildren, Directive, Input, NgModule, OnChanges, QueryList, SimpleChanges, TemplateRef, ViewChild, ViewContainerRef} from '@angular/core';
 import {SimpleChange} from '@angular/core/src/core';
 import {TestBed} from '@angular/core/testing';
 import {By} from '@angular/platform-browser';
@@ -3597,6 +3597,59 @@ describe('onDestroy', () => {
     expect(events).toEqual(['comp']);
   });
 
+  it('should not produce errors if change detection is triggered during ngOnDestroy', () => {
+    @Component({selector: 'child', template: `<ng-content></ng-content>`})
+    class Child {
+    }
+
+    @Component({selector: 'parent', template: `<ng-content></ng-content>`})
+    class Parent {
+      @ContentChildren(Child, {descendants: true}) child !: QueryList<Child>;
+    }
+
+    @Component({
+      selector: 'app',
+      template: `
+        <ng-template #tpl>
+          <parent>
+            <child></child>
+          </parent>
+        </ng-template>
+        <div #container dir></div>
+      `
+    })
+    class App {
+      @ViewChild('container', {read: ViewContainerRef, static: true})
+      container !: ViewContainerRef;
+
+      @ViewChild('tpl', {read: TemplateRef, static: true})
+      tpl !: TemplateRef<any>;
+
+      ngOnInit() { this.container.createEmbeddedView(this.tpl); }
+    }
+
+    @Directive({selector: '[dir]'})
+    class Dir {
+      constructor(public cdr: ChangeDetectorRef) {}
+
+      ngOnDestroy() {
+        // Important: calling detectChanges in destroy hook like that
+        // doesn’t have practical purpose, but in real-world cases it might
+        // happen, for example as a result of "focus()" call on a DOM element,
+        // in case ZoneJS is active.
+        this.cdr.detectChanges();
+      }
+    }
+
+    TestBed.configureTestingModule({
+      declarations: [App, Parent, Child, Dir],
+    });
+    const fixture = TestBed.createComponent(App);
+    fixture.autoDetectChanges();
+
+    expect(() => fixture.destroy()).not.toThrow();
+  });
+
   onlyInIvy(
       'View Engine has the opposite behavior, where it calls destroy on the directives first, then the components')
       .it('should be called on directives after component', () => {