Issue 3208 - Inject content policy implementation into all processes

Issue 3208 - Inject content policy implementation into all processes

[Wladimir Palant, 2015-10-22 21:51:11]

This is a massive patch, sorry. However, this seems to be the minimal amount of
changes required to allow the code to work *somewhat* correctly after splitting
up the current contentPolicy.js. In fact, lots of functionality remains broken
after this patch - see "Blocking" list in
https://issues.adblockplus.org/ticket/3208. This change isn't supposed to be
pushed until the rest of the codebase is adjusted as well.

I tried to avoid changing functionality but often it simply wasn't possible.
I've added corresponding notes to the code.

https://codereview.adblockplus.org/29329336/diff/29329346/chrome/locale/en-US...
File chrome/locale/en-US/global.properties (right):

https://codereview.adblockplus.org/29329336/diff/29329346/chrome/locale/en-US...
chrome/locale/en-US/global.properties:47: type_label_genericblock=generic block
This is required so that generichide/genericblock hits can show up in the
blockable items list (not that it works right now).

https://codereview.adblockplus.org/29329336/diff/29329346/lib/child/contentPo...
File lib/child/contentPolicy.js (left):

https://codereview.adblockplus.org/29329336/diff/29329346/lib/child/contentPo...
lib/child/contentPolicy.js:556: }
The code above was added because of AVG Secure Search extension which simply
cleared the content-policy category. Luckily, the latest version no longer does
anything that stupid. This hack caused Firefox to hang up on update (we forgot
to consider the scenario where we removed the category entry ourselves) so I
simply removed it.

https://codereview.adblockplus.org/29329336/diff/29329346/lib/child/contentPo...
lib/child/contentPolicy.js:601: }
In bug 467514 I confirmed that the issue is gone in Firefox 28. Regardless of
when exactly it was fixed, we no longer have to care.

https://codereview.adblockplus.org/29329336/diff/29329346/lib/child/contentPo...
File lib/child/contentPolicy.js (right):

https://codereview.adblockplus.org/29329336/diff/29329346/lib/child/contentPo...
lib/child/contentPolicy.js:49: let types = new Map();
The original implementation used three maps for translating types. This got
simpler now: this module always uses numerical type IDs, the backend module uses
string IDs only. Conversion is only necessary to translate the former to the
latter when sending data - that's what this map is for.

https://codereview.adblockplus.org/29329336/diff/29329346/lib/child/contentPo...
lib/child/contentPolicy.js:76: function checkRequest(window, node, contentType,
location)
This is mostly equivalent to the old Policy.processNode() function. location is
now a string, not nsIURI (sending the latter to another process would be
non-trivial). The last parameter (collapse) has been removed as this one is only
being used by Policy.refilterNodes() (called by the filter assistant) which
hasn't been updated yet. Also, the return value has been reversed - it's block
rather than allow now.

https://codereview.adblockplus.org/29329336/diff/29329346/lib/child/contentPo...
lib/child/contentPolicy.js:143: 
Bug 924340 has been fixed in Firefox 26, bug 753687 in Firefox 27. So I removed
the async factory registration/unregistration hack.

https://codereview.adblockplus.org/29329336/diff/29329346/lib/child/contentPo...
lib/child/contentPolicy.js:169: addMessageListener("AdblockPlus:CollapsedClass",
setCollapsedClass);
This class name has to be identical in all processes (user stylesheets are only
registered once for all of them), so it isn't being generated here but rather
received from the backend.

https://codereview.adblockplus.org/29329336/diff/29329346/lib/child/contentPo...
lib/child/contentPolicy.js:193: let window = getWindow(node);
I'm not using Utils.getWindow() here for a reason - backend code should never
access elements or windows directly, and Utils.getWindow() should be considered
deprecated. Code running in the content process can use getWindow() from
child/utils module.

https://codereview.adblockplus.org/29329336/diff/29329346/lib/child/contentPo...
lib/child/contentPolicy.js:203: node = node.ownerDocument;
The checks had to be shuffled around somewhat. Checking whitelisted schemes and
unknown types has to be done in the backend code, it depends on the
configuration available there. Correcting parameter values for objects on the
other hand had to be done here, it depends on the DOM elements.

