Precaching temp (#1342)

* Caches to a temp directory until activate event occurs

* Remove private

* Changing to actual activation vs just controller change

* Delete entries as we move them acorss

* Adding extra logic to wait for controller change on top of activate

infra/testing/activate-and-control.js

@@ -0,0 +1,66 @@
+const activateSWSafari = require('./activate-sw-safari');
+
+module.exports = async (swUrl) => {
+  if (global.__workbox.seleniumBrowser.getId() === 'safari') {
+    return activateSWSafari(swUrl);
+  }
+
+  const error = await global.__workbox.webdriver.executeAsyncScript((swUrl, cb) => {
+    function _onStateChangePromise(registration, desiredState) {
+      return new Promise((resolve, reject) => {
+        if (registration.installing === null) {
+          throw new Error('Service worker is not installing.');
+        }
+
+        let serviceWorker = registration.installing;
+
+        // We unregister all service workers after each test - this should
+        // always trigger an install state change
+        let stateChangeListener = function(evt) {
+          if (evt.target.state === desiredState) {
+            serviceWorker.removeEventListener('statechange', stateChangeListener);
+            resolve();
+            return;
+          }
+
+          if (evt.target.state === 'redundant') {
+            serviceWorker.removeEventListener('statechange', stateChangeListener);
+
+            // Must call reject rather than throw error here due to this
+            // being inside the scope of the callback function stateChangeListener
+            reject(new Error('Installing servier worker became redundant'));
+            return;
+          }
+        };
+
+        serviceWorker.addEventListener('statechange', stateChangeListener);
+      });
+    }
+
+    navigator.serviceWorker.register(swUrl)
+    .then((registration) => {
+      return _onStateChangePromise(registration, 'activated');
+    })
+    .then(() => {
+      // Ensure the page is being controlled by the SW.
+      if (navigator.serviceWorker.controller &&
+        navigator.serviceWorker.controller.scriptURL === swUrl) {
+        return;
+      } else {
+        return new Promise((resolve) => {
+          navigator.serviceWorker.addEventListener('controllerchange', () => {
+            if (navigator.serviceWorker.controller.scriptURL === swUrl) {
+              resolve();
+            }
+          });
+        });
+      }
+    })
+    .then(() => cb())
+    .catch((err) => cb(err));
+  }, swUrl);
+
+  if (error) {
+    throw error;
+  }
+};

packages/workbox-precaching/_default.mjs

@@ -165,7 +165,7 @@ moduleExports.precache = (entries) => {
     }));
   });
   self.addEventListener('activate', (event) => {
-    event.waitUntil(precacheController.cleanup());
+    event.waitUntil(precacheController.activate());
   });
 };
 

packages/workbox-precaching/controllers/PrecacheController.mjs

@@ -164,11 +164,15 @@ class PrecacheController {
       }
     }
 
