Opened on 06/30/2015 at 11:35:25 AM
Closed on 07/14/2015 at 03:05:16 PM
Last modified on 07/22/2015 at 09:57:14 AM
#2738 closed change (fixed)
Make RegExpFilter.matches() take a bit mask rather than string for type
Reported by: | trev | Assignee: | kzar |
---|---|---|---|
Priority: | P2 | Milestone: | Adblock-Plus-2.6.10-for-Firefox |
Module: | Core | Keywords: | |
Cc: | kzar, sebastian | Blocked By: | |
Blocking: | #616, #647 | Platform: | Unknown / Cross platform |
Ready: | yes | Confidential: | no |
Tester: | Unknown | Verified working: | no |
Review URL(s): |
https://codereview.adblockplus.org/29321478/ |
Description (last modified by kzar)
Background
We occasionally need to check a URL with multiple document types, e.g. DOCUMENT and ELEMHIDE. Currently we have to call RegExpFilter.matches() multiple times which is inefficient.
What to change
RegExpFilter translates the incoming type into a bit mask internally. We should instead have the type passed in as a bit mask - that will allow specifying multiple types in one go.
So the changes to adblockplus in detail:
- RegExpFilter.prototype.matches: contentType parameter becomes a numerical typeMask. We can just compare typeMask to this.contentType there without any additional conversion.
- Matcher.prototype.matchesAny: contentType parameter becomes a numerical typeMask and that is passed through all the way to RegExpFilter.prototype.matches.
- All callers to Matcher.prototype.matchesAny need to be adapted in order to call pass a numerical typeMask instead of contentType string (there are plenty). For the Policy object we want to cache the value however - add a typeMask object mapping numerical types to the corresponding masks (similar to the existing typeDescr). So defaultMatcher.matchesAny(..., item.typeDescr, ...) becomes defaultMatcher.matchesAny(..., Policy.typeMask[item.typeDescr], ...).
- Unit tests have to be adapted for this API change.
For adblockpluschrome we will need to simultaneously update the adblockplus dependency and update any calling code to be compatible. Specifically the dependency should be adblockplus = adblockplus hg:b7c6ed7c2137 git:7dce96a. There were no other changes since the previous dependency update.
Attachments (0)
Change History (9)
comment:1 Changed on 06/30/2015 at 11:39:21 AM by sebastian
- Cc sebastian added
comment:2 Changed on 07/08/2015 at 12:35:56 PM by kzar
- Owner set to kzar
comment:3 Changed on 07/09/2015 at 01:49:03 PM by kzar
- Review URL(s) modified (diff)
- Status changed from new to reviewing
comment:6 Changed on 07/12/2015 at 04:46:41 PM by kzar
- Blocking 647 added
comment:8 Changed on 07/14/2015 at 03:05:16 PM by kzar
- Resolution set to fixed
- Status changed from reviewing to closed
comment:9 Changed on 07/22/2015 at 09:57:14 AM by trev
- Milestone set to Adblock-Plus-2.6.10-for-Firefox
https://hg.adblockplus.org/adblockplus/rev/b7c6ed7c2137
https://hg.adblockplus.org/adblockplustests/rev/ae6c7a2cc723
https://hg.adblockplus.org/adblockpluschrome/rev/c2a297cd2884