Crawler frontend

lib/crawler.js

 Cu.import("resource://gre/modules/Services.jsm");
-
-function require(module)
-{

[Wladimir Palant, 2012/09/21 15:36:18]

This is unnecessary - module in lib/ directory can just use require() to get to
other modules of the same extension.

-  let result = {};
-  result.wrappedJSObject = result;
-  Services.obs.notifyObservers(result, "abpcrawler-require", module);
-  return result.exports;
-}
 
 function abprequire(module)
 {
   let result = {};
   result.wrappedJSObject = result;
   Services.obs.notifyObservers(result, "adblockplus-require", module);
-  if ("exports" in result)
-    return result.exports;
-  else
-    return Cu.import("chrome://adblockplus-modules/content/" +

[Wladimir Palant, 2012/09/21 15:36:18]

This is unnecessary - Diagnostics is currently compatible with old Adblock Plus
versions (ones that used JSM modules), this extension doesn't need to do that.

-                     module[0].toUpperCase() + module.substr(1) + ".jsm", null);
+  return result.exports;
 }
 
 let {Storage} = require("storage");
 let {Client} = require("client");
 
 let {Policy} = abprequire("contentPolicy");
 let {Filter} = abprequire("filterClasses");
+let {Utils} = abprequire("utils");
 
 let origProcessNode = Policy.processNode;
 
 let siteTabs;
 let currentTabs;
 
 function processNode(wnd, node, contentType, location, collapse)
 {
   let result = origProcessNode.apply(this, arguments);
-  let url = location.spec;
-  if (url)

[Wladimir Palant, 2012/09/21 15:36:18]

I think that you can remove that check - an nsIURI is always a valid URL. And
invalid URLs don't even result in a call to processNode().

There is a special case: for calls with type == Policy.type.ELEMHIDE the
location parameter is a filter (element hiding hits don't have a real location).
So you probably want to sent location.text then.

+  let url = (contentType === Policy.type.ELEMHIDE) ? location.text :
+      location.spec;
+
+  let topWindow = wnd.top;
+  if (!topWindow.document)
   {
-    let site = siteTabs[wnd.top.location.href];
-    let filtered = !result;
-    Storage.write([url, site, filtered]);
+    Cu.reportError("No document associated with the node's top window");
+    return result;
   }
+
+  let tabbrowser = Utils.getChromeWindow(topWindow).gBrowser;
+  if (!tabbrowser)
+  {
+    Cu.reportError("Unable to get a tabbrowser reference");
+    return result;
+  }
+
+  let browser = tabbrowser.getBrowserForDocument(topWindow.document);
+  if (!browser)
+  {
+    Cu.reportError("Unable to get browser for the tab");
+    return result;
+  }
+
+  let site = siteTabs.get(browser);
+  let filtered = !result;
+  Storage.write([url, site, filtered]);
   return result;
 }
 
 function loadSite(site, window, callback)
 {
   if (!site)
     return;
 
   let tabbrowser = window.gBrowser;
   let tab = tabbrowser.addTab(site);
   let browser = tabbrowser.getBrowserForTab(tab);
+
+  siteTabs.set(browser, site);
 
   let progressListener = {
     onStateChange: function(aBrowser, aWebProgress, aRequest, aStateFlags, aStatus)
     {
       if (browser !== aBrowser)
         return;
 
       if (!(aStateFlags & Ci.nsIWebProgressListener.STATE_STOP))
         return;
 
       tabbrowser.removeTabsProgressListener(progressListener);
       tabbrowser.removeTab(tab);
       callback();
-    },
-    onLocationChange: function(aBrowser, aWebProgress, aRequest, aLocation, aFlags)
-    {
-      if (browser === aBrowser)
-        siteTabs[aLocation.spec] = site;

[Wladimir Palant, 2012/09/21 15:36:18]

Ok, that's a hack. What if two webpages in different tabs redirect to the same
URL?

Associating data with the <browser> element should definitely work and it is the
cleaner/more reliable approach.

     }
   };
   tabbrowser.addTabsProgressListener(progressListener);
 }
 
 function loadSites(backendUrl, parallelTabs, window, sites, callback)
 {
   while (currentTabs < parallelTabs && sites.length)
   {
     currentTabs++;
-    let site = sites[0];
-    sites = sites.slice(1);

[Wladimir Palant, 2012/09/21 15:36:18]

let site = sites.shift()?

+    let site = sites.shift();
     loadSite(site, window, function()
     {
       currentTabs--;
       if (!sites.length && !currentTabs)
       {
         Storage.finish();
         let dataFilePath = Storage.dataFile.path;
-        Client.sendCrawlerDataFile(backendUrl, window, dataFilePath, function()
+        Client.sendCrawlerDataFile(backendUrl, dataFilePath, function()
         {
           Storage.destroy();
           callback();
         });
       }
       else
         loadSites(backendUrl, parallelTabs, window, sites, callback);
     });
   }
 }
 
 let Crawler = exports.Crawler = {};
 
 Crawler.crawl = function(backendUrl, parallelTabs, window, callback)
 {
   if (Policy.processNode != origProcessNode)
     return;
 
   Policy.processNode = processNode;
 
-  siteTabs = {};
+  siteTabs = new WeakMap();
   currentTabs = 0;
 
   Storage.init();
 
   Client.fetchCrawlableSites(backendUrl, function(sites)
   {
     loadSites(backendUrl, parallelTabs, window, sites, function()
     {
       Policy.processNode = origProcessNode;
+      siteTabs = null;
       callback();
     });
   });
 };