fix(aio): wait for the app to stabilize before registering the SW (#23093)

This commit also waits for the app to stabilize, before starting to
check for ServiceWorker updates. This avoids setting up a long timeout,
which would prevent the app from stabilizing and thus cause issues with
Protractor.

PR Close #23093

aio/src/app/sw-updates/sw-updates.service.spec.ts

@@ -1,4 +1,4 @@
-import { ReflectiveInjector } from '@angular/core';
+import { ApplicationRef, ReflectiveInjector } from '@angular/core';
 import { fakeAsync, tick } from '@angular/core/testing';
 import { NgServiceWorker } from '@angular/service-worker';
 import { Subject } from 'rxjs/Subject';
@@ -9,23 +9,26 @@ import { SwUpdatesService } from './sw-updates.service';
 
 describe('SwUpdatesService', () => {
   let injector: ReflectiveInjector;
+  let appRef: MockApplicationRef;
   let service: SwUpdatesService;
   let sw: MockNgServiceWorker;
   let checkInterval: number;
 
   // Helpers
   // NOTE:
-  //   Because `SwUpdatesService` uses the `debounceTime` operator, it needs to be instantiated
+  //   Because `SwUpdatesService` uses the `debounceTime` operator, it needs to be instantiated and
-  //   inside the `fakeAsync` zone (when `fakeAsync` is used for the test). Thus, we can't run
+  //   destroyed inside the `fakeAsync` zone (when `fakeAsync` is used for the test). Thus, we can't
-  //   `setup()` in a `beforeEach()` block. We use the `run()` helper to call `setup()` inside each
+  //   run `setup()`/`tearDown()` in `beforeEach()`/`afterEach()` blocks. We use the `run()` helper
-  //   test's zone.
+  //   to call them inside each test's zone.
   const setup = () => {
     injector = ReflectiveInjector.resolveAndCreate([
+      { provide: ApplicationRef, useClass: MockApplicationRef },
       { provide: Logger, useClass: MockLogger },
       { provide: NgServiceWorker, useClass: MockNgServiceWorker },
       SwUpdatesService
     ]);
 
+    appRef = injector.get(ApplicationRef);
     service = injector.get(SwUpdatesService);
     sw = injector.get(NgServiceWorker);
     checkInterval = (service as any).checkInterval;
@@ -42,11 +45,18 @@ describe('SwUpdatesService', () => {
     expect(service).toBeTruthy();
   }));
 
-  it('should immediately check for updates when instantiated', run(() => {
+  it('should start checking for updates when instantiated (once the app stabilizes)', run(() => {
+    expect(sw.checkForUpdate).not.toHaveBeenCalled();
+
+    appRef.isStable.next(false);
+    expect(sw.checkForUpdate).not.toHaveBeenCalled();
+
+    appRef.isStable.next(true);
     expect(sw.checkForUpdate).toHaveBeenCalled();
   }));
 
   it('should schedule a new check if there is no update available', fakeAsync(run(() => {
+    appRef.isStable.next(true);
     sw.checkForUpdate.calls.reset();
 
     sw.$$checkForUpdateSubj.next(false);
@@ -58,6 +68,7 @@ describe('SwUpdatesService', () => {
   })));
 
   it('should activate new updates immediately', fakeAsync(run(() => {
+    appRef.isStable.next(true);
     sw.checkForUpdate.calls.reset();
 
     sw.$$checkForUpdateSubj.next(true);
@@ -69,6 +80,7 @@ describe('SwUpdatesService', () => {
   })));
 
   it('should not pass a specific version to `NgServiceWorker.activateUpdate()`', fakeAsync(run(() => {
+    appRef.isStable.next(true);
     sw.$$checkForUpdateSubj.next(true);
     tick(checkInterval);
 
@@ -76,6 +88,7 @@ describe('SwUpdatesService', () => {
   })));
 
   it('should schedule a new check after activating the update', fakeAsync(run(() => {
+    appRef.isStable.next(true);
     sw.checkForUpdate.calls.reset();
     sw.$$checkForUpdateSubj.next(true);
 
@@ -103,6 +116,7 @@ describe('SwUpdatesService', () => {
 
   describe('when destroyed', () => {
     it('should not schedule a new check for update (after current check)', fakeAsync(run(() => {
+      appRef.isStable.next(true);
       sw.checkForUpdate.calls.reset();
 
       service.ngOnDestroy();
@@ -113,6 +127,7 @@ describe('SwUpdatesService', () => {
     })));
 
     it('should not schedule a new check for update (after activating an update)', fakeAsync(run(() => {
+      appRef.isStable.next(true);
       sw.checkForUpdate.calls.reset();
 
       sw.$$checkForUpdateSubj.next(true);
@@ -141,6 +156,10 @@ describe('SwUpdatesService', () => {
 });
 
 // Mocks
+class MockApplicationRef {
+  isStable = new Subject<boolean>();
+}
+
 class MockLogger {
   log = jasmine.createSpy('MockLogger.log');
 }

aio/src/app/sw-updates/sw-updates.service.ts

@@ -1,14 +1,14 @@
-import { Injectable, OnDestroy } from '@angular/core';
+import { ApplicationRef, Injectable, OnDestroy } from '@angular/core';
 import { NgServiceWorker } from '@angular/service-worker';
-import { Observable } from 'rxjs/Observable';
 import { Subject } from 'rxjs/Subject';
-import 'rxjs/add/observable/of';
 import 'rxjs/add/operator/concat';
 import 'rxjs/add/operator/debounceTime';
+import 'rxjs/add/operator/defaultIfEmpty';
+import 'rxjs/add/operator/do';
 import 'rxjs/add/operator/filter';
+import 'rxjs/add/operator/first';
 import 'rxjs/add/operator/map';
 import 'rxjs/add/operator/startWith';
-import 'rxjs/add/operator/take';
 import 'rxjs/add/operator/takeUntil';
 
 import { Logger } from 'app/shared/logger.service';
@@ -29,18 +29,20 @@ import { Logger } from 'app/shared/logger.service';
 @Injectable()
 export class SwUpdatesService implements OnDestroy {
   private checkInterval = 1000 * 60 * 60 * 6;   // 6 hours
-  private onDestroy = new Subject();
+  private onDestroy = new Subject<void>();
-  private checkForUpdateSubj = new Subject();
+  private checkForUpdateSubj = new Subject<void>();
   updateActivated = this.sw.updates
       .takeUntil(this.onDestroy)
       .do(evt => this.log(`Update event: ${JSON.stringify(evt)}`))
       .filter(({type}) => type === 'activation')
       .map(({version}) => version);
 
-  constructor(private logger: Logger, private sw: NgServiceWorker) {
+  constructor(appRef: ApplicationRef, private logger: Logger, private sw: NgServiceWorker) {
-    this.checkForUpdateSubj
+    const appIsStable$ = appRef.isStable.first(v => v);
-        .debounceTime(this.checkInterval)
+    const checkForUpdates$ = this.checkForUpdateSubj.debounceTime(this.checkInterval).startWith<void>(undefined);
-        .startWith(null)
+
+    appIsStable$
+        .concat(checkForUpdates$)
         .takeUntil(this.onDestroy)
         .subscribe(() => this.checkForUpdate());
   }
@@ -60,7 +62,8 @@ export class SwUpdatesService implements OnDestroy {
     this.sw.checkForUpdate()
         // Temp workaround for https://github.com/angular/mobile-toolkit/pull/137.
         // TODO (gkalpak): Remove once #137 is fixed.
-        .concat(Observable.of(false)).take(1)
+        .defaultIfEmpty(false)
+        .first()
         .do(v => this.log(`Update available: ${v}`))
         .subscribe(v => v ? this.activateUpdate() : this.scheduleCheckForUpdate());
   }

aio/src/main.ts

@@ -1,5 +1,6 @@
 import { enableProdMode, ApplicationRef } from '@angular/core';
 import { platformBrowserDynamic } from '@angular/platform-browser-dynamic';
+import 'rxjs/add/operator/first';
 
 import { AppModule } from './app/app.module';
 import { environment } from './environments/environment';
@@ -11,7 +12,7 @@ if (environment.production) {
 platformBrowserDynamic().bootstrapModule(AppModule).then(ref => {
   if (environment.production && 'serviceWorker' in (navigator as any)) {
     const appRef: ApplicationRef = ref.injector.get(ApplicationRef);
-    appRef.isStable.first().subscribe(() => {
+    appRef.isStable.first(v => v).subscribe(() => {
       (navigator as any).serviceWorker.register('/worker-basic.min.js');
     });
   }