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.sort().filter((selector, index, sortedSelectors) =>
+  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 208 matching lines @@
   test.equal(filtersByDomain.size, 2);
 
   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"));
+
+  // 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"));
+
+  // 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"));
+
+  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"));
+
+  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"));
+  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();
+};