ServiceWorker: Add layout tests for fetch event response

ServiceWorker: Add layout tests for fetch event response body

Multi-sided patch to implement blob-type fetch event 
response bodies:
1: blink-side, https://codereview.chromium.org/307063002/
2: chromium-side, https://codereview.chromium.org/315023003/
3: blink-side, THIS PATCH

BUG=379074
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=175538

[michaeln, 2014-06-02 23:05:45]

it'd be good if somebody with better js chops than me took a look

[jsbell, 2014-06-02 23:27:34]

I think michaeln@ was paging me...

Mostly l*g*t*m but I think the reject test is using the wrong worker, so not
actually getting tested?

https://codereview.chromium.org/304233017/diff/1/LayoutTests/http/tests/servi...
File LayoutTests/http/tests/serviceworker/fetch-event-expected.txt (right):

https://codereview.chromium.org/304233017/diff/1/LayoutTests/http/tests/servi...
LayoutTests/http/tests/serviceworker/fetch-event-expected.txt:1: 
This will need to be regenerated, following http://crrev.com/175288

https://codereview.chromium.org/304233017/diff/1/LayoutTests/http/tests/servi...
File LayoutTests/http/tests/serviceworker/fetch-event.html (right):

https://codereview.chromium.org/304233017/diff/1/LayoutTests/http/tests/servi...
LayoutTests/http/tests/serviceworker/fetch-event.html:22:
assert_equals(frame.contentDocument.body.textContent, "hello, world");
Please add a description to the assertion, e.g. 'Worker should have responded to
fetch with a fixed string'

https://codereview.chromium.org/304233017/diff/1/LayoutTests/http/tests/servi...
LayoutTests/http/tests/serviceworker/fetch-event.html:43:
assert_equals(frame.contentDocument.body.textContent, "Here's a simple html
file.\n");
Ditto, e.g. 'Worker should not have intercepted fetch'

https://codereview.chromium.org/304233017/diff/1/LayoutTests/http/tests/servi...
LayoutTests/http/tests/serviceworker/fetch-event.html:54: t,
'resources/empty-worker.js', scope, onRegister);
Shouldn't this be reject-fetch-event-worker.js ?

https://codereview.chromium.org/304233017/diff/1/LayoutTests/http/tests/servi...
LayoutTests/http/tests/serviceworker/fetch-event.html:64:
assert_equals(frame.contentDocument.body.textContent, "Here's a simple html
file.\n");
Ditto.

https://codereview.chromium.org/304233017/diff/1/LayoutTests/http/tests/servi...
File LayoutTests/http/tests/serviceworker/resources/hello-world-worker.js
(right):

https://codereview.chromium.org/304233017/diff/1/LayoutTests/http/tests/servi...
LayoutTests/http/tests/serviceworker/resources/hello-world-worker.js:2:
event.respondWith(new Response(new Blob(["hello, world"])));
This is just so awesome to see working! :)

https://codereview.chromium.org/304233017/diff/1/LayoutTests/http/tests/servi...
File LayoutTests/http/tests/serviceworker/resources/simple.html (right):

https://codereview.chromium.org/304233017/diff/1/LayoutTests/http/tests/servi...
LayoutTests/http/tests/serviceworker/resources/simple.html:2: Here's a simple
html file.
Really pedantic nit: should have a <title></title> to be valid HTML :)

https://codereview.chromium.org/304233017/diff/1/LayoutTests/virtual/servicew...
File
LayoutTests/virtual/serviceworker/http/tests/serviceworker/fetch-event-expected.txt
(right):

https://codereview.chromium.org/304233017/diff/1/LayoutTests/virtual/servicew...
LayoutTests/virtual/serviceworker/http/tests/serviceworker/fetch-event-expected.txt:1:

This will need to be regenerated, following http://crrev.com/175288

[falken, 2014-06-03 02:26:29]

Thanks! PTAL

https://codereview.chromium.org/304233017/diff/1/LayoutTests/http/tests/servi...
File LayoutTests/http/tests/serviceworker/fetch-event-expected.txt (right):

https://codereview.chromium.org/304233017/diff/1/LayoutTests/http/tests/servi...
LayoutTests/http/tests/serviceworker/fetch-event-expected.txt:1: 
On 2014/06/02 23:27:34, jsbell wrote:
> This will need to be regenerated, following http://crrev.com/175288

Done.

https://codereview.chromium.org/304233017/diff/1/LayoutTests/http/tests/servi...
File LayoutTests/http/tests/serviceworker/fetch-event.html (right):

https://codereview.chromium.org/304233017/diff/1/LayoutTests/http/tests/servi...
LayoutTests/http/tests/serviceworker/fetch-event.html:22:
assert_equals(frame.contentDocument.body.textContent, "hello, world");
On 2014/06/02 23:27:34, jsbell wrote:
> Please add a description to the assertion, e.g. 'Worker should have responded
to
> fetch with a fixed string'

Done.

https://codereview.chromium.org/304233017/diff/1/LayoutTests/http/tests/servi...
LayoutTests/http/tests/serviceworker/fetch-event.html:43:
assert_equals(frame.contentDocument.body.textContent, "Here's a simple html
file.\n");
On 2014/06/02 23:27:34, jsbell wrote:
> Ditto, e.g. 'Worker should not have intercepted fetch'

Done.

https://codereview.chromium.org/304233017/diff/1/LayoutTests/http/tests/servi...
LayoutTests/http/tests/serviceworker/fetch-event.html:54: t,
'resources/empty-worker.js', scope, onRegister);
On 2014/06/02 23:27:34, jsbell wrote:
> Shouldn't this be reject-fetch-event-worker.js ?

