fix(core): markDirty() should only mark flags when really scheduling …

…tick.

Close angular#39296

Fix an issue that `markDirty()` will not trigger change detection.

The case is for example we have the following component.

```
export class AppComponent implements OnInit {
  constructor(private router: Router) {}

  ngOnInit() {
    this.router.events
      .pipe(filter((e) => e instanceof NavigationEnd))
      .subscribe(() => ɵmarkDirty(this));
  }
}

export class CounterComponent implements OnInit, OnDestroy {
  ngOnInit() {
    this.countSubject.pipe(takeUntil(this.destroy)).subscribe((count) => {
      this.count = count;
      ɵmarkDirty(this);
    });
  }
```

Then the app navigate from `AppComponent` to `CounterComponent`,
so there are 2 `markDirty()` call at in a row.

The `1st` call is from `AppComponent` when router changed, the
`2nd` call is from `CounterComponent.ngOnInit()`.

And the `markDirty()->scheduleTick()` code look like this

```
function scheduleTick(rootContext, flags) {
    const nothingScheduled = rootContext.flags === 0 /* Empty */;
    rootContext.flags |= flags;
    if (nothingScheduled && rootContext.clean == _CLEAN_PROMISE) {
      rootContext.schedule(() => {
	...
        if (rootContext.flags & RootContextFlags.DetectChanges)
          rootContext.flags &= ~RootContextFlags.DetectChanges;
          tickContext();

        rootContext.clean = _CLEAN_PROMISE;
        ...
      });
```

So in this case, the `1st` markDirty() will
1. set rootContext.flags = 1
2. before `tickContext()`, reset rootContext.flags = 0
3. inside `tickContext()`, it will call `CounterComponent.ngOnint()`,
   so the `2nd` markDirty() is called.
4. and the `2nd` scheduleTick is called, `nothingScheduled` is true,
   but rootContext.clean is not `_CLEAN_PROMISE` yet, since the `1st` markDirty tick
   is still running.
5. So nowhere will reset the `rootContext.flags`.
6. then in the future, any other `markDirty()` call will not trigger the tick, since
   `nothingScheduled` is always false.

So `nothingScheduled` means no tick is scheduled, `rootContext.clean === _CLEAN_PROMISE`
means no tick is running.
So we should set the flags to `rootContext` only when `no tick is scheudled or running`.

packages/core/src/render3/instructions/shared.ts

