Bug 775676 - Fix leak in nsWebShellWindow. r=roc

After nsWebShellWindow::Destroy clears mSPTimer, it calls
nsXULWindow::Destroy, which runs script.  That script might cause us to
call nsWebShellWindow::SetPersistenceTimer.

If that happens, SetPersistenceTimer will create mSPTimer (it was nulled
out during nsWebShellWindow::Destroy) and addref this.  But there is no
corresponding release.

Let this be a lesson to all ye who try to be clever with manual
addref/release!

xpfe/appshell/src/nsWebShellWindow.cpp

@@ -473,28 +473,63 @@ static void LoadNativeMenus(nsIDOMDocument *aDOMDoc, nsIWidget *aParentWindow)
 }
 #endif
 
+namespace mozilla {
+
+class WebShellWindowTimerCallback : public nsITimerCallback
+{
+public:
+  WebShellWindowTimerCallback(nsWebShellWindow* aWindow)
+    : mWindow(aWindow)
+  {}
+
+  NS_DECL_ISUPPORTS
+
+  NS_IMETHOD Notify(nsITimer* aTimer)
+  {
+    // Although this object participates in a refcount cycle (this -> mWindow
+    // -> mSPTimer -> this), mSPTimer is a one-shot timer and releases this
+    // after it fires.  So we don't need to release mWindow here.
+
+    mWindow->FirePersistenceTimer();
+    return NS_OK;
+  }
+
+private:
+  nsRefPtr<nsWebShellWindow> mWindow;
+};
+
+NS_IMPL_THREADSAFE_ADDREF(WebShellWindowTimerCallback)
+NS_IMPL_THREADSAFE_RELEASE(WebShellWindowTimerCallback)
+NS_IMPL_THREADSAFE_QUERY_INTERFACE1(WebShellWindowTimerCallback,
+                                    nsITimerCallback)
+
+} // namespace mozilla
+
 void
 nsWebShellWindow::SetPersistenceTimer(PRUint32 aDirtyFlags)
 {
   MutexAutoLock lock(mSPTimerLock);
   if (!mSPTimer) {
-    nsresult rv;
+    mSPTimer = do_CreateInstance("@mozilla.org/timer;1");
-    mSPTimer = do_CreateInstance("@mozilla.org/timer;1", &rv);
+    if (!mSPTimer) {
-    if (NS_SUCCEEDED(rv)) {
+      NS_WARNING("Couldn't create @mozilla.org/timer;1 instance?");
-      NS_ADDREF_THIS(); // for the timer, which holds a reference to this window
+      return;
     }
   }
-  mSPTimer->InitWithFuncCallback(FirePersistenceTimer, this,
+
-                                 SIZE_PERSISTENCE_TIMEOUT, nsITimer::TYPE_ONE_SHOT);
+  nsRefPtr<WebShellWindowTimerCallback> callback =
+    new WebShellWindowTimerCallback(this);
+  mSPTimer->InitWithCallback(callback, SIZE_PERSISTENCE_TIMEOUT,
+                             nsITimer::TYPE_ONE_SHOT);
+
   PersistentAttributesDirty(aDirtyFlags);
 }
 
 void
-nsWebShellWindow::FirePersistenceTimer(nsITimer *aTimer, void *aClosure)
+nsWebShellWindow::FirePersistenceTimer()
 {
-  nsWebShellWindow *win = static_cast<nsWebShellWindow *>(aClosure);
+  MutexAutoLock lock(mSPTimerLock);
-  MutexAutoLock lock(win->mSPTimerLock);
+  SavePersistentAttributes();
-  win->SavePersistentAttributes();
 }
 
 
@@ -747,7 +782,6 @@ NS_IMETHODIMP nsWebShellWindow::Destroy()
       mSPTimer->Cancel();
       SavePersistentAttributes();
       mSPTimer = nsnull;
-      NS_RELEASE_THIS(); // the timer held a reference to us
     }
   }
   return nsXULWindow::Destroy();

xpfe/appshell/src/nsWebShellWindow.h

@@ -16,6 +16,10 @@
 /* Forward declarations.... */
 class nsIURI;
 
+namespace mozilla {
+class WebShellWindowTimerCallback;
+} // namespace mozilla
+
 class nsWebShellWindow : public nsXULWindow,
                          public nsIWebProgressListener
 {
@@ -41,6 +45,7 @@ public:
   NS_IMETHOD Destroy();
 
 protected:
+  friend class mozilla::WebShellWindowTimerCallback;
   
   virtual ~nsWebShellWindow();
 
@@ -54,7 +59,7 @@ protected:
   mozilla::Mutex          mSPTimerLock;
 
   void        SetPersistenceTimer(PRUint32 aDirtyFlags);
-  static void FirePersistenceTimer(nsITimer *aTimer, void *aClosure);
+  void        FirePersistenceTimer();
 };