Issue 3499 - Use new messaging API in the content policy implementation

lib/child/contentPolicy.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-2016 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 19 matching lines @@
     delete proto.Components;
 }
 catch (e)
 {
   Cu.reportError(e);
 }
 
 let {XPCOMUtils} = Cu.import("resource://gre/modules/XPCOMUtils.jsm", {});
 let {Services} = Cu.import("resource://gre/modules/Services.jsm", {});
 
+let {port} = require("messaging");
 let {Utils} = require("utils");
 let {getFrames, isPrivate} = require("child/utils");
 let {objectMouseEventHander} = require("child/objectTabs");
 let {RequestNotifier} = require("child/requestNotifier");
 
 /**
  * Randomly generated class name, to be applied to collapsed nodes.
  * @type string
  */
 let collapsedClass = null;
@@ skipping 16 matching lines @@
  * @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);
-});
+port.on("deleteNodes", onDeleteNodes);
+port.on("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)
 {
@@ skipping 36 matching lines @@
 /**
  * Checks whether a request should be allowed, hides it if necessary
  * @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)
 {
-  return processPolicyResponse(window, node, sendSyncMessage("AdblockPlus:ShouldAllow", {
+  return processPolicyResponse(window, node, port.emitSync("shouldAllow", {
     contentType,
     location,
     frames: getFrames(window),
     isPrivate: isPrivate(window)
   }));
 };
 
 /**
  * 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", {
+  port.emitWithResponse("shouldAllow", {
     contentType,
     location,
     frames: getFrames(window),
     isPrivate: isPrivate(window)
-  }, response => callback(processPolicyResponse(window, node, response)));
+  }).then(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)
 {
   let id = nodesIDPrefix + (++maxNodesID);
   storedNodes.set(id, nodes);
   return id;
 };
 
 /**
  * Called via message whenever Policy.deleteNodes() is called in the parent.
  */
-function onDeleteNodes(message)
+function onDeleteNodes(id, sender)
 {
-  storedNodes.delete(message.data);
+  storedNodes.delete(id);
 }
 
 /**
  * Called via message whenever Policy.refilterNodes() is called in the parent.
  */
-function onRefilterNodes(message)
+function onRefilterNodes({nodesID, entry}, sender)
 {
-  let {nodesID, entry} = message.data;
   let nodes = storedNodes.get(nodesID);
   if (nodes)
     for (let node of nodes)
       if (node.nodeType == Ci.nsIDOMNode.ELEMENT_NODE)
         Utils.runAsync(refilterNode.bind(this, node, entry));
 }
 
 /**
  * Re-checks filters on an element.
  */
@@ skipping 248 matching lines @@
     Utils.runAsync(postProcessNodes);
   }
 }
 
 /**
  * Processes nodes scheduled for post-processing (typically hides them).
  */
 function postProcessNodes()
 {
   if (!collapsedClass)
-    collapsedClass = sendSyncMessage("AdblockPlus:GetCollapsedClass");
+  {
+    port.emitWithResponse("getCollapsedClass").then(cls =>
+    {
+      // We might have sent this message multiple times, ignore response if a
+      // previous response was already processed.
+      if (collapsedClass)
+        return;
+
+      collapsedClass = cls;
+      postProcessNodes();
+    });
+    return;
+  }
 
   let nodes = scheduledNodes;
   scheduledNodes = null;

[Erik, 2016/03/15 20:57:41]

It appears that `postProcessNodes` is run twice now instead of once before
during the first execution, and won't this `scheduledNodes` value be incorrect
when `postProcessNodes` is run a second time?

Either way the code is a little confusing to me, I wonder if there is a less
confusing way to do this?

Perhaps we can generate the class when the add-on starts up and pass that to the
`lib/child/bootstrap.js` file when it is loaded? or leave this to another time
and just use a sync message here for the time being?

[Wladimir Palant, 2016/03/16 10:13:00]

No, we don't run this code multiple times (return statement above is important)
- we merely delay it until the class is available. While scheduledNodes list
might increase in the meantime, that's not an issue because all nodes will be
processed.

Passing parameters to child/bootstrap.js will break code locality - all the
sudden the involved parties are no longer contentPolicy.js and
child/contentPolicy.js but also main.js and child/bootstrap.js. Using
synchronous messaging is also not really an option, this was only synchronous
here originally because async messaging with responses was very complex.

Let's try promises, this will hopefully make this easier to follow...

 
-  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);
   }
 }