Upstream service worker `fetch` tests to WPT

Upstream service worker `fetch` tests to WPT

**fetch-request-css-base-url**

The Chromium-specific version of this test is almost identical to the
version provided by the Web Platform Test suite. The upstream version
differs only in its use of Promises for control flow. However, the
upstream version also suffers from timeout issues in the build
environment.

Rename the Chromium-specific version to reflect its deprecated status,
and re-enable the upstream version as an expected timeout.

**fetch-request-xhr**

Update the upstream version to improve failure reporting and to include
novel tests from the Chromium version for HTTP header values (modified
to comply with the Fetch specification).

Update the Chromium-specific version to document its deprecated status
and invalid assertions.

Update both versions to improve state management via `Test#add_cleanup`.

**fetch-response-xhr**

Update the upstream version to avoid a race condition.

Update the Chromium-specific version to document its deprecated status.

Update both versions to improve state management via `Test#add_cleanup`.

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

Review-Url: https://codereview.chromium.org/2862353002
Cr-Commit-Position: refs/heads/master@{#470395}
Committed: https://chromium.googlesource.com/chromium/src/+/6a3e413c07f490356a2a9b5cf623954ed9d35827

[mike3, 2017-05-05 19:38:03]

Hi Mek. I think this is the last of the `fetch` tests.

[Marijn Kruisselbrink, 2017-05-05 23:59:00]

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

https://codereview.chromium.org/2862353002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/fetch-request-xhr.https.html:6:
<script src="resources/test-helpers.sub.js?pipe=sub"></script>
nit: no need for pipe=sub

https://codereview.chromium.org/2862353002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/fetch-request-xhr.https.html:20:
channel.port1.onmessage = t.step_func(function(e) {
random thought: it seems like this test could be cleaned up a bit by using
fetch_tests_from_window (i.e. rather than the custom MessageChannel/postMessage
protocol, you could just use that to have regular testharness.js tests in the
iframe, and load the results that way). Not sure how cleanup/uninstalling the
service worker would be dealt with in that case though...

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

https://codereview.chromium.org/2862353002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/fetch-response-xhr.https.html:6:
<script src="resources/test-helpers.sub.js?pipe=sub"></script>
nit: don't need pipe=sub

https://codereview.chromium.org/2862353002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/fetch-response-xhr.https.html:15:
t.step(function() {
why this t.step? on_message is already wrapped in t.step_func, so there
shouldn't be an need for t.step here, right?

[mike3, 2017-05-08 18:41:41]

Thanks, Mek. This one's ready for another look.

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

https://codereview.chromium.org/2862353002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/fetch-request-xhr.https.html:6:
<script src="resources/test-helpers.sub.js?pipe=sub"></script>
On 2017/05/05 23:59:00, Marijn Kruisselbrink wrote:
> nit: no need for pipe=sub

Done.

https://codereview.chromium.org/2862353002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/fetch-request-xhr.https.html:20:
channel.port1.onmessage = t.step_func(function(e) {
On 2017/05/05 23:59:00, Marijn Kruisselbrink wrote:
> random thought: it seems like this test could be cleaned up a bit by using
> fetch_tests_from_window (i.e. rather than the custom
MessageChannel/postMessage
> protocol, you could just use that to have regular testharness.js tests in the
> iframe, and load the results that way). Not sure how cleanup/uninstalling the
> service worker would be dealt with in that case though...

I've been working on an extension to the "multiglobal" tests to support Service
Workers:

https://github.com/bocoup/web-platform-tests/commit/f6775c1991db37973b162d769...

Since this test requires an iframe, it's not directly applicable, but it does
introduce a callback to `fetch_test_from_worker`, which suggests that a similar
extension to `fetch_test_from_window` might make sense. Still, that's a little
ways off, though, so I think I'd like to land this particular test as-is.

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

https://codereview.chromium.org/2862353002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/fetch-response-xhr.https.html:6:
<script src="resources/test-helpers.sub.js?pipe=sub"></script>
On 2017/05/05 23:59:00, Marijn Kruisselbrink wrote:
> nit: don't need pipe=sub

Done.

https://codereview.chromium.org/2862353002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/fetch-response-xhr.https.html:15:
t.step(function() {
On 2017/05/05 23:59:00, Marijn Kruisselbrink wrote:
> why this t.step? on_message is already wrapped in t.step_func, so there
> shouldn't be an need for t.step here, right?

Using `t.step` like this ensures that the protocol continues even if the
assertion fails. This doesn't seem to be necessary, though, so I'll remove it.

[Marijn Kruisselbrink, 2017-05-08 19:05:13]

lgtm

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

https://codereview.chromium.org/2862353002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/fetch-request-xhr.https.html:20:
channel.port1.onmessage = t.step_func(function(e) {
On 2017/05/08 at 18:41:40, mike3 wrote:
> On 2017/05/05 23:59:00, Marijn Kruisselbrink wrote:
> > random thought: it seems like this test could be cleaned up a bit by using
> > fetch_tests_from_window (i.e. rather than the custom
MessageChannel/postMessage
> > protocol, you could just use that to have regular testharness.js tests in
the
> > iframe, and load the results that way). Not sure how cleanup/uninstalling
the
> > service worker would be dealt with in that case though...
> 
> I've been working on an extension to the "multiglobal" tests to support
Service
> Workers:
> 
>
https://github.com/bocoup/web-platform-tests/commit/f6775c1991db37973b162d769...
> 
> Since this test requires an iframe, it's not directly applicable, but it does
> introduce a callback to `fetch_test_from_worker`, which suggests that a
similar
> extension to `fetch_test_from_window` might make sense. Still, that's a little
> ways off, though, so I think I'd like to land this particular test as-is.

Agreed, landing as is is fine. Adding a done callback like in that CL does seem
like a good idea, yes. Or maybe a generic async cleanup mechanism...

[mike3, 2017-05-08 21:04:59]

The CQ bit was checked by mike@mikepennisi.com

[commit-bot: I haz the power, 2017-05-08 21:05:57]

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-08 21:12:33]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-08 21:12:35]

Try jobs failed on following builders:
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  android_cronet on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_cron...)

[mike3, 2017-05-09 16:35:56]

This latest failure seems to be due to another conflict in the
`enable-blink-features=LayoutNG` file. I've rebased the patch, resolving the
conflict. Re-starting the build now.

[mike3, 2017-05-09 16:36:02]

The CQ bit was checked by mike@mikepennisi.com

[mike3, 2017-05-09 16:36:03]

The patchset sent to the CQ was uploaded after l-g-t-m from mek@chromium.org
Link to the patchset: https://codereview.chromium.org/2862353002/#ps40001
(title: "Rebase and resolve conflict in 'expectations' file")

[commit-bot: I haz the power, 2017-05-09 16:36:47]

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-09 18:51:59]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1494347762693650,
"parent_rev": "14bd822963d10989b62db8bd675e3743b5780fa5", "commit_rev":
"6a3e413c07f490356a2a9b5cf623954ed9d35827"}

[commit-bot: I haz the power, 2017-05-09 18:52:15]

Description was changed from

==========
Upstream service worker `fetch` tests to WPT

**fetch-request-css-base-url**

The Chromium-specific version of this test is almost identical to the
version provided by the Web Platform Test suite. The upstream version
differs only in its use of Promises for control flow. However, the
upstream version also suffers from timeout issues in the build
environment.

Rename the Chromium-specific version to reflect its deprecated status,
and re-enable the upstream version as an expected timeout.

**fetch-request-xhr**

Update the upstream version to improve failure reporting and to include
novel tests from the Chromium version for HTTP header values (modified
to comply with the Fetch specification).

Update the Chromium-specific version to document its deprecated status
and invalid assertions.

Update both versions to improve state management via `Test#add_cleanup`.

**fetch-response-xhr**

Update the upstream version to avoid a race condition.

Update the Chromium-specific version to document its deprecated status.

Update both versions to improve state management via `Test#add_cleanup`.

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

to

==========
Upstream service worker `fetch` tests to WPT

**fetch-request-css-base-url**

The Chromium-specific version of this test is almost identical to the
version provided by the Web Platform Test suite. The upstream version
differs only in its use of Promises for control flow. However, the
upstream version also suffers from timeout issues in the build
environment.

Rename the Chromium-specific version to reflect its deprecated status,
and re-enable the upstream version as an expected timeout.

**fetch-request-xhr**

Update the upstream version to improve failure reporting and to include
novel tests from the Chromium version for HTTP header values (modified
to comply with the Fetch specification).

Update the Chromium-specific version to document its deprecated status
and invalid assertions.

Update both versions to improve state management via `Test#add_cleanup`.

**fetch-response-xhr**

Update the upstream version to avoid a race condition.

Update the Chromium-specific version to document its deprecated status.

Update both versions to improve state management via `Test#add_cleanup`.

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

Review-Url: https://codereview.chromium.org/2862353002
Cr-Commit-Position: refs/heads/master@{#470395}
Committed:
https://chromium.googlesource.com/chromium/src/+/6a3e413c07f490356a2a9b5cf623...
==========

[commit-bot: I haz the power, 2017-05-09 18:52:18]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/6a3e413c07f490356a2a9b5cf623...