Bug 1157323 - Part 4: Stop modifying mTimeout/mDelay from the TimerThread, plus some simplification. draft
authorByron Campen [:bwc] <docfaraday@gmail.com>
Fri, 05 Aug 2016 12:50:00 -0500
changeset 417175 445af08464069f4ce400adeffedb25ada4d71f85
parent 417174 86690a6c6411d2eb6ad12017ee52b74b7db3d2fa
child 417176 9c41628d5a0ec07e343b4e0edba361cefd70d1f1
push id30354
push userbcampen@mozilla.com
push dateFri, 23 Sep 2016 19:02:51 +0000
bugs1157323
milestone52.0a1
Bug 1157323 - Part 4: Stop modifying mTimeout/mDelay from the TimerThread, plus some simplification. MozReview-Commit-ID: 1pMCKLi9DLZ
xpcom/threads/TimerThread.cpp
xpcom/threads/nsTimerImpl.cpp
xpcom/threads/nsTimerImpl.h
--- a/xpcom/threads/TimerThread.cpp
+++ b/xpcom/threads/TimerThread.cpp
@@ -680,22 +680,16 @@ TimerThread::PostTimerEvent(already_AddR
   if (!event) {
     return timer.forget();
   }
 
   if (MOZ_LOG_TEST(GetTimerLog(), LogLevel::Debug)) {
     event->mInitTime = TimeStamp::Now();
   }
 
-  // If this is a repeating precise timer, we need to calculate the time for
-  // the next timer to fire before we make the callback. But don't re-arm.
-  if (timer->IsRepeatingPrecisely()) {
-    timer->SetDelayInternal(timer->mDelay);
-  }
-
 #ifdef MOZ_TASK_TRACER
   // During the dispatch of TimerEvent, we overwrite the current TraceInfo
   // partially with the info saved in timer earlier, and restore it back by
   // AutoSaveCurTraceInfo.
   AutoSaveCurTraceInfo saveCurTraceInfo;
   (timer->GetTracedTask()).SetTLSTraceInfo();
 #endif
 
--- a/xpcom/threads/nsTimerImpl.cpp
+++ b/xpcom/threads/nsTimerImpl.cpp
@@ -402,18 +402,16 @@ nsTimerImpl::GetClosure(void** aClosure)
 }
 
 
 NS_IMETHODIMP
 nsTimerImpl::GetCallback(nsITimerCallback** aCallback)
 {
   if (mCallbackType == CallbackType::Interface) {
     NS_IF_ADDREF(*aCallback = mCallback.i);
-  } else if (mTimerCallbackWhileFiring) {
-    NS_ADDREF(*aCallback = mTimerCallbackWhileFiring);
   } else {
     *aCallback = nullptr;
   }
 
   return NS_OK;
 }
 
 
@@ -471,93 +469,64 @@ nsTimerImpl::Fire()
     MOZ_LOG(GetTimerLog(), LogLevel::Debug,
            ("[this=%p]     delta           %4dms\n",
             this, (a > b) ? (int32_t)d : -(int32_t)d));
 
     mStart = mStart2;
     mStart2 = TimeStamp();
   }
 
