Issue 3222 - Split up requestNotifier module into a parent and child part

lib/requestNotifier.js

Index: lib/requestNotifier.js
===================================================================
--- a/lib/requestNotifier.js
+++ b/lib/requestNotifier.js
@@ -14,67 +14,82 @@
  * You should have received a copy of the GNU General Public License
  * along with Adblock Plus.  If not, see <http://www.gnu.org/licenses/>.
  */
 
 /**
  * @fileOverview Stores Adblock Plus data to be attached to a window.
  */
 
-let {Utils} = require("utils");
-
-let nodeData = new WeakMap();
-let windowStats = new WeakMap();
 let windowSelection = new WeakMap();
 let requestNotifierMaxId = 0;
-let requestEntryMaxId = 0;
 
 /**
  * Active RequestNotifier instances by their ID
  * @type Map
  */
 let notifiers = new Map();
 
+let messageManager = Cc["@mozilla.org/parentprocessmessagemanager;1"]
+                       .getService(Ci.nsIMessageListenerManager)
+                       .QueryInterface(Ci.nsIMessageBroadcaster);
+messageManager.addMessageListener("AdblockPlus:FoundNodeData", onNodeData);
+messageManager.addMessageListener("AdblockPlus:ScanComplete", onScanComplete);
+
+onShutdown.add(() => {
+  messageManager.removeMessageListener("AdblockPlus:FoundNodeData", onNodeData);
+  messageManager.removeMessageListener("AdblockPlus:ScanComplete", onScanComplete);
+});
+
+function onNodeData(message)
+{
+  let {notifierID, data} = message.data;
+  let notifier = notifiers.get(notifierID);
+  if (notifier)
+    notifier.notifyListener(data);
+}
+
+function onScanComplete(message)
+{
+  let notifier = notifiers.get(message.data);
+  if (notifier)
+    notifier.onComplete();
+}
+
 /**
  * Creates a notifier object for a particular window. After creation the window
  * will first be scanned for previously saved requests. Once that scan is
  * complete only new requests for this window will be reported.
- * @param {Window} wnd  window to attach the notifier to
+ * @param {Integer} outerWindowID  ID of the window to attach the notifier to
  * @param {Function} listener  listener to be called whenever a new request is found
  * @param {Object} [listenerObj]  "this" pointer to be used when calling the listener
  */
