Add LayoutTests for the behavior of navigation redirect with Service Worker.

Add LayoutTests for the behavior of navigation redirect with Service Worker.

This CL depends on https://codereview.chromium.org/1271733002/ (chromium) and https://codereview.chromium.org/1280733002/ (blink).

BUG=518713, 510650
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=200881

[horo, 2015-08-13 11:49:26]

Patchset #1 (id:1) has been deleted

[horo, 2015-08-13 11:54:49]

Patchset #1 (id:20001) has been deleted

[horo, 2015-08-14 03:08:03]

Patchset #2 (id:60001) has been deleted

[horo, 2015-08-14 03:08:48]

Patchset #2 (id:70033) has been deleted

[horo, 2015-08-14 05:10:50]

Patchset #1 (id:40001) has been deleted

[horo, 2015-08-14 05:13:12]

horo@chromium.org changed reviewers:
+ falken@chromium.org

[horo, 2015-08-14 05:13:13]

falken@
Could you please review this?

[falken, 2015-08-17 06:18:29]

lgtm

Ah you beat me to the navigation tests, sorry I didn't get them done in time.

https://codereview.chromium.org/1286153003/diff/90001/LayoutTests/http/tests/...
File LayoutTests/http/tests/serviceworker/navigation-redirect.html (right):

https://codereview.chromium.org/1286153003/diff/90001/LayoutTests/http/tests/...
LayoutTests/http/tests/serviceworker/navigation-redirect.html:69: OTHER_BASE_URL
+'resources/navigation-redirect-scope1.php?';
nit: space after +

https://codereview.chromium.org/1286153003/diff/90001/LayoutTests/http/tests/...
LayoutTests/http/tests/serviceworker/navigation-redirect.html:337: // SW once
stores the opaque redirect response and returns.
"once" was a bit confusing. Maybe just "SW responds with an opaque redirect
response from the Cache API." ?

https://codereview.chromium.org/1286153003/diff/90001/LayoutTests/http/tests/...
File
LayoutTests/http/tests/serviceworker/resources/navigation-redirect-other-origin.html
(right):

https://codereview.chromium.org/1286153003/diff/90001/LayoutTests/http/tests/...
LayoutTests/http/tests/serviceworker/resources/navigation-redirect-other-origin.html:26:
});
+2 indent

https://codereview.chromium.org/1286153003/diff/90001/LayoutTests/http/tests/...
LayoutTests/http/tests/serviceworker/resources/navigation-redirect-other-origin.html:27:
});
can this just be return wait_for_state(worker, 'activated')?

https://codereview.chromium.org/1286153003/diff/90001/LayoutTests/http/tests/...
LayoutTests/http/tests/serviceworker/resources/navigation-redirect-other-origin.html:48:
console.error('invalid origin: ' << e.origin);
<< should be +

https://codereview.chromium.org/1286153003/diff/90001/LayoutTests/http/tests/...
LayoutTests/http/tests/serviceworker/resources/navigation-redirect-other-origin.html:62:
})
missing ;

https://codereview.chromium.org/1286153003/diff/90001/LayoutTests/http/tests/...
File
LayoutTests/http/tests/serviceworker/resources/navigation-redirect-scope1.php
(right):

https://codereview.chromium.org/1286153003/diff/90001/LayoutTests/http/tests/...
LayoutTests/http/tests/serviceworker/resources/navigation-redirect-scope1.php:3:
header ("HTTP/1.1 302");
nit: space before (

https://codereview.chromium.org/1286153003/diff/90001/LayoutTests/http/tests/...
LayoutTests/http/tests/serviceworker/resources/navigation-redirect-scope1.php:16:
'http://127.0.0.1:8000');
will this work in wptserve?

[horo, 2015-08-19 01:21:05]

Patchset #2 (id:110001) has been deleted

[horo, 2015-08-19 08:33:01]

https://codereview.chromium.org/1286153003/diff/90001/LayoutTests/http/tests/...
File LayoutTests/http/tests/serviceworker/navigation-redirect.html (right):

https://codereview.chromium.org/1286153003/diff/90001/LayoutTests/http/tests/...
LayoutTests/http/tests/serviceworker/navigation-redirect.html:69: OTHER_BASE_URL
+'resources/navigation-redirect-scope1.php?';
On 2015/08/17 06:18:28, falken wrote:
> nit: space after +

Done.

https://codereview.chromium.org/1286153003/diff/90001/LayoutTests/http/tests/...
LayoutTests/http/tests/serviceworker/navigation-redirect.html:337: // SW once
stores the opaque redirect response and returns.
On 2015/08/17 06:18:28, falken wrote:
> "once" was a bit confusing. Maybe just "SW responds with an opaque redirect
> response from the Cache API." ?

Done.

https://codereview.chromium.org/1286153003/diff/90001/LayoutTests/http/tests/...
File
LayoutTests/http/tests/serviceworker/resources/navigation-redirect-other-origin.html
(right):

