From b369dab0964357aa3ac13469de949d76e01c683f Mon Sep 17 00:00:00 2001
From: Asami Doi <asamidoi@google.com>
Date: Thu, 1 Nov 2018 03:54:32 -0700
Subject: [PATCH 1/2] ServiceWorker: Add new WPT tests to make sure to update a
 registration with different script type and identical script content.

These tests check that a registration is updated correctly with
different script type. At first Service Worker is registered as
classic script type, then it is re-registered as module script type,
and vice versa. A main script is identical.

Bug: 824647
Change-Id: I2a3f87da1013f84c6e9495f362899dfe6ab97b45
Reviewed-on: https://chromium-review.googlesource.com/c/1298822
Commit-Queue: Asami Doi <asamidoi@google.com>
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#604551}
---
 .../resources/classic-worker.js               |   1 +
 .../service-worker/resources/module-worker.js |   1 +
 .../update-registration-with-type.https.html  | 134 +++++++++++++++++-
 3 files changed, 134 insertions(+), 2 deletions(-)
 create mode 100644 service-workers/service-worker/resources/classic-worker.js
 create mode 100644 service-workers/service-worker/resources/module-worker.js

diff --git a/service-workers/service-worker/resources/classic-worker.js b/service-workers/service-worker/resources/classic-worker.js
new file mode 100644
index 00000000000000..36a32b1a1f84da
--- /dev/null
+++ b/service-workers/service-worker/resources/classic-worker.js
@@ -0,0 +1 @@
+importScripts('./imported-classic-script.js');
diff --git a/service-workers/service-worker/resources/module-worker.js b/service-workers/service-worker/resources/module-worker.js
new file mode 100644
index 00000000000000..385fe7101503cb
--- /dev/null
+++ b/service-workers/service-worker/resources/module-worker.js
@@ -0,0 +1 @@
+import * as module from './imported-module-script.js';
diff --git a/service-workers/service-worker/update-registration-with-type.https.html b/service-workers/service-worker/update-registration-with-type.https.html
index 00c8a3345bb3d8..0f50ae20b971cf 100644
--- a/service-workers/service-worker/update-registration-with-type.https.html
+++ b/service-workers/service-worker/update-registration-with-type.https.html
@@ -7,8 +7,8 @@
 <script src="/resources/testharnessreport.js"></script>
 <script src="resources/test-helpers.sub.js"></script>
 <script>
-// These tests check that a registration is updated correctly with
-// different script type. At first Service Worker is register as
+// The following two tests check that a registration is updated correctly
+// with different script type. At first Service Worker is registered as
 // classic script type, then it is re-registered as module script type,
 // and vice versa. A main script is also updated at the same time.
 promise_test(async t => {
@@ -70,5 +70,135 @@
   assert_equals(msgEvent.data, 'A classic script.');
   assert_equals(firstRegistration, secondRegistration);
 }, 'Update the registration with a different script type (module => classic).');
