Upstream service worker `client` tests to WPT

third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/clients-get-cross-origin.https.html

Index: third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/clients-get-cross-origin.https.html
diff --git a/third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/clients-get-cross-origin.https.html b/third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/clients-get-cross-origin.https.html
index d5622054de8280e5b6ff2a0251de42c94ee49bd8..650c1ef0f92e1fdbc1322eaaf67e8c20005940bf 100644
--- a/third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/clients-get-cross-origin.https.html
+++ b/third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/clients-get-cross-origin.https.html
@@ -7,36 +7,42 @@
 <script>
 var host_info = get_host_info();
 
-var scope = 'resources/blank.html?clients-get';
-var t = async_test('Test Clients.get() cross origin');
+var scope = 'resources/clients-get-frame.html';
 var other_origin_iframe = host_info['HTTPS_REMOTE_ORIGIN'] + base_path() +
-                          'resources/clients-get-other-origin.html';
-var myOriginClientId;
-t.step(function() {
-    service_worker_unregister_and_register(
+                          'resources/clients-get-cross-origin-frame.html';
+var my_origin_client_id;
+promise_test(function(t) {

[falken, 2017/03/28 07:24:39]

I think this test could benefit from a high level comment about what it's doing.
It's cumbersome to open the iframe file, the service worker file, and this file
and try to piece it together.

[mike3, 2017/03/28 20:45:08]

Acknowledged.

+    return service_worker_unregister_and_register(
         t, 'resources/clients-get-worker.js', scope)
       .then(function(registration) {
+          add_completion_callback(function() { registration.unregister(); });
           return wait_for_state(t, registration.installing, 'activated');
         })
       .then(function() {
           return with_iframe(scope);
         })
       .then(function(frame1) {
-          myOriginClientId = frame1.contentDocument.body.textContent;
+          add_completion_callback(function() { frame1.remove(); });
+          return new Promise(function(resolve, reject) {
+            function get_client_id(e) {
+              window.removeEventListener('message', get_client_id);
+              resolve(e.data.clientId);
+            }
+            window.addEventListener('message', get_client_id, false);
+          });
+        })
+      .then(function(client_id) {
+          my_origin_client_id = client_id;
           return with_iframe(other_origin_iframe);
         })
       .then(function(frame2) {
-          window.addEventListener('message', on_message_other_origin, false);
+          add_completion_callback(function() { frame2.remove(); });
+          window.addEventListener('message', function(e) {
+            assert_equals(e.data.result, undefined);

[falken, 2017/03/28 07:24:39]

It's hard to tell what this test is trying to test, by reading this. Can we have
comments or assert_equals description?

[mike3, 2017/03/28 20:45:09]

Agreed. I have also moved the single assertion from the iframe source into this
file. This makes the test file more clear and improves error reporting at the
expense of extra complexity in the parent-to-iframe protocol. I think it's
worth it.

+            t.done();

[falken, 2017/03/28 07:24:39]

This t.done() doesn't look right, because this is a promise_test. I think this
test would end immediately after the postMessage in line 44.

[mike3, 2017/03/28 20:45:08]

You are correct: that is a vestage of the WPT version which I neglected to
update. That assertion was still evaluated because the iframe is *also* running
the testing harness, so it produces a number of irrelevant `message` events.
Resolving this required formalizing the postMessage protocol (adding a `type`
attribute to all messages) so that we could wait on the correct message to
detect test completion.

+          });
           frame2.contentWindow.postMessage(
-              {clientId: myOriginClientId,
-               message: 'get_client_id'},
-              host_info['HTTPS_REMOTE_ORIGIN']);
-        })
-      .catch(unreached_rejection(t));
-  });
-
-function on_message_other_origin(e) {
-  assert_equals(e.data.result, undefined);
-  t.done();
-}
+              {clientId: my_origin_client_id}, host_info['HTTP_REMOTE_ORIGIN']);
+        });
+  }, 'Test Clients.get() cross origin');
 </script>