Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#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/
https://codereview.adblockplus.org/29321487/
https://codereview.adblockplus.org/29321504/

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.

Change History (9)

comment:1 Changed 3 years ago by sebastian

  • Cc sebastian added

comment:2 Changed 3 years ago by kzar

  • Owner set to kzar

comment:3 Changed 3 years ago by kzar

  • Review URL(s) modified (diff)
  • Status changed from new to reviewing

comment:4 Changed 3 years ago by kzar

  • Review URL(s) modified (diff)

comment:5 Changed 3 years ago by kzar

  • Description modified (diff)

comment:6 Changed 3 years ago by kzar

  • Blocking 647 added

comment:7 Changed 3 years ago by kzar

  • Description modified (diff)

comment:9 Changed 3 years ago by trev

  • Milestone set to Adblock-Plus-2.6.10-for-Firefox
Note: See TracTickets for help on using tickets.