Upstream Service Worker window test to WPT

Upstream Service Worker window test to WPT

The Chromium infrastructure runs tests through the `localhost` address,
allowing tests there to include insecure iframe resources without
triggering "mixed content" errors. The WPT infrastructure executes tests
via an aliased host name, precluding this approach.

Re-factor the test to create insecure browsing contexts as new windows
since windows are not subject to the same constraints. Update the
inter-context messaging protocol to account for this change.

Update the `claim-worker.js` "resource" script to extend worker
lifecycle until the asynchronous operation is complete.

Because the modification from using iframes to using windows is
non-trivial and may expose distinct behaviors, the WPT-compatible
version is not a suitable replacement for the original Chromium version.
Persist the Chromium version of the test, adding a comment to document
the distinction.

BUG=688116
R=mek@chromium.org

[mike3, 2017-05-09 16:20:07]

Hi Mek,

This test is a little overloaded, and my inclination is to split it into
two sub-tests. Given the current capabilities of `testharness.js`, though,
I think that would end up adding more noise than it is worth.

Here's the diff as generated with `-M10`:
https://gist.github.com/jugglinmike/ca56024218bc00a994f886dd966c9fd5

[Marijn Kruisselbrink, 2017-05-16 18:31:25]

https://codereview.chromium.org/2869093002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/resources/insecure-parent.html
(right):

https://codereview.chromium.org/2869093002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/resources/insecure-parent.html:8:
iframe.src = get_host_info().HTTP_ORIGIN +
Is this the right origin? If I understand things correctly for the test to test
what it tries to test this needs to be the same origin as what the main test
script runs at, which I don't believe is HTTP_ORIGIN?

[falken, 2017-05-17 05:47:56]

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

[falken, 2017-05-17 05:47:57]

Overall this looks good with mek's comment addressed. Because this is testing a
security thing, I'd like to verify that the test in facts fails if the security
mechanism is removed.

I'm a bit ambivalent about changing from an iframe to a window to fit the WPT
infrastructure... having three nested browsing contexts: https -> http -> https
seems like a more vigorous test than http -> https. It might be worth keeping
the Chromium around as a stricter test.  Also, I don't think Chrome's secure
context checker traverses window.opener, so iframes is in the only way to set up
such a chain.

https://codereview.chromium.org/2869093002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/resources/insecure-parent.html
(right):

https://codereview.chromium.org/2869093002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/resources/insecure-parent.html:8:
iframe.src = get_host_info().HTTP_ORIGIN +
On 2017/05/16 18:31:25, Marijn Kruisselbrink wrote:
> Is this the right origin? If I understand things correctly for the test to
test
> what it tries to test this needs to be the same origin as what the main test
> script runs at, which I don't believe is HTTP_ORIGIN?

Good catch, I think you're right.

When I disable the parent frame security check in Blink (in
ServiceWorkerNetworkProvider, set all instances of is_parent_frame_secure to
true), this test still passes, while the original Chromium test fails.

[mike3, 2017-05-18 15:48:00]

> It might be worth keeping the Chromium around as a stricter test.

Sure thing. I've re-named the WPT version to
"insecure-parent-window.https.html" to help differentiate it. The Chromium
version now has the "chromium." file name prefix along with a comment
explaining our reasoning for keeping it around.

> Because this is testing a security thing, I'd like to verify that the test in
> facts fails if the security mechanism is removed.

It sounds as though it's significantly faster for you to re-build the
project--that takes hours on my system :/ Would you mind verifying this on my
behalf?

https://codereview.chromium.org/2869093002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/resources/insecure-parent.html
(right):

https://codereview.chromium.org/2869093002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/resources/insecure-parent.html:8:
iframe.src = get_host_info().HTTP_ORIGIN +
On 2017/05/16 18:31:25, Marijn Kruisselbrink wrote:
> Is this the right origin? If I understand things correctly for the test to
test
> what it tries to test this needs to be the same origin as what the main test
> script runs at, which I don't believe is HTTP_ORIGIN?

Acknowledged.

[mike3, 2017-05-18 15:50:52]

Description was changed from

==========
Upstream Service Worker window test to WPT

The Chromium infrastructure runs tests through the `localhost` address,
allowing tests there to include insecure iframe resources without
triggering "mixed content" errors. The WPT infrastructure executes tests
via an aliased host name, precluding this approach.

Re-factor the test to create insecure browsing contexts as new windows
since windows are not subject to the same constraints. Update the
inter-context messaging protocol to account for this change.

Update the `claim-worker.js` "resource" script to extend worker
lifecycle until the asynchronous operation is complete. Remove
Chromium's copy of that script as there are no further tests that
reference it.

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

to

==========
Upstream Service Worker window test to WPT