+
+// The following two tests change the script type while keeping
+// the script identical.
+promise_test(async t => {
+  const script = 'resources/empty-worker.js';
+  const scope = 'resources/update-registration-with-type';
+  await service_worker_unregister(t, scope);
+  t.add_cleanup(() => service_worker_unregister(t, scope));
+
+  // Register with classic script type.
+  const firstRegistration = await navigator.serviceWorker.register(script, {
+    scope: scope,
+    type: 'classic'
+  });
+  await wait_for_state(t, firstRegistration.installing, 'activated');
+  const firstActiveWorker = firstRegistration.active;
+
+  // Re-register with module script type.
+  const secondRegistration = await navigator.serviceWorker.register(script, {
+    scope: scope,
+    type: 'module'
+  });
+  await wait_for_state(t, secondRegistration.installing, 'activated');
+  const secondActiveWorker = secondRegistration.active;
+
+  assert_not_equals(firstActiveWorker, secondActiveWorker);
+  assert_equals(firstRegistration, secondRegistration);
+}, 'Update the registration with a different script type (classic => module) '
+    + 'and with a same main script.');
+
+promise_test(async t => {
+  const script = 'resources/empty-worker.js';
+  const scope = 'resources/update-registration-with-type';
+  await service_worker_unregister(t, scope);
+  t.add_cleanup(() => service_worker_unregister(t, scope));
+
+  // Register with module script type.
+  const firstRegistration = await navigator.serviceWorker.register(script, {
+    scope: scope,
+    type: 'module'
+  });
+  await wait_for_state(t, firstRegistration.installing, 'activated');
+  const firstActiveWorker = firstRegistration.active;
+
+  // Re-register with classic script type.
+  const secondRegistration = await navigator.serviceWorker.register(script, {
+    scope: scope,
+    type: 'classic'
+  });
+  await wait_for_state(t, secondRegistration.installing, 'activated');
+  const secondActiveWorker = secondRegistration.active;
+
+  assert_not_equals(firstActiveWorker, secondActiveWorker);
+  assert_equals(firstRegistration, secondRegistration);
+}, 'Update the registration with a different script type (module => classic) '
+    + 'and with a same main script.');
+
+// This test checks that a registration is not updated with the same script
+// type and the same main script.
+promise_test(async t => {
+  const script = 'resources/empty-worker.js';
+  const scope = 'resources/update-registration-with-type';
+  await service_worker_unregister(t, scope);
+  t.add_cleanup(() => service_worker_unregister(t, scope));
+
+  // Register with module script type.
+  const firstRegistration = await navigator.serviceWorker.register(script, {
+    scope: scope,
+    type: 'module'
+  });
+  await wait_for_state(t, firstRegistration.installing, 'activated');
+
+  // Re-register with module script type.
+  const secondRegistration = await navigator.serviceWorker.register(script, {
+    scope: scope,
+    type: 'module'
+  });
+  assert_equals(secondRegistration.installing, null);
+
+  assert_equals(firstRegistration, secondRegistration);
+}, 'Does not update the registration with the same script type and '
+    + 'the same main script.');
+
+// In the case (classic => module), a worker script contains importScripts()
+// that is disallowed on module scripts, so the second registration is
+// expected to fail script evaluation.
+promise_test(async t => {
+  const script = 'resources/classic-worker.js';
+  const scope = 'resources/update-registration-with-type';
+  await service_worker_unregister(t, scope);
+  t.add_cleanup(() => service_worker_unregister(t, scope));
+
+  // Register with classic script type.
+  const firstRegistration = await navigator.serviceWorker.register(script, {
+    scope: scope,
+    type: 'classic'
+  });
+  assert_not_equals(firstRegistration.installing, null);
+
+  // Re-register with module script type and expect TypeError.
+  return promise_rejects(t, new TypeError, navigator.serviceWorker.register(script, {
+    scope: scope,
+    type: 'module'
+  }), 'Registering with invalid evaluation should be failed.');
+}, 'Update the registration with a different script type (classic => module) '
+    + 'and with a same main script. Expect evaluation failed.');
+
+// In the case (module => classic), a worker script contains static-import
+// that is disallowed on classic scripts, so the second registration is
+// expected to fail script evaluation.
+promise_test(async t => {
+  const script = 'resources/module-worker.js';
+  const scope = 'resources/update-registration-with-type';
+  await service_worker_unregister(t, scope);
+  t.add_cleanup(() => service_worker_unregister(t, scope));
+
+  // Register with module script type.
+  const firstRegistration = await navigator.serviceWorker.register(script, {
+    scope: scope,
+    type: 'module'
+  });
+  assert_not_equals(firstRegistration.installing, null);
+
+  // Re-register with classic script type and expect TypeError.
+  return promise_rejects(t, new TypeError, navigator.serviceWorker.register(script, {
+    scope: scope,
+    type: 'classic'
+  }), 'Registering with invalid evaluation should be failed.');
+}, 'Update the registration with a different script type (module => classic) '
+    + 'and with a same main script. Expect evaluation failed.');
 </script>
 </body>

From 55045bdbf4d47329ef37b1a8ec4304a075f692e3 Mon Sep 17 00:00:00 2001
From: Hiroki Nakagawa <nhiroki@chromium.org>
Date: Tue, 13 Nov 2018 04:59:29 +0000
Subject: [PATCH 2/2] ServiceWorker: Deflake update-with-script-type.https.html

To sheriff: Please re-disable the test if it's still flaky.

In this test file, some tests start re-registering the second service worker
before the first service worker gets activated. In my theory, this sometimes
squashes the second registration job into the first registration job, and
results in test flakiness. This CL makes sure the second register runs after
the first service worker gets activated.

Note that it would be the correct behavior that the second registration job
isn't squashed when the script type is changed. There is a spec issue about
this: https://github.com/w3c/ServiceWorker/issues/1358

Bug: 901317
Change-Id: I7ce379071a35ef9aeb98e4492d651ee6fc4714ec
Reviewed-on: https://chromium-review.googlesource.com/c/1328546
Reviewed-by: Makoto Shimazu <shimazu@chromium.org>
Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607494}
---
 .../update-registration-with-type.https.html  | 56 ++++++++++---------
 1 file changed, 30 insertions(+), 26 deletions(-)

diff --git a/service-workers/service-worker/update-registration-with-type.https.html b/service-workers/service-worker/update-registration-with-type.https.html
index 0f50ae20b971cf..b712c30f71e9a8 100644
--- a/service-workers/service-worker/update-registration-with-type.https.html
+++ b/service-workers/service-worker/update-registration-with-type.https.html
@@ -16,16 +16,17 @@
   const script = `resources/update-registration-with-type.py?classic_first=1&key=${key}`;
   const scope = 'resources/update-registration-with-type';
   await service_worker_unregister(t, scope);
+  t.add_cleanup(() => service_worker_unregister(t, scope));
 
   // Register with classic script type.
   const firstRegistration = await navigator.serviceWorker.register(script, {
     scope: scope,
     type: 'classic'
   });
