Issue 3224 - Unbreak filter assistant

lib/child/contentPolicy.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-2015 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 40 matching lines @@
 /**
  * Maps numerical content type IDs to strings.
  * @type Map.<number,string>
  */
 let types = new Map();
 
 /**
  * Contains nodes stored by storeNodes() mapped by their IDs.
  * @type Map.<string,DOMNode[]>
  */
 let storedNodes = new Map();

[tschuster, 2015/12/01 16:42:40]

So, this is cleaned up by DeleteNodes, which is called when you close the UI,
right? Shouldn't we clean this up when the content navigates in the child? (I
assume we can't leak because of the DeadObjectProxy, but still something to
think about)

[Wladimir Palant, 2015/12/01 19:15:31]

Nodes can be removed without navigating away (yes, we used to have memory leaks
on websites that did this). So cleaning up when the UI is closed isn't optional.
Doing the same when the child navigates away can be done as well - but it would
add complexity and the gain is rather questionable (typically, the page lives
longer than our composer UI).

 
 /**
  * Process-dependent prefix to be used for unique nodes identifiers returned
  * by storeNodes().
  * @type string
  */
 let nodesIDPrefix = Services.appinfo.processID + " ";
 
 /**
  * Counter used to generate unique nodes identifiers in storeNodes().
  * @type number
  */
 let maxNodesID = 0;
 
 addMessageListener("AdblockPlus:DeleteNodes", onDeleteNodes);
 addMessageListener("AdblockPlus:RefilterNodes", onRefilterNodes);
 
 onShutdown.add(() => {
   removeMessageListener("AdblockPlus:DeleteNodes", onDeleteNodes);
   removeMessageListener("AdblockPlus:RefilterNodes", onRefilterNodes);
 });
 
 /**
+ * Processes parent's response to the ShouldAllow message.
+ * @param {nsIDOMWindow} window window that the request is associated with
+ * @param {nsIDOMElement} node  DOM element that the request is associated with
+ * @param {Object|undefined} response  object received as response
+ * @return {Boolean} false if the request should be blocked
+ */
+function processPolicyResponse(window, node, response)
+{
+  if (typeof response == "undefined")
+    return true;
+
+  let {allow, collapse, hits} = response;
+  let isObject = false;
+  for (let hit of hits)
+  {
+    if (hit.contentType == "OBJECT")
+      isObject = true;
+
+    let context = node;
+    if (typeof hit.frameIndex == "number")
+    {
+      context = window;
+      for (let i = 0; i < hit.frameIndex; i++)
+        context = context.parent;
+      context = context.document;
+    }
+    RequestNotifier.addNodeData(context, window.top, hit);
+  }
+
+  if (node.nodeType == Ci.nsIDOMNode.ELEMENT_NODE)
+  {
+    // Track mouse events for objects
+    if (allow && isObject)
+    {
+      node.addEventListener("mouseover", objectMouseEventHander, true);
+      node.addEventListener("mouseout", objectMouseEventHander, true);
+    }
+
+    if (collapse)
+      schedulePostProcess(node);
+  }
+  return allow;
+}
+
+/**
  * Checks whether a request should be allowed, hides it if necessary
- * @param wnd {nsIDOMWindow}
- * @param node {nsIDOMElement}
- * @param contentType {String}
- * @param location {String} location of the request, filter key if contentType is ELEMHIDE
- * @param [callback] {Function} If present, the request will be sent
- *                              asynchronously and callback called with the
- *                              response
+ * @param {nsIDOMWindow} window
+ * @param {nsIDOMElement} node
+ * @param {String} contentType
+ * @param {String} location location of the request, filter key if contentType is ELEMHIDE
  * @return {Boolean} false if the request should be blocked
  */
