Issue 3792 - Fix to support multiprocess firefox

lib/crawler.js

 /*
  * This Source Code is subject to the terms of the Mozilla Public License
  * version 2.0 (the "License"). You can obtain a copy of the License at
  * http://mozilla.org/MPL/2.0/.
  */
 
+"use strict";
+
 /**
  * @module crawler
  */
 
-Cu.import("resource://gre/modules/Services.jsm");
-Cu.import("resource://gre/modules/Task.jsm");
-Cu.import("resource://gre/modules/Promise.jsm");
-Cu.import("resource://gre/modules/Timer.jsm");
+const {Services} = Cu.import("resource://gre/modules/Services.jsm", {});
+const {XPCOMUtils} = Cu.import("resource://gre/modules/XPCOMUtils.jsm", {});
+const {Task} = Cu.import("resource://gre/modules/Task.jsm", {});
+const {setTimeout, clearTimeout} = Cu.import("resource://gre/modules/Timer.jsm", {});
 
 function abprequire(module)
 {
   let result = {};
   result.wrappedJSObject = result;
   Services.obs.notifyObservers(result, "adblockplus-require", module);
   return result.exports;
 }
 
 let {RequestNotifier} = abprequire("requestNotifier");
 let {FilterNotifier} = abprequire("filterNotifier");
 let {FilterStorage} = abprequire("filterStorage");
 
 /**
- * Creates a pool of tabs and allocates them to tasks on request.
+ * Allocates tabs on request but not more than maxtabs at the same time.
  *
  * @param {tabbrowser} browser
  *    The tabbed browser where tabs should be created
  * @param {int} maxtabs
  *    The maximum number of tabs to be allocated
  * @constructor
  */
 function TabAllocator(browser, maxtabs)
 {
-  browser.removeAllTabsBut(browser.tabs[0])
-
-  this._tabs = [];
-  for (let i = 0; i < maxtabs; i++)
-    this._tabs.push(browser.addTab("about:blank"));
-
-  browser.removeTab(browser.tabs[0]);
-
-  this._deferred = [];
+  this._browser = browser;
+  this._tabs = 0;
+  this._maxtabs = maxtabs;
+  // The queue containing resolve functions of promises waiting for a tab.
+  this._resolvers = [];
+  // Keep at least one tab alive to prevent browser from closing itself.
+  this._tabKeepingWindowAlive = this._browser.tabs[0];
+  this._browser.removeAllTabsBut(this._tabKeepingWindowAlive);
 }
 TabAllocator.prototype = {
+  _removeTabKeepingWindowAlive: function()
+  {
+    if (!this._tabKeepingWindowAlive)
+      return;
+    this._browser.removeTab(this._tabKeepingWindowAlive);
+    delete this._tabKeepingWindowAlive;
+  },
+
   /**
-   * Returns a promise that will resolve into a tab once a tab can be allocated.
+   * Creates a blank tab in this._browser.
+   *
+   * @return {Promise.<tab>} promise which resolves once the tab is fully initialized.
+   */
+  _createTab: function()
+  {
+    this._tabs++;
+    let tab = this._browser.addTab("about:blank");
+    if (tab.linkedBrowser.outerWindowID)
+    {
+      this._removeTabKeepingWindowAlive();
+      return Promise.resolve(tab);
+    }
+    return new Promise((resolve, reject) =>
+    {
+      let onBrowserInit = (msg) =>
+      {
+        tab.linkedBrowser.messageManager.removeMessageListener("Browser:Init", onBrowserInit);
+        this._removeTabKeepingWindowAlive();
+        resolve(tab);
+      };
+      // "Browser:Init" message is sent once the browser is ready, see
+      // https://bugzil.la/1256602#c1
+      tab.linkedBrowser.messageManager.addMessageListener("Browser:Init", onBrowserInit);
+    });
+  },
+
+  /**
+   * Returns a promise that will resolve into a tab once a tab is allocated.
    * The tab cannot be used by other tasks until releaseTab() is called.
    *
-   * @result {Promise}
+   * @result {Promise.<tab>}
    */
   getTab: function()
   {
-    if (this._tabs.length)
-      return this._tabs.shift();
-    else
-    {
-      let deferred = Promise.defer();
-      this._deferred.push(deferred);
-      return deferred.promise;
-    }
+    if (this._tabs < this._maxtabs)
+      return this._createTab();
+    return new Promise((resolve, reject) => this._resolvers.push(resolve));
   },
 
   /**
    * Adds a tab back to the pool so that it can be used by other tasks.
    *
    * @param {tab} tab
    */
   releaseTab: function(tab)
   {
-    let browser = tab.parentNode.tabbrowser;
-    browser.removeTab(tab);
-    tab = browser.addTab("about:blank");
-
-    if (this._deferred.length)
-      this._deferred.shift().resolve(tab);
+    // If we are about to close last tab don't close it immediately to keep
+    // the window alive. It will be closed when a new tab is created.
+    if (this._tabs > 1)
+      this._browser.removeTab(tab);
     else
-      this._tabs.push(tab);
-  }
+    {
+      // navigate away from previously opened URL
+      tab.linkedBrowser.loadURI("about:blank", null, null);
+      this._tabKeepingWindowAlive = tab;
+    }
+
+    this._tabs--;
+    if (this._resolvers.length && this._tabs < this._maxtabs)
+    {
+      this._resolvers.shift()(this._createTab());
+    }
+  },
 };
 
 /**
  * Once created, this object will make sure all new windows are dismissed
  * immediately.
  *
  * @constructor
  */
 function WindowCloser()
 {
@@ skipping 16 matching lines @@
     {
       if (window.document.documentElement.localName == 'dialog')
         window.document.documentElement.acceptDialog();
       else
         window.close();
     }, false);
   },
 
   QueryInterface: XPCOMUtils.generateQI([Ci.nsIObserver, Ci.nsISupportsWeakReference])
 };
