Issue 6538, 6781 - Allow snippets only from circumvention lists

Based on https://codereview.adblockplus.org/29737558/ and https://codereview.adblockplus.org/29824575/

[Manish Jethani, 2018-07-06 15:23:10]

Patch Set 1

As per the plan for the initial "alpha" release, we will execute snippets only
based on filters from the circumvention list or one of the user's own filters.

[Manish Jethani, 2018-07-10 07:17:24]

Patch Set 2: Remove redundant check in for loop
Patch Set 3: Improve unit tests

[kzar, 2018-07-10 14:38:36]

https://codereview.adblockplus.org/29824589/diff/29826558/lib/filterListener.js
File lib/filterListener.js (right):

https://codereview.adblockplus.org/29824589/diff/29826558/lib/filterListener....
lib/filterListener.js:145: for (let i = 0; i < filter.subscriptions.length; i++)
Nit: `for (let subscription of filter.subscriptions)`

[Manish Jethani, 2018-07-10 18:33:22]

https://codereview.adblockplus.org/29824589/diff/29826558/lib/filterListener.js
File lib/filterListener.js (right):

https://codereview.adblockplus.org/29824589/diff/29826558/lib/filterListener....
lib/filterListener.js:145: for (let i = 0; i < filter.subscriptions.length; i++)
On 2018/07/10 14:38:35, kzar wrote:
> Nit: `for (let subscription of filter.subscriptions)`

I did this initially and then reverted it. There's a similar loop in the
removeFilter function. The way I see it, this would be an unrelated change. I
think it should be done separately as a "no issue", also in the removeFilter
function.

[kzar, 2018-07-10 18:56:00]

https://codereview.adblockplus.org/29824589/diff/29826558/lib/filterListener.js
File lib/filterListener.js (right):

https://codereview.adblockplus.org/29824589/diff/29826558/lib/filterListener....
lib/filterListener.js:145: for (let i = 0; i < filter.subscriptions.length; i++)
On 2018/07/10 18:33:22, Manish Jethani wrote:
> On 2018/07/10 14:38:35, kzar wrote:
> > Nit: `for (let subscription of filter.subscriptions)`
> 
> I did this initially and then reverted it. There's a similar loop in the
> removeFilter function. The way I see it, this would be an unrelated change. I
> think it should be done separately as a "no issue", also in the removeFilter
> function.

Sure it's an unrelated change, but you're changing the entire loop anyway, it
seems crazy not to fix this at the same time IMO.

[Manish Jethani, 2018-07-11 15:07:04]

https://codereview.adblockplus.org/29824589/diff/29826558/lib/filterListener.js
File lib/filterListener.js (right):

https://codereview.adblockplus.org/29824589/diff/29826558/lib/filterListener....
lib/filterListener.js:145: for (let i = 0; i < filter.subscriptions.length; i++)
On 2018/07/10 18:56:00, kzar wrote:
> On 2018/07/10 18:33:22, Manish Jethani wrote:
> > On 2018/07/10 14:38:35, kzar wrote:
> > > Nit: `for (let subscription of filter.subscriptions)`
> > 
> > I did this initially and then reverted it. There's a similar loop in the
> > removeFilter function. The way I see it, this would be an unrelated change.
I
> > think it should be done separately as a "no issue", also in the removeFilter
> > function.
> 
> Sure it's an unrelated change, but you're changing the entire loop anyway, it
> seems crazy not to fix this at the same time IMO.

There are some performance implications of for..of [1] which really matters on
mobile. I would like to properly test/benchmark it and make the change in both
places.

[1]:
https://www.incredible-web.com/blog/performance-of-for-loops-with-javascript/

[kzar, 2018-07-11 17:22:07]

LGTM

https://codereview.adblockplus.org/29824589/diff/29826558/lib/filterListener.js
File lib/filterListener.js (right):

https://codereview.adblockplus.org/29824589/diff/29826558/lib/filterListener....
lib/filterListener.js:145: for (let i = 0; i < filter.subscriptions.length; i++)
On 2018/07/11 15:07:03, Manish Jethani wrote:
> On 2018/07/10 18:56:00, kzar wrote:
> > On 2018/07/10 18:33:22, Manish Jethani wrote:
> > > On 2018/07/10 14:38:35, kzar wrote:
> > > > Nit: `for (let subscription of filter.subscriptions)`
> > > 
> > > I did this initially and then reverted it. There's a similar loop in the
> > > removeFilter function. The way I see it, this would be an unrelated
change.
> I
> > > think it should be done separately as a "no issue", also in the
removeFilter
> > > function.
> > 
> > Sure it's an unrelated change, but you're changing the entire loop anyway,
it
> > seems crazy not to fix this at the same time IMO.
> 
> There are some performance implications of for..of [1] which really matters on
> mobile. I would like to properly test/benchmark it and make the change in both
> places.
> 
> [1]:
> https://www.incredible-web.com/blog/performance-of-for-loops-with-javascript/

Fine.