-  TimeStamp timeout = mTimeout;
-  if (IsRepeatingPrecisely()) {
-    // Precise repeating timers advance mTimeout by mDelay without fail before
-    // calling Fire().
-    timeout -= TimeDuration::FromMilliseconds(mDelay);
+  if (MOZ_LOG_TEST(GetTimerFiringsLog(), LogLevel::Debug)) {
+    LogFiring(mCallbackType, mCallback);
   }
 
-  if (mCallbackType == CallbackType::Interface) {
-    mTimerCallbackWhileFiring = mCallback.i;
-  }
-  mFiring = true;
+  int32_t oldGeneration = mGeneration;
 
-  // Handle callbacks that re-init the timer, but avoid leaking.
-  // See bug 330128.
-  CallbackUnion callback = mCallback;
-  CallbackType callbackType = mCallbackType;
-  if (callbackType == CallbackType::Interface) {
-    NS_ADDREF(callback.i);
-  } else if (callbackType == CallbackType::Observer) {
-    NS_ADDREF(callback.o);
-  }
-  ReleaseCallback();
-
-  if (MOZ_LOG_TEST(GetTimerFiringsLog(), LogLevel::Debug)) {
-    LogFiring(callbackType, callback);
-  }
-
-  switch (callbackType) {
+  switch (mCallbackType) {
     case CallbackType::Function:
-      callback.c(mITimer, mClosure);
+      mCallback.c(mITimer, mClosure);
       break;
     case CallbackType::Interface:
-      callback.i->Notify(mITimer);
+      {
+        nsCOMPtr<nsITimerCallback> keepalive(mCallback.i);
+        keepalive->Notify(mITimer);
+      }
       break;
     case CallbackType::Observer:
-      callback.o->Observe(mITimer,
-                          NS_TIMER_CALLBACK_TOPIC,
-                          nullptr);
+      {
+        nsCOMPtr<nsIObserver> keepalive(mCallback.o);
+        keepalive->Observe(mITimer,
+                           NS_TIMER_CALLBACK_TOPIC,
+                           nullptr);
+      }
       break;
     default:
       ;
   }
 
-  // If the callback didn't re-init the timer, and it's not a one-shot timer,
-  // restore the callback state.
-  if (mCallbackType == CallbackType::Unknown &&
-      mType != nsITimer::TYPE_ONE_SHOT && !mCanceled) {
-    mCallback = callback;
-    mCallbackType = callbackType;
-  } else {
-    // The timer was a one-shot, or the callback was reinitialized.
-    if (callbackType == CallbackType::Interface) {
-      NS_RELEASE(callback.i);
-    } else if (callbackType == CallbackType::Observer) {
-      NS_RELEASE(callback.o);
+  if (oldGeneration == mGeneration && mCallbackType != CallbackType::Unknown) {
+    // Timer has not been re-init or canceled; clear out one-shot timers, and
+    // re-schedule repeating timers.
+    if (IsRepeating()) {
+      if (mType == nsITimer::TYPE_REPEATING_SLACK) {
+        SetDelayInternal(mDelay);
+      } else {
+        SetDelayInternal(mDelay, mTimeout);
+      }
+      if (gThread) {
+        gThread->AddTimer(this);
+      }
+    } else {
+      ReleaseCallback();
     }
   }
 
-  mFiring = false;
-  mTimerCallbackWhileFiring = nullptr;
-
   MOZ_LOG(GetTimerLog(), LogLevel::Debug,
          ("[this=%p] Took %fms to fire timer callback\n",
           this, (TimeStamp::Now() - now).ToMilliseconds()));
-
-  // Reschedule repeating timers, but make sure that we aren't armed already
-  // (which can happen if the callback reinitialized the timer).
-  if (IsRepeating() && !mArmed) {
-    if (mType == nsITimer::TYPE_REPEATING_SLACK) {
-      SetDelayInternal(mDelay);  // force mTimeout to be recomputed.  For
-    }
-    // REPEATING_PRECISE_CAN_SKIP timers this has
-    // already happened.
-    if (gThread) {
-      gThread->AddTimer(this);
-    }
-  }
 }
 
 #if defined(HAVE_DLADDR) && defined(HAVE___CXA_DEMANGLE)
 #define USE_DLADDR 1
 #endif
 
 #ifdef USE_DLADDR
 #include <cxxabi.h>
@@ -666,32 +635,31 @@ nsTimerImpl::LogFiring(CallbackType aCal
               ("[%d]   ??? timer (%s, %5d ms)\n",
                getpid(), typeStr, mDelay));
       break;
     }
   }
 }
 
 void
-nsTimerImpl::SetDelayInternal(uint32_t aDelay)
+nsTimerImpl::SetDelayInternal(uint32_t aDelay, TimeStamp aBase)
 {
   TimeDuration delayInterval = TimeDuration::FromMilliseconds(aDelay);
 
   mDelay = aDelay;
 
-  TimeStamp now = TimeStamp::Now();
-  mTimeout = now;
+  mTimeout = aBase;
 
   mTimeout += delayInterval;
 
   if (MOZ_LOG_TEST(GetTimerLog(), LogLevel::Debug)) {
     if (mStart.IsNull()) {
-      mStart = now;
+      mStart = aBase;
     } else {
-      mStart2 = now;
+      mStart2 = aBase;
     }
   }
 }
 
 nsTimer::~nsTimer()
 {
 }
 
--- a/xpcom/threads/nsTimerImpl.h
+++ b/xpcom/threads/nsTimerImpl.h
@@ -43,21 +43,17 @@ public:
 
   explicit nsTimerImpl(nsITimer* aTimer);
   NS_INLINE_DECL_THREADSAFE_REFCOUNTING(nsTimerImpl)
   NS_DECL_NON_VIRTUAL_NSITIMER
 
   static nsresult Startup();
   static void Shutdown();
 
-  friend class TimerThread;
-  friend class nsTimerEvent;
-  friend struct TimerAdditionComparator;
-
-  void SetDelayInternal(uint32_t aDelay);
+  void SetDelayInternal(uint32_t aDelay, TimeStamp aBase = TimeStamp::Now());
 
   void Fire();
 
 #ifdef MOZ_TASK_TRACER
   void GetTLSTraceInfo();
   mozilla::tasktracer::TracedTaskCommon GetTracedTask();
 #endif