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

lib/child/contentPolicy.js

Index: lib/child/contentPolicy.js
===================================================================
--- a/lib/child/contentPolicy.js
+++ b/lib/child/contentPolicy.js
@@ -32,26 +32,27 @@ try
 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
+ * @type Promise.<string>
  */
-let collapsedClass = null;
+let collapsedClass = port.emitWithResponse("getCollapsedClass");
 
 /**
  * Maps numerical content type IDs to strings.
  * @type Map.<number,string>
  */
 let types = new Map();
 
 /**
@@ -68,23 +69,18 @@ let storedNodes = new Map();
 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
  */
@@ -131,17 +127,17 @@ function processPolicyResponse(window, n
  * @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)
   }));
 };
 
 /**
@@ -150,22 +146,25 @@ let shouldAllow = exports.shouldAllow = 
  * @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
@@ -175,27 +174,26 @@ let storeNodes = exports.storeNodes = fu
   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));
 }
 
 /**
@@ -453,42 +451,44 @@ function schedulePostProcess(/**Element*
   }
 }
 
 /**
  * Processes nodes scheduled for post-processing (typically hides them).
  */
 function postProcessNodes()
 {
-  if (!collapsedClass)
-    collapsedClass = sendSyncMessage("AdblockPlus:GetCollapsedClass");
+  collapsedClass.then(cls =>

[Thomas Greiner, 2016/03/23 18:47:59]

Detail: Anything wrong with calling it "collapsedClass" instead of "cls"?
Because the promise is just a different representation of that same value.

[Wladimir Palant, 2016/03/24 07:27:26]

They are not the same thing however. I would consider this highly confusing if
collapsedClass could refer to both the promise (variable outside the closure)
and actual value (parameter).

+  {
+    let nodes = scheduledNodes;
+    scheduledNodes = null;
 
-  let nodes = scheduledNodes;
-  scheduledNodes = null;
+    // Resolving class is async initially so the nodes might have already been
+    // processed in the meantime.
+    if (!nodes)
+      return;
 
-  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++;
 
-  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(",");
+          let property = (hasCols ? "cols" : "rows");
+          let weights = parentNode[property].split(",");
+          weights[index] = "0";
+          parentNode[property] = weights.join(",");
+        }
       }
+      else
+        node.classList.add(cls);
     }
-    else
-      node.classList.add(collapsedClass);
-  }
+  });
 }