Issue 6955 - Avoid making copies of common selector list

test/elemHide.js

Index: test/elemHide.js
===================================================================
--- a/test/elemHide.js
+++ b/test/elemHide.js
@@ -39,17 +39,17 @@
 
   callback();
 };
 
 function normalizeSelectors(selectors)
 {
   // getSelectorsForDomain is currently allowed to return duplicate selectors
   // for performance reasons, so we need to remove duplicates here.
-  return selectors.sort().filter((selector, index, sortedSelectors) =>
+  return selectors.slice().sort().filter((selector, index, sortedSelectors) =>
   {
     return index == 0 || selector != sortedSelectors[index - 1];
   });
 }
 
 function testResult(test, domain, expectedSelectors, specificOnly)
 {
   let normalizedExpectedSelectors = normalizeSelectors(expectedSelectors);
@@ -268,8 +268,121 @@
   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)
+{
+  // 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"));
+
+  // Domain-specific filter.
+  ElemHide.add(Filter.fromText("foo.example.com##test4"));
+
+  test.notEqual(ElemHide.getSelectorsForDomain("foo.example.com"),
+                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"));
+
+  // Generic filter with domain excluded.
+  ElemHide.add(Filter.fromText("~foo.example.com##test4"));
+
+  test.notEqual(ElemHide.getSelectorsForDomain("foo.example.com"),
+                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"));
+
+  // 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"));
+
+  // 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();
+};