-  firstRegistration.installing.postMessage(' ');
-  let msgEvent = await new Promise(resolve => {
-    navigator.serviceWorker.onmessage = resolve;
-  });
+  const firstWorker = firstRegistration.installing;
+  await wait_for_state(t, firstWorker, 'activated');
+  firstWorker.postMessage(' ');
+  let msgEvent = await new Promise(r => navigator.serviceWorker.onmessage = r);
   assert_equals(msgEvent.data, 'A classic script.');
 
   // Re-register with module script type.
@@ -33,11 +34,12 @@
     scope: scope,
     type: 'module'
   });
-  secondRegistration.installing.postMessage(' ');
-  msgEvent = await new Promise(resolve => {
-    navigator.serviceWorker.onmessage = resolve;
-  });
+  const secondWorker = secondRegistration.installing;
+  secondWorker.postMessage(' ');
+  msgEvent = await new Promise(r => navigator.serviceWorker.onmessage = r);
   assert_equals(msgEvent.data, 'A module script.');
+
+  assert_not_equals(firstWorker, secondWorker);
   assert_equals(firstRegistration, secondRegistration);
 }, 'Update the registration with a different script type (classic => module).');
 
@@ -46,16 +48,17 @@
   const script = `resources/update-registration-with-type.py?classic_first=0&key=${key}`;
   const scope = 'resources/update-registration-with-type';
   await service_worker_unregister(t, scope);
+  t.add_cleanup(() => service_worker_unregister(t, scope));
 
   // Register with module script type.
   const firstRegistration = await navigator.serviceWorker.register(script, {
     scope: scope,
     type: 'module'
   });
-  firstRegistration.installing.postMessage(' ');
-  let msgEvent = await new Promise(resolve => {
-    navigator.serviceWorker.onmessage = resolve;
-  });
+  const firstWorker = firstRegistration.installing;
+  await wait_for_state(t, firstWorker, 'activated');
+  firstWorker.postMessage(' ');
+  let msgEvent = await new Promise(r => navigator.serviceWorker.onmessage = r);
   assert_equals(msgEvent.data, 'A module script.');
 
   // Re-register with classic script type.
@@ -63,11 +66,12 @@
     scope: scope,
     type: 'classic'
   });
-  secondRegistration.installing.postMessage(' ');
-  msgEvent = await new Promise(resolve => {
-    navigator.serviceWorker.onmessage = resolve;
-  });
+  const secondWorker = secondRegistration.installing;
+  secondWorker.postMessage(' ');
+  msgEvent = await new Promise(r => navigator.serviceWorker.onmessage = r);
   assert_equals(msgEvent.data, 'A classic script.');
+
+  assert_not_equals(firstWorker, secondWorker);
   assert_equals(firstRegistration, secondRegistration);
 }, 'Update the registration with a different script type (module => classic).');
 
@@ -84,18 +88,17 @@
     scope: scope,
     type: 'classic'
   });
-  await wait_for_state(t, firstRegistration.installing, 'activated');
-  const firstActiveWorker = firstRegistration.active;
+  const firstWorker = firstRegistration.installing;
+  await wait_for_state(t, firstWorker, 'activated');
 
   // Re-register with module script type.
   const secondRegistration = await navigator.serviceWorker.register(script, {
     scope: scope,
     type: 'module'
   });
-  await wait_for_state(t, secondRegistration.installing, 'activated');
-  const secondActiveWorker = secondRegistration.active;
+  const secondWorker = secondRegistration.installing;
 
-  assert_not_equals(firstActiveWorker, secondActiveWorker);
+  assert_not_equals(firstWorker, secondWorker);
   assert_equals(firstRegistration, secondRegistration);
 }, 'Update the registration with a different script type (classic => module) '
     + 'and with a same main script.');
@@ -111,18 +114,17 @@
     scope: scope,
     type: 'module'
   });
-  await wait_for_state(t, firstRegistration.installing, 'activated');
-  const firstActiveWorker = firstRegistration.active;
+  const firstWorker = firstRegistration.installing;
+  await wait_for_state(t, firstWorker, 'activated');
 
   // Re-register with classic script type.
   const secondRegistration = await navigator.serviceWorker.register(script, {
     scope: scope,
     type: 'classic'
   });
-  await wait_for_state(t, secondRegistration.installing, 'activated');
-  const secondActiveWorker = secondRegistration.active;
+  const secondWorker = secondRegistration.installing;
 
-  assert_not_equals(firstActiveWorker, secondActiveWorker);
+  assert_not_equals(firstWorker, secondWorker);
   assert_equals(firstRegistration, secondRegistration);
 }, 'Update the registration with a different script type (module => classic) '
     + 'and with a same main script.');
@@ -168,6 +170,7 @@
     type: 'classic'
   });
   assert_not_equals(firstRegistration.installing, null);
+  await wait_for_state(t, firstRegistration.installing, 'activated');
 
   // Re-register with module script type and expect TypeError.
   return promise_rejects(t, new TypeError, navigator.serviceWorker.register(script, {
@@ -192,6 +195,7 @@
     type: 'module'
   });
   assert_not_equals(firstRegistration.installing, null);
+  await wait_for_state(t, firstRegistration.installing, 'activated');
 
   // Re-register with classic script type and expect TypeError.
   return promise_rejects(t, new TypeError, navigator.serviceWorker.register(script, {