Upstream service worker `fetch` test to WPT

Upstream service worker `fetch` test to WPT

**fetch-request-css-cross-origin-mime-check**

Update resource file paths as appropriate and use HTTPS version of
absolute URLs.

**fetch-request-fallback**

Both the Chromium version and upstream version of this test cover the
following conditions:

- same origin XHR should succeed
- CORS-unsupported other origin XHR should fail
- CORS-supported other origin XHR should succeed
- redirected XHR should succeed
- XHR which is redirected to CORS-unsupported other origin should fail
- XHR which is redirected to CORS-supported other origin should succeed

However, the Chromium version includes additional tests for image
requests:

- image request should succeed
- other origin image request should succeed
- CORS-unsupported other origin image request should fail
- CORS-supported other origin image request should succeed
- redirected image request should succeed
- image request which is redirected to other origin should succeed
- image request which is redirected to CORS-unsupported other origin should fail
- image request which is redirected to CORS-supported other origin should succeed

The two versions differ significantly in test structure, as well. Adopt
the Chromium version's structure because that implements more granular
assertion messages.

BUG=688116
R=mek@chromium.org

Review-Url: https://codereview.chromium.org/2858933003
Cr-Commit-Position: refs/heads/master@{#470248}
Committed: https://chromium.googlesource.com/chromium/src/+/906aedf74008e1d36dfe1d48fdf236afb3cc5572

[mike3, 2017-05-03 20:44:00]

Would you mind taking a look, Mek?

[Marijn Kruisselbrink, 2017-05-04 00:28:25]

Sorry, haven't looked in too much detail yet, will do so tomorrow.

Just a general thought though, in a way fetch-request-fallback.https.html is
very similar in structure to fetch-response-taint.https.html. Some shared setup
code, followed by a whole bunch of mostly independent tests. It seems like this
test would be a lot nicer if like the other it would consist of a bunch of
separate promise_tests, rather than the one long promise chain (that would also
eliminate the need for the weird assert_resolves thing, as with test names we
wouldn't need assertion descriptions to document which test didn't pass).

But rewriting it like that is probably a not insignificant amount of work...

https://codereview.chromium.org/2858933003/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/fetch-request-css-cross-origin-mime-check.https.html
(right):

https://codereview.chromium.org/2858933003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/fetch-request-css-cross-origin-mime-check.https.html:5:
<script src="/common/get-host-info.sub.js?pipe=sub"></script>
nit: here too no need for the pipe=sub

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

https://codereview.chromium.org/2858933003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/fetch-request-fallback.https.html:10:
promise
Is this any different from promise.catch(t.unreached_func(description)), or
promise.catch(() => assert_unreached(description)) if you want to avoid needing
the Test?

https://codereview.chromium.org/2858933003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/fetch-request-fallback.https.html:17:
function assert_rejects(promise, description) {
assert_rejects is pretty much promise_rejects(t, null, promise, description)
right? Might as well use that one then? (also promise_rejects was deliberately
not called assert_promise_rejects, as unlike all assert_* methods, it doesn't
synchronously assert anything).

[mike3, 2017-05-05 18:56:12]

> But rewriting it like that is probably a not insignificant amount of work...

I'm game :)

https://codereview.chromium.org/2858933003/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/fetch-request-css-cross-origin-mime-check.https.html
(right):

https://codereview.chromium.org/2858933003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/fetch-request-css-cross-origin-mime-check.https.html:5:
<script src="/common/get-host-info.sub.js?pipe=sub"></script>
On 2017/05/04 00:28:24, Marijn Kruisselbrink wrote:
> nit: here too no need for the pipe=sub

Acknowledged.

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

https://codereview.chromium.org/2858933003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/fetch-request-fallback.https.html:10:
promise
On 2017/05/04 00:28:25, Marijn Kruisselbrink wrote:
> Is this any different from promise.catch(t.unreached_func(description)), or
> promise.catch(() => assert_unreached(description)) if you want to avoid
needing
> the Test?

No difference. I'll use that approach.

https://codereview.chromium.org/2858933003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/fetch-request-fallback.https.html:17:
function assert_rejects(promise, description) {
On 2017/05/04 00:28:25, Marijn Kruisselbrink wrote:
> assert_rejects is pretty much promise_rejects(t, null, promise, description)
> right? Might as well use that one then? (also promise_rejects was deliberately
> not called assert_promise_rejects, as unlike all assert_* methods, it doesn't
> synchronously assert anything).

Acknowledged.

[Marijn Kruisselbrink, 2017-05-05 23:50:26]

thanks, lgtm

https://codereview.chromium.org/2858933003/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/fetch-request-fallback.https.html
(right):

https://codereview.chromium.org/2858933003/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/fetch-request-fallback.https.html:84:
'The SW must intercept the request for a main resource.');
I think this was supposed to be the test name on the next line, rather then some
ignored third parameter to check_urls?

[mike3, 2017-05-08 18:16:59]

https://codereview.chromium.org/2858933003/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/fetch-request-fallback.https.html
(right):

https://codereview.chromium.org/2858933003/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/fetch-request-fallback.https.html:84:
'The SW must intercept the request for a main resource.');
On 2017/05/05 23:50:25, Marijn Kruisselbrink wrote:
> I think this was supposed to be the test name on the next line, rather then
some
> ignored third parameter to check_urls?

Done.

[mike3, 2017-05-08 18:17:20]

The CQ bit was checked by mike@mikepennisi.com

[mike3, 2017-05-08 18:17:20]

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/2858933003/#ps60001
(title: "Correct specification of sub-test title")

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

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 18:24:53]

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

