Issue 3251 - Add shouldAllowAsync() function for non-urgent policy checks

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 37 matching lines @@
  */
 let collapsedClass = null;
 
 /**
  * Maps numerical content type IDs to strings.
  * @type Map.<number,string>
  */
 let types = new Map();
 
 /**
+ * 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 {frameIndex, contentType, docDomain, thirdParty, location, filter} of hits)
+  {
+    if (contentType == "OBJECT")
+      isObject = true;
+
+    let context = node;
+    if (typeof frameIndex == "number")
+    {
+      context = window;
+      for (let i = 0; i < frameIndex; i++)
+        context = context.parent;
+      context = context.document;
+    }
+    RequestNotifier.addNodeData(context, window.top, contentType, docDomain, thirdParty, location, filter);
+  }
+
+  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}
- * @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
  * @return {Boolean} false if the request should be blocked
  */
-function shouldAllow(window, node, contentType, location, callback)

[tschuster, 2015/11/12 13:38:13]

This should really be two functions. If you reuse processResponse the
duplication should be small.

[Wladimir Palant, 2015/11/12 15:01:13]

Done.

-{
-  function processResponse(response)
-  {
-    if (typeof response == "undefined")
-      return true;
-
-    let {allow, collapse, hits} = response;
-    for (let {frameIndex, contentType, docDomain, thirdParty, location, filter} of hits)
-    {
-      let context = node;
-      if (typeof frameIndex == "number")
-      {
-        context = window;
-        for (let i = 0; i < frameIndex; i++)
-          context = context.parent;
-        context = context.document;
-      }
-      RequestNotifier.addNodeData(context, window.top, contentType, docDomain, thirdParty, location, filter);
-    }
-
-    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;
-  }

[Wladimir Palant, 2015/11/04 15:05:22]

The diff is messy but all the code above has merely been moved around.

-
-  let data = {
-    contentType: contentType,
-    location: location,
+function shouldAllow(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
+ * @param {Function} callback  callback to be called with a boolean value, if
+ *                             false the request should be blocked
+ */
+function shouldAllowAsync(window, node, contentType, location, callback)
+{
+  sendAsyncMessage("AdblockPlus:ShouldAllow", {
+    contentType,
+    location,
+    frames: getFrames(window),
+    isPrivate: isPrivate(window)
+  }, response => callback(processPolicyResponse(window, node, response)));
 }
 
 /**
  * Actual nsIContentPolicy and nsIChannelEventSink implementation
  * @class
  */
 var PolicyImplementation =
 {
   classDescription: "Adblock Plus content policy",
   classID: Components.ID("cfeaabe6-1dd1-11b2-a0c6-cb5c268894c9"),
@@ skipping 132 matching lines @@
         return;
 
       let wnd = Utils.getRequestWindow(newChannel);
       if (!wnd)
         return;
 
       if (contentType == Ci.nsIContentPolicy.TYPE_DOCUMENT)
       {
         if (wnd.history.length <= 1 && wnd.opener)
         {
-          // Special treatment for pop-up windows. Note that we might not have
+          // 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)
-  {
-    let response = sendSyncMessage("AdblockPlus:GetCollapsedClass");
-    if (response.length)
-      collapsedClass = response[0];
-
-    if (!collapsedClass)
-    {
-      Utils.runAsync(postProcessNodes);
-      return;
-    }
-  }
+    collapsedClass = sendSyncMessage("AdblockPlus:GetCollapsedClass");
 
   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);
   }
 }