From 7bc0d4741c6345d9c19cd53d3607de17a563c834 Mon Sep 17 00:00:00 2001
From: Jake Archibald <jaffathecake@gmail.com>
Date: Fri, 23 Jun 2017 16:46:09 +0100
Subject: [PATCH] Fixing 'all' case. Also testing reg.updateViaCache.

---
 .../registration-updateviacache.https.html    | 38 ++++++++++++-------
 .../resources/update-max-aged-worker.py       |  2 +-
 2 files changed, 26 insertions(+), 14 deletions(-)

diff --git a/service-workers/service-worker/registration-updateviacache.https.html b/service-workers/service-worker/registration-updateviacache.https.html
index f109af38efedb5..0df401efcc6786 100644
--- a/service-workers/service-worker/registration-updateviacache.https.html
+++ b/service-workers/service-worker/registration-updateviacache.https.html
@@ -17,10 +17,6 @@
     };
   }
 
-  function wait(ms) {
-    return new Promise(r => setTimeout(r, ms));
-  }
-
   function getScriptTimes(sw, testName) {
     return new Promise(resolve => {
       navigator.serviceWorker.addEventListener('message', function listener(event) {
@@ -110,7 +106,7 @@
     [registerViaLinkHeader, 'via-link-header']
   ];
 
-  // Test creating registrations.
+  // Test creating registrations & triggering an update.
   for (const [registrationMethod, registrationMethodName] of registrationMethods) {
     for (const updateViaCache of UPDATE_VIA_CACHE_VALUES) {
       const testName = `register-${registrationMethodName}-updateViaCache-${updateViaCache}`;
@@ -127,6 +123,8 @@
           opts
         );
 
+        assert_equals(reg.updateViaCache, updateViaCache || 'imports', "reg.updateViaCache");
+
         const sw = reg.installing || reg.waiting || reg.active;
         await wait_for_state(t, sw, 'activated');
         const values = await getScriptTimes(sw, testName);
@@ -144,10 +142,15 @@
             assert_not_equals(values.mainTime, newValues.mainTime, "Main script should have updated");
             assert_equals(values.importTime, newValues.importTime, "Imported script should be the same");
           }
-          else { // updateViaCache == 'none'
+          else if (updateViaCache == 'none') {
             assert_not_equals(values.mainTime, newValues.mainTime, "Main script should have updated");
             assert_not_equals(values.importTime, newValues.importTime, "Imported script should have updated");
           }
+          else {
+            // We should have handled all of the possible values for updateViaCache.
+            // If this runs, something's gone very wrong.
+            throw Error(`Unexpected updateViaCache value: ${updateViaCache}`);
+          }
         }
 
         await cleanup();
@@ -178,9 +181,17 @@
 
         await navigator.serviceWorker.register(fullScriptUrl, opts);
 
+        assert_equals(reg.updateViaCache, updateViaCache2 || 'imports', "reg.updateViaCache updated");
+
+        // If the update happens via the cache, the scripts will come back byte-identical.
+        // We bypass the byte-identical check if the script URL has changed, but not if
+        // only the updateViaCache value has changed.
+        if (updateViaCache2 == 'all') {
+          assert_equals(reg.installing, null, "No new service worker");
+        }
         // If there's no change to the updateViaCache value, register should be a no-op.
         // The default value should behave as 'imports'.
-        if ((updateViaCache1 || 'imports') == (updateViaCache2 || 'imports')) {
+        else if ((updateViaCache1 || 'imports') == (updateViaCache2 || 'imports')) {
           assert_equals(reg.installing, null, "No new service worker");
         }
         else {
@@ -188,18 +199,19 @@
           assert_true(!!newWorker, "New worker installing");
           const newValues = await getScriptTimes(newWorker, testName);
 
-          if (updateViaCache2 == 'all') {
-            assert_equals(values.mainTime, newValues.mainTime, "Main script should be the same");
-            assert_equals(values.importTime, newValues.importTime, "Imported script should be the same");
-          }
-          else if (updateViaCache2 == 'imports') {
+          if (!updateViaCache2 || updateViaCache2 == 'imports') {
             assert_not_equals(values.mainTime, newValues.mainTime, "Main script should have updated");
             assert_equals(values.importTime, newValues.importTime, "Imported script should be the same");
           }
-          else { // updateViaCache == 'none'
+          else if (updateViaCache2 == 'none') {
             assert_not_equals(values.mainTime, newValues.mainTime, "Main script should have updated");
             assert_not_equals(values.importTime, newValues.importTime, "Imported script should have updated");
           }
+          else {
+            // We should have handled all of the possible values for updateViaCache2.
+            // If this runs, something's gone very wrong.
+            throw Error(`Unexpected updateViaCache value: ${updateViaCache}`);
+          }
         }
 
         await cleanup();
diff --git a/service-workers/service-worker/resources/update-max-aged-worker.py b/service-workers/service-worker/resources/update-max-aged-worker.py
index a9acf1b6de547c..0fc112d8b0f033 100644
--- a/service-workers/service-worker/resources/update-max-aged-worker.py
+++ b/service-workers/service-worker/resources/update-max-aged-worker.py
@@ -6,7 +6,7 @@ def main(request, response):
                ('Cache-Control', 'max-age=86400'),
                ('Last-Modified', time.strftime("%a, %d %b %Y %H:%M:%S GMT", time.gmtime()))]
 
-    test = request.GET['test'];
+    test = request.GET['test']
 
     body = '''
         const mainTime = {time};