Share functionality and optimizations between ElemHide and ElemHideEmulation

[mjethani]

Background

We are doing some optimizations to element hiding emulation to make it perform better (hide elements as soon as they appear, don't waste CPU, etc.) (see #6437). We have also recently done some optimizations to element hiding selector lookups, while some others are on the way (see #6562, #6652). It may be possible now to merge the code in lib/elemHide.js and lib/elemHideEmulation.js so that ElemHideEmulation can have all the functionality of ElemHide, including the optimizations.

Once this is done, both ElemHide and ElemHideEmulation as well as the upcoming Snippets module (#6538) will be able to do fast lookups of CSS selectors, emulation rules, and snippet scripts.

What to change

Split out code that is related to exceptions into its own lib/elemHideExceptions.js file and export the functionality as a new ElemHideExceptions module.

In lib/elemHide.js, separate out code that is common to both ElemHide and ElemHideEmulation, including the data structures, into a new ElemHideTemplate object. Make both ElemHide and ElemHideEmulation instances of this "class" or "template", with their own functions added on top (e.g. ElemHide.getSelectorsForDomain and ElemHideEmulation.getRulesForDomain).

Both ElemHide and ElemHideEmulation should use the same logic via ElemHideTemplate but have their own copies of the data structures that are used for bookkeeping and optimization. Exceptions should be shared between the two via the new ElemHideExceptions object.

Hints for testers

After changeset ​3fa9d74b5e3f, element hiding exceptions should work correctly as before. For example, given two filters example.com##.foo and www.example.com#@#.foo, the element <div class="foo">Ad</div> should be hidden on example.com but should remain visible on www.example.com. Excluding domains in a filter should work too; for example, given only the filter example.com,~www.example.com##.foo, the element <div class="foo">Ad</div> should be hidden on example.com but remain visible on www.example.com.

[mjethani]

@kzar, @hfiguiere does this sound like a good idea?

Right now a filter like ~cool.example.com,example.com#?#div:-abp-contains(Ad) will still apply on cool.example.com even though it shouldn't.

[mjethani]

Replying to mjethani:

Right now a filter like ~cool.example.com,example.com#?#div:-abp-contains(Ad) will still apply on cool.example.com even though it shouldn't.

My bad, this already works.

[kzar]

It sounds like a good idea to me, if we can reuse ElemHide.getSelectorsForDomain for the emulation hiding filters then why not? It's fast and well tested. That said I'm not as familiar with the emulation hiding code, so I'm not sure what this would look like in practice.

[mjethani]

@kzar since you agree that doing at least some work towards this is a good idea, I'm setting this to Ready. I also think that it would be better to do this first so the Snippets implementation can benefit from it.

[kzar]

Yes, you're free to triage issues in the Core module now.

[mjethani]

This is the only issue that will keep us from making full use of element hiding emulation, we should make it a priority.

[abpbot]

A commit referencing this issue has landed:
​Issue 6665 - Split out element hiding exceptions into their own module

[Ross]

Done. Working as described.

ABP 3.3.2.2175
Firefox 62 / 51 / Windows 10
Chrome 69 / 49 / Windows 10
Opera 56 / 36 / Windows 10

[sebastian]

Sorry, but we switched to GitLab. If this issue is still relevant, please file it again in the ​new issue tracker.