Open Bug 380556 Opened 14 years ago Updated 6 years ago
IContent Policy .Should Process is useless
I was asked today what nsIContentPolicy.ShouldLoad does and I found it really difficult to answer. The comment in the interface says: ShouldProcess will be called once all the information passed to it has been determined about the resource, typically after part of the resource has been loaded. In my understanding that would mean that any ShouldLoad call should have a corresponding ShouldProcess call once the load started and more information is available. So much about theory. However, from what I see there is exactly one caller for ShouldProcess in the code base (nsImageDocument) and zero consumers (all consumers I could find either always accept or delegate the call to ShouldLoad). Not even the documented ShouldProcess use case (TYPE_REFRESH) works, there are no content policy calls for META Refresh. I see two possible courses of action: 1. Fix up TYPE_REFRESH (should it be fixed?), fix bug 305699 and bug 309524, add a few dozen ShouldProcess calls in other places as well to make this feature somewhat consistent/useful. 2. Remove TYPE_REFRESH and ShouldLoad altogether and stop pretending to have a feature that is plainly broken. I tend to choose the second solution since there doesn't seem to be enough use cases for ShouldLoad to justify calling it everywhere. Please comment.
I forgot the third possible solution: Remove TYPE_REFRESH and fix the documentation to tell what this feature is really good for.
Did you mean "ShouldProcess" most of the times you typed "ShouldLoad" in comment 0 and the summary? The right solution is whatever the consumers want, really. I'd go with #1, myself, but if that wouldn't be useful we could consider removing it instead... Part of the issue for me is that content policy calls are really quite expensive (in that the policies do things that take a _lot_ of time during pageload). Doubling that stuff by also adding ShouldProcess calls is not that appealing in some ways; I would be happier if we had a faster solution somehow. :( Perhaps if more of the built-in content policies made their ShouldProcess impls no-ops instead of synonyms for ShouldLoad?
Summary: nsIContentPolicy.ShouldLoad is useless → nsIContentPolicy.ShouldProcess is useless
It would mostly be useful for cases where ShouldLoad is never called (inline stylesheets, inline scripts, meta refresh) and for cases where ShouldLoad doesn't know what you're loading (iframes and objects)...
The Tor Project / Electronic Frontier Foundation is paying to have this bug fixed. "If you know C++ and/or Firefox internals, we should be able to pay you for your time to address these issues and shepherd the relevant patches through Mozilla's review process." Source: https://blog.torproject.org/blog/web-developers-and-firefox-hackers-help-us-firefox-4
You need to log in before you can comment on or make changes to this bug.