Bug 338693: move safe browsing timer code into C++ on doc nav event.

r=darin,bryner

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

@@ -117,6 +117,8 @@ function PROT_PhishingWarden() {
       .getService(Ci.nsIDocNavStartProgressListener);
   this.progressListener_.callback = this;
   this.progressListener_.enabled = this.phishWardenEnabled_;
+  // ms to wait after a request has started before firing JS callback
+  this.progressListener_.delay = 1500;
 
   G_Debug(this, "phishWarden initialized");
 }
@@ -139,6 +141,7 @@ PROT_PhishingWarden.prototype.QueryInterface = function(iid) {
  */
 PROT_PhishingWarden.prototype.shutdown = function() {
   this.progressListener_.callback = null;
+  this.progressListener_ = null;
 }
 
 /**
@@ -255,12 +258,8 @@ PROT_PhishingWarden.prototype.onPhishWardenEnabledPrefChanged = function(
  * @param url
  */ 
 PROT_PhishingWarden.prototype.onDocNavStart = function(request, url) {
-  //G_Debug(this, "phishWarden: " + 
-  //        (this.phishWardenEnabled_ ? "enabled" : "disabled"));
-  //G_Debug(this, "checkRemote: " +
-  //        (this.checkRemote_ ? "yes" : "no"));
-  //G_Debug(this, "isTestURL: " +
-  //        (this.isBlacklistTestURL(url) ? "yes" : "no"));
+  G_Debug(this, "checkRemote: " +
+          (this.checkRemote_ ? "yes" : "no"));
 
   // This logic is a bit involved. In some instances of SafeBrowsing
   // (the stand-alone extension, for example), the user might yet have
@@ -288,14 +287,11 @@ PROT_PhishingWarden.prototype.onDocNavStart = function(request, url) {
     }
   } else {
     // Check the local lists for a match.
-    // XXX This is to not slow down Tp.  The real solution is to
-    // move all the logic in isEvilURL_ to the background thread.
-    // This involves moving the method into the listmanager.
     var evilCallback = BindToObject(this.localListMatch_,
                                     this,
                                     url,
                                     request);
-    new G_Alarm(BindToObject(this.checkUrl_, this, url, evilCallback), 1000);
+    this.checkUrl_(url, evilCallback);
   }
 }
 

browser/components/safebrowsing/public/nsIDocNavStartProgressListener.idl

@@ -46,7 +46,7 @@ interface nsIDocNavStartProgressCallback : nsISupports
   void onDocNavStart(in nsIRequest aRequest, in AUTF8String aUri);
 };
 