-function RequestNotifier(wnd, listener, listenerObj)
+function RequestNotifier(outerWindowID, listener, listenerObj)
 {
-  this.window = wnd;
   this.listener = listener;
   this.listenerObj = listenerObj || null;
   this.id = ++requestNotifierMaxId;
   notifiers.set(this.id, this);
-  if (wnd)
-    this.startScan(wnd);
-  else
-    this.scanComplete = true;
+
+  messageManager.broadcastAsyncMessage("AdblockPlus:StartWindowScan", {
+    notifierID: this.id,
+    outerWindowID: outerWindowID
+  });
 }
 exports.RequestNotifier = RequestNotifier;
 
 RequestNotifier.prototype =
 {
   /**
    * The unique ID of this notifier.
    * @type Integer
    */
   id: null,
 
   /**
-   * The window this notifier is associated with.
-   * @type Window
-   */
-  window: null,
-
-  /**
    * The listener to be called when a new request is found.
    * @type Function
    */
   listener: null,
 
   /**
    * "this" pointer to be used when calling the listener.
    * @type Object
@@ -88,161 +103,49 @@ RequestNotifier.prototype =
   scanComplete: false,
 
   /**
    * Shuts down the notifier once it is no longer used. The listener
    * will no longer be called after that.
    */
   shutdown: function()
   {
-    delete this.window;
-    delete this.listener;
-    delete this.listenerObj;
-
     notifiers.delete(this.id);
+    messageManager.broadcastAsyncMessage("AdblockPlus:ShutdownNotifier", this.id);
   },
 
   /**
    * Notifies listener about a new request.
-   * @param {Window} wnd
-   * @param {Node} node
    * @param {Object} entry
    */
-  notifyListener: function(wnd, node, entry)
+  notifyListener: function(entry)
   {
-    this.listener.call(this.listenerObj, wnd, node, entry, this.scanComplete);

[Thomas Greiner, 2015/11/12 12:24:06]

From what I see, this will break stuff (namely
chrome/content/ui/{sendReport,sidebar}.js) because the listeners are expecting
those parameters to be present. Is that something that's already planned to get
fixed in other reviews?

Or are our own pages not even getting notified anymore? Because I also noticed
that the change in the constructor signature has also not been considered in
those files.

[Wladimir Palant, 2015/11/12 12:40:57]

Of course that will break stuff - we change the API here, not merely the
listeners but also RequestNotifier constructor parameters. There are follow-up
issues for all users of this API, all of them under review. All of that stuff is
broken by the changes in https://codereview.adblockplus.org/29329571/ already,
which is why these changes are supposed to land on a branch until everything is
working again.

Note that the wnd parameter isn't actually used anywhere, so removing this one
isn't an issue. Designing a proper replacement for the node parameter on the
other hand was rather tricky.

+    this.listener.call(this.listenerObj, entry, this.scanComplete);
   },
 
-  /**
-   * Number of currently posted scan events (will be 0 when the scan finishes
-   * running).
-   */
-  eventsPosted: 0,
-
-  /**
-   * Starts the initial scan of the window (will recurse into frames).
-   * @param {Window} wnd  the window to be scanned
-   */
-  startScan: function(wnd)
+  onComplete: function()
   {
-    let doc = wnd.document;
-    let walker = doc.createTreeWalker(doc, Ci.nsIDOMNodeFilter.SHOW_ELEMENT, null, false);
-
-    let process = function()
-    {
-      if (!this.listener)
-        return;
-
-      let node = walker.currentNode;
-      let data = nodeData.get(node);
-      if (typeof data != "undefined")
-        for (let k in data)
-          this.notifyListener(wnd, node, data[k]);
-
-      if (walker.nextNode())
-        Utils.runAsync(process);
-      else
-      {
-        // Done with the current window, start the scan for its frames
-        for (let i = 0; i < wnd.frames.length; i++)
-          this.startScan(wnd.frames[i]);
-
-        this.eventsPosted--;
-        if (!this.eventsPosted)
-        {
-          this.scanComplete = true;
-          this.notifyListener(wnd, null, null);
-        }
-      }
-    }.bind(this);
-
-    // Process each node in a separate event to allow other events to process
-    this.eventsPosted++;
-    Utils.runAsync(process);
+    this.scanComplete = true;
+    this.notifyListener(null);
   }
 };
 
 RequestNotifier.storeSelection = function(/**Window*/ wnd, /**String*/ selection)
 {
   windowSelection.set(wnd.document, selection);
 };
 RequestNotifier.getSelection = function(/**Window*/ wnd) /**String*/
 {
   if (windowSelection.has(wnd.document))
     return windowSelection.get(wnd.document);
   else
     return null;
 };
 
 /**
- * Attaches request data to a DOM node.
- * @param {Node} node   node to attach data to
- * @param {Window} topWnd   top-level window the node belongs to
- * @param {String} contentType   request type, e.g. "IMAGE"
- * @param {String} docDomain  domain of the document that initiated the request
- * @param {Boolean} thirdParty  will be true if a third-party server has been requested
- * @param {String} location   the address that has been requested
- * @param {Filter} filter   filter applied to the request or null if none
- */
-RequestNotifier.addNodeData = function(/**Node*/ node, /**Window*/ topWnd, /**String*/ contentType, /**String*/ docDomain, /**Boolean*/ thirdParty, /**String*/ location, /**Filter*/ filter)
-{
-  let entry = {
-    id: ++requestEntryMaxId,
-    type: contentType,
-    docDomain, thirdParty, location, filter
-  }
-
-  let existingData = nodeData.get(node);
-  if (typeof existingData == "undefined")
-  {
-    existingData = {};
-    nodeData.set(node, existingData);
-  }
-
-  // Add this request to the node data
-  existingData[contentType + " " + location] = entry;
-
-  // Update window statistics
-  if (!windowStats.has(topWnd.document))
-  {
-    windowStats.set(topWnd.document, {
-      items: 0,
-      hidden: 0,
-      blocked: 0,
-      whitelisted: 0,
-      filters: {}
-    });
-  }
-
-  let stats = windowStats.get(topWnd.document);
-  let filterType = (filter ? filter.type : null);
-  if (filterType != "elemhide" && filterType != "elemhideexception" && filterType != "cssproperty")
-    stats.items++;
-  if (filter)
-  {
-    if (filterType == "blocking")
-      stats.blocked++;
-    else if (filterType == "whitelist" || filterType == "elemhideexception")
-      stats.whitelisted++;
-    else if (filterType == "elemhide" || filterType == "cssproperty")
-      stats.hidden++;
-
-    if (filter.text in stats.filters)
-      stats.filters[filter.text]++;
-    else
-      stats.filters[filter.text] = 1;
-  }
-
-  // Notify listeners
-  for (let notifier of notifiers.values())
-    if (!notifier.window || notifier.window == topWnd)
-      notifier.notifyListener(topWnd, node, entry);
-}
-
-/**
  * Retrieves the statistics for a window.
  * @result {Object} Object with the properties items, blocked, whitelisted, hidden, filters containing statistics for the window (might be null)
  */
 RequestNotifier.getWindowStatistics = function(/**Window*/ wnd)
 {
   if (windowStats.has(wnd.document))
     return windowStats.get(wnd.document);
   else