fix(core): error if detectChanges is called at the wrong time under specific circumstances (#40206)

Internally we store lifecycle hooks in the format `[index, hook, index, hook]` and when
iterating over them, we check one place ahead to figure out whether we've hit found
a hook or an index. The problem is that the loop is set up to iterate up to `hooks.length`
which means that we may go out of bounds on the last iteration, depending on where
we started. This appears to happen under a specific set of circumstances where a
directive calls `detectChanges` from an input setter while it has `ngOnChanges` and
`ngAfterViewInit` hooks.

These changes resolve the issue by only iterating up to `length - 1` which guarantees that
we can always look one place ahead.

This appears to have regressed some time in version 10.

Fixes #38611.

PR Close #40206

packages/core/src/render3/hooks.ts

@@ -212,9 +212,10 @@ function callHooks(
       (currentView[PREORDER_HOOK_FLAGS] & PreOrderHookFlags.IndexOfTheNextPreOrderHookMaskMask) :
       0;
   const nodeIndexLimit = currentNodeIndex != null ? currentNodeIndex : -1;
+  const max = arr.length - 1;  // Stop the loop at length - 1, because we look for the hook at i + 1
   let lastNodeIndexFound = 0;
-  for (let i = startIndex; i < arr.length; i++) {
-    const hook = arr[i + 1] as () => void;
+  for (let i = startIndex; i < max; i++) {
+    const hook = arr[i + 1] as number | (() => void);
     if (typeof hook === 'number') {
       lastNodeIndexFound = arr[i] as number;
       if (currentNodeIndex != null && lastNodeIndexFound >= currentNodeIndex) {
@@ -250,8 +251,7 @@ function callHook(currentView: LView, initPhase: InitPhaseState, arr: HookData,
   const directive = currentView[directiveIndex];
   if (isInitHook) {
     const indexWithintInitPhase = currentView[FLAGS] >> LViewFlags.IndexWithinInitPhaseShift;
-    // The init phase state must be always checked here as it may have been recursively
-    // updated
+    // The init phase state must be always checked here as it may have been recursively updated.
     if (indexWithintInitPhase <
             (currentView[PREORDER_HOOK_FLAGS] >> PreOrderHookFlags.NumberOfInitHooksCalledShift) &&
         (currentView[FLAGS] & LViewFlags.InitPhaseStateMask) === initPhase) {

packages/core/test/acceptance/lifecycle_spec.ts

@@ -7,8 +7,7 @@
  */
 
 import {CommonModule} from '@angular/common';
-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 {AfterViewInit, ChangeDetectorRef, Component, ComponentFactoryResolver, ContentChildren, Directive, DoCheck, Input, NgModule, OnChanges, QueryList, SimpleChange, SimpleChanges, TemplateRef, ViewChild, ViewContainerRef} from '@angular/core';
 import {TestBed} from '@angular/core/testing';
 import {By} from '@angular/platform-browser';
 import {onlyInIvy} from '@angular/private/testing';
@@ -4378,4 +4377,72 @@ describe('non-regression', () => {
 
     expect(destroyed).toBeTruthy();
   });
+
+  onlyInIvy('Use case is not supported in ViewEngine')
+      .it('should not throw when calling detectChanges from a setter in the presence of a data binding, ngOnChanges and ngAfterViewInit',
+          () => {
+            const hooks: string[] = [];
+
+            @Directive({selector: '[testDir]'})
+            class TestDirective implements OnChanges, AfterViewInit {
+              constructor(private _changeDetectorRef: ChangeDetectorRef) {}
+
+              @Input('testDir')
+              set value(_value: any) {
+                this._changeDetectorRef.detectChanges();
+              }
+              ngOnChanges() {
+                hooks.push('ngOnChanges');
+              }
+              ngAfterViewInit() {
+                hooks.push('ngAfterViewInit');
+              }
+            }
+
+            @Component({template: `<div [testDir]="value">{{value}}</div>`})
+            class App {
+              value = 1;
+            }
+
+            TestBed.configureTestingModule({declarations: [App, TestDirective]});
+            const fixture = TestBed.createComponent(App);
+            expect(() => fixture.detectChanges()).not.toThrow();
+            expect(hooks).toEqual(['ngOnChanges', 'ngAfterViewInit']);
+            expect(fixture.nativeElement.textContent.trim()).toBe('1');
+          });
+
+  onlyInIvy('Use case is not supported in ViewEngine')
+      .it('should call hooks in the correct order when calling detectChanges in a setter', () => {
+        const hooks: string[] = [];
+
+        @Directive({selector: '[testDir]'})
+        class TestDirective implements OnChanges, DoCheck, AfterViewInit {
+          constructor(private _changeDetectorRef: ChangeDetectorRef) {}
+
+          @Input('testDir')
+          set value(_value: any) {
+            this._changeDetectorRef.detectChanges();
+          }
+          ngOnChanges() {
+            hooks.push('ngOnChanges');
+          }
+          ngDoCheck() {
+            hooks.push('ngDoCheck');
+          }
+          ngAfterViewInit() {
+            hooks.push('ngAfterViewInit');
+          }
+        }
+
+        @Component({template: `<div [testDir]="value">{{value}}</div>`})
+        class App {
+          value = 1;
+        }
+
+        TestBed.configureTestingModule({declarations: [App, TestDirective]});
+        const fixture = TestBed.createComponent(App);
+        expect(() => fixture.detectChanges()).not.toThrow();
+        expect(hooks).toEqual(['ngOnChanges', 'ngDoCheck', 'ngAfterViewInit']);
+        expect(fixture.nativeElement.textContent.trim()).toBe('1');
+      });
 });