https://codereview.chromium.org/1286153003/diff/90001/LayoutTests/http/tests/...
LayoutTests/http/tests/serviceworker/resources/navigation-redirect-other-origin.html:26:
});
On 2015/08/17 06:18:29, falken wrote:
> +2 indent

Done.

https://codereview.chromium.org/1286153003/diff/90001/LayoutTests/http/tests/...
LayoutTests/http/tests/serviceworker/resources/navigation-redirect-other-origin.html:27:
});
On 2015/08/17 06:18:28, falken wrote:
> can this just be return wait_for_state(worker, 'activated')?

I think we can use wait_for_state() only when we have a Test object.
This file is used in a iframe.
So we can't use wait_for_state().

https://codereview.chromium.org/1286153003/diff/90001/LayoutTests/http/tests/...
LayoutTests/http/tests/serviceworker/resources/navigation-redirect-other-origin.html:48:
console.error('invalid origin: ' << e.origin);
On 2015/08/17 06:18:28, falken wrote:
> << should be +

Done.

https://codereview.chromium.org/1286153003/diff/90001/LayoutTests/http/tests/...
LayoutTests/http/tests/serviceworker/resources/navigation-redirect-other-origin.html:62:
})
On 2015/08/17 06:18:29, falken wrote:
> missing ;

Done.

https://codereview.chromium.org/1286153003/diff/90001/LayoutTests/http/tests/...
File
LayoutTests/http/tests/serviceworker/resources/navigation-redirect-scope1.php
(right):

https://codereview.chromium.org/1286153003/diff/90001/LayoutTests/http/tests/...
LayoutTests/http/tests/serviceworker/resources/navigation-redirect-scope1.php:3:
header ("HTTP/1.1 302");
On 2015/08/17 06:18:29, falken wrote:
> nit: space before (

Done.

https://codereview.chromium.org/1286153003/diff/90001/LayoutTests/http/tests/...
LayoutTests/http/tests/serviceworker/resources/navigation-redirect-scope1.php:16:
'http://127.0.0.1:8000');
On 2015/08/17 06:18:29, falken wrote:
> will this work in wptserve?

No.
We will have to rewrite this PHP file to Python for wptserve.

[bkelly, 2015-08-19 14:21:06]

https://codereview.chromium.org/1286153003/diff/130001/LayoutTests/http/tests...
File LayoutTests/http/tests/serviceworker/navigation-redirect.html (right):

https://codereview.chromium.org/1286153003/diff/130001/LayoutTests/http/tests...
LayoutTests/http/tests/serviceworker/navigation-redirect.html:149: return
test_redirect(
If we intend to uplift these as WPT tests, it would be really helpful if each
case in the test was in a separate async_test().  This lets individuals test
cases be marked as "expected fail".  This current style of chaining them all
together in a single async_test() makes the entire test all-or-nothing in terms
of passing.

[horo, 2015-08-20 06:35:51]

https://codereview.chromium.org/1286153003/diff/130001/LayoutTests/http/tests...
File LayoutTests/http/tests/serviceworker/navigation-redirect.html (right):

https://codereview.chromium.org/1286153003/diff/130001/LayoutTests/http/tests...
LayoutTests/http/tests/serviceworker/navigation-redirect.html:149: return
test_redirect(
On 2015/08/19 14:21:06, bkelly wrote:
> If we intend to uplift these as WPT tests, it would be really helpful if each
> case in the test was in a separate async_test().  This lets individuals test
> cases be marked as "expected fail".  This current style of chaining them all
> together in a single async_test() makes the entire test all-or-nothing in
terms
> of passing.

Acknowledged.
I will create crbug for this issue after submitting this cl.

[horo, 2015-08-20 06:35:55]

The CQ bit was checked by horo@chromium.org

[horo, 2015-08-20 06:35:55]

The patchset sent to the CQ was uploaded after l-g-t-m from falken@chromium.org
Link to the patchset: https://codereview.chromium.org/1286153003/#ps130001
(title: "incorporated falken's comment")

[commit-bot: I haz the power, 2015-08-20 06:36:12]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1286153003/130001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1286153003/130001

[commit-bot: I haz the power, 2015-08-20 07:19:30]

Committed patchset #2 (id:130001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=200881

[horo, 2015-08-21 05:35:34]

On 2015/08/19 14:21:06, bkelly wrote:
>
https://codereview.chromium.org/1286153003/diff/130001/LayoutTests/http/tests...
> File LayoutTests/http/tests/serviceworker/navigation-redirect.html (right):
> 
>
https://codereview.chromium.org/1286153003/diff/130001/LayoutTests/http/tests...
> LayoutTests/http/tests/serviceworker/navigation-redirect.html:149: return
> test_redirect(
> If we intend to uplift these as WPT tests, it would be really helpful if each
> case in the test was in a separate async_test().  This lets individuals test
> cases be marked as "expected fail".  This current style of chaining them all
> together in a single async_test() makes the entire test all-or-nothing in
terms
> of passing.

Created a cl: https://codereview.chromium.org/1304073002/