Remove hard-coded origins in layout tests.

Remove hard-coded origins in layout tests.

Because the W3C's web-platform-tests are run using a
different origin (http://web-platform.test versus
http://127.0.0.1), we should use the existing host-
detection code in get-host-info.js.

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

[jeremyarcher, 2015-08-26 05:54:56]

jeremyarcher@google.com changed reviewers:
+ nhiroki@chromium.org

[nhiroki, 2015-08-26 09:02:29]

lgtm with nits

Can you explain why we need to remove hard-coded origins on the CL description?

https://codereview.chromium.org/1314123002/diff/1/LayoutTests/http/tests/serv...
File LayoutTests/http/tests/serviceworker/chromium/service-worker-allowed.html
(right):

https://codereview.chromium.org/1314123002/diff/1/LayoutTests/http/tests/serv...
LayoutTests/http/tests/serviceworker/chromium/service-worker-allowed.html:76:
host_info.HTTP_REMOTE_ORIGIN + '/cross-origin/';
nit: 2-spaces indent seems not necessary.

https://codereview.chromium.org/1314123002/diff/1/LayoutTests/http/tests/serv...
File
LayoutTests/http/tests/serviceworker/resources/fetch-request-html-imports-worker.js
(right):

https://codereview.chromium.org/1314123002/diff/1/LayoutTests/http/tests/serv...
LayoutTests/http/tests/serviceworker/resources/fetch-request-html-imports-worker.js:18:
'/dummy-dir/same-other.html">'));
indent-nit

https://codereview.chromium.org/1314123002/diff/1/LayoutTests/http/tests/serv...
LayoutTests/http/tests/serviceworker/resources/fetch-request-html-imports-worker.js:26:
'/dummy-dir/other-other.html">'));
indent-nit

https://codereview.chromium.org/1314123002/diff/1/LayoutTests/http/tests/serv...
File LayoutTests/http/tests/serviceworker/resources/interfaces-worker.js
(right):

https://codereview.chromium.org/1314123002/diff/1/LayoutTests/http/tests/serv...
LayoutTests/http/tests/serviceworker/resources/interfaces-worker.js:95: var req
= new Request('https://www.google.com/', {method: 'POST'});
example.com could be better because this will be upstreamed.

https://codereview.chromium.org/1314123002/diff/1/LayoutTests/http/tests/serv...
File
LayoutTests/http/tests/serviceworker/resources/service-worker-csp-worker.php
(right):

https://codereview.chromium.org/1314123002/diff/1/LayoutTests/http/tests/serv...
LayoutTests/http/tests/serviceworker/resources/service-worker-csp-worker.php:29:
'/serviceworker/resources/fetch-access-control.php?ACAOrigin=*',
nit: can you add 4-spaces indent?

(host_info...
   '/serviceworker/...
 {mode: 'cors'})

https://codereview.chromium.org/1314123002/diff/1/LayoutTests/http/tests/serv...
LayoutTests/http/tests/serviceworker/resources/service-worker-csp-worker.php:80:
'/serviceworker/resources/fetch-access-control.php?ACAOrigin=*',
ditto.

https://codereview.chromium.org/1314123002/diff/1/LayoutTests/http/tests/serv...
LayoutTests/http/tests/serviceworker/resources/service-worker-csp-worker.php:131:
'/serviceworker/resources/fetch-access-control.php?ACAOrigin=*',
ditto.

[jeremyarcher, 2015-08-27 07:45:44]

I, er, think this is right- PTAL, I guess, since I'm not sure.

https://codereview.chromium.org/1314123002/diff/1/LayoutTests/http/tests/serv...
File
LayoutTests/http/tests/serviceworker/resources/fetch-request-html-imports-worker.js
(right):

https://codereview.chromium.org/1314123002/diff/1/LayoutTests/http/tests/serv...
LayoutTests/http/tests/serviceworker/resources/fetch-request-html-imports-worker.js:18:
'/dummy-dir/same-other.html">'));
On 2015/08/26 09:02:28, nhiroki wrote:
> indent-nit

Done.

https://codereview.chromium.org/1314123002/diff/1/LayoutTests/http/tests/serv...
LayoutTests/http/tests/serviceworker/resources/fetch-request-html-imports-worker.js:26:
'/dummy-dir/other-other.html">'));
On 2015/08/26 09:02:28, nhiroki wrote:
> indent-nit

Done.

https://codereview.chromium.org/1314123002/diff/1/LayoutTests/http/tests/serv...
File LayoutTests/http/tests/serviceworker/resources/interfaces-worker.js
(right):

https://codereview.chromium.org/1314123002/diff/1/LayoutTests/http/tests/serv...
LayoutTests/http/tests/serviceworker/resources/interfaces-worker.js:95: var req
= new Request('https://www.google.com/', {method: 'POST'});
On 2015/08/26 09:02:28, nhiroki wrote:
> http://example.com could be better because this will be upstreamed.

