Bug 472258 - Reinitializing one-shot timers by resetting delay (->SetDelay) doesn't work anymore - fix callers; r+sr=bzbarsky
authorNickolay_Ponomarev <asqueella@gmail.com>
Tue, 03 Feb 2009 15:42:21 +0100
changeset 24552 cdd059f3d2005defcc2ebf66206fe264ac5e4191
parent 24551 a206aff7a9c64104a2720ba597b75f88333672e2
child 24553 39219f4ec8101f1ff11c5d7175b5337a21ef4503
push id5113
push usersgautherie.bz@free.fr
push dateTue, 03 Feb 2009 14:44:19 +0000
treeherdermozilla-central@c23d77b0d600 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs472258
milestone1.9.2a1pre
Bug 472258 - Reinitializing one-shot timers by resetting delay (->SetDelay) doesn't work anymore - fix callers; r+sr=bzbarsky
js/src/xpconnect/loader/mozJSComponentLoader.cpp
xpcom/threads/nsITimer.idl
xpcom/threads/nsTimerImpl.cpp
xpfe/appshell/src/nsWebShellWindow.cpp
--- a/js/src/xpconnect/loader/mozJSComponentLoader.cpp
+++ b/js/src/xpconnect/loader/mozJSComponentLoader.cpp
@@ -981,16 +981,18 @@ mozJSComponentLoader::StartFastLoad(nsIF
         mFastLoadTimer = do_CreateInstance(NS_TIMER_CONTRACTID, &rv);
         NS_ENSURE_SUCCESS(rv, rv);
 
         rv = mFastLoadTimer->InitWithFuncCallback(&mozJSComponentLoader::CloseFastLoad,
                                                   this,
                                                   kFastLoadWriteDelay,
                                                   nsITimer::TYPE_ONE_SHOT);
     } else {
+        // Note, that since CloseFastLoad nulls out mFastLoadTimer,
+        // SetDelay() will only be called on a timer that hasn't fired.
         rv = mFastLoadTimer->SetDelay(kFastLoadWriteDelay);
     }
 
     return rv;
 }
 
 nsresult
 mozJSComponentLoader::ReadScript(nsIFastLoadService *flSvc,
--- a/xpcom/threads/nsITimer.idl
+++ b/xpcom/threads/nsITimer.idl
@@ -74,19 +74,19 @@ interface nsITimerCallback : nsISupports
    * @param aTimer the timer which has expired
    */
   void notify(in nsITimer timer);
 };
 
 
 /**
  * nsITimer instances must be initialized by calling one of the "init" methods
- * documented below.  You may also re-initialize an existing instance with new
- * delay to avoid the overhead of destroying and creating a timer.  It is not
- * necessary to cancel the timer in that case.
+ * documented below.  You may also re-initialize (using one of the init()
+ * methods) an existing instance to avoid the overhead of destroying and
+ * creating a timer.  It is not necessary to cancel the timer in that case.
  */
 [scriptable, uuid(193fc37a-8aa4-4d29-aa57-1acd87c26b66)]
 interface nsITimer : nsISupports
 {
   /* Timer types */
 
   /**
    * Type of a timer that fires once only.
@@ -165,22 +165,26 @@ interface nsITimer : nsISupports
    * Cancel the timer.  This method works on all types, not just on repeating
    * timers -- you might want to cancel a TYPE_ONE_SHOT timer, and even reuse
    * it by re-initializing it (to avoid object destruction and creation costs
    * by conserving one timer instance).
    */
   void cancel();
   
   /**
-   * The millisecond delay of the timeout
+   * The millisecond delay of the timeout.
+   *
+   * NOTE: Re-setting the delay on a one-shot timer that has already fired
+   * doesn't restart the timer. Call one of the init() methods to restart
+   * a one-shot timer.
    */
   attribute unsigned long delay;
   
   /**
-   * The timer type : one shot or repeating
+   * The timer type - one of the above TYPE_* constants.
    */  
   attribute unsigned long type;
 
   /**
    * The opaque pointer pass to initWithFuncCallback.
    */  
   [noscript] readonly attribute voidPtr closure;
 
--- a/xpcom/threads/nsTimerImpl.cpp
+++ b/xpcom/threads/nsTimerImpl.cpp
@@ -287,16 +287,24 @@ NS_IMETHODIMP nsTimerImpl::Cancel()
 
   ReleaseCallback();
 
   return NS_OK;
 }
 
 NS_IMETHODIMP nsTimerImpl::SetDelay(PRUint32 aDelay)
 {
+  if (mCallbackType == CALLBACK_TYPE_UNKNOWN && mType == TYPE_ONE_SHOT) {
+    // This may happen if someone tries to re-use a one-shot timer
+    // by re-setting delay instead of reinitializing the timer.
+    NS_ERROR("nsITimer->SetDelay() called when the "
+             "one-shot timer is not set up.");
+    return NS_ERROR_NOT_INITIALIZED;
+  }
+
   // If we're already repeating precisely, update mTimeout now so that the
   // new delay takes effect in the future.
   if (mTimeout != 0 && mType == TYPE_REPEATING_PRECISE)
     mTimeout = PR_IntervalNow();
 
   SetDelayInternal(aDelay);
 
   if (!mFiring && gThread)
--- a/xpfe/appshell/src/nsWebShellWindow.cpp
+++ b/xpfe/appshell/src/nsWebShellWindow.cpp
@@ -567,29 +567,26 @@ static void LoadNativeMenus(nsIDOMDocume
 
 void
 nsWebShellWindow::SetPersistenceTimer(PRUint32 aDirtyFlags)
 {
   if (!mSPTimerLock)
     return;
 
   PR_Lock(mSPTimerLock);
-  if (mSPTimer) {
-    mSPTimer->SetDelay(SIZE_PERSISTENCE_TIMEOUT);
-    PersistentAttributesDirty(aDirtyFlags);
-  } else {
+  if (!mSPTimer) {
     nsresult rv;
     mSPTimer = do_CreateInstance("@mozilla.org/timer;1", &rv);
     if (NS_SUCCEEDED(rv)) {
       NS_ADDREF_THIS(); // for the timer, which holds a reference to this window
-      mSPTimer->InitWithFuncCallback(FirePersistenceTimer, this,
-                                     SIZE_PERSISTENCE_TIMEOUT, nsITimer::TYPE_ONE_SHOT);
-      PersistentAttributesDirty(aDirtyFlags);
     }
   }
+  mSPTimer->InitWithFuncCallback(FirePersistenceTimer, this,
+                                 SIZE_PERSISTENCE_TIMEOUT, nsITimer::TYPE_ONE_SHOT);
+  PersistentAttributesDirty(aDirtyFlags);
   PR_Unlock(mSPTimerLock);
 }
 
 void
 nsWebShellWindow::FirePersistenceTimer(nsITimer *aTimer, void *aClosure)
 {
   nsWebShellWindow *win = static_cast<nsWebShellWindow *>(aClosure);
   if (!win->mSPTimerLock)