We should write a browser test that registers a new about page. Then we load that about page and make sure that our assertion keeps working correctly. We can annotate that test with 'expected assertions' 1 or something to make sure it fires in such a case.

I started writing a test but ran into some problems with BrowserTestUtils.registerAboutPage. See also: https://bugzilla.mozilla.org/show_bug.cgi?id=1459544#c13

Hey Smaug, we would like to make sure that our assertion keeps firing correctly in case an about: page ships without a CSP. In the end it seems that a mochitest is the best solution to achieve that. Please note that the assertion only runs in debug builds anyway, so I guess adding Preferences::GetBool() should be fine. Thanks!

Ah, this is not working properly: https://treeherder.mozilla.org/#/jobs?repo=try&revision=e51016ed7cc7cf7f983acfd4f689be5059fdab99&selectedJob=199869214 but then how do we annotate a test where we expect an assertion?

Doesn't expectAssertions help with NS_ASSERTIONs? NS_ASSERTIONs are more like warnings.

Since this is debug-only code, would you be willing to accept that patch where we branch on the pref and either do NS_ASSERT for the test VS MOZ_ASSERT in the regular case?

Comment on attachment 9009928 [details] [diff] [review] bug_1490977_assert_about_no_csp_assert.patch >+ if (Preferences::GetBool("csp.overrule_content_privileged_about_uris_without_csp_whitelist")) { >+ NS_ASSERTION(parsedPolicyStr.Find("default-src") >= 0, >+ "about: page must contain a CSP including default-src"); odd indentation But, another option, perhaps a bit nicer, would be to dispatch some event for testing case when the pref is set. That would mean the method should take nsIDocument* as param and then dispatch nsContentUtils::DispatchEvent(aDocument, aDocument, NS_LITERAL_STRING("testing-only-about-csp-check"), ...); However, I can live with this too

Pushed by mozilla@christophkerschbaumer.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/b4ac15e18538 Assert content privileged about page has CSP. r=smaug

Backout by toros@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/d48fc7e23aa7 Backed out changeset b4ac15e18538 for assertion failures at build/build/src/dom/base/nsDocument.cpp on a CLOSED TREE

Backed out changeset b4ac15e18538 for assertion failures at build/build/src/dom/base/nsDocument.cpp Backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/d48fc7e23aa71ef831048823cc435db20264dc45 Push with the failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=b4ac15e185381a8e410b9c9515d15a26a4719520 Failure log file: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=b4ac15e185381a8e410b9c9515d15a26a4719520&selectedJob=199953563

(In reply to Tiberius Oros[:tiberius_oros] from comment #9) > Push with the failures: > https://treeherder.mozilla.org/#/jobs?repo=mozilla- > inbound&revision=b4ac15e185381a8e410b9c9515d15a26a4719520 Ah, once the whitelist is initialized it doesn't get overriden. In that case we are stuck with a whitelist of "". We have to explicitly clear that cache before resettting the old whitelist.

Works correctly locally, but let's make sure TRY is happy as well: https://treeherder.mozilla.org/#/jobs?repo=try&revision=49bfc0fdba4cc6b491a84de5930de176eff6db35

Pushed by mozilla@christophkerschbaumer.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/8da145a5caaf Assert content privileged about page has CSP. r=smaug