This is needed for adblock. It uses the shouldLoad handler as its main mechanism for blocking ads. There's a little complexity here because: 1. Some builtin content policies already run in the child, so there's no reason to run them in the parent. This patch avoids doing so. 2. We don't want to have to send messages on every load if adblock isn't installed. This patch ensures that we don't do any extra work if the parent doesn't have any non-builtin content policies installed.

Same comment here as in bug 905441 - can we make this work without front-end code that is disconnected from the underlying content-policy triggering code?

First, I don't think this bug is security-related. There are a number of content policies for security, but this patch purposely avoids touching those. I don't know what we'll end up doing about those policies--that's supposed to be the subject of bug 896639 if I understand it. The patch in this bug is only supposed to address addon-installed content policies. Second, I think it makes a lot more sense to implement the code for this bug in JS than in C++. We're still experimenting with how the whole thing is going to work. I suspect that the CPOWs that we pass to the shouldLoad handler may need to be customized in some ways to handle certain calls like setUserData specially. It would be a lot more unwieldy to do that in C++. Also, the code technically lives in toolkit/, so it's not really part of the frontend. I guess I should have filed this bug differently though. Is there a toolkit component in bugzilla?

(In reply to Bill McCloskey (:billm) from comment #2) > First, I don't think this bug is security-related. There are a number of > content policies for security, but this patch purposely avoids touching > those. I don't know what we'll end up doing about those policies--that's > supposed to be the subject of bug 896639 if I understand it. The patch in > this bug is only supposed to address addon-installed content policies. Well, addons can have native-code content policies, so I'm not sure I understand the distinction you're trying to make. It also seems rather hacky to have a solution for a specific subset of content policies rather than a more general solution that addresses the core problem. If it's really just short-term, I guess I can live with that, but let's be clear about our intentions. > Second, I think it makes a lot more sense to implement the code for this bug > in JS than in C++. We're still experimenting with how the whole thing is > going to work. I suspect that the CPOWs that we pass to the shouldLoad > handler may need to be customized in some ways to handle certain calls like > setUserData specially. It would be a lot more unwieldy to do that in C++. The code can be JS but still live next to e.g. nsContentPolicy.cpp. I suppose the term "frontend" conflates a bunch of stuff - I just don't want this to be a completely separate "connector" that people can end up forgetting to update when making changes to the core code.

I spoke to bill about this in person. I'm still a bit doubtful that this will be a complete/maintainable solution (even as a compatibility shim) when it comes to actually shipping this, but we don't need to pre-judge that now, so I guess I'll withdraw my objection for the moment.

Comment on attachment 790518 [details] [diff] [review] shouldload Review of attachment 790518 [details] [diff] [review]: ----------------------------------------------------------------- ::: toolkit/modules/RemoteAddonsParent.jsm @@ +72,5 @@ > + .getService(Ci.nsIMessageBroadcaster); > + ppmm.addMessageListener("Addons:ShouldLoad:IgnorePolicies", this); > + ppmm.addMessageListener("Addons:ShouldLoad:Run", this); > + > + Services.obs.addObserver(this, "xpcom-category-entry-added", false); nit: you should be able to have a weak observer here which is usually better.. Just generate a QI function for this obj with nsISupportsWeakReference, and have the 3rd param here be true

Landed this after renaming ShouldLoad to ContentPolicy, which I think is clearer. https://hg.mozilla.org/integration/mozilla-inbound/rev/c70bedada68f

Er... just accepting for ShouldProcess is broken for at least image documents and <object>, no?

This is just an experimental thing so far. Adblock doesn't use ShouldProcess so I didn't implement it. I don't think there's any reason why we couldn't forward it in the same way though.

OK. We have internal content policies that do use ShouldProcess, but maybe those are already running in the child?

Yes, we run some internal content policies from the child. Not sure about all of them, but I don't see why any would be excepted.