Issue 6699 - Support the "circumvention" filter list as default subscription

lib/subscriptionInit.js

Index: lib/subscriptionInit.js
===================================================================
--- a/lib/subscriptionInit.js
+++ b/lib/subscriptionInit.js
@@ -82,61 +82,74 @@
         subscription.filters.length > 0)
       return false;
   }
 
   return true;
 }
 
 /**
- * Finds the element for the default ad blocking filter subscription based
+ * @typedef {object} DefaultSubscriptions
+ * @property {?Element} ads
+ * @property {?Element} cv
+ */
+/**
+ * Finds the elements for the default ad blocking filter subscriptions based
  * on the user's locale.
  *
  * @param {HTMLCollection} subscriptions
- * @return {Element}
+ * @return {DefaultSubscriptions}
  */
-function chooseFilterSubscription(subscriptions)
+function chooseFilterSubscriptions(subscriptions)
 {
-  let selectedItem = null;
+  let selectedItem = {};
   let selectedPrefix = null;
   let matchCount = 0;
   for (let subscription of subscriptions)
   {
-    if (!selectedItem)
-      selectedItem = subscription;
-
     let prefixes = subscription.getAttribute("prefixes");
     let prefix = prefixes && prefixes.split(",").find(
       lang => new RegExp("^" + lang + "\\b").test(Utils.appLocale)
     );
 
     let subscriptionType = subscription.getAttribute("type");
 
-    if (prefix && subscriptionType == "ads")
+    if (subscriptionType in ["ads", "cv"] && !selectedItem[subscriptionType])
+      selectedItem[subscriptionType] = subscription;
+
+    if (prefix)
     {
-      if (!selectedPrefix || selectedPrefix.length < prefix.length)
-      {
-        selectedItem = subscription;
-        selectedPrefix = prefix;
-        matchCount = 1;
-      }
-      else if (selectedPrefix && selectedPrefix.length == prefix.length)
+      // The "ads" subscription is the one driving the selection.
+      if (subscriptionType == "ads")
       {
-        matchCount++;
+        if (!selectedPrefix || selectedPrefix.length < prefix.length)
+        {
+          selectedItem[subscriptionType] = subscription;
+          selectedPrefix = prefix;
+          matchCount = 1;
+        }
+        else if (selectedPrefix && selectedPrefix.length == prefix.length)
+        {
+          matchCount++;
 
-        // If multiple items have a matching prefix of the same length:
-        // Select one of the items randomly, probability should be the same
-        // for all items. So we replace the previous match here with
-        // probability 1/N (N being the number of matches).
-        if (Math.random() * matchCount < 1)
-        {
-          selectedItem = subscription;
-          selectedPrefix = prefix;
+          // If multiple items have a matching prefix of the same length:
+          // Select one of the items randomly, probability should be the same
+          // for all items. So we replace the previous match here with
+          // probability 1/N (N being the number of matches).
+          if (Math.random() * matchCount < 1)
+          {
+            selectedItem[subscriptionType] = subscription;
+            selectedPrefix = prefix;
+          }
         }
       }
+      else if (subscriptionType == "cv")
+      {
+        selectedItem[subscriptionType] = subscription;
+      }
     }
   }
   return selectedItem;
 }
 
 function supportsNotificationsWithButtons()
 {
   // Microsoft Edge (as of EdgeHTML 16) doesn't have the notifications API.
@@ -201,27 +214,35 @@
   {
     return fetch("subscriptions.xml")
       .then(response => response.text())
       .then(text =>
       {
         let doc = new DOMParser().parseFromString(text, "application/xml");
         let nodes = doc.getElementsByTagName("subscription");
 
-        let node = chooseFilterSubscription(nodes);
-        if (node)
+        let subs = chooseFilterSubscriptions(nodes);
+        if (subs)
         {
-          let url = node.getAttribute("url");
-          if (url)
+          for (let name in subs)

[Manish Jethani, 2018/06/13 16:58:12]

By the way, if you want to keep the return type a plain object rather than a Map
object, I think Object.keys would be faster and more appropriate because it only
returns the _own_ enumerable properties.

[hub, 2018/06/13 21:58:19]

There are at most two properties.

[Manish Jethani, 2018/06/14 05:07:34]

Acknowledged.

Since the calling code is now assuming the number of properties, maybe it could
assume the names as well? So `for (let name of ["ads", "circumvention"])`.

[Manish Jethani, 2018/06/14 05:42:56]

OK, forget this, it makes it worse.

Like you said it doesn't matter in this case, but when the names of the keys are
unknown (coming from an XML file for example), a Map object performs
significantly better (regardless of the number of keys).

Here's a test:

  (function()
  {
    function choose()
    {
      //return new Map([
      //  ["" + Math.random(), (Math.random() * 2) | 0],
      //  ["" + Math.random(), (Math.random() * 3) | 0]
      //]);
      return {
        ["" + Math.random()]: (Math.random() * 2) | 0,
        ["" + Math.random()]: (Math.random() * 3) | 0
      };
    }

    let array = new Array(100000);
    for (let i = 0; i < array.length; i++)
      array[i] = choose();

    let s = Date.now();
    let n = 0;
    for (let i = 0; i < array.length; i++)
    {
      let item = array[i];
      //for (let [, value] of item)
      //  n += value;
      for (let prop in item)
        n += item[prop];
    }
    console.log(Date.now() - s, n);
  })();

[hub, 2018/06/14 19:59:57]

Acknowledged.

           {
-            let subscription = Subscription.fromURL(url);
-            subscription.disabled = false;
-            subscription.title = node.getAttribute("title");
-            subscription.homepage = node.getAttribute("homepage");
-            subscriptions.push(subscription);
+            let node = subs[name];
+            if (!node)
+              continue;
+
+            let url = node.getAttribute("url");
+            if (url)
+            {
+              let subscription = Subscription.fromURL(url);
+              subscription.disabled = false;
+              subscription.title = node.getAttribute("title");
+              subscription.homepage = node.getAttribute("homepage");
+              subscription.type = node.getAttribute("type");
+              subscriptions.push(subscription);
+            }
           }
         }
 
         return subscriptions;
       });
   }
 
   return subscriptions;
@@ -301,8 +322,11 @@
  * that will effectively be added.
  *
  * @param {function} callback
  */
 exports.setSubscriptionsCallback = callback =>
 {
   subscriptionsCallback = callback;
 };
+
+// Exports for tests only
+exports.chooseFilterSubscriptions = chooseFilterSubscriptions;