[commit-bot: I haz the power, 2017-05-08 18:24:55]

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...)
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[mike3, 2017-05-08 21:16:34]

There was a conflict in the file named `enable-blink-features=LayoutNG`, and I
believe this is the cause of the errors on the previous "try" job.

If I enable the "Commit" checkbox in situations like this, will the subsequent
job result in a commit to `master`? Or will it simply perform a dry run?

[falken, 2017-05-09 00:30:01]

On 2017/05/08 21:16:34, mike3 wrote:
> There was a conflict in the file named `enable-blink-features=LayoutNG`, and I
> believe this is the cause of the errors on the previous "try" job.
> 
> If I enable the "Commit" checkbox in situations like this, will the subsequent
> job result in a commit to `master`? Or will it simply perform a dry run?

Clicking the commit checkbox is always "real", unless you click the CQ dry run
link.

It'll run another commit queue job, and if the job succeeds it'll commit to
master.

[falken, 2017-05-09 00:30:13]

The CQ bit was checked by falken@chromium.org

[falken, 2017-05-09 00:30:14]

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/2858933003/#ps80001
(title: "Resolve conflict in `enable-blink-features=LayoutNG`")

[commit-bot: I haz the power, 2017-05-09 00:40:10]

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 01:21:15]

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

[commit-bot: I haz the power, 2017-05-09 01:21:17]

Try jobs failed on following builders:
  win_chromium_compile_dbg_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
  win_clang on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)

[falken, 2017-05-09 05:09:51]

The CQ bit was checked by falken@chromium.org

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

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 06:26:32]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1494306591190550,
"parent_rev": "0979251b5a413aa37bb82e8d72d75cd7a1c5e106", "commit_rev":
"906aedf74008e1d36dfe1d48fdf236afb3cc5572"}

[commit-bot: I haz the power, 2017-05-09 06:26:45]

Description was changed from

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

**fetch-request-css-cross-origin-mime-check**

Update resource file paths as appropriate and use HTTPS version of
absolute URLs.

**fetch-request-fallback**

Both the Chromium version and upstream version of this test cover the
following conditions:

- same origin XHR should succeed
- CORS-unsupported other origin XHR should fail
- CORS-supported other origin XHR should succeed
- redirected XHR should succeed
- XHR which is redirected to CORS-unsupported other origin should fail
- XHR which is redirected to CORS-supported other origin should succeed

However, the Chromium version includes additional tests for image
requests:

- image request should succeed
- other origin image request should succeed
- CORS-unsupported other origin image request should fail
- CORS-supported other origin image request should succeed
- redirected image request should succeed
- image request which is redirected to other origin should succeed
- image request which is redirected to CORS-unsupported other origin should fail
- image request which is redirected to CORS-supported other origin should
succeed

The two versions differ significantly in test structure, as well. Adopt
the Chromium version's structure because that implements more granular
assertion messages.

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

to

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

**fetch-request-css-cross-origin-mime-check**

Update resource file paths as appropriate and use HTTPS version of
absolute URLs.

**fetch-request-fallback**

Both the Chromium version and upstream version of this test cover the
following conditions:

- same origin XHR should succeed
- CORS-unsupported other origin XHR should fail
- CORS-supported other origin XHR should succeed
- redirected XHR should succeed
- XHR which is redirected to CORS-unsupported other origin should fail
- XHR which is redirected to CORS-supported other origin should succeed

However, the Chromium version includes additional tests for image
requests:

- image request should succeed
- other origin image request should succeed
- CORS-unsupported other origin image request should fail
- CORS-supported other origin image request should succeed
- redirected image request should succeed
- image request which is redirected to other origin should succeed
- image request which is redirected to CORS-unsupported other origin should fail
- image request which is redirected to CORS-supported other origin should
succeed

The two versions differ significantly in test structure, as well. Adopt
the Chromium version's structure because that implements more granular
assertion messages.

BUG=688116
R=mek@chromium.org

Review-Url: https://codereview.chromium.org/2858933003
Cr-Commit-Position: refs/heads/master@{#470248}
Committed:
https://chromium.googlesource.com/chromium/src/+/906aedf74008e1d36dfe1d48fdf2...
==========

[commit-bot: I haz the power, 2017-05-09 06:26:47]

Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/906aedf74008e1d36dfe1d48fdf2...

[Marijn Kruisselbrink, 2017-05-09 18:09:09]

https://codereview.chromium.org/2858933003/diff/80001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/fetch-request-fallback.https.html
(right):

https://codereview.chromium.org/2858933003/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/fetch-request-fallback.https.html:133:
null,
my mistake, but as this landed, annevk landed a change that made passing "null"
to assert_throws or promise_rejects cause an assertion failure. So the
upstreamed version of this test doesn't actually pass... You probably want to
change "null" here (and wherever else you're passing it to promise_rejects) to
the correct exception (presumably new TypeError).

[mike3, 2017-05-09 18:53:40]

On 2017/05/09 18:09:09, Marijn Kruisselbrink wrote:
>
https://codereview.chromium.org/2858933003/diff/80001/third_party/WebKit/Layo...
> File
>
third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/fetch-request-fallback.https.html
> (right):
> 
>
https://codereview.chromium.org/2858933003/diff/80001/third_party/WebKit/Layo...
>
third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/fetch-request-fallback.https.html:133:
> null,
> my mistake, but as this landed, annevk landed a change that made passing
"null"
> to assert_throws or promise_rejects cause an assertion failure. So the
> upstreamed version of this test doesn't actually pass... You probably want to
> change "null" here (and wherever else you're passing it to promise_rejects) to
> the correct exception (presumably new TypeError).

I'm glad that Anne implemented that change. I should have done this in the
first place! I'll correct this in WPT directly.