From a1beba4b6e3b6ca020765aa6be01ce31eb35be54 Mon Sep 17 00:00:00 2001
From: Andrew Kushnir <akushnir@google.com>
Date: Fri, 6 Sep 2019 16:48:13 -0700
Subject: [PATCH] fix(ivy): restore global state after running refreshView
 (#32521)

Prior to this commit, the `previousOrParentTNode` was set to null after performing all operations within `refreshView` function. It's causing problems in more complex scenarios, for example when change detection is triggered during DI (see test added as a part of this commit). As a result, global state might be corrupted. This commit captures current value of `previousOrParentTNode` and restores it after `refreshView` call.

PR Close #32521
---
 .../core/src/render3/instructions/shared.ts   |  6 ++
 packages/core/test/acceptance/di_spec.ts      | 66 ++++++++++++++++++-
 2 files changed, 71 insertions(+), 1 deletion(-)

diff --git a/packages/core/src/render3/instructions/shared.ts b/packages/core/src/render3/instructions/shared.ts
index d438d70ea1..05da849d7b 100644
--- a/packages/core/src/render3/instructions/shared.ts
+++ b/packages/core/src/render3/instructions/shared.ts
@@ -471,6 +471,8 @@ export function renderComponentOrTemplate<T>(
   const rendererFactory = hostView[RENDERER_FACTORY];
   const normalExecutionPath = !getCheckNoChangesMode();
   const creationModeIsActive = isCreationMode(hostView);
+  const previousOrParentTNode = getPreviousOrParentTNode();
+  const isParent = getIsParent();
   try {
     if (normalExecutionPath && !creationModeIsActive && rendererFactory.begin) {
       rendererFactory.begin();
@@ -484,6 +486,7 @@ export function renderComponentOrTemplate<T>(
     if (normalExecutionPath && !creationModeIsActive && rendererFactory.end) {
       rendererFactory.end();
     }
+    setPreviousOrParentTNode(previousOrParentTNode, isParent);
   }
 }
 
@@ -1642,6 +1645,8 @@ export function tickRootContext(rootContext: RootContext) {
 
 export function detectChangesInternal<T>(view: LView, context: T) {
   const rendererFactory = view[RENDERER_FACTORY];
+  const previousOrParentTNode = getPreviousOrParentTNode();
+  const isParent = getIsParent();
 
   if (rendererFactory.begin) rendererFactory.begin();
   try {
@@ -1652,6 +1657,7 @@ export function detectChangesInternal<T>(view: LView, context: T) {
     throw error;
   } finally {
     if (rendererFactory.end) rendererFactory.end();
+    setPreviousOrParentTNode(previousOrParentTNode, isParent);
   }
 }
 
diff --git a/packages/core/test/acceptance/di_spec.ts b/packages/core/test/acceptance/di_spec.ts
index b37b520feb..48fdc21415 100644
--- a/packages/core/test/acceptance/di_spec.ts
+++ b/packages/core/test/acceptance/di_spec.ts
@@ -7,11 +7,12 @@
  */
 
 import {CommonModule} from '@angular/common';
-import {Attribute, ChangeDetectorRef, Component, Directive, ElementRef, EventEmitter, Host, HostBinding, INJECTOR, Inject, Injectable, InjectionToken, Injector, Input, LOCALE_ID, ModuleWithProviders, NgModule, Optional, Output, Pipe, PipeTransform, Self, SkipSelf, TemplateRef, ViewChild, ViewContainerRef, forwardRef, ɵDEFAULT_LOCALE_ID as DEFAULT_LOCALE_ID} from '@angular/core';
+import {Attribute, ChangeDetectorRef, Component, ComponentFactoryResolver, ComponentRef, Directive, ElementRef, EventEmitter, Host, HostBinding, INJECTOR, Inject, Injectable, InjectionToken, Injector, Input, LOCALE_ID, ModuleWithProviders, NgModule, NgZone, Optional, Output, Pipe, PipeTransform, Self, SkipSelf, TemplateRef, ViewChild, ViewContainerRef, forwardRef, ɵDEFAULT_LOCALE_ID as DEFAULT_LOCALE_ID} from '@angular/core';
 import {ɵINJECTOR_SCOPE} from '@angular/core/src/core';
 import {ViewRef} from '@angular/core/src/render3/view_ref';
 import {TestBed} from '@angular/core/testing';
 import {ivyEnabled, onlyInIvy} from '@angular/private/testing';
+import {BehaviorSubject} from 'rxjs';
 
 describe('di', () => {
   describe('no dependencies', () => {
@@ -1118,6 +1119,69 @@ describe('di', () => {
            // the nativeElement should be a comment
            expect(directive.elementRef.nativeElement.nodeType).toEqual(Node.COMMENT_NODE);
          });
+
+      it('should be available if used in conjunction with other tokens', () => {
+        @Injectable()
+        class ServiceA {
+          subject: any;
+          constructor(protected zone: NgZone) {
+            this.subject = new BehaviorSubject<any>(1);
+            // trigger change detection
+            zone.run(() => { this.subject.next(2); });
+          }
+        }
+
+        @Directive({selector: '[dir]'})
+        class DirectiveA {
+          constructor(public service: ServiceA, public elementRef: ElementRef) {}
+        }
+
+        @Component({
+          selector: 'child',
+          template: `<div id="test-id" dir></div>`,
+        })
+        class ChildComp {
+          @ViewChild(DirectiveA, {static: false}) directive !: DirectiveA;
+        }
+
+        @Component({
+          selector: 'root',
+          template: '...',
+        })
+        class RootComp {
+          public childCompRef !: ComponentRef<ChildComp>;
+
+          constructor(
+              public factoryResolver: ComponentFactoryResolver, public vcr: ViewContainerRef) {}
+
+          create() {
+            const factory = this.factoryResolver.resolveComponentFactory(ChildComp);
+            this.childCompRef = this.vcr.createComponent(factory);
+            this.childCompRef.changeDetectorRef.detectChanges();
+          }
+        }
+
+        // this module is needed, so that View Engine can generate factory for ChildComp
+        @NgModule({
+          declarations: [DirectiveA, RootComp, ChildComp],
+          entryComponents: [RootComp, ChildComp],
+        })
+        class ModuleA {
+        }
+
+        TestBed.configureTestingModule({
+          imports: [ModuleA],
+          providers: [ServiceA],
+        });
+
+        const fixture = TestBed.createComponent(RootComp);
+        fixture.autoDetectChanges();
+
+        fixture.componentInstance.create();
+
+        const {elementRef} = fixture.componentInstance.childCompRef.instance.directive;
+        expect(elementRef.nativeElement.id).toBe('test-id');
+      });
     });
 
     describe('TemplateRef', () => {