Done.

https://codereview.chromium.org/1314123002/diff/1/LayoutTests/http/tests/serv...
File
LayoutTests/http/tests/serviceworker/resources/service-worker-csp-worker.php
(right):

https://codereview.chromium.org/1314123002/diff/1/LayoutTests/http/tests/serv...
LayoutTests/http/tests/serviceworker/resources/service-worker-csp-worker.php:29:
'/serviceworker/resources/fetch-access-control.php?ACAOrigin=*',
On 2015/08/26 09:02:29, nhiroki wrote:
> nit: can you add 4-spaces indent?
> 
> (host_info...
>    '/serviceworker/...
>  {mode: 'cors'})

Done.

https://codereview.chromium.org/1314123002/diff/1/LayoutTests/http/tests/serv...
LayoutTests/http/tests/serviceworker/resources/service-worker-csp-worker.php:80:
'/serviceworker/resources/fetch-access-control.php?ACAOrigin=*',
On 2015/08/26 09:02:28, nhiroki wrote:
> ditto.

Done.

https://codereview.chromium.org/1314123002/diff/1/LayoutTests/http/tests/serv...
LayoutTests/http/tests/serviceworker/resources/service-worker-csp-worker.php:131:
'/serviceworker/resources/fetch-access-control.php?ACAOrigin=*',
On 2015/08/26 09:02:28, nhiroki wrote:
> ditto.

Done.

[jeremyarcher, 2015-08-28 06:25:00]

The CQ bit was checked by jeremyarcher@google.com

[jeremyarcher, 2015-08-28 06:25:01]

The patchset sent to the CQ was uploaded after l-g-t-m from nhiroki@chromium.org
Link to the patchset: https://codereview.chromium.org/1314123002/#ps20001
(title: "Fix indentation.")

[commit-bot: I haz the power, 2015-08-28 06:25:03]

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

[commit-bot: I haz the power, 2015-08-28 07:05:27]

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

[commit-bot: I haz the power, 2015-08-28 07:05:28]

Try jobs failed on following builders:
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[jeremyarcher, 2015-08-31 01:35:04]

The CQ bit was checked by jeremyarcher@google.com

[commit-bot: I haz the power, 2015-08-31 01:35:10]

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

[commit-bot: I haz the power, 2015-08-31 02:13:39]

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

[commit-bot: I haz the power, 2015-08-31 02:13:40]

Try jobs failed on following builders:
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[nhiroki, 2015-08-31 03:07:31]

fyi: This CL seems to be breaking fetch tests that depend on
serviceworker/resources/test-helpers.js.

[jeremyarcher, 2015-09-01 01:41:23]

On 2015/08/31 03:07:31, nhiroki wrote:
> fyi: This CL seems to be breaking fetch tests that depend on
> serviceworker/resources/test-helpers.js.

I've updated the fetch tests and now run-webkit-tests succeeds for everything
fetch or serviceworker related.

[nhiroki, 2015-09-01 02:28:43]

On 2015/09/01 01:41:23, jeremyarcher wrote:
> On 2015/08/31 03:07:31, nhiroki wrote:
> > fyi: This CL seems to be breaking fetch tests that depend on
> > serviceworker/resources/test-helpers.js.
> 
> I've updated the fetch tests and now run-webkit-tests succeeds for everything
> fetch or serviceworker related.

I cannot see diffs on Patch Set 3. Maybe you failed to upload?

[jeremyarcher, 2015-09-01 03:10:48]

Patchset #3 (id:40001) has been deleted

[jeremyarcher, 2015-09-01 03:11:49]

Patchset #3 (id:40001) has been deleted

[jeremyarcher, 2015-09-01 03:12:42]

On 2015/09/01 02:28:43, nhiroki wrote:
> On 2015/09/01 01:41:23, jeremyarcher wrote:
> > On 2015/08/31 03:07:31, nhiroki wrote:
> > > fyi: This CL seems to be breaking fetch tests that depend on
> > > serviceworker/resources/test-helpers.js.
> > 
> > I've updated the fetch tests and now run-webkit-tests succeeds for
everything
> > fetch or serviceworker related.
> 
> I cannot see diffs on Patch Set 3. Maybe you failed to upload?

I've reuploaded the commit, and now there are M's present on each file. Sorry
about that! Must've gotten in a weird state from the extra files.

[nhiroki, 2015-09-01 04:05:21]

nhiroki@chromium.org changed reviewers:
+ hiroshige@chromium.org

[nhiroki, 2015-09-01 04:06:43]

nhiroki@chromium.org changed reviewers:
- hiroshige@chromium.org

[nhiroki, 2015-09-01 04:06:44]

lgtm with a nit

https://codereview.chromium.org/1314123002/diff/60001/LayoutTests/http/tests/...
File LayoutTests/http/tests/serviceworker/resources/test-helpers.js (right):

