Issue 6955 - Avoid making copies of common selector list

test/elemHide.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-present 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 26 matching lines @@
     {Filter} = sandboxedRequire("../lib/filterClasses")
   );
 
   callback();
 };
 
 function normalizeSelectors(selectors)
 {
   // getSelectorsForDomain is currently allowed to return duplicate selectors
   // for performance reasons, so we need to remove duplicates here.
   return selectors.slice().sort().filter((selector, index, sortedSelectors) =>

[Manish Jethani, 2018/09/17 09:28:43]

Need to make a copy now because the return value of getSelectorsForDomain is a
read-only array.

   {
     return index == 0 || selector != sortedSelectors[index - 1];
   });
 }
 
 function testResult(test, domain, expectedSelectors, specificOnly)
 {
   let normalizedExpectedSelectors = normalizeSelectors(expectedSelectors);
 
   test.deepEqual(
@@ skipping 209 matching lines @@
 
   ElemHide.remove(Filter.fromText("example.com##test"));
   test.equal(filtersByDomain.size, 2);
 
   ElemHide.remove(Filter.fromText("example.com,~www.example.com##test"));
   test.equal(filtersByDomain.size, 0);
 
   test.done();
 };
 
 exports.testCommonSelectorsDedup = function(test)

[Manish Jethani, 2018/09/17 09:28:43]

Read this first.

 {
   // Here we compare references to verify that the selector lookup function
   // returns the same object for the common case.
 
   ElemHide.add(Filter.fromText("##test1"));
   ElemHide.add(Filter.fromText("##test2"));
   ElemHide.add(Filter.fromText("##test3"));
 
   test.equal(ElemHide.getSelectorsForDomain("foo.example.com"),
              ElemHide.getSelectorsForDomain("bar.example.com"));
@@ skipping 16 matching lines @@
                 ElemHide.getSelectorsForDomain("bar.example.com"));
 
   ElemHide.remove(Filter.fromText("~foo.example.com##test4"));
 
   test.equal(ElemHide.getSelectorsForDomain("foo.example.com"),
              ElemHide.getSelectorsForDomain("bar.example.com"));
 
   // Domain-specific exception.
   ElemHideExceptions.add(Filter.fromText("foo.example.com#@#test4"));
 
-  // Since the exception does not apply to any filters yet, the objects
-  // returned for foo.example.com and bar.example.com are one and the same.
-  test.equal(ElemHide.getSelectorsForDomain("foo.example.com"),
-             ElemHide.getSelectorsForDomain("bar.example.com"));
+  // Even though the exception does not apply to any filters yet, the objects
+  // returned for foo.example.com and bar.example.com are not one and the same.
+  test.notEqual(ElemHide.getSelectorsForDomain("foo.example.com"),
+                ElemHide.getSelectorsForDomain("bar.example.com"));
 
   ElemHide.add(Filter.fromText("##test4"));
 
   test.notEqual(ElemHide.getSelectorsForDomain("foo.example.com"),
                 ElemHide.getSelectorsForDomain("bar.example.com"));
 
   ElemHide.remove(Filter.fromText("##test4"));
   ElemHideExceptions.remove(Filter.fromText("foo.example.com#@#test4"));
 
   test.equal(ElemHide.getSelectorsForDomain("foo.example.com"),
              ElemHide.getSelectorsForDomain("bar.example.com"));
 
   // Generic exception with domain excluded.
   ElemHideExceptions.add(Filter.fromText("~foo.example.com#@#test4"));
 
-  test.equal(ElemHide.getSelectorsForDomain("foo.example.com"),
-             ElemHide.getSelectorsForDomain("bar.example.com"));
+  // Even though the exception does not apply to any filters yet, the objects
+  // returned for foo.example.com and bar.example.com are not one and the same.
+  test.notEqual(ElemHide.getSelectorsForDomain("foo.example.com"),
+                ElemHide.getSelectorsForDomain("bar.example.com"));
 
   ElemHide.add(Filter.fromText("##test4"));
 
   test.notEqual(ElemHide.getSelectorsForDomain("foo.example.com"),
                 ElemHide.getSelectorsForDomain("bar.example.com"));
 
   ElemHide.remove(Filter.fromText("##test4"));
   ElemHideExceptions.remove(Filter.fromText("~foo.example.com#@#test4"));
 
   test.equal(ElemHide.getSelectorsForDomain("foo.example.com"),
              ElemHide.getSelectorsForDomain("bar.example.com"));
 
   // Generic exception.
   ElemHide.add(Filter.fromText("##test4"));
   ElemHideExceptions.add(Filter.fromText("#@#test4"));
 
   test.equal(ElemHide.getSelectorsForDomain("foo.example.com"),
              ElemHide.getSelectorsForDomain("bar.example.com"));
 
   ElemHideExceptions.add(Filter.fromText("example.com#@#test4"));
 
   // In this case, even though the exception applies to both domains, we get
   // different objects.
   test.notEqual(ElemHide.getSelectorsForDomain("foo.example.com"),
                 ElemHide.getSelectorsForDomain("bar.example.com"));
   test.deepEqual(ElemHide.getSelectorsForDomain("foo.example.com"),
                  ElemHide.getSelectorsForDomain("bar.example.com"));
 
   ElemHideExceptions.remove(Filter.fromText("example.com#@#test4"));
+
+  // Generic exception with unrelated domain excluded.

[Manish Jethani, 2018/09/17 12:21:15]

This particular test would not pass with Patch Set 2, and this reflects the
situation in EasyList+AA (unrelated domains are excluded from generic filters).

To fix this Patch Set 3 keeps count of the number of exceptions related to a
domain and uses that to decide whether a domain falls in the common case or not.

+  ElemHideExceptions.add(Filter.fromText("~x4aabd7c0ddcd.com#@#test4"));
+
+  test.equal(ElemHide.getSelectorsForDomain("foo.example.com"),
+             ElemHide.getSelectorsForDomain("bar.example.com"));
+
+  ElemHideExceptions.remove(Filter.fromText("~x4aabd7c0ddcd.com#@#test4"));
   ElemHideExceptions.remove(Filter.fromText("#@#test4"));
   ElemHide.remove(Filter.fromText("##test4"));
 
   test.equal(ElemHide.getSelectorsForDomain("foo.example.com"),
              ElemHide.getSelectorsForDomain("bar.example.com"));
 
   ElemHide.remove(Filter.fromText("##test1"));
   ElemHide.remove(Filter.fromText("##test2"));
   ElemHide.remove(Filter.fromText("##test3"));
 
   test.equal(ElemHide.getSelectorsForDomain("foo.example.com"),
              ElemHide.getSelectorsForDomain("bar.example.com"));
 
   test.done();
 };