-let shouldAllow = exports.shouldAllow = function(window, node, contentType, location, callback)
-{
-  function processResponse(response)
-  {
-    if (typeof response == "undefined")
-      return true;
-
-    let {allow, collapse, hits} = response;
-    for (let hit of hits)
-    {
-      let context = node;
-      if (typeof hit.frameIndex == "number")
-      {
-        context = window;
-        for (let i = 0; i < hit.frameIndex; i++)
-          context = context.parent;
-        context = context.document;
-      }
-      RequestNotifier.addNodeData(context, window.top, hit);
-    }
-
-    if (node.nodeType == Ci.nsIDOMNode.ELEMENT_NODE)
-    {
-      // Track mouse events for objects
-      if (allow && contentType == "OBJECT")
-      {
-        node.addEventListener("mouseover", objectMouseEventHander, true);
-        node.addEventListener("mouseout", objectMouseEventHander, true);
-      }
-
-      if (collapse)
-        schedulePostProcess(node);
-    }
-    return allow;
-  }
-
-  let data = {
-    contentType: contentType,
-    location: location,
+let shouldAllow = exports.shouldAllow = function(window, node, contentType, location)
+{
+  return processPolicyResponse(window, node, sendSyncMessage("AdblockPlus:ShouldAllow", {
+    contentType,
+    location,
     frames: getFrames(window),
     isPrivate: isPrivate(window)
-  };
-  if (typeof callback == "function")
-  {
-    sendAsyncMessage("AdblockPlus:ShouldAllow", data, (data) => {
-      callback(processResponse(data));
-    });
-  }
-  else
-    return processResponse(sendSyncMessage("AdblockPlus:ShouldAllow", data));
+  }));
+};
+
+/**
+ * Asynchronously checks whether a request should be allowed.
+ * @param {nsIDOMWindow} window
+ * @param {nsIDOMElement} node
+ * @param {String} contentType
+ * @param {String} location location of the request, filter key if contentType is ELEMHIDE
+ * @param {Function} callback  callback to be called with a boolean value, if
+ *                             false the request should be blocked
+ */
+let shouldAllowAsync = exports.shouldAllowAsync = function(window, node, contentType, location, callback)
+{
+  sendAsyncMessage("AdblockPlus:ShouldAllow", {
+    contentType,
+    location,
+    frames: getFrames(window),
+    isPrivate: isPrivate(window)
+  }, response => callback(processPolicyResponse(window, node, response)));
 };
 
 /**
  * Stores nodes and generates a unique ID for them that can be used for
  * Policy.refilterNodes() later. It's important that Policy.deleteNodes() is
  * called later, otherwise the nodes will be leaked.
  * @param {DOMNode[]} nodes  list of nodes to be stored
  * @return {string}  unique ID for the nodes
  */
 let storeNodes = exports.storeNodes = function(nodes)
@@ skipping 199 matching lines @@
           // Special treatment for pop-up windows - this will close the window
           // rather than preventing the redirect. Note that we might not have
           // seen the original channel yet because the redirect happened before
           // the async code in observe() had a chance to run.
           this.observe(wnd, "content-document-global-created", null, oldChannel.URI.spec);
           this.observe(wnd, "content-document-global-created", null, newChannel.URI.spec);
         }
         return;
       }
 
-      shouldAllow(wnd, wnd.document, types.get(contentType), newChannel.URI.spec, function(allow)
+      shouldAllowAsync(wnd, wnd.document, types.get(contentType), newChannel.URI.spec, function(allow)
       {
         callback.onRedirectVerifyCallback(allow ? Cr.NS_OK : Cr.NS_BINDING_ABORTED);
       });
       async = true;
     }
     catch (e)
     {
       // We shouldn't throw exceptions here - this will prevent the redirect.
       Cu.reportError(e);
     }
@@ skipping 36 matching lines @@
     Utils.runAsync(postProcessNodes);
   }
 }
 
 /**
  * Processes nodes scheduled for post-processing (typically hides them).
  */
 function postProcessNodes()
 {
   if (!collapsedClass)
-  {
     collapsedClass = sendSyncMessage("AdblockPlus:GetCollapsedClass");
-    if (!collapsedClass)
-    {
-      Utils.runAsync(postProcessNodes);
-      return;
-    }
-  }
 
   let nodes = scheduledNodes;
   scheduledNodes = null;
+
+  if (!collapsedClass)
+    return;
 
   for (let node of nodes)
   {
     // adjust frameset's cols/rows for frames
     let parentNode = node.parentNode;
     if (parentNode && parentNode instanceof Ci.nsIDOMHTMLFrameSetElement)
     {
       let hasCols = (parentNode.cols && parentNode.cols.indexOf(",") > 0);
       let hasRows = (parentNode.rows && parentNode.rows.indexOf(",") > 0);
       if ((hasCols || hasRows) && !(hasCols && hasRows))
       {
         let index = -1;
         for (let frame = node; frame; frame = frame.previousSibling)
           if (frame instanceof Ci.nsIDOMHTMLFrameElement || frame instanceof Ci.nsIDOMHTMLFrameSetElement)
             index++;
 
         let property = (hasCols ? "cols" : "rows");
         let weights = parentNode[property].split(",");
         weights[index] = "0";
         parentNode[property] = weights.join(",");
       }
     }
     else
       node.classList.add(collapsedClass);
   }
 }