https://codereview.chromium.org/1314123002/diff/60001/LayoutTests/http/tests/...
LayoutTests/http/tests/serviceworker/resources/test-helpers.js:191: function
login(local, remote, test) {
nit: can you move |test| to the first argument to keep consistency with other
functions in this file?

[nhiroki, 2015-09-01 04:07:32]

nhiroki@chromium.org changed reviewers:
+ hiroshige@chromium.org

[nhiroki, 2015-09-01 04:07:33]

+hiroshige@ for http/tests/fetch/*

Please wait for his review.

[hiroshige, 2015-09-01 06:22:41]

> +hiroshige@ for http/tests/fetch/*
Basically Looks good for http/tests/fetch changes, but I think there was a
discussion about the meaning of HTTP_ORIGIN, i.e. what does it mean when the
test runs on HTTPS origin.
From the CL description, the test might run on http://web-platform.test, but
http://web-platform.test is not a secure origin, so serviceworker registration
fails.
If we run the test on https://web-platform.test, then the tests probably fails
because HTTP_ORIGIN seems to be used as 'the origin the test runs'.
Is the issue resolved?

https://codereview.chromium.org/1314123002/diff/60001/LayoutTests/http/tests/...
File
LayoutTests/http/tests/fetch/script-tests/thorough/TEMPLATE-serviceworker-proxied.html
(right):

https://codereview.chromium.org/1314123002/diff/60001/LayoutTests/http/tests/...
LayoutTests/http/tests/fetch/script-tests/thorough/TEMPLATE-serviceworker-proxied.html:6:
<script src = "/resources/get-host-info.js"></script>
Isn't "?pipe=sub" also needed here?

[nhiroki, 2015-09-01 07:41:04]

On 2015/09/01 06:22:41, hiroshige (slow) wrote:
> > +hiroshige@ for http/tests/fetch/*
> Basically Looks good for http/tests/fetch changes, but I think there was a
> discussion about the meaning of HTTP_ORIGIN, i.e. what does it mean when the
> test runs on HTTPS origin.

The issue hasn't been resolved yet. IIUC, this CL just canonicalizes urls used
in layout tests to make it easier to prepare for upstream.

> From the CL description, the test might run on http://web-platform.test, but
> http://web-platform.test is not a secure origin, so serviceworker registration
> fails.
> If we run the test on https://web-platform.test, then the tests probably fails
> because HTTP_ORIGIN seems to be used as 'the origin the test runs'.
> Is the issue resolved?

Maybe...
  (1) adding SECURE_ORIGIN/SECURE_REMOTE_ORIGIN parameters that should be able
to run service workers (eg. "http://localhost", "https://example.com") into
get_host_info(),
  (2) replacing HTTP_ORIGIN etc with SECURE_ORIGIN etc, and
  (2) making the testrunner open foo-test.html on SECURE_ORIGIN
can work?

[hiroshige, 2015-09-01 08:14:05]

On 2015/09/01 07:41:04, nhiroki wrote:
> The issue hasn't been resolved yet. IIUC, this CL just canonicalizes urls used
> in layout tests to make it easier to prepare for upstream.
I see. So we canonicalize URLs in this CL, and then will address the issue of
running non-HTTP/non-localhost origins in the future.
Then I'll l-g-t-m after my comment on Patch Set 3 is addressed.

[jeremyarcher, 2015-09-02 03:22:49]

On 2015/09/01 08:14:05, hiroshige (slow) wrote:
> On 2015/09/01 07:41:04, nhiroki wrote:
> > The issue hasn't been resolved yet. IIUC, this CL just canonicalizes urls
used
> > in layout tests to make it easier to prepare for upstream.
> I see. So we canonicalize URLs in this CL, and then will address the issue of
> running non-HTTP/non-localhost origins in the future.
> Then I'll l-g-t-m after my comment on Patch Set 3 is addressed.

I've restructured login(...) so that the test appears as the first parameter,
and added the ?pipe=sub query string flag to get-host-info.js. If I have spare
brain cycles, I'll take a look at the extra origins, as well -- earlier, I just
hacked get-host-info.js so that host_info["HTTP_ORIGIN"] actually pointed to
the HTTPS URL.

[hiroshige, 2015-09-02 03:28:35]

lgtm. Thanks!

[jeremyarcher, 2015-09-02 03:35:21]

The CQ bit was checked by jeremyarcher@google.com

[jeremyarcher, 2015-09-02 03:35:22]

The patchset sent to the CQ was uploaded after l-g-t-m from nhiroki@chromium.org
Link to the patchset: https://codereview.chromium.org/1314123002/#ps80001
(title: "Add ?pipe=sub and make test first parameter.")

[commit-bot: I haz the power, 2015-09-02 03:35:27]

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

[commit-bot: I haz the power, 2015-09-02 04:18:20]

Committed patchset #4 (id:80001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=201611