Upstream service worker registration tests to WPT

Upstream service worker registration tests to WPT

The test for `getRegistration` in Chromium is identical to the
corresponding test in the Web Platform Tests project and may therefore
be removed.

The test for `getRegistrations` is likewise highly similar.
Schedule iframe removal to consistently occur following
sub-test completion (regardless of test results).

BUG=688116, 658997
R=mek@chromium.org

Review-Url: https://codereview.chromium.org/2853383004
Cr-Commit-Position: refs/heads/master@{#469071}
Committed: https://chromium.googlesource.com/chromium/src/+/4d4ce2a062601d712fcc1dc52f4e106fe4618579

[Marijn Kruisselbrink, 2017-05-03 16:31:41]

I don't think any of the changes (other than possibly removing the done() at the
end) to getRegistrations should be needed...

https://codereview.chromium.org/2853383004/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/getregistrations.https.html
(right):

https://codereview.chromium.org/2853383004/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/getregistrations.https.html:31:
.then(waitForUnregistered)
I don't think this waitForUnregistered change (rather than in the old WPT test
just asserting that all registrations are gone) should be needed either. I
believe the comment in the chromium version of this test ("As
registration.unregister() promises resolve before the corresponding
registrations are deleted from the storage, we must wait until the registrations
are actually removed from the storage.") is wrong. Yes, unregister() resolves
before the registration is deleted from storage, but getRegistrations is never
supposed to return registrations whose uninstalling flag is set (and that flag
is set before the promise is resolved).

https://codereview.chromium.org/2853383004/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/getregistrations.https.html:33:
// executed) after all registrations are known to be cleared.
promise_tests are ran sequentially anyway (i.e. the next promise test won't
start running until this promise test has completed), so I'm not sure what the
benefit is of having this defineTests thing? Having the waitForUnregistered
above should be enough. Similarly the explicit_done doesn't seem like it would
add anything?

[mike3, 2017-05-03 17:26:19]

> I don't think any of the changes (other than possibly removing
> the done() at the end) to getRegistrations should be needed...

You are correct! However, while rebasing this work, I seem to
have omitted a small modification mentioned in the description:
the use of `t.add_cleanup` to schedule iframe removal. I have
re-introduced that change in addition to implementing your
feedback.

Thanks!

https://codereview.chromium.org/2853383004/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/getregistrations.https.html
(right):

https://codereview.chromium.org/2853383004/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/getregistrations.https.html:31:
.then(waitForUnregistered)
On 2017/05/03 16:31:41, Marijn Kruisselbrink wrote:
> I don't think this waitForUnregistered change (rather than in the old WPT test
> just asserting that all registrations are gone) should be needed either. I
> believe the comment in the chromium version of this test ("As
> registration.unregister() promises resolve before the corresponding
> registrations are deleted from the storage, we must wait until the
registrations
> are actually removed from the storage.") is wrong. Yes, unregister() resolves
> before the registration is deleted from storage, but getRegistrations is never
> supposed to return registrations whose uninstalling flag is set (and that flag
> is set before the promise is resolved). 

Ah, right. I'll remove `waitForUnregistered`.

https://codereview.chromium.org/2853383004/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/getregistrations.https.html:33:
// executed) after all registrations are known to be cleared.
On 2017/05/03 16:31:41, Marijn Kruisselbrink wrote:
> promise_tests are ran sequentially anyway (i.e. the next promise test won't
> start running until this promise test has completed), so I'm not sure what the
> benefit is of having this defineTests thing? Having the waitForUnregistered
> above should be enough. Similarly the explicit_done doesn't seem like it would
> add anything?

I made both changes according to the mistaken belief that promise_tests
were run in parallel. That is not correct, so I will revert those.

[mike3, 2017-05-03 17:28:42]

Description was changed from

==========
Upstream service worker registration tests to WPT

The test for `getRegistration` in Chromium is identical to the
corresponding test in the Web Platform Tests project and may therefore
be removed.

The test for `getRegistrations` is likewise highly similar, although the
Chromium version is more explicit in its management of sub-test
sequencing. Implement equivalent logic in the Web Platform Tests version
in order to ensure that global state is properly initialized. In
addition, schedule iframe removal to consistently occur following
sub-test completion (regardless of test results).

BUG=688116, 658997
R=mek@chromium.org
==========

to

==========
Upstream service worker registration tests to WPT

The test for `getRegistration` in Chromium is identical to the
corresponding test in the Web Platform Tests project and may therefore
be removed.

The test for `getRegistrations` is likewise highly similar.
Schedule iframe removal to consistently occur following
sub-test completion (regardless of test results).

BUG=688116, 658997
R=mek@chromium.org
==========

[Marijn Kruisselbrink, 2017-05-03 17:39:17]

thanks, lgtm

[mike3, 2017-05-03 17:46:30]

The CQ bit was checked by mike@mikepennisi.com

[commit-bot: I haz the power, 2017-05-03 17:47:09]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-03 19:26:42]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1493833590268120,
"parent_rev": "9a3f5f993acb59b16a6e1d5f157a46f6c7a11df8", "commit_rev":
"4d4ce2a062601d712fcc1dc52f4e106fe4618579"}

[commit-bot: I haz the power, 2017-05-03 19:27:11]

Description was changed from

==========
Upstream service worker registration tests to WPT

The test for `getRegistration` in Chromium is identical to the
corresponding test in the Web Platform Tests project and may therefore
be removed.

The test for `getRegistrations` is likewise highly similar.
Schedule iframe removal to consistently occur following
sub-test completion (regardless of test results).

BUG=688116, 658997
R=mek@chromium.org
==========

to

==========
Upstream service worker registration tests to WPT

The test for `getRegistration` in Chromium is identical to the
corresponding test in the Web Platform Tests project and may therefore
be removed.

The test for `getRegistrations` is likewise highly similar.
Schedule iframe removal to consistently occur following
sub-test completion (regardless of test results).

BUG=688116, 658997
R=mek@chromium.org

Review-Url: https://codereview.chromium.org/2853383004
Cr-Commit-Position: refs/heads/master@{#469071}
Committed:
https://chromium.googlesource.com/chromium/src/+/4d4ce2a062601d712fcc1dc52f4e...
==========

[commit-bot: I haz the power, 2017-05-03 19:27:12]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/4d4ce2a062601d712fcc1dc52f4e...