I noticed while working on csp tests that often nsIContentPolicy::ShouldLoad is called twice for resources: once during preloading and once again during the actual load. Should we cache the results of the first check? The duplicate calls don't always happen (there's probably some race that the actual load occasionally wins), so this results in a non-deterministic number of violation reports going out when CSP policies are violated -- makes it hard to test, and I bet web devs who are getting these reports really don't want duplicates. Here's a sample pair of stacks from two calls to NS_CheckContentPolicy on the same resource: #0 CSPService::ShouldLoad (this=0x46044780, aContentType=4, aContentLocation=0x4d7d3cc0, aRequestOrigin=0x4d7d3600, aRequestContext=0x43db6400, aMimeTypeGuess=..., aExtra=0x0, aDecision=0xbfffe70a) at /home/sstamm/src/build-csp/content/base/src/nsCSPService.cpp:121 #1 0x4096e335 in nsContentPolicy::CheckPolicy (this=0x4604f790, policyMethod=&virtual nsIContentPolicy::ShouldLoad(unsigned int, nsIURI*, nsIURI*, nsISupports*, nsACString_internal const&, nsISupports*, short*), contentType=4, contentLocation=0x4d7d3cc0, requestingLocation=0x4d7d3600, requestingContext=0x43db6400, mimeType=..., extra=0x0, decision=0xbfffe70a) at /home/sstamm/src/build-csp/content/base/src/nsContentPolicy.cpp:157 #2 0x4096d630 in nsContentPolicy::ShouldLoad (this=0x4604f790, contentType=4, contentLocation=0x4d7d3cc0, requestingLocation=0x4d7d3600, requestingContext=0x43db6400, mimeType=..., extra=0x0, decision=0xbfffe70a) at /home/sstamm/src/build-csp/content/base/src/nsContentPolicy.cpp:218 #3 0x405964d0 in NS_CheckContentLoadPolicy (contentType=4, contentLocation=0x4d7d3cc0, originPrincipal=0x48df5dd0, context=0x43db6400, mimeType=..., extra=0x0, decision=0xbfffe70a, policyService=0x4604f790, aSecMan=0x44b97be0) at ../../../dist/include/nsContentPolicyUtils.h:221 #4 0x40800a02 in mozilla::css::Loader::CheckLoadAllowed (this=0x4d79bf80, aSourcePrincipal=0x48df5dd0, aTargetURI=0x4d7d3cc0, aContext=0x43db6400) at /home/sstamm/src/build-csp/layout/style/Loader.cpp:1042 #5 0x40805026 in mozilla::css::Loader::InternalLoadNonDocumentSheet (this=0x4d79bf80, aURL=0x4d7d3cc0, aAllowUnsafeRules=0, aUseSystemPrincipal=0, aOriginPrincipal=0x48df5dd0, aCharset=..., aSheet=0x0, aObserver=0x481c6df0) at /home/sstamm/src/build-csp/layout/style/Loader.cpp:2164 #6 0x40805541 in mozilla::css::Loader::LoadSheet (this=0x4d79bf80, aURL=0x4d7d3cc0, aOriginPrincipal=0x48df5dd0, aCharset=..., aObserver=0x481c6df0) at /home/sstamm/src/build-csp/layout/style/Loader.cpp:2135 #7 0x409d6e57 in nsDocument::PreloadStyle (this=0x43db6400, uri=0x4d7d3cc0, charset=...) at /home/sstamm/src/build-csp/content/base/src/nsDocument.cpp:7778 #8 0x40f195bb in nsHtml5TreeOpExecutor::PreloadStyle (this=0x4d741400, aURL=..., aCharset=...) at /home/sstamm/src/build-csp/parser/html/nsHtml5TreeOpExecutor.cpp:887 #9 0x40f233a6 in nsHtml5SpeculativeLoad::Perform (this=0x4725e768, aExecutor=0x4d741400) at /home/sstamm/src/build-csp/parser/html/nsHtml5SpeculativeLoad.cpp:70 #10 0x40f19a69 in nsHtml5TreeOpExecutor::FlushSpeculativeLoads (this=0x4d741400) at /home/sstamm/src/build-csp/parser/html/nsHtml5TreeOpExecutor.cpp:359 #11 0x40f225cc in nsHtml5LoadFlusher::Run (this=0x4d79a870) at /home/sstamm/src/build-csp/parser/html/nsHtml5StreamParser.cpp:168 #12 0x41b43e18 in nsThread::ProcessNextEvent (this=0x43dc9ac0, mayWait=0, result=0xbfffea20) at /home/sstamm/src/build-csp/xpcom/threads/nsThread.cpp:626 #13 0x41ac8a48 in NS_ProcessNextEvent_P (thread=0x43dc9ac0, mayWait=0) at nsThreadUtils.cpp:250 #14 0x4190ea18 in mozilla::ipc::MessagePump::Run (this=0x43d973d0, aDelegate=0x43d338a0) at /home/sstamm/src/build-csp/ipc/glue/MessagePump.cpp:110 #15 0x41bae7cb in MessageLoop::RunInternal (this=0x43d338a0) at /home/sstamm/src/build-csp/ipc/chromium/src/base/message_loop.cc:219 #16 0x41bae7e3 in MessageLoop::RunHandler (this=0x43d338a0) at /home/sstamm/src/build-csp/ipc/chromium/src/base/message_loop.cc:202 #17 0x41bae847 in MessageLoop::Run (this=0x43d338a0) at /home/sstamm/src/build-csp/ipc/chromium/src/base/message_loop.cc:176 #18 0x41798000 in nsBaseAppShell::Run (this=0x458496a0) at /home/sstamm/src/build-csp/widget/src/xpwidgets/nsBaseAppShell.cpp:181 #19 0x414ba889 in nsAppStartup::Run (this=0x458a5640) at /home/sstamm/src/build-csp/toolkit/components/startup/src/nsAppStartup.cpp:191 #20 0x4038a14e in XRE_main (argc=5, argv=0xbffff1b4, aAppData=0x43d10380) at /home/sstamm/src/build-csp/toolkit/xre/nsAppRunner.cpp:3691 #21 0x08049912 in main (argc=5, argv=0xbffff1b4) at /home/sstamm/src/build-csp/browser/app/nsBrowserApp.cpp:158 #0 CSPService::ShouldLoad (this=0x46044780, aContentType=4, aContentLocation=0x4d7d3e40, aRequestOrigin=0x4d7d3600, aRequestContext=0x4d73c390, aMimeTypeGuess=..., aExtra=0x0, aDecision=0xbfffde8a) at /home/sstamm/src/build-csp/content/base/src/nsCSPService.cpp:121 #1 0x4096e335 in nsContentPolicy::CheckPolicy (this=0x4604f790, policyMethod=&virtual nsIContentPolicy::ShouldLoad(unsigned int, nsIURI*, nsIURI*, nsISupports*, nsACString_internal const&, nsISupports*, short*), contentType=4, contentLocation=0x4d7d3e40, requestingLocation=0x4d7d3600, requestingContext=0x4d73c390, mimeType=..., extra=0x0, decision=0xbfffde8a) at /home/sstamm/src/build-csp/content/base/src/nsContentPolicy.cpp:157 #2 0x4096d630 in nsContentPolicy::ShouldLoad (this=0x4604f790, contentType=4, contentLocation=0x4d7d3e40, requestingLocation=0x4d7d3600, requestingContext=0x4d73c390, mimeType=..., extra=0x0, decision=0xbfffde8a) at /home/sstamm/src/build-csp/content/base/src/nsContentPolicy.cpp:218 #3 0x405964d0 in NS_CheckContentLoadPolicy (contentType=4, contentLocation=0x4d7d3e40, originPrincipal=0x48df5dd0, context=0x4d73c390, mimeType=..., extra=0x0, decision=0xbfffde8a, policyService=0x4604f790, aSecMan=0x44b97be0) at ../../../dist/include/nsContentPolicyUtils.h:221 #4 0x40800a02 in mozilla::css::Loader::CheckLoadAllowed (this=0x4d79bf80, aSourcePrincipal=0x48df5dd0, aTargetURI=0x4d7d3e40, aContext=0x4d73c390) at /home/sstamm/src/build-csp/layout/style/Loader.cpp:1042 #5 0x40805a0a in mozilla::css::Loader::LoadStyleLink (this=0x4d79bf80, aElement=0x4d73c390, aURL=0x4d7d3e40, aTitle=..., aMedia=..., aHasAlternateRel=0, aObserver=0x4d741400, aIsAlternate=0xbfffe324) at /home/sstamm/src/build-csp/layout/style/Loader.cpp:1914 #6 0x40a53e50 in nsStyleLinkElement::DoUpdateStyleSheet (this=0x4d73c3cc, aOldDocument=0x0, aObserver=0x4d741400, aWillNotify=0xbfffe504, aIsAlternate=0xbfffe500, aForceUpdate=0) at /home/sstamm/src/build-csp/content/base/src/nsStyleLinkElement.cpp:303 #7 0x40a53fe0 in nsStyleLinkElement::UpdateStyleSheet (this=0x4d73c3cc, aObserver=0x4d741400, aWillNotify=0xbfffe504, aIsAlternate=0xbfffe500) at /home/sstamm/src/build-csp/content/base/src/nsStyleLinkElement.cpp:200 #8 0x40f19bab in nsHtml5TreeOpExecutor::UpdateStyleSheet (this=0x4d741400, aElement=0x4d73c390) at /home/sstamm/src/build-csp/parser/html/nsHtml5TreeOpExecutor.cpp:310 #9 0x40f169e3 in nsHtml5TreeOperation::Perform (this=0x4d6264d0, aBuilder=0x4d741400, aScriptElement=0xbfffe938) at /home/sstamm/src/build-csp/parser/html/nsHtml5TreeOperation.cpp:679 #10 0x40f1a8ea in nsHtml5TreeOpExecutor::RunFlushLoop (this=0x4d741400) at /home/sstamm/src/build-csp/parser/html/nsHtml5TreeOpExecutor.cpp:489 #11 0x40f225a8 in nsHtml5ExecutorFlusher::Run (this=0x4d79a860) at /home/sstamm/src/build-csp/parser/html/nsHtml5StreamParser.cpp:153 #12 0x41b43e18 in nsThread::ProcessNextEvent (this=0x43dc9ac0, mayWait=0, result=0xbfffea20) at /home/sstamm/src/build-csp/xpcom/threads/nsThread.cpp:626 #13 0x41ac8a48 in NS_ProcessNextEvent_P (thread=0x43dc9ac0, mayWait=0) at nsThreadUtils.cpp:250 #14 0x4190ea18 in mozilla::ipc::MessagePump::Run (this=0x43d973d0, aDelegate=0x43d338a0) at /home/sstamm/src/build-csp/ipc/glue/MessagePump.cpp:110 #15 0x41bae7cb in MessageLoop::RunInternal (this=0x43d338a0) at /home/sstamm/src/build-csp/ipc/chromium/src/base/message_loop.cc:219 #16 0x41bae7e3 in MessageLoop::RunHandler (this=0x43d338a0) at /home/sstamm/src/build-csp/ipc/chromium/src/base/message_loop.cc:202 #17 0x41bae847 in MessageLoop::Run (this=0x43d338a0) at /home/sstamm/src/build-csp/ipc/chromium/src/base/message_loop.cc:176 #18 0x41798000 in nsBaseAppShell::Run (this=0x458496a0) at /home/sstamm/src/build-csp/widget/src/xpwidgets/nsBaseAppShell.cpp:181 #19 0x414ba889 in nsAppStartup::Run (this=0x458a5640) at /home/sstamm/src/build-csp/toolkit/components/startup/src/nsAppStartup.cpp:191 #20 0x4038a14e in XRE_main (argc=5, argv=0xbffff1b4, aAppData=0x43d10380) at /home/sstamm/src/build-csp/toolkit/xre/nsAppRunner.cpp:3691 #21 0x08049912 in main (argc=5, argv=0xbffff1b4) at /home/sstamm/src/build-csp/browser/app/nsBrowserApp.cpp:158

Instead of "two calls to NS_CheckContentPolicy" I meant to say "two calls to NS_CheckContentLoadPolicy".

> Should we cache the results of the first check? You mean in the CSP code, right? The two checks are not equivalent, so the caller can't cache the result.