Issue 3108 - Inject our about: module into all processes

Issue 3108 - Inject our about: module into all processes

[Wladimir Palant, 2015-10-16 12:15:11]

https://codereview.adblockplus.org/29329246/diff/29329247/lib/child/bootstrap.js
File lib/child/bootstrap.js (right):

https://codereview.adblockplus.org/29329246/diff/29329247/lib/child/bootstrap...
lib/child/bootstrap.js:59: Components, Cc, Ci, Cu, Cr, atob, btoa, onShutdown,
bootstrap.js from buildtools also exposes File and XMLHttpRequest but I think
that we better remove both from the global scope there as well. We don't use
File, and XMLHttpRequest is better accessed via XPCOM explicitly.

The other difference is the Components property - SDK loader will
unconditionally mask it by a getter throwing an exception (see hack in
elemHideHitRegistration.js). So far I couldn't find a better solution than
removing the getter explicitly in the modules that need to access Components.

https://codereview.adblockplus.org/29329246/diff/29329247/lib/contentPolicy.js
File lib/contentPolicy.js (right):

https://codereview.adblockplus.org/29329246/diff/29329247/lib/contentPolicy.j...
lib/contentPolicy.js:362: var PolicyImplementation =
This is only marginally related - it unbreaks Diagnostics extension (see
https://blog.mozilla.org/addons/2015/10/14/breaking-changes-let-const-firefox...),
almost impossible to test otherwise.

https://codereview.adblockplus.org/29329246/diff/29329247/lib/elemHide.js
File lib/elemHide.js (right):

https://codereview.adblockplus.org/29329246/diff/29329247/lib/elemHide.js#new...
lib/elemHide.js:242: return !Policy.processNode(fakeFrame, fakeFrame.document,
Policy.type.ELEMHIDE, filter);
Side-effect here: the hit info is attached to a fake window, meaning that it
won't appear in the blockable items list. Not really feasible to fix until we
start injecting the content policy into all processes as well (at this point we
will also be able to stop faking DOM structures here).

https://codereview.adblockplus.org/29329246/diff/29329247/metadata.gecko
File metadata.gecko (right):

https://codereview.adblockplus.org/29329246/diff/29329247/metadata.gecko#newc...
metadata.gecko:36: toolkit=38.0/45.0
This is because of our usage of process scripts. The SDK loader on the other
hand is available starting with Firefox 23.

[Thomas Greiner, 2015-10-16 15:40:25]

LGTM
Also the comments you attached were quite helpful so thanks for adding those.

https://codereview.adblockplus.org/29329246/diff/29329247/lib/child/bootstrap.js
File lib/child/bootstrap.js (right):

https://codereview.adblockplus.org/29329246/diff/29329247/lib/child/bootstrap...
lib/child/bootstrap.js:59: Components, Cc, Ci, Cu, Cr, atob, btoa, onShutdown,
On 2015/10/16 12:15:10, Wladimir Palant wrote:
> The other difference is the Components property - SDK loader will
> unconditionally mask it by a getter throwing an exception (see hack in
> elemHideHitRegistration.js). So far I couldn't find a better solution than
> removing the getter explicitly in the modules that need to access Components.

I couldn't find a better one either. We could implement our own module resolver
but that would end up being even more effort.

[Wladimir Palant, 2015-10-16 17:52:03]

https://codereview.adblockplus.org/29329246/diff/29329247/lib/child/bootstrap.js
File lib/child/bootstrap.js (right):

https://codereview.adblockplus.org/29329246/diff/29329247/lib/child/bootstrap...
lib/child/bootstrap.js:59: Components, Cc, Ci, Cu, Cr, atob, btoa, onShutdown,
On 2015/10/16 15:40:25, Thomas Greiner wrote:
> I couldn't find a better one either. We could implement our own module
resolver
> but that would end up being even more effort.

The alternative is exposing this property as components rather than Components
(the SDK does that) but I'd rather keep it consistent with the current
bootstrap.js in buildtools. We could use the SDK loader there as well, but
currently I have doubts that this would make sense (requires significant effort,
e.g. testing the performance impact, yet doesn't give us anything other than
being consistent with the SDK which seems to be doomed as well).