1222105 - Get dom/security/test/csp/test_report.html and test_blocked_uri_in_reports.html working with e10s

Status: RESOLVED FIXED

(Core :: DOM: Security, defect)

Comment 1

[Andrew McCreight [:mccr8]]

test_blocked_uri_in_reports.html is basically the same thing, so I can fix it at the same time.

Comment 2

[Andrew McCreight [:mccr8]]

Attachment: Make test_report.html and test_blocked_uri_in_reports.html work with e10s.

This moves the code that analyzes the netwerking stuff into the parent process. It then sends the string down to the child for analysis, or an error if there was one. It looks like that part of the code is identical between these two tests so it has the nice benefit of reducing the over all size of the tests.

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=7d7f5729a60b

Comment 3

[Christoph Kerschbaumer [:ckerschb]]

Comment on attachment 8683988 [details] [diff] [review]
Make test_report.html and test_blocked_uri_in_reports.html work with e10s.

Review of attachment 8683988 [details] [diff] [review]:
-----------------------------------------------------------------

Nicely done, that means that all CSP tests are enabled for e10s now - which is great - thanks Andrew.

::: dom/security/test/csp/test_blocked_uri_in_reports.html
@@ +51,5 @@
> +      // blocked-uri should only be the asciiHost instead of:
> +      // http://test1.example.com/tests/dom/security/test/csp/file_path_matching.js
> +      is(cspReport["blocked-uri"], "http://test1.example.com", "Incorrect blocked-uri");
> +    } catch (e) {
> +      ok(false, "Could not query report (exception: " + e + ")");

same nit applies as for test_report.html

::: dom/security/test/csp/test_report.html
@@ +78,5 @@
> +    try {
> +      // test for the proper values in the report object
> +      window.checkResults(reportObj);
> +    } catch (e) {
> +      ok(false, "Could not query report (exception: " + e + ")");

Super nit: I usually prefer early returns. Would you mind fixing that up so whenever we encounter an error or end up in a catch block we log the error and return early?

Comment 4

[Andrew McCreight [:mccr8]]

The drawback of the early return is that some code will have to be added before every return:
  script.removeMessageListener('opening-request-completed', ml);
  SimpleTest.finish();
I guess the removeMessageListener can go at the top of the message listener, but the SimpleTest.finish() still has to go before the returns. Is that okay?

Comment 5

[Christoph Kerschbaumer [:ckerschb]]

(In reply to Andrew McCreight [:mccr8] from comment #4)
> The drawback of the early return is that some code will have to be added
> before every return:
>   script.removeMessageListener('opening-request-completed', ml);
>   SimpleTest.finish();
> I guess the removeMessageListener can go at the top of the message listener,
> but the SimpleTest.finish() still has to go before the returns. Is that okay?

Ah, I haven't realized that that code is still before the closing curly brace. In that case I would say, just keep what you have. It was only a nitpicky thing anyway.

Comment 6

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ff2125aab1c2

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/ff2125aab1c2