Bug 1631356 [wpt PR 23105] - ServiceWorker: Fix timeout on client-navigate.https.html, a=testonly
authorHiroki Nakagawa <nhiroki@chromium.org>
Tue, 28 Apr 2020 11:37:45 +0000
changeset 527576 e4f2dd72c208f41d4cccb483eeead7523e60e0fc
parent 527575 4cdada5d87dffd05db728efe8e1c5d43bc6cd7f8
child 527577 c810496118ef82f0f29650c7deebfda23ab36ea9
push id37368
push userbtara@mozilla.com
push dateFri, 01 May 2020 21:45:51 +0000
treeherdermozilla-central@0f9c5a59e45d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstestonly
bugs1631356, 23105, 21162, 22086, 1057682, 2155760, 760974
milestone77.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1631356 [wpt PR 23105] - ServiceWorker: Fix timeout on client-navigate.https.html, a=testonly Automatic update from web-platform-tests ServiceWorker: Fix timeout on client-navigate.https.html This CL fixes timeout on client-navigate.https.html, and removes unused code in testharness.js. This change is a follow-up for: [1] https://github.com/web-platform-tests/wpt/pull/21162 [2] https://github.com/web-platform-tests/wpt/pull/22086 After the change [1], fetch_tests_from_workers() helper uses ExtendableMessageEvent.source instead of MessageChannel to communicate between a window and a service worker. This change works well for most of cases, but fails some tests. The fetch_tests_from_workers() helper takes a service worker object to post a "connect" message to the worker, and then passes the worker object to RemoteContext in testharness.js. RemoteContext adds an onmessage event handler on self.navigator.serviceWorker. This works well as long as the given worker object is associated with `self`. However, these failing tests pass the worker object associated with the inner frame to the helper. In this case, the helper uses an inner frame's service worker object for postMessage(), while the helper waits on the main frame's navigator.serviceWorker.onmessage. Consequently event.source is the inner frame, and a reply from the service worker is dispatched on the inner frame's navigator.serviceWorker.onmessage. This results in timeout. To fix this, this CL makes the main frame pass its own service worker object instead of the inner frame's service worker object. Those objects should be equal other than associated contexts, and this change doesn't affect what the tests verify. Bug: 1057682 Change-Id: I0f30f1fe9c54c3de1006276f3445c5e2b92ea5a7 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2155760 Reviewed-by: Makoto Shimazu <shimazu@chromium.org> Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org> Cr-Commit-Position: refs/heads/master@{#760974} -- wpt-commits: 0a9ee5f5458c922aab15c25339db79e93df5c707 wpt-pr: 23105
testing/web-platform/tests/resources/testharness.js
testing/web-platform/tests/service-workers/service-worker/client-navigate.https.html
--- a/testing/web-platform/tests/resources/testharness.js
+++ b/testing/web-platform/tests/resources/testharness.js
@@ -393,30 +393,17 @@ policies and contribution forms [3].
     function ServiceWorkerTestEnvironment() {
         WorkerTestEnvironment.call(this);
         this.all_loaded = false;
         this.on_loaded_callback = null;
         var this_obj = this;
         self.addEventListener("message",
                 function(event) {
                     if (event.data && event.data.type && event.data.type === "connect") {
-                        if (event.ports && event.ports[0]) {
-                            // If a MessageChannel was passed, then use it to
-                            // send results back to the main window.  This
-                            // allows the tests to work even if the browser
-                            // does not fully support MessageEvent.source in
-                            // ServiceWorkers yet.
-                            this_obj._add_message_port(event.ports[0]);
-                            event.ports[0].start();
-                        } else {
-                            // If there is no MessageChannel, then attempt to
-                            // use the MessageEvent.source to send results
-                            // back to the main window.
-                            this_obj._add_message_port(event.source);
-                        }
+                        this_obj._add_message_port(event.source);
                     }
                 }, false);
 
         // The oninstall event is received after the service worker script and
         // all imported scripts have been fetched and executed. It's the
         // equivalent of an onload event for a document. All tests should have
         // been added by the time this event is received, thus it's not
         // necessary to wait until the onactivate event. However, tests for
--- a/testing/web-platform/tests/service-workers/service-worker/client-navigate.https.html
+++ b/testing/web-platform/tests/service-workers/service-worker/client-navigate.https.html
@@ -33,21 +33,23 @@
   }
 
   promise_test(function(t) {
     var worker = "resources/client-navigate-worker.js";
     var scope = "resources/client-navigate-frame.html";
     var controller, frame, clientId;
 
     return service_worker_unregister_and_register(t, worker, scope)
-      .then(reg => wait_for_state(t, reg.installing, "activated"))
+      .then(reg => {
+        controller = reg.installing;
+        return wait_for_state(t, reg.installing, "activated");
+      })
       .then(___ => with_iframe(scope))
       .then(f => {
         frame = f;
-        controller = frame.contentWindow.navigator.serviceWorker.controller;
         fetch_tests_from_worker(controller);
         return wait_for_message()
       })
       .then(({id}) => clientId = id)
       .then(___ => run_test(controller, clientId, "test_client_navigate_success"))
       .then(({result, url}) => {
         assert_equals(result, "test_client_navigate_success");
         assert_equals(
@@ -64,21 +66,23 @@
   }, "Frame location should update on successful navigation");
 
   promise_test(function(t) {
     var worker = "resources/client-navigate-worker.js";
     var scope = "resources/client-navigate-frame.html";
     var controller, frame, clientId;
 
     return service_worker_unregister_and_register(t, worker, scope)
-      .then(reg => wait_for_state(t, reg.installing, "activated"))
+      .then(reg => {
+        controller = reg.installing;
+        return wait_for_state(t, reg.installing, "activated");
+      })
       .then(___ => with_iframe(scope))
       .then(f => {
         frame = f;
-        controller = frame.contentWindow.navigator.serviceWorker.controller;
         fetch_tests_from_worker(controller);
         return wait_for_message()
       })
       .then(({id}) => clientId = id)
       .then(___ => run_test(controller, clientId, "test_client_navigate_redirect"))
       .then(({result, url}) => {
         assert_equals(result, "test_client_navigate_redirect");
         assert_equals(url, "");
@@ -90,21 +94,23 @@
   }, "Frame location should not be accessible after redirect");
 
   promise_test(function(t) {
     var worker = "resources/client-navigate-worker.js";
     var scope = "resources/client-navigate-frame.html";
     var controller, frame, clientId;
 
     return service_worker_unregister_and_register(t, worker, scope)
-      .then(reg => wait_for_state(t, reg.installing, "activated"))
+      .then(reg => {
+        controller = reg.installing;
+        return wait_for_state(t, reg.installing, "activated");
+      })
       .then(___ => with_iframe(scope))
       .then(f => {
         frame = f;
-        controller = frame.contentWindow.navigator.serviceWorker.controller;
         fetch_tests_from_worker(controller);
         return wait_for_message()
       })
       .then(({id}) => clientId = id)
       .then(___ => run_test(controller, clientId, "test_client_navigate_cross_origin"))
       .then(({result, url}) => {
         assert_equals(result, "test_client_navigate_cross_origin");
         assert_equals(url, "");
@@ -116,21 +122,23 @@
   }, "Frame location should not be accessible after cross-origin navigation");
 
   promise_test(function(t) {
     var worker = "resources/client-navigate-worker.js";
     var scope = "resources/client-navigate-frame.html";
     var controller, frame, clientId;
 
     return service_worker_unregister_and_register(t, worker, scope)
-      .then(reg => wait_for_state(t, reg.installing, "activated"))
+      .then(reg => {
+        controller = reg.installing;
+        return wait_for_state(t, reg.installing, "activated");
+      })
       .then(___ => with_iframe(scope))
       .then(f => {
         frame = f;
-        controller = frame.contentWindow.navigator.serviceWorker.controller;
         fetch_tests_from_worker(controller);
         return wait_for_message()
       })
       .then(({id}) => clientId = id)
       .then(___ => run_test(controller, clientId, "test_client_navigate_about_blank"))
       .then(({result, url}) => {
         assert_equals(result, "test_client_navigate_about_blank");
         assert_equals(
@@ -146,22 +154,22 @@
 
   promise_test(function(t) {
     var worker = "resources/client-navigate-worker.js";
     var scope = "resources/client-navigate-frame.html";
     var controller, frame, clientId;
 
     return service_worker_unregister_and_register(t, worker, scope)
       .then(reg => {
+        controller = reg.installing;
         return wait_for_state(t, reg.installing, "activated");
       })
       .then(___ => with_iframe(scope))
       .then(f => {
         frame = f;
-        controller = frame.contentWindow.navigator.serviceWorker.controller;
         fetch_tests_from_worker(controller);
         return wait_for_message()
       })
       .then(({id}) => clientId = id)
       .then(___ => run_test(controller, clientId, "test_client_navigate_mixed_content"))
       .then(({result, url}) => {
         assert_equals(result, "test_client_navigate_mixed_content");
         assert_equals(