Resolve getSW() to a matching waiting SW (#1961)

packages/workbox-window/Workbox.mjs

@@ -105,10 +105,10 @@ class Workbox extends EventTargetShim {
 
     this._registration = await this._registerScript();
 
-    // Only resolve deferreds now if we know we have a compatible controller.
+    // If we have a compatible controller, store the controller as the "own"
+    // SW, resolve active/controlling deferreds and add necessary listeners.
     if (this._compatibleControllingSW) {
       this._sw = this._compatibleControllingSW;
-      this._swDeferred.resolve(this._compatibleControllingSW);
       this._activeDeferred.resolve(this._compatibleControllingSW);
       this._controllingDeferred.resolve(this._compatibleControllingSW);
 
@@ -118,17 +118,21 @@ class Workbox extends EventTargetShim {
     }
 
     // If there's a waiting service worker with a matching URL before the
-    // `updatefound` event fires, it likely means the this site is open
-    // in another tab, or the user refreshed the page without unloading it
-    // first.
+    // `updatefound` event fires, it likely means that this site is open
+    // in another tab, or the user refreshed the page (and thus the prevoius
+    // page wasn't fully unloaded before this page started loading).
     // https://developers.google.com/web/fundamentals/primers/service-workers/lifecycle#waiting
-    if (this._registration.waiting &&
-        urlsMatch(this._registration.waiting.scriptURL, this._scriptURL)) {
+    const waitingSW = this._registration.waiting;
+    if (waitingSW && urlsMatch(waitingSW.scriptURL, this._scriptURL)) {
+      // Store the waiting SW as the "own" Sw, even if it means overwriting
+      // a compatible controller.
+      this._sw = waitingSW;
+
       // Run this in the next microtask, so any code that adds an event
       // listener after awaiting `register()` will get this event.
       Promise.resolve().then(() => {
         this.dispatchEvent(new WorkboxEvent('waiting', {
-          sw: this._registration.waiting,
+          sw: waitingSW,
           wasWaitingBeforeRegister: true,
         }));
         if (process.env.NODE_ENV !== 'production') {
@@ -138,6 +142,11 @@ class Workbox extends EventTargetShim {
       });
     }
 
+    // If an "own" SW is already set, resolve the deferred.
+    if (this._sw) {
+      this._swDeferred.resolve(this._sw);
+    }
+
     if (process.env.NODE_ENV !== 'production') {
       logger.log('Successfully registered service worker.', this._scriptURL);
 
@@ -211,11 +220,14 @@ class Workbox extends EventTargetShim {
    * Resolves with a reference to a service worker that matches the script URL
    * of this instance, as soon as it's available.
    *
-   * If, at registration time, there’s already an active service worker with a
-   * matching script URL, that will be what is resolved. If there’s no active
-   * and matching service worker at registration time then the promise will
-   * not resolve until an update is found and starts installing, at which
-   * point the installing service worker is resolved.
+   * If, at registration time, there's already an active or waiting service
+   * worker with a matching script URL, it will be used (with the waiting
+   * service worker taking precedence over the active service worker if both
+   * match, since the waiting service worker would have been registered more
+   * recently).
+   * If there's no matching active or waiting service worker at registration
+   * time then the promise will not resolve until an update is found and starts
+   * installing, at which point the installing service worker is used.
    *
    * @return {Promise<ServiceWorker>}
    */
@@ -227,7 +239,8 @@ class Workbox extends EventTargetShim {
 
   /**
    * Sends the passed data object to the service worker registered by this
-   * instance and resolves with a response (if any).
+   * instance (via [`getSW()`]{@link module:workbox-window.Workbox#getSW}) and resolves
+   * with a response (if any).
    *
    * A response can be set in a message handler in the service worker by
    * calling `event.ports[0].postMessage(...)`, which will resolve the promise
@@ -239,31 +252,19 @@ class Workbox extends EventTargetShim {
    */
   async messageSW(data) {
     const sw = await this.getSW();
-    return new Promise((resolve) => {
-      let messageChannel = new MessageChannel();
-      messageChannel.port1.onmessage = (evt) => resolve(evt.data);
-      sw.postMessage(data, [messageChannel.port2]);
-    });
+    return messageSW(sw, data);
   }
 
   /**
    * Checks for a service worker already controlling the page and returns
-   * it if its script URL (and optionally script version) match. The
-   * script version is determined by sending a message to the controlling
-   * service worker and waiting for a response. If no response is returned
-   * the service worker is assumed to not have a version.
+   * it if its script URL matchs.
    *
    * @private
    * @return {ServiceWorker|undefined}
    */
   _getControllingSWIfCompatible() {
     const controller = navigator.serviceWorker.controller;
-
     if (controller && urlsMatch(controller.scriptURL, this._scriptURL)) {
-      // If the URLs match and no script version is specified, assume the
-      // SW is the same. NOTE: without a script version, this isn't a
-      // particularly good test. Using a script version is encouraged if
-      // you need to send messages to your service worker on all page loads.
       return controller;
     }
   }

test/workbox-window/unit/test-Workbox.mjs

@@ -384,7 +384,7 @@ describe(`[workbox-window] Workbox`, function() {
       expect(sw).to.equal(reg.installing);
     });
 
-    it(`resolves before updating if a SW with the same script URL is already active`, async function() {
+    it(`resolves before updating if a SW with the same script URL is already controlling`, async function() {
       const scriptURL = navigator.serviceWorker.controller.scriptURL;
       const wb = new Workbox(scriptURL);
 
@@ -396,7 +396,31 @@ describe(`[workbox-window] Workbox`, function() {
       expect(sw).to.equal(navigator.serviceWorker.controller);
     });
 
-    it(`resolves as soon as an an update is found (if no active SW exists)`, async function() {
+    it(`resolves before updating if a SW with the same script URL is already waiting to install`, async function() {
+      const scriptURL = uniq('sw-no-skip-waiting.tmp.js');
+
+      const wb1 = new Workbox(scriptURL);
+      const reg1 = await wb1.register();
+
+      await nextEvent(wb1, 'waiting');
+      expect(reg1.waiting.scriptURL).to.equal(scriptURL);
+
+      // Stub the controlling SW's scriptURL so it matches the SW that is
+      // about to be waiting. This is done to assert that if a matching
+      // controller *and* waiting SW are found at registration time, the
+      // `getSW()` method resolves to the waiting SW.
+      sandbox.stub(navigator.serviceWorker.controller, 'scriptURL')
+          .value(scriptURL);
+
+      const wb2 = new Workbox(scriptURL);
+      const reg2Promise = wb2.register();
+
+      const sw = await wb2.getSW();
+      const reg2 = await reg2Promise;
+      expect(sw).to.equal(reg2.waiting);
+    });
+
+    it(`resolves as soon as an an update is found (if not already resolved)`, async function() {
       const wb = new Workbox(uniq('sw-clients-claim.tmp.js'));
       wb.register();