Issue 3208 - Unify handling of $document and $elemhide exceptions and improve performance

Issue 3208 - Unify handling of $document and $elemhide exceptions and improve performance

[Thomas Greiner, 2015-11-10 18:22:34]

https://codereview.adblockplus.org/29329419/diff/29329420/lib/contentPolicy.js
File lib/contentPolicy.js (right):

https://codereview.adblockplus.org/29329419/diff/29329420/lib/contentPolicy.j...
lib/contentPolicy.js:177: function cleanWindowLocation(wnd)
This function doesn't need to be inside `processNode` so better move it outside
to declare it only once.

https://codereview.adblockplus.org/29329419/diff/29329420/lib/contentPolicy.j...
lib/contentPolicy.js:202: let whitelistMatch =
defaultMatcher.matchesAny(testWndLocation, typeMap, parentDocDomain, false,
sitekey);
`sitekey` is the bottom window's sitekey so shouldn't we instead pass the parent
window's sitekey since we're checking a request that originated from the parent
window?

[Wladimir Palant, 2015-11-10 19:13:15]

https://codereview.adblockplus.org/29329419/diff/29329420/lib/contentPolicy.js
File lib/contentPolicy.js (right):

https://codereview.adblockplus.org/29329419/diff/29329420/lib/contentPolicy.j...
lib/contentPolicy.js:177: function cleanWindowLocation(wnd)
On 2015/11/10 18:22:33, Thomas Greiner wrote:
> This function doesn't need to be inside `processNode` so better move it
outside
> to declare it only once.

Given that this is a temporary solution I'd rather not do this - this job will
be done by getFrames() in child/utils module.

https://codereview.adblockplus.org/29329419/diff/29329420/lib/contentPolicy.j...
lib/contentPolicy.js:202: let whitelistMatch =
defaultMatcher.matchesAny(testWndLocation, typeMap, parentDocDomain, false,
sitekey);
On 2015/11/10 18:22:34, Thomas Greiner wrote:
> `sitekey` is the bottom window's sitekey so shouldn't we instead pass the
parent
> window's sitekey since we're checking a request that originated from the
parent
> window?

Yes, this should really be testSitekey - this was a bug in the $elemhide
whitelisting code I merged here. I fixed this in
https://codereview.adblockplus.org/29329571/ but I'll move that change here
tomorrow.

[Wladimir Palant, 2015-11-11 07:54:21]

https://codereview.adblockplus.org/29329419/diff/29329420/lib/contentPolicy.js
File lib/contentPolicy.js (right):

https://codereview.adblockplus.org/29329419/diff/29329420/lib/contentPolicy.j...
lib/contentPolicy.js:202: let whitelistMatch =
defaultMatcher.matchesAny(testWndLocation, typeMap, parentDocDomain, false,
sitekey);
On 2015/11/10 18:22:34, Thomas Greiner wrote:
> `sitekey` is the bottom window's sitekey so shouldn't we instead pass the
parent
> window's sitekey since we're checking a request that originated from the
parent
> window?

Done.

[Thomas Greiner, 2015-11-11 13:26:10]

LGTM