Fix http-to-https-redirect-and-register.html for --site-per-process.

Fix http-to-https-redirect-and-register.html for --site-per-process.

The test was assuming that the onload callback would be dispatched
before the postMessage from SW registration, which wasn't happening
when the frame in the second promise_test was an OOPIF.  This CL
changes the onload and postMessage handlers to be independent of
ordering.  See bug for more details.

BUG=622887
Committed: https://crrev.com/033dafcf829151ce612ed36460049e0a282fbf38
Cr-Commit-Position: refs/heads/master@{#401900}

[alexmos, 2016-06-24 01:05:17]

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

[alexmos, 2016-06-24 01:05:18]

falken@: since you wrote the original test, can you please take a look at
whether this fix makes sense?  This gets the test working with
--site-per-process.

[falken, 2016-06-24 01:18:55]

Yes this looks good. Just style nits.

https://codereview.chromium.org/2092953003/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/http/tests/serviceworker/http-to-https-redirect-and-register.html
(right):

https://codereview.chromium.org/2092953003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/http/tests/serviceworker/http-to-https-redirect-and-register.html:18:
});
serviceworker test style uses snake_case and somewhat strange indents:
https://www.chromium.org/blink/serviceworker/testing#TOC-Layout-Tests-Style

Also, you can just omit the braces for arrow operators with single statements
bodies.

So this could become:
var message_promise = new Promise(resolve => {
    window.addEventListener('message', e => resolve(e.data));
  });

Similiar comments for below.

[alexmos, 2016-06-24 01:52:37]

Thanks!

https://codereview.chromium.org/2092953003/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/http/tests/serviceworker/http-to-https-redirect-and-register.html
(right):

https://codereview.chromium.org/2092953003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/http/tests/serviceworker/http-to-https-redirect-and-register.html:18:
});
On 2016/06/24 01:18:54, falken wrote:
> serviceworker test style uses snake_case and somewhat strange indents:
> https://www.chromium.org/blink/serviceworker/testing#TOC-Layout-Tests-Style
> 
> Also, you can just omit the braces for arrow operators with single statements
> bodies.
> 
> So this could become:
> var message_promise = new Promise(resolve => {
>     window.addEventListener('message', e => resolve(e.data));
>   });
> 
> Similiar comments for below.

Done.

[falken, 2016-06-24 01:55:22]

lgtm with nits

https://codereview.chromium.org/2092953003/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/serviceworker/http-to-https-redirect-and-register.html
(right):

https://codereview.chromium.org/2092953003/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/serviceworker/http-to-https-redirect-and-register.html:18:
});
line 17-18 should be indented an additional two spaces. (it is a bit
unconventional, but that's our style).

https://codereview.chromium.org/2092953003/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/serviceworker/http-to-https-redirect-and-register.html:24:
});
lines 22-24 should be indented an additional two spaces.

[alexmos, 2016-06-24 16:33:11]

Thanks, I definitely wasn't familiar with the intricacies of SW coding style.

https://codereview.chromium.org/2092953003/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/serviceworker/http-to-https-redirect-and-register.html
(right):

https://codereview.chromium.org/2092953003/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/serviceworker/http-to-https-redirect-and-register.html:18:
});
On 2016/06/24 01:55:22, falken wrote:
> line 17-18 should be indented an additional two spaces. (it is a bit
> unconventional, but that's our style).

Done.

https://codereview.chromium.org/2092953003/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/serviceworker/http-to-https-redirect-and-register.html:24:
});
On 2016/06/24 01:55:22, falken wrote:
> lines 22-24 should be indented an additional two spaces.

Done.

[alexmos, 2016-06-24 16:33:27]

The CQ bit was checked by alexmos@chromium.org

[alexmos, 2016-06-24 16:33:27]

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/2092953003/#ps40001
(title: "Fix indent")

[commit-bot: I haz the power, 2016-06-24 16:34:13]

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, 2016-06-24 17:48:02]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-06-24 17:51:11]

Description was changed from

==========
Fix http-to-https-redirect-and-register.html for --site-per-process.

The test was assuming that the onload callback would be dispatched
before the postMessage from SW registration, which wasn't happening
when the frame in the second promise_test was an OOPIF.  This CL
changes the onload and postMessage handlers to be independent of
ordering.  See bug for more details.

BUG=622887
==========

to

==========
Fix http-to-https-redirect-and-register.html for --site-per-process.

The test was assuming that the onload callback would be dispatched
before the postMessage from SW registration, which wasn't happening
when the frame in the second promise_test was an OOPIF.  This CL
changes the onload and postMessage handlers to be independent of
ordering.  See bug for more details.

BUG=622887

Committed: https://crrev.com/033dafcf829151ce612ed36460049e0a282fbf38
Cr-Commit-Position: refs/heads/master@{#401900}
==========

[commit-bot: I haz the power, 2016-06-24 17:51:12]

Patchset 3 (id:??) landed as
https://crrev.com/033dafcf829151ce612ed36460049e0a282fbf38
Cr-Commit-Position: refs/heads/master@{#401900}