fix(core): Microtask scheduling should be used after any application synchronization

Previously, Angular would switch from the macrotask to a microtask
scheduler _only_ when the scheduler was the trigger for the
synchronization. This microtask scheduling is to ensure patterns such as
`Promise.resolve().then(() => updateAppStateAgain())` _during_
synchronization are caught and synchronized again within the same event
loop (guaranteeing that they aren't split across multiple browser paints).

The microtask scheduler should be used after any tick, not just from
those than run within the scheduler to always account for the promises
within synchronization. This is encountered most frequently during
bootstrap, which triggers the tick directly.

In this change we exempt `TestBed.tick` and
`ComponentFixture.detectChanges` from this behavior. Doing so would affect
the timing of stability and tests are quite sensitive to this (e.g.
`fixture.whenStable`). It is somewhat unfortunate that we have "special" test-only
behavior. However, it is important to acknowledge that this only affects
the test-only APIs as well. Any code in the application under test that
triggers `ApplicationRef.tick` directly would still use the microtask
scheduling behavior.

fixes angular#65444

Author: atscott

packages/core/src/application/application_ref.ts

@@ -293,7 +293,8 @@ export class ApplicationRef {
   // Eventually the hostView of the fixture should just attach to ApplicationRef.
   private allTestViews: Set<InternalViewRef<unknown>> = new Set();
   private autoDetectTestViews: Set<InternalViewRef<unknown>> = new Set();
-  private includeAllTestViews = false;
+  /** @internal */
+  includeAllTestViews = false;
   /** @internal */
   afterTick = new Subject<void>();
   /** @internal */

packages/core/src/change_detection/scheduling/zoneless_scheduling_impl.ts

@@ -82,12 +82,28 @@ export class ChangeDetectionSchedulerImpl implements ChangeDetectionScheduler {
   constructor() {
     this.subscriptions.add(
       this.appRef.afterTick.subscribe(() => {
+        // Prevent stabilization if cleanup causes the last task to be removed
+        // before we can switch to the microtask scheduler.
+        const task = this.taskService.add();
         // If the scheduler isn't running a tick but the application ticked, that means
         // someone called ApplicationRef.tick manually. In this case, we should cancel
         // any change detections that had been scheduled so we don't run an extra one.
         if (!this.runningTick) {
           this.cleanup();
+          // Ticks that happen when ZoneJS is present do not get the microtask scheduling treatment.
+          // ZoneJS is responsible for rerunning change detection on microtask queue empty.
+          // Ticks initiated from tests also do not get microtask treatment so those ticks
+          // do not affect stability timing, which tests are quite sensitive to.
+          // TODO(atscott): we really should not use microtask scheduler
+          // _ever_ when ZoneJS is enabled because ZoneJS is responsible for rerunning change
+          // detection on microtask queue empty. This change breaks some tests
+          if (!this.zonelessEnabled || this.appRef.includeAllTestViews) {
+            this.taskService.remove(task);
+            return;
+          }
         }
+        this.switchToMicrotaskScheduler();
+        this.taskService.remove(task);
       }),
     );
     this.subscriptions.add(
@@ -102,6 +118,22 @@ export class ChangeDetectionSchedulerImpl implements ChangeDetectionScheduler {
     );
   }
 
+  // If we're notified of a change within 1 microtask of running change
+  // detection, run another round in the same event loop. This allows code
+  // which uses Promise.resolve (see NgModel) to avoid
+  // ExpressionChanged...Error to still be reflected in a single browser
+  // paint, even if that spans multiple rounds of change detection.
+  private switchToMicrotaskScheduler(): void {
+    this.ngZone.runOutsideAngular(() => {
+      const task = this.taskService.add();
+      this.useMicrotaskScheduler = true;
+      queueMicrotask(() => {
+        this.useMicrotaskScheduler = false;
+        this.taskService.remove(task);
+      });
+    });
+  }
+
   notify(source: NotificationSource): void {
     if (!this.zonelessEnabled && source === NotificationSource.Listener) {
       // When the notification comes from a listener, we skip the notification unless the
@@ -264,21 +296,11 @@ export class ChangeDetectionSchedulerImpl implements ChangeDetectionScheduler {
         this.schedulerTickApplyArgs,
       );
     } catch (e: unknown) {
-      this.taskService.remove(task);
       this.applicationErrorHandler(e);
     } finally {
+      this.taskService.remove(task);
       this.cleanup();
     }
-    // If we're notified of a change within 1 microtask of running change
-    // detection, run another round in the same event loop. This allows code
-    // which uses Promise.resolve (see NgModel) to avoid
-    // ExpressionChanged...Error to still be reflected in a single browser
-    // paint, even if that spans multiple rounds of change detection.
-    this.useMicrotaskScheduler = true;
-    scheduleCallbackWithMicrotask(() => {
-      this.useMicrotaskScheduler = false;
-      this.taskService.remove(task);
-    });
   }
 
   ngOnDestroy() {

packages/core/test/acceptance/pending_tasks_spec.ts

@@ -78,6 +78,9 @@ describe('public PendingTasks', () => {
     // stability is delayed until a tick happens
     await expectAsync(applicationRefIsStable(appRef)).toBeResolvedTo(false);
     TestBed.inject(ApplicationRef).tick();
+    // Stability is not synchronous after a tick. We wait for a microtask
+    // in case there is a Promise inside tick that requires tick again
+    await Promise.resolve();
     await expectAsync(applicationRefIsStable(appRef)).toBeResolvedTo(true);
   });
 

packages/core/test/render3/reactivity_spec.ts

@@ -91,6 +91,7 @@ describe('reactivity', () => {
       expect(isStable).toEqual([true, false]);
 
       appRef.tick();
+      await appRef.whenStable();
 
       expect(isStable).toEqual([true, false, true]);
     });

packages/core/testing/src/test_bed.ts

@@ -880,6 +880,8 @@ export class TestBedImpl implements TestBed {
       // The behavior should be that TestBed.tick, ComponentFixture.detectChanges, and ApplicationRef.tick all result in the test fixtures
       // getting synchronized, regardless of whether they are autoDetect: true.
       // Automatic scheduling (zone or zoneless) will call _tick which will _not_ include fixtures with autoDetect: false
+      // If this does get changed, we will need a new flag for the scheduler to use to omit the microtask scheduling
+      // from a tick initiated by tests.
       (appRef as any).includeAllTestViews = true;
       appRef.tick();
     } finally {