-[scriptable, uuid(8b8474f3-2091-4afa-bbe0-f113e0472661)]
+[scriptable, uuid(c21b40c0-be77-4328-a822-24f539391445)]
 interface nsIDocNavStartProgressListener : nsISupports
 {
   /**
@@ -56,6 +56,13 @@ interface nsIDocNavStartProgressListener : nsISupports
    */
   attribute boolean enabled;
 
+  /**
+   * Number of ms to wait after receiving a doc load event and calling the
+   * callback.  Even when set to zero, we set a timer so the call will fire
+   * asynchronously.  Defaults to 0.
+   */
+  attribute PRUint32 delay;
+
   /**
    * Callback object to be used when enabled=true.
    * NULL when there is no callback.

browser/components/safebrowsing/src/nsDocNavStartProgressListener.cpp

@@ -39,17 +39,19 @@
 #include "nsDocNavStartProgressListener.h"
 #include "nsIChannel.h"
 #include "nsIRequest.h"
+#include "nsITimer.h"
 #include "nsIWebProgress.h"
 #include "nsServiceManagerUtils.h"
 #include "nsString.h"
 
-NS_IMPL_ISUPPORTS3(nsDocNavStartProgressListener,
+NS_IMPL_ISUPPORTS4(nsDocNavStartProgressListener,
                    nsIDocNavStartProgressListener,
                    nsIWebProgressListener,
+                   nsIObserver,
                    nsISupportsWeakReference)
 
 nsDocNavStartProgressListener::nsDocNavStartProgressListener() :
-  mEnabled(PR_FALSE)
+  mEnabled(PR_FALSE), mDelay(0), mRequests(nsnull), mTimers(nsnull)
 {
 
 }
@@ -57,10 +59,19 @@ nsDocNavStartProgressListener::nsDocNavStartProgressListener() :
 
 nsDocNavStartProgressListener::~nsDocNavStartProgressListener()
 {
+  // Clean up items left in our queues.
+  mRequests.Clear();
 
+  // Cancel pending timers.
+  PRUint32 length = mTimers.Count();
+
+  for (PRUint32 i = 0; i < length; ++i) {
+    mTimers[i]->Cancel();
+  }
+
+  mTimers.Clear();
 }
 
-
 // nsDocNavStartProgressListener::AttachListeners
 
 nsresult
@@ -118,6 +129,19 @@ nsDocNavStartProgressListener::SetEnabled(PRBool aEnabled)
   return NS_OK; // nothing to do
 }
 
+NS_IMETHODIMP
+nsDocNavStartProgressListener::GetDelay(PRUint32* aDelay)
+{
+  *aDelay = mDelay;
+  return NS_OK;
+}
+
+NS_IMETHODIMP
+nsDocNavStartProgressListener::SetDelay(PRUint32 aDelay)
+{
+  mDelay = aDelay;
+  return NS_OK;
+}
 
 // nsDocNavStartProgressListener::GetCallback
 
@@ -167,9 +191,21 @@ nsDocNavStartProgressListener::OnStateChange(nsIWebProgress *aWebProgress,
     nsCAutoString name;
     rv = aRequest->GetName(name);
     if (NS_FAILED(rv))
-      return NS_OK; // ignore requests with no name
-    if (mCallback)
-      mCallback->OnDocNavStart(aRequest, name);
+      return NS_OK; // ignore requests with no name (url)
+
+    // We store the request and a timer in queue.  When the timer fires,
+    // we use the request in the front of the queue.
+
+    nsCOMPtr<nsITimer> timer = do_CreateInstance("@mozilla.org/timer;1", &rv);
+    NS_ENSURE_TRUE(timer, rv);
+
+    rv = timer->Init(this, mDelay, nsITimer::TYPE_ONE_SHOT);
+    if (NS_SUCCEEDED(rv)) {
+      mRequests.AppendObject(aRequest);
+      mTimers.AppendObject(timer);
+    } else {
+      return NS_ERROR_FAILURE;
+    }
   }
   return NS_OK;
 }
@@ -221,3 +257,33 @@ nsDocNavStartProgressListener::OnSecurityChange(nsIWebProgress *aWebProgress,
   return NS_OK;
 }
 
+// nsIObserver ****************************************************************
+
+NS_IMETHODIMP
+nsDocNavStartProgressListener::Observe(nsISupports *subject, const char *topic,
+                                       const PRUnichar *data)
+{
+  if (strcmp(topic, NS_TIMER_CALLBACK_TOPIC) == 0) {
+    // Timer callback, pop the front of the request queue and call the callback.
+#ifdef DEBUG
+    PRUint32 length = mRequests.Count();
+    NS_ASSERTION(length > 0, "timer callback with empty request queue?");
+    length = mTimers.Count();
+    NS_ASSERTION(length > 0, "timer callback with empty timer queue?");
+#endif
+
+    nsIRequest* request = mRequests[0];
+
+    if (mCallback) {
+      nsCAutoString name;
+      nsresult rv = request->GetName(name);
+      NS_ENSURE_SUCCESS(rv, rv);
+
+      mCallback->OnDocNavStart(request, name);
+    }
+
+    mRequests.RemoveObjectAt(0);
+    mTimers.RemoveObjectAt(0);
+  }
+  return NS_OK;
+}

browser/components/safebrowsing/src/nsDocNavStartProgressListener.h

@@ -35,12 +35,19 @@
  * ***** END LICENSE BLOCK ***** */
 
 #include "nsIDocNavStartProgressListener.h"
+#include "nsIObserver.h"
 #include "nsIWebProgressListener.h"
+#include "nsCOMArray.h"
 #include "nsCOMPtr.h"
 #include "nsWeakReference.h"
 
+// Forward declare template types.
+class nsITimer;
+class nsIRequest;
+
 class nsDocNavStartProgressListener : public nsIDocNavStartProgressListener,
                                       public nsIWebProgressListener,
+                                      public nsIObserver,
                                       public nsSupportsWeakReference
 {
 public:
@@ -50,12 +57,18 @@ public:
   NS_DECL_ISUPPORTS
   NS_DECL_NSIDOCNAVSTARTPROGRESSLISTENER
   NS_DECL_NSIWEBPROGRESSLISTENER
+  NS_DECL_NSIOBSERVER
 
 protected:
 
   PRBool mEnabled;
+  PRUint32 mDelay;
   nsCOMPtr<nsIDocNavStartProgressCallback> mCallback;
 
+  // queue of pending requests; should we use nsDeque instead?
+  nsCOMArray<nsIRequest> mRequests;
+  nsCOMArray<nsITimer> mTimers;
+
   nsresult AttachListeners();
   nsresult DetachListeners();
 };