Issue 6727 - Use string rather than map for single-domain filters

lib/filterClasses.js

Index: lib/filterClasses.js
===================================================================
--- a/lib/filterClasses.js
+++ b/lib/filterClasses.js
@@ -395,19 +395,20 @@
   /**
    * Determines whether domainSource is already upper-case,
    * can be overridden by subclasses.
    * @type {boolean}
    */
   domainSourceIsUpperCase: false,
 
   /**
-   * Map containing domains that this filter should match on/not match
-   * on or null if the filter should match on all domains
-   * @type {?Map.<string,boolean>}
+   * String specifying the domain that this filter should match on, or map
+   * containing domains that this filter should match on/not match on, or null
+   * if the filter should match on all domains
+   * @type {?Map.<string,boolean>|string}
    */
   get domains()
   {
     // Despite this property being cached, the getter is called
     // several times on Safari, due to WebKit bug 132872
     let prop = Object.getOwnPropertyDescriptor(this, "domains");
     if (prop)
       return prop.value;
@@ -423,17 +424,17 @@
         source = source.toUpperCase();
       }
       let list = source.split(this.domainSeparator);
       if (list.length == 1 && list[0][0] != "~")
       {
         // Fast track for the common one-domain scenario
         if (this.ignoreTrailingDot)
           list[0] = list[0].replace(/\.+$/, "");
-        domains = new Map([["", false], [list[0], true]]);
+        domains = list[0];
       }
       else
       {
         let hasIncludes = false;
         for (let i = 0; i < list.length; i++)
         {
           let domain = list[i];
           if (this.ignoreTrailingDot)
@@ -494,22 +495,28 @@
 
     // If no domains are set the rule matches everywhere
     if (!this.domains)
       return true;
 
     // If the document has no host name, match only if the filter
     // isn't restricted to specific domains
     if (!docDomain)
-      return this.domains.get("");
+      return typeof this.domains != "string" && this.domains.get("");
 
     if (this.ignoreTrailingDot)
       docDomain = docDomain.replace(/\.+$/, "");
     docDomain = docDomain.toUpperCase();
 
+    if (typeof this.domains == "string")
+    {
+      return docDomain == this.domains ||
+             docDomain.endsWith("." + this.domains);
+    }
+
     while (true)
     {
       let isDomainIncluded = this.domains.get(docDomain);
       if (typeof isDomainIncluded != "undefined")
         return isDomainIncluded;
 
       let nextDot = docDomain.indexOf(".");
       if (nextDot < 0)
@@ -521,23 +528,32 @@
 
   /**
    * Checks whether this filter is active only on a domain and its subdomains.
    * @param {string} docDomain
    * @return {boolean}
    */
   isActiveOnlyOnDomain(docDomain)
   {
-    if (!docDomain || !this.domains || this.domains.get(""))
+    if (!docDomain || !this.domains ||
+        typeof this.domains != "string" && this.domains.get(""))

[sergei, 2018/05/28 09:22:07]

IMO, perhaps it would be safer to rather check that `this.domains instanceof
Map` instead of typeof.

BTW, despite it's correct, wouldn't it be better to wrap the logical AND into
the braces () in order to improve readability because I'm not sure how many
people actually remember that && has a higher priority than || in JS?

[kzar, 2018/06/05 16:51:30]

Disagree on this one, people that don't understand JavaScript's logical
operators shouldn't really be messing with this code IMO.

[Manish Jethani, 2018/06/06 11:16:31]

This is one of those things that can lead to an endless debate, so in the
interest of consistency I'll leave it as it is. It seems in the rest of the code
we assume knowledge of logical operator precedence. I personally do prefer to
group things logically though.

[Manish Jethani, 2018/06/06 12:31:18]

typeof is typically faster than instanceof, so where there's a choice I think it
makes sense to use the former.

+    {
       return false;
+    }
 
     if (this.ignoreTrailingDot)
       docDomain = docDomain.replace(/\.+$/, "");
     docDomain = docDomain.toUpperCase();
 
+    if (typeof this.domains == "string")
+    {
+      return docDomain == this.domains ||
+             this.domains.endsWith("." + docDomain);
+    }
+
     for (let [domain, isIncluded] of this.domains)
     {
       if (isIncluded && domain != docDomain)
       {
         if (domain.length <= docDomain.length)
           return false;
 
         if (!domain.endsWith("." + docDomain))
@@ -550,17 +566,18 @@
 
   /**
    * Checks whether this filter is generic or specific
    * @return {boolean}
    */
   isGeneric()
   {
     return !(this.sitekeys && this.sitekeys.length) &&
-            (!this.domains || this.domains.get(""));
+            (!this.domains ||
+             typeof this.domains != "string" && this.domains.get(""));
   },
 
   /**
    * See Filter.serialize()
    * @inheritdoc
    */
   serialize(buffer)
   {