@@ -1863,9 +1863,10 @@ export function markViewDirty(lView: LView): LView|null {
  */
 export function scheduleTick(rootContext: RootContext, flags: RootContextFlags) {
   const nothingScheduled = rootContext.flags === RootContextFlags.Empty;
-  rootContext.flags |= flags;
-
   if (nothingScheduled && rootContext.clean == _CLEAN_PROMISE) {
+    // https://github.com/angular/angular/issues/39296
+    // should only attach the flags when really scheduling a tick
+    rootContext.flags |= flags;
     let res: null|((val: null) => void);
     rootContext.clean = new Promise<null>((r) => res = r);
     rootContext.scheduler(() => {

packages/core/test/render3/change_detection_spec.ts

@@ -8,18 +8,20 @@
 
 import {withBody} from '@angular/private/testing';
 
-import {ChangeDetectionStrategy, DoCheck} from '../../src/core';
+import {ChangeDetectionStrategy, DoCheck, OnInit} from '../../src/core';
 import {whenRendered} from '../../src/render3/component';
-import {getRenderedText, LifecycleHooksFeature, ɵɵadvance, ɵɵdefineComponent, ɵɵgetCurrentView, ɵɵproperty, ɵɵtextInterpolate1, ɵɵtextInterpolate2} from '../../src/render3/index';
-import {detectChanges, markDirty, tick, ɵɵelement, ɵɵelementEnd, ɵɵelementStart, ɵɵlistener, ɵɵtext, ɵɵtextInterpolate} from '../../src/render3/instructions/all';
+import {AttributeMarker, getRenderedText, LifecycleHooksFeature, ɵɵadvance, ɵɵdefineComponent, ɵɵgetCurrentView, ɵɵproperty, ɵɵtextInterpolate1, ɵɵtextInterpolate2} from '../../src/render3/index';
+import {detectChanges, markDirty, tick, ɵɵelement, ɵɵelementEnd, ɵɵelementStart, ɵɵlistener, ɵɵtemplate, ɵɵtext, ɵɵtextInterpolate} from '../../src/render3/instructions/all';
 import {RenderFlags} from '../../src/render3/interfaces/definition';
 import {Renderer3, RendererFactory3} from '../../src/render3/interfaces/renderer';
 import {FLAGS, LViewFlags} from '../../src/render3/interfaces/view';
 
+import {NgIf} from './common_with_def';
 import {containerEl, createComponent, renderComponent, requestAnimationFrame} from './render_util';
 
 describe('change detection', () => {
   describe('markDirty, detectChanges, whenRendered, getRenderedText', () => {
+    let mycompOninit: MyComponentWithOnInit;
     class MyComponent implements DoCheck {
       value: string = 'works';
       doCheckCount = 0;
@@ -48,6 +50,84 @@ describe('change detection', () => {
       });
     }
 
+    class MyComponentWithOnInit implements OnInit, DoCheck {
+      value: string = 'works';
+      doCheckCount = 0;
+
+      ngOnInit() {
+        markDirty(this);
+      }
+
+      ngDoCheck(): void {
+        this.doCheckCount++;
+      }
+
+      click() {
+        this.value = 'click works';
+        markDirty(this);
+      }
+
+      static ɵfac = () => mycompOninit = new MyComponentWithOnInit();
+      static ɵcmp = ɵɵdefineComponent({
+        type: MyComponentWithOnInit,
+        selectors: [['my-comp-oninit']],
+        decls: 2,
+        vars: 1,
+        template:
+            (rf: RenderFlags, ctx: MyComponentWithOnInit) => {
+              if (rf & RenderFlags.Create) {
+                ɵɵelementStart(0, 'span');
+                ɵɵtext(1);
+                ɵɵelementEnd();
+              }
+              if (rf & RenderFlags.Update) {
+                ɵɵadvance(1);
+                ɵɵtextInterpolate(ctx.value);
+              }
+            }
+      });
+    }
+
+    class MyParentComponent implements OnInit {
+      show = false;
+      value = 'parent';
+      mycomp: any = undefined;
+
+      ngOnInit() {}
+
+      click() {
+        this.show = true;
+        markDirty(this);
+      }
+
+      static ɵfac = () => new MyParentComponent();
+      static ɵcmp = ɵɵdefineComponent({
+        type: MyParentComponent,
+        selectors: [['my-parent-comp']],
+        decls: 2,
+        vars: 1,
+        directives: [NgIf, MyComponentWithOnInit],
+        consts: [[AttributeMarker.Template, 'ngIf']],
+        template:
+            (rf: RenderFlags, ctx: MyParentComponent) => {
+              if (rf & RenderFlags.Create) {
+                ɵɵtext(0, ' -->\n');
+                ɵɵtemplate(1, (rf, ctx) => {
+                  if (rf & RenderFlags.Create) {
+                    ɵɵelementStart(0, 'div');
+                    ɵɵelement(1, 'my-comp-oninit');
+                    ɵɵelementEnd();
+                  }
+                }, 2, 0, 'div', 0);
+              }
+              if (rf & RenderFlags.Update) {
+                ɵɵadvance(1);
+                ɵɵproperty('ngIf', ctx.show);
+              }
+            }
+      })
+    }
+
     it('should mark a component dirty and schedule change detection', withBody('my-comp', () => {
          const myComp = renderComponent(MyComponent, {hostFeatures: [LifecycleHooksFeature]});
          expect(getRenderedText(myComp)).toEqual('works');
@@ -66,6 +146,24 @@ describe('change detection', () => {
          expect(getRenderedText(myComp)).toEqual('updated');
        }));
 
+    fit('should detectChanges after markDirty is called multiple times within ngOnInit',
+        withBody('my-comp-oninit', () => {
+          const myParentComp =
+              renderComponent(MyParentComponent, {hostFeatures: [LifecycleHooksFeature]});
+          expect(myParentComp.show).toBe(false);
+          myParentComp.click();
+          requestAnimationFrame.flush();
+          expect(myParentComp.show).toBe(true);
+          const myComp = mycompOninit;
+          expect(getRenderedText(myComp)).toEqual('works');
+          expect(myComp.doCheckCount).toBe(1);
+          myComp.click();
+          expect(getRenderedText(myComp)).toEqual('works');
+          requestAnimationFrame.flush();
+          expect(getRenderedText(myComp)).toEqual('click works');
+          expect(myComp.doCheckCount).toBe(2);
+        }));
+
     it('should detectChanges only once if markDirty is called multiple times',
        withBody('my-comp', () => {
          const myComp = renderComponent(MyComponent, {hostFeatures: [LifecycleHooksFeature]});