+
+function configureFrameScript()
+{
+  const info = require("info");
+  let frameScriptPath = info.addonRoot + "/lib/child/frameScript.js";
+  Services.mm.loadFrameScript(frameScriptPath, true);
+
+  onShutdown.add(() =>
+  {
+    Services.mm.removeDelayedFrameScript(frameScriptPath);
+  });
+}
 
 /**
  * Starts the crawling session. The crawler opens each URL in a tab and stores
  * the results.
  *
  * @param {Window} window
  *    The browser window we're operating in
  * @param {String[]} urls
  *    URLs to be crawled
- * @param {int} number_of_tabs
+ * @param {int} timeout
+ *    Load timeout in milliseconds
+ * @param {int} maxtabs
  *    Maximum number of tabs to be opened
  * @param {String} targetURL
  *    URL that should receive the results
+ * @param {Function} onDone
+ *    The callback which is called after finishing of crawling of all URLs.
  */
 function run(window, urls, timeout, maxtabs, targetURL, onDone)
+{
+  configureFrameScript();
+  new Promise((resolve, reject) =>
+  {
+    if (FilterStorage.subscriptions.length > 0)
+    {
+      resolve();
+      return;
+    }
+    let onFiltersLoaded = (action, item, newValue, oldValue) =>
+    {
+      if (action == "load")
+      {
+        FilterNotifier.removeListener(onFiltersLoaded);
+        resolve();
+      }
+    };
+    FilterNotifier.addListener(onFiltersLoaded);
+  }).then(() => crawl_urls(window, urls, timeout, maxtabs, targetURL, onDone))
+  .catch(reportException);
+}
+exports.run = run;
+
+/**
+ * Spawns a {Task} task to crawl each url from urls argument and calls
+ * onDone when all tasks are finished.
+ * @param {Window} window
+ *   The browser window we're operating in
+ * @param {String[]} urls
+ *   URLs to be crawled
+ * @param {int} timeout
+ *    Load timeout in milliseconds
+ * @param {int} maxtabs
+ *    Maximum number of tabs to be opened
+ * @param {String} targetURL
+ *    URL that should receive the results
+ * @param {Function} onDone
+ *    The callback which is called after finishing of all tasks.
+ */
+function crawl_urls(window, urls, timeout, maxtabs, targetURL, onDone)
 {
   let tabAllocator = new TabAllocator(window.getBrowser(), maxtabs);
 
   let running = 0;
   let windowCloser = new WindowCloser();
   let taskDone = function()
   {
     running--;
     if (running <= 0)
     {
       windowCloser.stop();
       onDone();
     }
   };
 
-  new Promise(function(resolve, reject)
-  {
-    if (FilterStorage.subscriptions.length > 0 && !FilterStorage._loading)
-    {
-      resolve();
-      return;
-    }
-    FilterNotifier.addListener((action, item, newValue, oldValue) =>
-    {
-      if (action === "load")
-      {
-        resolve();
-      }
-    });
-  }).then(_ =>
-  {
-    for (let url of urls)
-    {
-      running++;
-      Task.spawn(crawl_url.bind(null, url, tabAllocator, timeout)).then(function(result)
-      {
-        let request = new XMLHttpRequest();
-        request.open("POST", targetURL);
-        request.addEventListener("load", taskDone, false);
-        request.addEventListener("error", taskDone, false);
-        request.send(JSON.stringify(result));
-      }, function(url, exception)
-      {
-        reportException(exception);
-
-        let request = new XMLHttpRequest();
-        request.open("POST", targetURL);
-        request.addEventListener("load", taskDone, false);
-        request.addEventListener("error", taskDone, false);
-        request.send(JSON.stringify({
-          url: url,
-          startTime: Date.now(),
-          error: String(exception)
-        }));
-      }.bind(null, url));
-    }
-  // Be careful, `catch` does not catch exeptions from this `then` handler because
-  // the latter one does not return an array of promises of asynchrounous tasks
-  // and does not contain any waiting code.
-  }).catch(reportException);
-}
-exports.run = run;
+  for (let url of urls)
+  {
+    running++;
+    Task.spawn(crawl_url.bind(null, url, tabAllocator, timeout)).then(function(result)
+    {
+      let request = new XMLHttpRequest();
+      request.open("POST", targetURL);
+      request.addEventListener("load", taskDone, false);
+      request.addEventListener("error", taskDone, false);
+      request.send(JSON.stringify(result));
+    }, function(url, exception)
+    {
+      reportException(exception);
+
+      let request = new XMLHttpRequest();
+      request.open("POST", targetURL);
+      request.addEventListener("load", taskDone, false);
+      request.addEventListener("error", taskDone, false);
+      request.send(JSON.stringify({
+        url: url,
+        startTime: Date.now(),
+        error: String(exception)
+      }));
+    }.bind(null, url));
+  }
+}
+
+/**
+ * Expects to receive page info gathered in a content process for the specified
+ * `tab`. If there is no relevant message within specified `timeout` then
+ * the result promise is resolved with error object.
+ * @param tab
+ *    Tab in which we are interested in
+ * @param {int} timeout
+ *    Timeout in milliseconds
+ * @return {Promise} promise which will be resolved with the received page info
+ */
+function getPageInfo(tab, timeout)
+{
+  return new Promise((resolve, result) =>
+  {
+    let mm = tab.linkedBrowser.messageManager;
+    let timerID;
+    let onDone = (msg) =>
+    {
+      mm.removeMessageListener("abpcrawler:pageInfoGathered", onDone);
+      clearTimeout(timerID);
+      resolve(msg.data);
+    }
+    mm.addMessageListener("abpcrawler:pageInfoGathered", onDone);
+    timerID = setTimeout(() => onDone({data: {error: "timeout"}}), timeout);
+  });
+}
 
 /**
  * Crawls a URL. This is a generator meant to be used via a Task object.
  *
  * @param {String} url
  * @param {TabAllocator} tabAllocator
+ * @param {int} timeout
+ *    Load timeout in milliseconds
  * @result {Object}
  *    Crawling result
  */
 function* crawl_url(url, tabAllocator, timeout)
 {
   let tab = yield tabAllocator.getTab();
   let result = {url, requests: []};
-
+  let requestNotifier;
   try
   {
     result.startTime = Date.now();
-    let requestNotifier = new RequestNotifier(tab.linkedBrowser.outerWindowID, function({type, location, filter}, scanComplete)

[sergei, 2016/03/15 16:40:10]

BTW, in addition, this part stops to work, I have filed an issue about it
https://bugzilla.mozilla.org/show_bug.cgi?id=1256602

[sergei, 2016/03/16 14:44:23]

https://issues.adblockplus.org/ticket/3815

-    {
-      result.requests.push({location, contentType: type, filter});
+    requestNotifier = new RequestNotifier(tab.linkedBrowser.outerWindowID,
+      function(entry, scanComplete)
+    {
+      if (!entry)
+        return;
+      let {type: contentType, location, filter} = entry;
+      result.requests.push({location, contentType, filter});
     });
 
     tab.linkedBrowser.loadURI(url, null, null);
 
-    let mm = tab.linkedBrowser.messageManager;
-    let pageInfoFuture = new Promise((resolve, result) =>
-    {
-      let timerID;
-      let onDone = (pageInfo) =>
-      {
-        mm.removeMessageListener("abpcrawler:pageInfoGathered", onDone);

[Wladimir Palant, 2016/03/15 10:07:10]

So, which tab did you get the page info for?

The response should contain the outerWindowID so that you can check whether it
applies to "your" tab.

[sergei, 2016/03/16 14:44:23]

For the `tab`, it's "browser message manager" it allows to avoid additional
manual fiddling with `outerWindowID`.

-        clearTimeout(timerID);
-        resolve(pageInfo);
-      }
-      mm.addMessageListener("abpcrawler:pageInfoGathered", (msg) => onDone(msg.data));;
-      timerID = setTimeout(onDone.bind(this, {error: "timeout"}), timeout);
-    });

[Wladimir Palant, 2016/03/15 10:07:10]

Please move this functionality into a separate function. Here it should be only:

  Object.assign(result, yield getPageInfo(tab));

[sergei, 2016/03/16 14:44:23]

Done.

-
-    let pageInfo = yield pageInfoFuture;
-     
+    Object.assign(result, yield getPageInfo(tab, timeout));
     result.finalUrl = tab.linkedBrowser.currentURI.spec;
-    Object.assign(result, pageInfo);
     result.endTime = Date.now();
   }
   finally
   {
+    if (requestNotifier)
+      requestNotifier.shutdown();
     tabAllocator.releaseTab(tab);
   }
   return result;
 }
 
 function reportException(e)
 {
   let stack = "";
   if (e && typeof e == "object" && "stack" in e)
     stack = e.stack + "\n";
 
   Cu.reportError(e);
   dump(e + "\n" + stack + "\n");
 }
-
-let {addonRoot} = require("info");
-let frameScriptPath = addonRoot + "/lib/child/frameScript.js";
-let globalMessageManager = Services.mm;
-globalMessageManager.loadFrameScript(frameScriptPath, true);

[Wladimir Palant, 2016/03/15 10:07:10]

This should be a process script, no point using a frame script here. You can
send the outerWindowID as parameter, the process script can then call
nsIWindowMediator.getOuterWindowWithId() to retrieve it.

-
-let onReportException = function(msg)
-{
-  reportException(msg.objects);

[Wladimir Palant, 2016/03/15 10:07:10]

Please don't use msg.objects - ever. That's a wrapper which will cause
additional IPC traffic whenever you access its properties.

This is also completely unnecessary, just report the exception from the process
script. Both Cu.reportError() and dump() are available.

[sergei, 2016/03/16 14:44:23]

Acknowledged. Actually, I wanted to avoid duplicating of `reportException`, may
be it's not necessary.

[sergei, 2016/03/16 14:44:23]

I know, I used `msg.objects` because we don't know anything about exception
object, it's not necessarily that its properties are accessible from chrome
process, so we should rather access it from chrome process through CPOW.

Just wonder, what is the issue with additional IPC traffic in case of exception
which should actually never happen and when it happens it likely means that
there is a bug in our code?

[Wladimir Palant, 2016/09/14 16:11:46]

Worst-case scenario: deadlocks because all of that traffic needs to be
synchronous.

-}
-globalMessageManager.addMessageListener("abpcrawler:reportException", onReportException);
-
-onShutdown.add(() =>
-{
-  globalMessageManager.removeMessageListener("abpcrawler:reportException", onReportException);
-  globalMessageManager.removeDelayedFrameScript(frameScriptPath);
-});