ServiceWorker: Check the equality of JS-level SW/SWR objects

third_party/WebKit/LayoutTests/http/tests/serviceworker/synced-state.html

Index: third_party/WebKit/LayoutTests/http/tests/serviceworker/synced-state.html
diff --git a/third_party/WebKit/LayoutTests/http/tests/serviceworker/synced-state.html b/third_party/WebKit/LayoutTests/http/tests/serviceworker/synced-state.html
index 63a6e5b23226e2cc655dd3473da658a965f77d2e..25641ed4be5bc400929e5ba9a678697ae6873f4b 100644
--- a/third_party/WebKit/LayoutTests/http/tests/serviceworker/synced-state.html
+++ b/third_party/WebKit/LayoutTests/http/tests/serviceworker/synced-state.html
@@ -4,18 +4,37 @@
 <script src="../resources/testharnessreport.js"></script>
 <script src="resources/test-helpers.js"></script>
 <script>
+
+function multiple_resolver(num_of_waiter, resolve) {

[nhiroki, 2016/04/13 05:44:10]

Optional: 'barrier' (barrier_resolver?) would be common name for such a feature:
https://en.wikipedia.org/wiki/Barrier_(computer_science)
https://code.google.com/p/chromium/codesearch#chromium/src/base/barrier_closu...

num_of_waiter -> num_of_waiters

[shimazu (google), 2016/04/15 05:19:03]

Done.

+  var counter = 0;
+  return function() {
+    counter++;
+    if (counter === num_of_waiter)
+      resolve();
+  }
+}
+
 // Tests that ServiceWorker objects representing the same Service Worker
-// entity have the same state. JS object equality is not tested, since the spec
-// does not require it.
+// entity have the same state. JS-level equality is now required according to
+// the spec.
 promise_test(function(t) {
     var scope = 'resources/synced-state';
     var script = 'resources/empty-worker.js';
     return service_worker_unregister_and_register(t, script, scope)
       .then(function(registration) {

[nhiroki, 2016/04/13 05:44:10]

Can you add this?

  add_completion_callback(function() { registration.unregister(); });

[shimazu (google), 2016/04/15 05:19:03]

Done.

           return new Promise(function(resolve) {
-              var step = 0;
-              registration.installing.addEventListener('statechange',
-                                                       function(e) {
+            var step = 0;
+            var multiple_resolve = multiple_resolver(2, resolve);
+            registration.installing.addEventListener(
+                'statechange', function(e) {
+                  navigator.serviceWorker.getRegistration(scope)
+                    .then(function(r) {
+                        assert_equals(r, registration,
+                                      'Equality of registration should ' +
+                                      'be kept all the time: ' + step);

[nhiroki, 2016/04/13 05:44:10]

Hmm... this should work, but I'd prefer not to have assertions that could be
non-deterministic. I suspect this assertion could not be tested on some steps
because the spec does not ensure the execution order of statechange events and
getRegistration() calls.

If you really want to check the equality of registrations before/after the
statechange event, how about checking it only on statechange('activated') like
this?

  var registration;
  register().then(r => {
        registration = r;
        return new Promise(resolve => {
            r.installing.addEventListener('statechange', e => {
                // Test the step 1, 2, 3;

                if (step == 3)  // .state is 'activated'
                  resolve();
            });
        });
      })
    .then(() => {
        // statechange event does not happen anymore,
        // so we can deterministically check the equality.
        return getRegistration();
      })
    .then(r => {
        assert_equals(r, registration);
      })

[shimazu (google), 2016/04/15 05:19:03]

Done.

+                        if (step == 3)
+                          multiple_resolve();
+                      });
                   step++;
                   if (step == 1) {
                     assert_equals(e.currentTarget.state, 'installed',
@@ -23,9 +42,13 @@ promise_test(function(t) {
                     assert_equals(registration.installing, null,
                                   'in installed, .installing should be null');
                     assert_equals(registration.waiting.state, 'installed',
-                                  'in installed, .waiting should be installed');
+                                  'in installed, the state of .waiting ' +
+                                  'should be installed');
                     assert_equals(registration.active, null,
                                   'in installed, .active should be null');
+                    assert_equals(registration.waiting, e.currentTarget,
+                                  '.waiting should be equal to the original ' +
+                                  'SW in installed');
                   } else if (step == 2) {
                     assert_equals(e.currentTarget.state, 'activating',
                                   'original SW should be activating');
@@ -33,9 +56,12 @@ promise_test(function(t) {
                                   'in activating, .installing should be null');
                     assert_equals(registration.waiting, null,
                                   'in activating, .waiting should be null');
-                    assert_equals(
-                        registration.active.state, 'activating',
-                        'in activating, .active should be activating');
+                    assert_equals(registration.active.state, 'activating',
+                                  'in activating, the state of .active ' +
+                                  'should be activating');
+                    assert_equals(registration.active, e.currentTarget,
+                                  '.active should be equal to the original ' +
+                                  'SW in activating');
                   } else if (step == 3) {
                     assert_equals(e.currentTarget.state, 'activated',
                                   'original SW should be activated');
@@ -44,11 +70,15 @@ promise_test(function(t) {
                     assert_equals(registration.waiting, null,
                                   'in activated, .waiting should be null');
                     assert_equals(registration.active.state, 'activated',
-                                  'in activated .active should be activated');
-                    resolve();
+                                  'in activated, the state of .active should ' +
+                                  'be activated');
+                    assert_equals(registration.active, e.currentTarget,
+                                  '.active should be equal to the original ' +
+                                  'SW in activated');
+                    multiple_resolve();
                   }
-                })
-            })
+                });
+          });
         })
       .then(function() {
           return service_worker_unregister_and_done(t, scope);

[nhiroki, 2016/04/13 05:44:10]

(Not related to your changes)

We don't have to call done() here bacause promise_test() internally calls it.
After you add "add_completion_callback" as my comment at line 24, you can just
remove this .then() chain.

[shimazu (google), 2016/04/15 05:19:03]

Done.