Issue 3780 - wait for the loading of filters and only afterwards start to fetch pages

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/.
  */
 
 /**
  * @module crawler
  */
 
 Cu.import("resource://gre/modules/Services.jsm");
 Cu.import("resource://gre/modules/Task.jsm");
 Cu.import("resource://gre/modules/Promise.jsm");
 
 function abprequire(module)
 {
   let result = {};
   result.wrappedJSObject = result;
   Services.obs.notifyObservers(result, "adblockplus-require", module);
   return result.exports;
 }
 
 let {RequestNotifier} = abprequire("requestNotifier");
-

[Wladimir Palant, 2016/03/15 13:44:22]

Nit: Why the empty line here?

[sergei, 2016/03/15 14:44:25]

Done.

 let {FilterNotifier} = abprequire("filterNotifier");
 let {FilterStorage} = abprequire("filterStorage");
 
 /**
  * Creates a pool of tabs and allocates them to tasks on request.
  *
  * @param {tabbrowser} browser
  *    The tabbed browser where tabs should be created
  * @param {int} maxtabs
  *    The maximum number of tabs to be allocated
@@ skipping 157 matching lines @@
 };
 
 /**
  * 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)
+{
+  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 loadListener = new LoadListener(window.getBrowser(), timeout);
   let running = 0;
   let windowCloser = new WindowCloser();
   let taskDone = function()
   {
     running--;
     if (running <= 0)
     {
       loadListener.stop();
       windowCloser.stop();
       onDone();
     }
   };
 
-  let crawl_urls = () =>

[Wladimir Palant, 2016/03/15 13:44:22]

I really meant having crawl_urls as a regular function - |function
clawl_urls(urls)|, just above |function* crawl_url|.

[sergei, 2016/03/15 14:44:25]

Done.

-  {
-    for (let url of urls)
-    {
-      running++;
-      Task.spawn(crawl_url.bind(null, url, tabAllocator, loadListener)).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));
-    }
-  };
-  new Promise((resolve, reject) =>
-  {
-    if (FilterStorage.subscriptions.length > 0 && !FilterStorage._loading)
-    {
-      resolve();
-      return;
-    }
-    FilterNotifier.addListener((action, item, newValue, oldValue) =>
-    {
-      if (action == "load")
-      {
-        resolve();
-      }
-    });
-  }).then(crawl_urls)
-  .catch(reportException);

[Wladimir Palant, 2016/03/15 13:44:22]

Nit: put catch() on the same line as then()?

[sergei, 2016/03/15 14:44:25]

Done.

-}
-exports.run = run;
+  for (let url of urls)
+  {
+    running++;
+    Task.spawn(crawl_url.bind(null, url, tabAllocator, loadListener)).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));
+  }
+}
 
 /**
  * Crawls a URL. This is a generator meant to be used via a Task object.
  *
  * @param {String} url
  * @param {TabAllocator} tabAllocator
  * @param {loadListener} loadListener
  * @result {Object}
  *    Crawling result
  */
@@ skipping 54 matching lines @@
 
 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");
 }