Bug 2021482 Comment 20 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

bug 543435 landed Nov 24 and on Nov 25, bug 2001842 adjusted the test expectations for `/content-security-policy/inheritance/frame-src-javascript-url.html` from timeout to fail. The attached patch caused it to timeout again, I suspect bug 543435 accidentally improved / changed our behavior here. I'm confused why that test didn't show up in my numerous try runs, but a change here is plausible.

This test operates on a transient `about:blank` (from `CreateAboutBlankDocumentViewer`). So before bug 543435, `SetRequestContextWithDocument` didn't happen yet, `nsCSPContext::mInnerWindowID` is the one of the parent. So the test timed out as the expected CSP violation is raised on the parent window. Bug 543435 changed this, the CSP violation is raised correctly in the iframe, but somehow still with wrong `blockedURI`.

To avoid timeouts on CI, we can just fail immediately if we get the violation on the wrong window. The attached patch is a regression nevertheless, but maybe it's better than blocking requests incorrectly?
bug 543435 landed Nov 24 and on Nov 25, bug 2001842 adjusted the test expectations for `/content-security-policy/inheritance/frame-src-javascript-url.html` from timeout to fail. The attached patch caused it to timeout again, I suspect bug 543435 accidentally improved / changed our behavior here. I'm confused why that test didn't show up in my numerous try runs, but a change here is plausible.

This test operates on a transient `about:blank` (from `CreateAboutBlankDocumentViewer`). So before bug 543435, `SetRequestContextWithDocument` didn't happen yet, `nsCSPContext::mInnerWindowID` is the one of the parent. So the test timed out as the expected CSP violation is raised on the parent window. Bug 543435 changed this, the CSP violation is raised correctly in the iframe, but somehow still with wrong `blockedURI`. The attached patch keeps calling `SetRequestContextWithDocument` for the transient `about:blank`, but at a later time so we end up with the CSP violation on the parent window again. I'm not quite sure yet why the call is too late. We could consider setting only `mSelfURI` later.

To avoid timeouts on CI, we can just fail immediately if we get the violation on the wrong window. The attached patch is a regression nevertheless, but maybe it's better than blocking requests incorrectly?
bug 543435 landed Nov 24 and on Nov 25, bug 2001842 adjusted the test expectations for `/content-security-policy/inheritance/frame-src-javascript-url.html` from timeout to fail. The attached patch caused it to timeout again, I suspect bug 543435 accidentally improved / changed our behavior here. I'm confused why that test didn't show up in my numerous try runs, but a change here is plausible.

This test operates on a transient `about:blank` (from `CreateAboutBlankDocumentViewer`). So before bug 543435, `SetRequestContextWithDocument` didn't happen yet, `nsCSPContext::mInnerWindowID` is the one of the parent. So the test timed out as the expected CSP violation is raised on the parent window. Bug 543435 changed this, the CSP violation is raised correctly in the iframe, but somehow still with wrong `blockedURI`. The attached patch keeps calling `SetRequestContextWithDocument` for the transient `about:blank`, but at a later time so we end up with the CSP violation on the parent window again. I'm not quite sure yet why the call is too late, but the CSP is checked in the parent process. So likely, when we update `mInnerWindowID`, the wrong CSP was already sent to the parent. We could consider setting only `mSelfURI` later.

To avoid timeouts on CI, we can just fail immediately if we get the violation on the wrong window. The attached patch is a regression nevertheless, but maybe it's better than blocking requests incorrectly?

Back to Bug 2021482 Comment 20