1475849 - Refactor worker tests within test_CSP.html

Status: RESOLVED FIXED

(Core :: DOM: Security, enhancement, P2)

Description

[Christoph Kerschbaumer [:ckerschb]]

The test test_CSP.html seems overly complicated, especially the worker tests within that test should be refactored and put into a separate test. We should create a new test for all of [1] and add nice documentation around how each test should behalf, especially the sub worker inheritance tests are hard to follow.

[1] https://dxr.mozilla.org/mozilla-central/source/dom/security/test/csp/test_CSP.html#32-55

Comment 1

[Thomas Nguyen (:tnguyen)]

Attachment: 0001-Bug-1475849-Refactor-worker-tests-within-test_CSP.ht.patch

Comment 2

[Thomas Nguyen (:tnguyen)]

Interestingly, if I put the meta CSP, the <script> that triggers new worker, the worker can not get the CSP from meta tag (I have no idea why). Then I have to put the test into an iframe then add Content-Security-Policy header in ^headers^ file.
Hi Christoph, could you please run the tests on the bug 965637 you are trying to fix and tell me your result?
It passed all the tests on my local machine.

Comment 3

[Thomas Nguyen (:tnguyen)]

Attachment: Bug 1475849 - Refactor worker tests within test_CSP.html

MozReview-Commit-ID: 8ACGbm2htCF

Comment 4

[Christoph Kerschbaumer [:ckerschb]]

(In reply to Thomas Nguyen[:tnguyen] ni? plz from comment #2)
> Interestingly, if I put the meta CSP, the <script> that triggers new worker,
> the worker can not get the CSP from meta tag (I have no idea why). Then I
> have to put the test into an iframe then add Content-Security-Policy header
> in ^headers^ file.

Let's investigate that as a follow up within Bug 1477178 - marking dependency for that. I guess with my changes from Bug 965637 it should also work correctly for meta CSP.

> Hi Christoph, could you please run the tests on the bug 965637 you are
> trying to fix and tell me your result?
> It passed all the tests on my local machine.

Tests work with my changes from Bug 965637 applied. From a first glance, tests look good, but I need to take a closer look when reviewing.

Pushing the latest changeset to TRY for Thomas:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=de284f702c557b8e2634f65b12701718dabdf4e1

Comment 5

[Phabricator Automation]

Comment on attachment 8993579 [details]
Bug 1475849 - Refactor worker tests within test_CSP.html

Christoph Kerschbaumer [:ckerschb] has approved the revision.

https://phabricator.services.mozilla.com/D2257

Comment 6

[Christoph Kerschbaumer [:ckerschb]]

Summing it up for the record what happened within this bug:
* We moved all the worker tests from test_CSP.html into test_csp_worker_inheritance.csp
* Thomas extended the test coverage for all kinds of sub-worker inheritance, where the worker is either delivered through with it's own CSP or the worker is supposed to inherit it's csp from the parent (e.g. in case of blob).
* Thomas added tests for top-level workers, sub-workers and sub-sub-workers
* Overall we extended the test coverage and simplified the tests adding more comments of expected behavior!

Thanks Thomas for doing that!

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f08dcbe14baf
Refactor worker tests within test_CSP.html r=ckerschb

Comment 8

[Cristina Coroiu [:ccoroiu]]

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