The Chromium infrastructure runs tests through the `localhost` address,
allowing tests there to include insecure iframe resources without
triggering "mixed content" errors. The WPT infrastructure executes tests
via an aliased host name, precluding this approach.

Re-factor the test to create insecure browsing contexts as new windows
since windows are not subject to the same constraints. Update the
inter-context messaging protocol to account for this change.

Update the `claim-worker.js` "resource" script to extend worker
lifecycle until the asynchronous operation is complete.

Because the modification from using iframes to using windows is
non-trivial and may expose distinct behaviors, the WPT-compatible
version is not a suitable replacement for the original Chromium version.
Persist the Chromium version of the test, adding a comment to document
the distinction.

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

[falken, 2017-05-19 05:41:47]

Thanks for the revisions and preserving the chromium version. I verified that
Chromium fails the Chromium version of the test when the security mechanism is
removed.

But the WPT behavior seems weird and I dug into it for some time but couldn't
totally figure it out. I'd expect when the security mechanism is removed, then
Chrome would load iframes with the controller and fail the asserts. Instead I
get a test timeout.

It looks like the post-registration iframe fail to load and we get stuck waiting
for the message from it. I think one thing to fix in the test is that the window
should wait for the iframe to load instead of posting 'loaded' in its own
onload.

With that change, the window gets stuck waiting for the iframe to load.

