Issue 2738 - Make RegExpFilter.matches() take a bit mask instead of content type string

lib/filterClasses.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-2015 Eyeo GmbH
  *
  * Adblock Plus is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
  * published by the Free Software Foundation.
  *
  * Adblock Plus is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
@@ skipping 615 matching lines @@
    * Tests whether the URL matches this filter
    * @param {String} location URL to be tested
    * @param {String} typeMask bitmask of content / request types to match
    * @param {String} docDomain domain name of the document that loads the URL
    * @param {Boolean} thirdParty should be true if the URL is a third-party request
    * @param {String} sitekey public key provided by the document
    * @return {Boolean} true in case of a match
    */
   matches: function(location, typeMask, docDomain, thirdParty, sitekey)
   {
-    if ((this.contentType & typeMask) != 0 &&

[Sebastian Noack, 2015/07/09 15:36:44]

Note that |!= 0| is unneeded here.

[kzar, 2015/07/12 13:59:39]

Done.

+    if (this.contentType & typeMask &&
         (this.thirdParty == null || this.thirdParty == thirdParty) &&
         this.isActiveOnDomain(docDomain, sitekey) && this.regexp.test(location))
     {
       return true;
     }
 
     return false;
   }
 };
 
@@ skipping 107 matching lines @@
   BACKGROUND: 4,    // Backwards compat, same as IMAGE
 
   POPUP: 0x10000000,
   ELEMHIDE: 0x40000000
 };
 
 // DOCUMENT, ELEMHIDE, POPUP options shouldn't be there by default
 RegExpFilter.prototype.contentType &= ~(RegExpFilter.typeMap.DOCUMENT | RegExpFilter.typeMap.ELEMHIDE | RegExpFilter.typeMap.POPUP);
 
 /**
- * Creates a content / request type mask from a content type string or
- * array of content type strings.
- * @param {String} [contentType] content / request type string(s)
- */
-RegExpFilter.toTypeMask = function (contentType)

[Sebastian Noack, 2015/07/09 15:36:44]

I feel that this function just adds unneeded complexity and overhead. From the
use cases I've seen so far, one could as well write..

  RegExpFilter.typeMap[mediatype]

..instead..

  RegExpFilter.toTypeMask(mediatype)

[kzar, 2015/07/09 17:44:18]

I've got to say I agree with you here, the few times we _do_ need multiple types
the calling code could just bitwise OR the types together itself. What do you
think Wladimir?

[Wladimir Palant, 2015/07/10 21:07:00]

Not really sure this is a good idea but well - let's treat RegExpFilter.typeMap
like a public enum (it's already being used in a few places in the code). And -
no, having it around just to convert arrays isn't worth the effort.
RegExpFilter.typeMask.DOCUMENT definitely reads better.

[kzar, 2015/07/12 13:59:39]

Done.

-{
-  if (typeof contentType == "string")
-    return RegExpFilter.typeMap[contentType];
-
-  var mask = 0;
-  for (let type of contentType)
-    mask |= RegExpFilter.typeMap[type];
-  return mask;
-};
-
-/**
  * Class for blocking filters
  * @param {String} text see Filter()
  * @param {String} regexpSource see RegExpFilter()
  * @param {Number} contentType see RegExpFilter()
  * @param {Boolean} matchCase see RegExpFilter()
  * @param {String} domains see RegExpFilter()
  * @param {Boolean} thirdParty see RegExpFilter()
  * @param {String} sitekeys see RegExpFilter()
  * @param {Boolean} collapse  defines whether the filter should collapse blocked content, can be null
  * @constructor
@@ skipping 247 matching lines @@
     // several times on Safari, due to WebKit bug 132872
     let prop = Object.getOwnPropertyDescriptor(this, "regexpString");
     if (prop)
       return prop.value;
 
     let regexp = Filter.toRegExp(this.regexpSource);
     Object.defineProperty(this, "regexpString", {value: regexp});
     return regexp;
   }
 };