+    // Clear any existing temp cache
+    await caches.delete(this._getTempCacheName());
+
     const entriesToPrecache = [];
     const entriesAlreadyPrecached = [];
 
     for (const precacheEntry of this._entriesToCacheMap.values()) {
-      if (await this._precacheDetailsModel._isEntryCached(precacheEntry)) {
+      if (await this._precacheDetailsModel._isEntryCached(
+        this._cacheName, precacheEntry)) {
         entriesAlreadyPrecached.push(precacheEntry);
       } else {
         entriesToPrecache.push(precacheEntry);
@@ -177,7 +181,7 @@ class PrecacheController {
 
     // Wait for all requests to be cached.
     await Promise.all(entriesToPrecache.map((precacheEntry) => {
-      return this._cacheEntry(precacheEntry, options.plugins);
+      return this._cacheEntryInTemp(precacheEntry, options.plugins);
     }));
 
     if (process.env.NODE_ENV !== 'production') {
@@ -190,6 +194,41 @@ class PrecacheController {
     };
   }
 
+  /**
+   * Takes the current set of temporary files and moves them to the final
+   * cache, deleting the temporary cache once copying is complete.
+   *
+   * @return {
+   * Promise<workbox.precaching.CleanupResult>}
+   * Resolves with an object containing details of the deleted cache requests
+   * and precache revision details.
+   */
+  async activate() {
+    const tempCache = await caches.open(this._getTempCacheName());
+
+    const requests = await tempCache.keys();
+    await Promise.all(requests.map(async (request) => {
+      const response = await tempCache.match(request);
+      await cacheWrapper.put(this._cacheName, request, response);
+      await tempCache.delete(request);
+    }));
+
+    await caches.delete(this._getTempCacheName());
+
+    return this._cleanup();
+  }
+
+  /**
+   * Returns the name of the temporary cache.
+   *
+   * @return {string}
+   *
+   * @private
+   */
+  _getTempCacheName() {
+    return `${this._cacheName}-temp`;
+  }
+
   /**
    * Requests the entry and saves it to the cache if the response
    * is valid.
@@ -203,7 +242,7 @@ class PrecacheController {
    * promise resolves with true if the entry was cached / updated and
    * false if the entry is already cached and up-to-date.
    */
-  async _cacheEntry(precacheEntry, plugins) {
+  async _cacheEntryInTemp(precacheEntry, plugins) {
     let response = await fetchWrapper.fetch(
       precacheEntry._networkRequest,
       null,
@@ -214,7 +253,7 @@ class PrecacheController {
       response = await cleanRedirect(response);
     }
 
-    await cacheWrapper.put(this._cacheName,
+    await cacheWrapper.put(this._getTempCacheName(),
       precacheEntry._cacheRequest, response, plugins);
 
     await this._precacheDetailsModel._addEntry(precacheEntry);
@@ -232,8 +271,10 @@ class PrecacheController {
    * Promise<workbox.precaching.CleanupResult>}
    * Resolves with an object containing details of the deleted cache requests
    * and precache revision details.
+   *
+   * @private
    */
-  async cleanup() {
+  async _cleanup() {
     const expectedCacheUrls = [];
     this._entriesToCacheMap.forEach((entry) => {
       const fullUrl = new URL(entry._cacheRequest.url, location).toString();

packages/workbox-precaching/models/PrecachedDetailsModel.mjs

@@ -15,7 +15,6 @@
 */
 
 import {DBWrapper} from 'workbox-core/_private/DBWrapper.mjs';
-import {cacheNames} from 'workbox-core/_private/cacheNames.mjs';
 import '../_version.mjs';
 
 // Allows minifier to mangle this name
@@ -32,12 +31,9 @@ class PrecachedDetailsModel {
   /**
    * Construct a new model for a specific cache.
    *
-   * @param {string} cacheName
-   *
    * @private
    */
-  constructor(cacheName) {
-    this._cacheName = cacheNames.getPrecacheName(cacheName);
+  constructor() {
     this._db = new DBWrapper(`workbox-precaching`, 2, {
       onupgradeneeded: this._handleUpgrade,
     });
@@ -70,18 +66,19 @@ class PrecachedDetailsModel {
    * Check if an entry is already cached. Returns false if
    * the entry isn't cached or the revision has changed.
    *
+   * @param {string} cacheName
    * @param {PrecacheEntry} precacheEntry
    * @return {boolean}
    *
    * @private
    */
-  async _isEntryCached(precacheEntry) {
+  async _isEntryCached(cacheName, precacheEntry) {
     const revisionDetails = await this._getRevision(precacheEntry._entryId);
     if (revisionDetails !== precacheEntry._revision) {
       return false;
     }
 
-    const openCache = await caches.open(this._cacheName);
+    const openCache = await caches.open(cacheName);
     const cachedResponse = await openCache.match(precacheEntry._cacheRequest);
     return !!cachedResponse;
   }

test/workbox-background-sync/integration/test-bg-sync.js

@@ -1,6 +1,6 @@
 const expect = require('chai').expect;
 
-const activateSW = require('../../../infra/testing/activate-sw');
+const activateAndControlSW = require('../../../infra/testing/activate-and-control');
 
 describe(`[workbox-background-sync] Load and use Background Sync`, function() {
   const testServerAddress = global.__workbox.server.getAddress();
@@ -27,7 +27,7 @@ describe(`[workbox-background-sync] Load and use Background Sync`, function() {
   it(`should load a page with service worker`, async function() {
     // Load the page and wait for the first service worker to register and activate.
     await global.__workbox.webdriver.get(testingUrl);
-    await activateSW(swUrl);
+    await activateAndControlSW(swUrl);
 
     const err = await global.__workbox.webdriver.executeAsyncScript((testingUrl, cb) => {
       return fetch(`${testingUrl}example.txt`)

test/workbox-broadcast-cache-update/integration/broadcast-cache-update-plugin.js

@@ -1,6 +1,6 @@
 const expect = require('chai').expect;
 
-const activateSW = require('../../../infra/testing/activate-sw');
+const activateAndControlSW = require('../../../infra/testing/activate-and-control');
 
 describe(`broadcastCacheUpdate.Plugin`, function() {
   const testServerAddress = global.__workbox.server.getAddress();
@@ -10,7 +10,7 @@ describe(`broadcastCacheUpdate.Plugin`, function() {
 
   it(`should broadcast a message on the expected channel when there's a cache update`, async function() {
     await global.__workbox.webdriver.get(testingUrl);
-    await activateSW(swUrl);
+    await activateAndControlSW(swUrl);
 
     const supported = await global.__workbox.webdriver.executeScript(() => {
       return 'BroadcastChannel' in window;

test/workbox-cache-expiration/integration/expiration-plugin.js

@@ -1,6 +1,6 @@
 const expect = require('chai').expect;
 
-const activateSW = require('../../../infra/testing/activate-sw');
+const activateAndControlSW = require('../../../infra/testing/activate-and-control');
 const cleanSWEnv = require('../../../infra/testing/clean-sw');
 
 describe(`expiration.Plugin`, function() {
@@ -30,7 +30,7 @@ describe(`expiration.Plugin`, function() {
     const swUrl = `${testingUrl}sw-max-entries.js`;
 
     // Wait for the service worker to register and activate.
-    await activateSW(swUrl);
+    await activateAndControlSW(swUrl);
 
     await global.__workbox.webdriver.executeAsyncScript((testingUrl, cb) => {
       fetch(`${testingUrl}example-1.txt`).then(() => cb()).catch((err) => cb(err.message));
@@ -86,7 +86,7 @@ describe(`expiration.Plugin`, function() {
 
     // Load the page and wait for the service worker to register and activate.
     await global.__workbox.webdriver.get(testingUrl);
-    await activateSW(swUrl);
+    await activateAndControlSW(swUrl);
 
     await global.__workbox.webdriver.executeAsyncScript((testingUrl, cb) => {
       fetch(`${testingUrl}example-1.txt`).then(() => cb()).catch((err) => cb(err.message));

test/workbox-cacheable-response/integration/test-cacheable-response-plugin.js

@@ -1,6 +1,6 @@
 const expect = require('chai').expect;
 
-const activateSW = require('../../../infra/testing/activate-sw');
+const activateAndControlSW = require('../../../infra/testing/activate-and-control');
 
 describe(`cacheableResponse.Plugin`, function() {
   const testServerAddress = global.__workbox.server.getAddress();
@@ -33,7 +33,7 @@ describe(`cacheableResponse.Plugin`, function() {
 
   it(`should load a page and cache entries`, async function() {
     // Wait for the service worker to register and activate.
-    await activateSW(swUrl);
+    await activateAndControlSW(swUrl);
 
     await global.__workbox.webdriver.executeAsyncScript((testingUrl, cb) => {
       fetch(`${testingUrl}example-1.txt`).then(() => cb()).catch((err) => cb(err.message));

test/workbox-core/integration/test-core.js

@@ -1,4 +1,4 @@
-const activateSW = require('../../../infra/testing/activate-sw');
+const activateAndControlSW = require('../../../infra/testing/activate-and-control');
 
 describe(`[workbox-core] Load core in the browser`, function() {
   const testServerAddress = global.__workbox.server.getAddress();
@@ -7,7 +7,7 @@ describe(`[workbox-core] Load core in the browser`, function() {
 
   it(`should load workbox-core in a service worker.`, async function() {
     await global.__workbox.webdriver.get(testingUrl);
-    await activateSW(swUrl);
+    await activateAndControlSW(swUrl);
 
     // If the service worker activated, it meant the assertions in sw.js were
     // met and workbox-core exposes the expected API and defaults that were

test/workbox-google-analytics/integration/basic-example.js

@@ -1,7 +1,7 @@
 const expect = require('chai').expect;
 const qs = require('qs');
 const {By} = require('selenium-webdriver');
-const activateSW = require('../../../infra/testing/activate-sw');
+const activateAndControlSW = require('../../../infra/testing/activate-and-control');
 
 describe(`[workbox-google-analytics] Load and use Google Analytics`,
     function() {
@@ -27,11 +27,14 @@ describe(`[workbox-google-analytics] Load and use Google Analytics`,
         data, [messageChannel.port2]);
   };
 
-  beforeEach(async function() {
+  before(async function() {
     // Load the page and wait for the first service worker to activate.
     await driver.get(testingUrl);
-    await activateSW(swUrl);
 
+    await activateAndControlSW(swUrl);
+  });
+
+  beforeEach(async function() {
     // Reset the spied requests array.
     await driver.executeAsyncScript(messageSw, {
       action: 'clear-spied-requests',