https://codereview.adblockplus.org/29329336/diff/29329346/lib/child/contentPo...
lib/child/contentPolicy.js:243: this.observe(subject, topic, data,
channel.URI.spec);
This relied on the http-on-opening-request notification originally, it could be
received immediately when the window started loading. That doesn't really work
with E10S as the http-on-opening-request is usually being sent out in another
process (content process doesn't do any networking). However, I found a better
way to figure out what URL is being loaded right now.

https://codereview.adblockplus.org/29329336/diff/29329346/lib/child/contentPo...
lib/child/contentPolicy.js:265: return;
Originally we used a complicated and rather fragile approach to attach the
content policy type to the channel (see previousRequest and abpRequestType in
http-on-opening-request observer). Luckily, I didn't have to fix this hack -
Mozilla's networking code attaches the content policy type now without us doing
anything.

https://codereview.adblockplus.org/29329336/diff/29329346/lib/child/contentPo...
lib/child/contentPolicy.js:273: if (wnd.history.length <= 1 && wnd.opener)
Since we no longer attach our content type to the channel we need a different
way to recognize popups - this one seems to be good enough, and much more sane.

https://codereview.adblockplus.org/29329336/diff/29329346/lib/child/contentPo...
lib/child/contentPolicy.js:276: this.observe(wnd,
"content-document-global-created", null);
This fixes a bug in the original code. It was merely blocking the redirect
instead of closing the pop-up - re-running the pop-up handling code on redirect
fixes that.

https://codereview.adblockplus.org/29329336/diff/29329346/lib/child/utils.js
File lib/child/utils.js (right):

https://codereview.adblockplus.org/29329336/diff/29329346/lib/child/utils.js#...
lib/child/utils.js:56: }
The block above is something we have been doing in contentPolicy.js but not in
elemHide.js. Now the behavior is consistent.

Otherwise the getFrames() function simply moved here from elemHide.js. Only one
further difference: private browsing flag is being sent separately now, see
isPrivate() below.

https://codereview.adblockplus.org/29329336/diff/29329346/lib/contentPolicy.js
File lib/contentPolicy.js (left):

https://codereview.adblockplus.org/29329336/diff/29329346/lib/contentPolicy.j...
lib/contentPolicy.js:85: nonVisual: {},
I removed that member - it isn't necessary if nonVisualTypes is a set.

https://codereview.adblockplus.org/29329336/diff/29329346/lib/contentPolicy.j...
lib/contentPolicy.js:218: contentType = Policy.type.OBJECT;
The two checks above moved to PolicyImplementation.shouldLoad().

https://codereview.adblockplus.org/29329336/diff/29329346/lib/contentPolicy.j...
lib/contentPolicy.js:281: }
This part moved to checkRequest() in the child.

https://codereview.adblockplus.org/29329336/diff/29329346/lib/contentPolicy.js
File lib/contentPolicy.js (right):

https://codereview.adblockplus.org/29329336/diff/29329346/lib/contentPolicy.j...
lib/contentPolicy.js:132: contentType = "OTHER";
This check was in PolicyImplementation.shouldLoad() originally.

https://codereview.adblockplus.org/29329336/diff/29329346/lib/contentPolicy.j...
lib/contentPolicy.js:179: }
This block was further down originally, having a separate yet mostly identical
loop for it makes little sense however.

https://codereview.adblockplus.org/29329336/diff/29329346/lib/contentPolicy.j...
lib/contentPolicy.js:195: contentType: genericType,
Note that this parameter changed: originally the hit would be recorded with the
type of the request, now with "GENERICHIDE" or "GENERICBLOCK" (note the two
strings I added to global.properties so that these could be displayed in the
blockable items list). IMHO that wasn't implemented correctly.

https://codereview.adblockplus.org/29329336/diff/29329346/lib/contentPolicy.j...
lib/contentPolicy.js:213: if (!match.isActiveOnDomain(docDomain) || (nogeneric
&& match.isGeneric()))
The nogeneric part of this check was further down originally.

https://codereview.adblockplus.org/29329336/diff/29329346/lib/contentPolicy.j...
lib/contentPolicy.js:317: refilterNodes: function(nodes, entry)
This functionality is intentionally left broken, fixing it is subject to
https://issues.adblockplus.org/ticket/3224.

https://codereview.adblockplus.org/29329336/diff/29329346/lib/contentPolicy.j...
lib/contentPolicy.js:371: return [null, null];
Note that the original code was returning sitekey variable but never setting it
- that was some left-over apparently.

https://codereview.adblockplus.org/29329336/diff/29329346/lib/filterStorage.js
File lib/filterStorage.js (right):

https://codereview.adblockplus.org/29329336/diff/29329346/lib/filterStorage.j...
lib/filterStorage.js:329: if (!Prefs.savestats || !(filter instanceof
ActiveFilter))
Having private browsing considered in filterStorage was always a hack, now
contentPolicy takes care of that.

[Wladimir Palant, 2015-10-26 20:44:48]

Never mind, I'm splitting up this patch - there are way too many unrelated
changes here.