When trying to debug locally by running wptserve locally and navigating to
http://127.0.0.1:8001 (and changing /etc/hosts as suggested in
https://github.com/w3c/web-platform-tests), which I'm not sure totally
replicates the testing script, I ran into two issues:
- The in-scope iframe on an https:// fails to load due to an SSL error. I could
escape this by --ignore-certificate-errors.
- Once loaded, the test passed. But this is because the "in-scope" iframe was in
a different origin (https://web-platform.test:8001).

Could we add asserts to the test that the in-scope iframe is the same-origin as
the test page? That would guard against the issue mek@ discovered above. I think
the in-scope iframe could just post its location along with the controller
attribute.

Unfortunately, I'm still not totally sure why the test is timing out without the
security mechanism... would need time to investigate more.

[mike3, 2017-05-19 16:47:09]

On 2017/05/19 05:41:47, falken wrote:
> Thanks for the revisions and preserving the chromium version. I verified that
> Chromium fails the Chromium version of the test when the security mechanism is
> removed.
> 
> But the WPT behavior seems weird and I dug into it for some time but couldn't
> totally figure it out. I'd expect when the security mechanism is removed, then
> Chrome would load iframes with the controller and fail the asserts. Instead I
> get a test timeout.
> 
> It looks like the post-registration iframe fail to load and we get stuck
waiting
> for the message from it. I think one thing to fix in the test is that the
window
> should wait for the iframe to load instead of posting 'loaded' in its own
> onload.
> 
> With that change, the window gets stuck waiting for the iframe to load.
> 
> When trying to debug locally by running wptserve locally and navigating to
> http://127.0.0.1:8001 (and changing /etc/hosts as suggested in
> https://github.com/w3c/web-platform-tests), which I'm not sure totally
> replicates the testing script, I ran into two issues:
> - The in-scope iframe on an https:// fails to load due to an SSL error. I
could
> escape this by --ignore-certificate-errors.
> - Once loaded, the test passed. But this is because the "in-scope" iframe was
in
> a different origin (https://web-platform.test:8001).
> 
> Could we add asserts to the test that the in-scope iframe is the same-origin
as
> the test page? That would guard against the issue mek@ discovered above. I
think
> the in-scope iframe could just post its location along with the controller
> attribute.
> 
> Unfortunately, I'm still not totally sure why the test is timing out without
the
> security mechanism... would need time to investigate more.

This reminds me of an issue I stumbled on in Firefox recently:

https://github.com/w3c/web-platform-tests/pull/5738

As reported there, Chromium did not exhibit any timeout, but the underlying
behavior may be related. Just like in that case, we're seeing timeouts when
waiting for Promises to resolve following document removal.

I'd rather not make changes to a patch that I'm not able to validate locally,
since I don't trust myself not to introduce bugs that way. I'm not certain
about whether you are interested in continuing the investigation yourself. I'd
be happy to re-build and pick up where you left off, but since that's a bit of
a time investment for me, I'd like to explicitly verify: is that what you had
in mind?

[falken, 2017-05-22 04:42:30]

On 2017/05/19 16:47:09, mike3 wrote:
> On 2017/05/19 05:41:47, falken wrote:
> > Thanks for the revisions and preserving the chromium version. I verified
that
> > Chromium fails the Chromium version of the test when the security mechanism
is
> > removed.
> > 
> > But the WPT behavior seems weird and I dug into it for some time but
couldn't
> > totally figure it out. I'd expect when the security mechanism is removed,
then
> > Chrome would load iframes with the controller and fail the asserts. Instead
I
> > get a test timeout.
> > 
> > It looks like the post-registration iframe fail to load and we get stuck
> waiting
> > for the message from it. I think one thing to fix in the test is that the
> window
> > should wait for the iframe to load instead of posting 'loaded' in its own
> > onload.
> > 
> > With that change, the window gets stuck waiting for the iframe to load.
> > 
> > When trying to debug locally by running wptserve locally and navigating to
> > http://127.0.0.1:8001 (and changing /etc/hosts as suggested in
> > https://github.com/w3c/web-platform-tests), which I'm not sure totally
> > replicates the testing script, I ran into two issues:
> > - The in-scope iframe on an https:// fails to load due to an SSL error. I
> could
> > escape this by --ignore-certificate-errors.
> > - Once loaded, the test passed. But this is because the "in-scope" iframe
was
> in
> > a different origin (https://web-platform.test:8001).
> > 
> > Could we add asserts to the test that the in-scope iframe is the same-origin
> as
> > the test page? That would guard against the issue mek@ discovered above. I
> think
> > the in-scope iframe could just post its location along with the controller
> > attribute.
> > 
> > Unfortunately, I'm still not totally sure why the test is timing out without
> the
> > security mechanism... would need time to investigate more.
> 
> This reminds me of an issue I stumbled on in Firefox recently:
> 
> https://github.com/w3c/web-platform-tests/pull/5738
> 
> As reported there, Chromium did not exhibit any timeout, but the underlying
> behavior may be related. Just like in that case, we're seeing timeouts when
> waiting for Promises to resolve following document removal.
> 
> I'd rather not make changes to a patch that I'm not able to validate locally,
> since I don't trust myself not to introduce bugs that way. I'm not certain
> about whether you are interested in continuing the investigation yourself. I'd
> be happy to re-build and pick up where you left off, but since that's a bit of
> a time investment for me, I'd like to explicitly verify: is that what you had
> in mind?

Sorry, I meant I'd be willing to investigate, but I not able to do so
immediately. Could we put this patch on hold for the time being?

[mike3, 2017-05-22 14:48:26]

On 2017/05/22 04:42:30, falken wrote:
> On 2017/05/19 16:47:09, mike3 wrote:
> > On 2017/05/19 05:41:47, falken wrote:
> > > Thanks for the revisions and preserving the chromium version. I verified
> that
> > > Chromium fails the Chromium version of the test when the security
mechanism
> is
> > > removed.
> > > 
> > > But the WPT behavior seems weird and I dug into it for some time but
> couldn't
> > > totally figure it out. I'd expect when the security mechanism is removed,
> then
> > > Chrome would load iframes with the controller and fail the asserts.
Instead
> I
> > > get a test timeout.
> > > 
> > > It looks like the post-registration iframe fail to load and we get stuck
> > waiting
> > > for the message from it. I think one thing to fix in the test is that the
> > window
> > > should wait for the iframe to load instead of posting 'loaded' in its own
> > > onload.
> > > 
> > > With that change, the window gets stuck waiting for the iframe to load.
> > > 
> > > When trying to debug locally by running wptserve locally and navigating to
> > > http://127.0.0.1:8001 (and changing /etc/hosts as suggested in
> > > https://github.com/w3c/web-platform-tests), which I'm not sure totally
> > > replicates the testing script, I ran into two issues:
> > > - The in-scope iframe on an https:// fails to load due to an SSL error. I
> > could
> > > escape this by --ignore-certificate-errors.
> > > - Once loaded, the test passed. But this is because the "in-scope" iframe
> was
> > in
> > > a different origin (https://web-platform.test:8001).
> > > 
> > > Could we add asserts to the test that the in-scope iframe is the
same-origin
> > as
> > > the test page? That would guard against the issue mek@ discovered above. I
> > think
> > > the in-scope iframe could just post its location along with the controller
> > > attribute.
> > > 
> > > Unfortunately, I'm still not totally sure why the test is timing out
without
> > the
> > > security mechanism... would need time to investigate more.
> > 
> > This reminds me of an issue I stumbled on in Firefox recently:
> > 
> > https://github.com/w3c/web-platform-tests/pull/5738
> > 
> > As reported there, Chromium did not exhibit any timeout, but the underlying
> > behavior may be related. Just like in that case, we're seeing timeouts when
> > waiting for Promises to resolve following document removal.
> > 
> > I'd rather not make changes to a patch that I'm not able to validate
locally,
> > since I don't trust myself not to introduce bugs that way. I'm not certain
> > about whether you are interested in continuing the investigation yourself.
I'd
> > be happy to re-build and pick up where you left off, but since that's a bit
of
> > a time investment for me, I'd like to explicitly verify: is that what you
had
> > in mind?
> 
> Sorry, I meant I'd be willing to investigate, but I not able to do so
> immediately. Could we put this patch on hold for the time being?

Certainly!