Issue 5438 - Observer DOM changes to reapply filters.

test/browser/elemHideEmulation.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
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  * GNU General Public License for more details.
  *
  * You should have received a copy of the GNU General Public License
  * along with Adblock Plus.  If not, see <http://www.gnu.org/licenses/>.
  */
 
 "use strict";
 
 const {ElemHideEmulation} = require("../../lib/content/elemHideEmulation");
 
+const REFRESH_INTERVAL = 200;
+
 let testDocument = null;
 
 exports.setUp = function(callback)
 {
-  testDocument = null;

[Wladimir Palant, 2017/08/23 13:23:31]

This assignment seems pointless.

[hub, 2017/08/23 16:38:00]

Done.

   let iframe = document.createElement("iframe");
-  iframe.id = "test-iframe";
   document.body.appendChild(iframe);
   testDocument = iframe.contentDocument;
 
   callback();
 };
 
 exports.tearDown = function(callback)
 {
-  let iframe = document.getElementById("test-iframe");

[Wladimir Palant, 2017/08/23 13:23:32]

Use `testDocument.defaultView.frameElement` here? Then you won't need the frame
id either.

[hub, 2017/08/23 16:38:00]

Done.

+  let iframe = testDocument.defaultView.frameElement;
   iframe.parentNode.removeChild(iframe);

[Wladimir Palant, 2017/08/23 13:23:31]

Please null out testDocument as well, so that trying to use it after this point
throws.

[hub, 2017/08/23 16:38:00]

Done.

+  testDocument = null;
 
   callback();
 };
+
+function timeout(delay)
+{
+  return new Promise((resolve, reject) =>
+  {
+    window.setTimeout(resolve, delay);
+  });
+}
 
 function unexpectedError(error)
 {
   console.error(error);
   this.ok(false, "Unexpected error: " + error);
 }
 
 function expectHidden(test, element)
 {
   test.equal(window.getComputedStyle(element).display, "none",
@@ skipping 65 matching lines @@
         let selector = newSelectors.join(", ");
         insertStyleRule(selector + "{display: none !important;}");
       },
       elems =>
       {
         for (let elem of elems)
           elem.style.display = "none";
       }
     );
 
