Bug 342196: ordering bug causes some urls to not be inspected on new windows

patch: setup the webprogresslistener immediately (before js load)
r=mmchew

browser/components/safebrowsing/content/controller.js

@@ -205,44 +205,3 @@ PROT_Controller.prototype.onTabSwitch = function(e) {
   if (this.browserView_.hasProblem(e.toBrowser))
     this.browserView_.problemBrowserSelected(e.toBrowser);
 }
-
-/**
- * Check all browsers (tabs) to see if any of them are phishy.
- * This isn't that clean of a design because as new wardens get
- * added, this method needs to be updated manually.  TODO: fix this
- * when we add a second warden and know what the needs are.
- */
-PROT_Controller.prototype.checkAllBrowsers = function() {
-  var browsers = this.tabWatcher_.getTabBrowser().browsers;
-  for (var i = 0, browser = null; browser = browsers[i]; ++i) {
-    // Check window and all frames.
-    this.checkAllHtmlWindows_(browser, browser.contentWindow)
-  }
-}
-
-/**
- * Check the HTML window and all containing frames for phishing urls.
- * @param browser ChromeWindow that contains the html window
- * @param win HTMLWindow
- */
-PROT_Controller.prototype.checkAllHtmlWindows_ = function(browser, win) {
-  // Check this window
-  var doc = win && win.document;
-  if (!doc)
-    return;
-
-  var url = doc.location.href;
-  
-  var callback = BindToObject(this.browserView_.isProblemDocument_,
-                              this.browserView_,
-                              browser,
-                              doc,
-                              this.phishingWarden_);
-
-  this.phishingWarden_.checkUrl_(url, callback);
-  
-  // Check all frames
-  for (var i = 0, frame = null; frame = win.frames[i]; ++i) {
-    this.checkAllHtmlWindows_(browser, frame);
-  }
-}

browser/components/safebrowsing/content/phishing-warden.js

@@ -79,9 +79,10 @@ const kTestUrls = {
  * Abtracts the checking of user/browser actions for signs of
  * phishing. 
  *
+ * @param progressListener nsIDocNavStartProgressListener
  * @constructor
  */
-function PROT_PhishingWarden() {
+function PROT_PhishingWarden(progressListener) {
   PROT_ListWarden.call(this);
 
   this.debugZone = "phishwarden";
@@ -119,8 +120,7 @@ function PROT_PhishingWarden() {
   this.prefs_.addObserver(kPhishWardenEnabledPref, phishWardenPrefObserver);
 
   // hook up our browser listener
-  this.progressListener_ = Cc["@mozilla.org/browser/safebrowsing/navstartlistener;1"]
-      .createInstance(Ci.nsIDocNavStartProgressListener);
+  this.progressListener_ = progressListener;
   this.progressListener_.callback = this;
   this.progressListener_.enabled = this.phishWardenEnabled_;
   // ms to wait after a request has started before firing JS callback

browser/components/safebrowsing/content/sb-loader.js

@@ -44,6 +44,20 @@ var safebrowsing = {
   controller: null,
   phishWarden: null,
 
+  // We set up the web progress listener immediately so we don't miss any
+  // phishing urls.  Since the phishing infrastructure isn't loaded yet, we
+  // just store the urls in a list.
+  progressListener: null,
+  progressListenerCallback: {
+    requests: [],
+    onDocNavStart: function(request, url) {
+      this.requests.push({
+        'request': request,
+        'url': url
+      });
+    }
+  },
+
   startup: function() {
     setTimeout(safebrowsing.deferredStartup, 2000);
 
@@ -60,7 +74,8 @@ var safebrowsing = {
 
     var contentArea = document.getElementById("content");
 
-    var phishWarden = new appContext.PROT_PhishingWarden();
+    var phishWarden = new appContext.PROT_PhishingWarden(
+          safebrowsing.progressListener);
     safebrowsing.phishWarden = phishWarden;
 
     // Register tables
@@ -83,8 +98,25 @@ var safebrowsing = {
     
     // The initial pages may be a phishing site (e.g., user clicks on a link
     // in an email message and it opens a new window with a phishing site),
-    // so we need to check all open tabs.
-    safebrowsing.controller.checkAllBrowsers();
+    // so we need to check all requests that fired before deferredStartup.
+    if (!phishWarden.phishWardenEnabled_) {
+      safebrowsing.progressListenerCallback.requests = null;
+      safebrowsing.progressListenerCallback = null;
+      safebrowsing.progressListener = null;
+      return;
+    }
+
+    var pendingRequests = safebrowsing.progressListenerCallback.requests;
+    for (var i = 0; i < pendingRequests.length; ++i) {
+      var request = pendingRequests[i].request;
+      var url = pendingRequests[i].url;
+
+      phishWarden.onDocNavStart(request, url);
+    }
+    // Cleanup
+    safebrowsing.progressListenerCallback.requests = null;
+    safebrowsing.progressListenerCallback = null;
+    safebrowsing.progressListener = null;
   },
 
   /**
@@ -103,5 +135,16 @@ var safebrowsing = {
   }
 }
 
+// Set up our request listener immediately so we don't miss
+// any url loads.  We do the actually checking in the deferredStartup
+// method.
+safebrowsing.progressListener =
+  Cc["@mozilla.org/browser/safebrowsing/navstartlistener;1"]
+  .createInstance(Ci.nsIDocNavStartProgressListener);
+safebrowsing.progressListener.callback =
+  safebrowsing.progressListenerCallback;
+safebrowsing.progressListener.enabled = true;
+safebrowsing.progressListener.delay = 0;
+
 window.addEventListener("load", safebrowsing.startup, false);
 window.addEventListener("unload", safebrowsing.shutdown, false);