Of course! Done.

https://codereview.chromium.org/304233017/diff/1/LayoutTests/http/tests/servi...
LayoutTests/http/tests/serviceworker/fetch-event.html:64:
assert_equals(frame.contentDocument.body.textContent, "Here's a simple html
file.\n");
On 2014/06/02 23:27:34, jsbell wrote:
> Ditto.

Done.

https://codereview.chromium.org/304233017/diff/1/LayoutTests/http/tests/servi...
File LayoutTests/http/tests/serviceworker/resources/hello-world-worker.js
(right):

https://codereview.chromium.org/304233017/diff/1/LayoutTests/http/tests/servi...
LayoutTests/http/tests/serviceworker/resources/hello-world-worker.js:2:
event.respondWith(new Response(new Blob(["hello, world"])));
On 2014/06/02 23:27:34, jsbell wrote:
> This is just so awesome to see working! :)

Yay!

https://codereview.chromium.org/304233017/diff/1/LayoutTests/http/tests/servi...
File LayoutTests/http/tests/serviceworker/resources/simple.html (right):

https://codereview.chromium.org/304233017/diff/1/LayoutTests/http/tests/servi...
LayoutTests/http/tests/serviceworker/resources/simple.html:2: Here's a simple
html file.
On 2014/06/02 23:27:34, jsbell wrote:
> Really pedantic nit: should have a <title></title> to be valid HTML :)

I like such nits :) Done

https://codereview.chromium.org/304233017/diff/1/LayoutTests/virtual/servicew...
File
LayoutTests/virtual/serviceworker/http/tests/serviceworker/fetch-event-expected.txt
(right):

https://codereview.chromium.org/304233017/diff/1/LayoutTests/virtual/servicew...
LayoutTests/virtual/serviceworker/http/tests/serviceworker/fetch-event-expected.txt:1:

On 2014/06/02 23:27:34, jsbell wrote:
> This will need to be regenerated, following http://crrev.com/175288

Done.

[falken, 2014-06-03 04:20:47]

Actually I must revise this patch a bit:
- Need to ensure tests that use the same request URL (simple.html) run in
sequence, or else it's racy which SW you're actually using
- Add a test for new Response with null body

[falken, 2014-06-03 08:13:25]

On 2014/06/03 04:20:47, falken wrote:
> Actually I must revise this patch a bit:
> - Need to ensure tests that use the same request URL (simple.html) run in
> sequence, or else it's racy which SW you're actually using
> - Add a test for new Response with null body

Revised as per above, PTAL. A lot of the test JS will be simplified when
request.url lands, and there won't be a need to register one SW per fetch event
test.

[jsbell, 2014-06-03 17:51:26]

lgtm, with a thought (not sure if it's a good idea or not)

https://codereview.chromium.org/304233017/diff/80001/LayoutTests/http/tests/s...
File LayoutTests/http/tests/serviceworker/fetch-event.html (right):

https://codereview.chromium.org/304233017/diff/80001/LayoutTests/http/tests/s...
LayoutTests/http/tests/serviceworker/fetch-event.html:78: // a non-existent
file. simple.html would be better but this test runs concurrently with
Would 'resources/simple.html?null' work? I'm not 100% sure how the HTTP server
handles query params, but they should (1) serve up the same file, but (2) be
treated as separate resources/scopes by the SW.

(If any of the tests use just 'resources/simple.html' it would break unless the
longest-scope matching patch has landed, though.)

Hrm... would that help avoid the racy stuff above that requires
Promise.then()-ing the tests, if each test uses a distinct scope?

[falken, 2014-06-04 08:50:29]

Thanks Joshua! It turned out that request.url landed before this patch, so I
changed the tests to make use of it.

The test portion of the patch has completely changed, so can someone take
another look?

Thanks for the query parameter trick idea, also. I ended up using it in the new
patch.

[jsbell, 2014-06-04 17:42:52]

lgtm...

...but half the test script ends up being custom plumbing to run and clean-up
after the tests. WDYT about just using a different registration/scope for each
test, where the scope is just the URL that's going to be requested? Then each
async test is completely independent and the overall script can be simpler?

[falken, 2014-06-05 02:25:57]

On 2014/06/04 17:42:52, jsbell wrote:
> lgtm...
> 
> ...but half the test script ends up being custom plumbing to run and clean-up
> after the tests. WDYT about just using a different registration/scope for each
> test, where the scope is just the URL that's going to be requested? Then each
> async test is completely independent and the overall script can be simpler?

Done. Yes that is much nicer, thanks. As the fetch-event.html was completely
rewritten (again), could someone review please?

[dominicc (has gone to gerrit), 2014-06-05 04:56:05]

On 2014/06/05 02:25:57, falken wrote:
> On 2014/06/04 17:42:52, jsbell wrote:
> > lgtm...
> > 
> > ...but half the test script ends up being custom plumbing to run and
clean-up
> > after the tests. WDYT about just using a different registration/scope for
each
> > test, where the scope is just the URL that's going to be requested? Then
each
> > async test is completely independent and the overall script can be simpler?
> 
> Done. Yes that is much nicer, thanks. As the fetch-event.html was completely
> rewritten (again), could someone review please?

fetch-event.html LGTM++

[falken, 2014-06-05 05:09:21]

The CQ bit was checked by falken@chromium.org

[commit-bot: I haz the power, 2014-06-05 05:09:35]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/falken@chromium.org/304233017/190001

[commit-bot: I haz the power, 2014-06-05 06:17:46]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  blink_presubmit on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/blink_presubmit/builds/6990)

[commit-bot: I haz the power, 2014-06-05 06:20:09]

Change committed as 175538