+    elemHideEmulation.MIN_INVOCATION_INTERVAL = REFRESH_INTERVAL / 2;
     elemHideEmulation.apply();
     return elemHideEmulation;
   });
 }
 
 exports.testVerbatimPropertySelector = function(test)
 {
   let toHide = createElementWithStyle("{background-color: #000}");
   applyElemHideEmulation(
     [":-abp-properties(background-color: rgb(0, 0, 0))"]
@@ skipping 103 matching lines @@
 
 exports.testDynamicallyChangedProperty = function(test)
 {
   let toHide = createElementWithStyle("{}");
   applyElemHideEmulation(
     [":-abp-properties(background-color: rgb(0, 0, 0))"]
   ).then(() =>
   {
     expectVisible(test, toHide);
     insertStyleRule("#" + toHide.id + " {background-color: #000}");
-    return new Promise((resolve, reject) =>
-    {
-      // Re-evaluation will only happen after a few seconds
-      expectVisible(test, toHide);
-      window.setTimeout(() =>
-      {
-        expectHidden(test, toHide);
-        resolve();
-      }, 4000);
-    });
+
+    return timeout(0);
+  }).then(() =>
+  {
+    // Re-evaluation will only happen after a delay
+    expectVisible(test, toHide);
+    return timeout(REFRESH_INTERVAL);
+  }).then(() =>
+  {
+    expectHidden(test, toHide);
   }).catch(unexpectedError.bind(test)).then(() => test.done());
 };
 
 exports.testPseudoClassWithPropBeforeSelector = function(test)
 {
   let parent = createElementWithStyle("{}");
   let child = createElementWithStyle("{background-color: #000}", parent);
   insertStyleRule(`#${child.id}::before {content: "publicite"}`);
 
   applyElemHideEmulation(
@@ skipping 228 matching lines @@
   let parent = createElementWithStyle("{}");
   let child = createElementWithStyle("{}", parent);
   applyElemHideEmulation(
     ["div:-abp-has(:-abp-properties(background-color: rgb(0, 0, 0)))"]
   ).then(() =>
   {
     expectVisible(test, child);
     expectVisible(test, parent);
 
     insertStyleRule("body #" + parent.id + " > div { background-color: #000}");
-
-    return new Promise((resolve, reject) =>
-    {
-      expectVisible(test, child);
-      expectVisible(test, parent);
-      window.setTimeout(() =>
-      {
-        expectVisible(test, child);
-        expectHidden(test, parent);
-        resolve();
-      }, 4000);
-    });

[Wladimir Palant, 2017/08/23 13:23:32]

Please use a helper to promisify timeouts:

  function timeout(delay)
  {
    return new Promise((resolve, reject) =>
    {
      window.setTimeout(resolve, delay);
    });
  }

Then you can write this and similar passages in a much clearer fashion:

  .then(() =>
  {
    ...
    return timeout(0);
  }).then(() =>
  {
    ...
    return timeout(4000);
  }).then(() =>
  {
    ...
  })

[hub, 2017/08/23 16:38:00]

Done.

+    return timeout(0);
+  }).then(() =>
+  {
+    expectVisible(test, child);
+    expectVisible(test, parent);
+    return timeout(REFRESH_INTERVAL);
+  }).then(() =>
+  {
+    expectVisible(test, child);
+    expectHidden(test, parent);
   }).catch(unexpectedError.bind(test)).then(() => test.done());
 };
 
 exports.testDomUpdatesContent = function(test)
 {
   let parent = createElementWithStyle("{}");
   let child = createElementWithStyle("{}", parent);
   applyElemHideEmulation(
     ["div > div:-abp-contains(hide me)"]
   ).then(() =>
   {
     expectVisible(test, parent);
     expectVisible(test, child);
 
-    child.innerText = "hide me";

[Wladimir Palant, 2017/08/23 13:23:32]

Please use child.textContent, innerText is non-standard.

[hub, 2017/08/23 16:38:00]

Done.

-    return new Promise((resolve, reject) =>
-    {
-      expectVisible(test, parent);
-      expectVisible(test, child);
-      window.setTimeout(() =>
-      {
-        expectVisible(test, parent);
-        expectHidden(test, child);
-        resolve();
-      }, 4000);
-    });
+    child.textContent = "hide me";
+    return timeout(0);
+  }).then(() =>
+  {
+    expectVisible(test, parent);
+    expectVisible(test, child);
+    return timeout(REFRESH_INTERVAL);
+  }).then(() =>
+  {
+    expectVisible(test, parent);
+    expectHidden(test, child);
   }).catch(unexpectedError.bind(test)).then(() => test.done());
 };
 
 exports.testDomUpdatesNewElement = function(test)
 {
   let parent = createElementWithStyle("{}");
   let child = createElementWithStyle("{ background-color: #000}", parent);
+  let sibling;
+  let child2;
   applyElemHideEmulation(
     ["div:-abp-has(:-abp-properties(background-color: rgb(0, 0, 0)))"]
   ).then(() =>
   {
     expectHidden(test, parent);
     expectVisible(test, child);
 
-    let sibling = createElementWithStyle("{}");
+    sibling = createElementWithStyle("{}");
+    return timeout(0);
+  }).then(() =>
+  {
+    expectHidden(test, parent);
+    expectVisible(test, child);
     expectVisible(test, sibling);
 
-    return new Promise((resolve, reject) =>
-    {
-      expectHidden(test, parent);
-      expectVisible(test, child);
-      expectVisible(test, sibling);
-      window.setTimeout(() =>
-      {
-        expectHidden(test, parent);
-        expectVisible(test, child);
-        expectVisible(test, sibling);
-
-        let child2 = createElementWithStyle("{ background-color: #000}",
-                                            sibling);
-        expectVisible(test, child2);
-        window.setTimeout(() =>
-        {
-          expectHidden(test, parent);
-          expectVisible(test, child);
-          expectHidden(test, sibling);
-          expectVisible(test, child2);
-
-          resolve();
-        }, 4000);
-      }, 4000);

[Wladimir Palant, 2017/08/23 13:23:32]

The huge delays here are an issue. We need to change MIN_INVOCATION_INTERVAL for
unit tests to a much lower value, like 100. Not sure what the best approach for
this would be, maybe string-replace-webpack-plugin?

[hub, 2017/08/23 16:38:00]

IMHO the best approach would be to have it configurable in elemHideEmulation.
This would be clean, and we could also have the possibility to make that a
preference for a reason or the other.

Done.

-    });
-  }).catch(unexpectedError.bind(test)).then(() => test.done());
-};
+    return timeout(REFRESH_INTERVAL);
+  }).then(() =>
+  {
+    expectHidden(test, parent);
+    expectVisible(test, child);
+    expectVisible(test, sibling);
+
+    child2 = createElementWithStyle("{ background-color: #000}",
+                                    sibling);
+    return timeout(0);
+  }).then(() =>
+  {
+    expectVisible(test, child2);
+    return timeout(REFRESH_INTERVAL);
+  }).then(() =>
+  {
+    expectHidden(test, parent);
+    expectVisible(test, child);
+    expectHidden(test, sibling);
+    expectVisible(test, child2);
+  }).catch(unexpectedError.bind(test)).then(() => test.done());
+};