Bug 1157323 - Part 4: Stop modifying mTimeout/mDelay from the TimerThread, plus some simplification. r=froydnj
authorByron Campen [:bwc] <docfaraday@gmail.com>
Fri, 05 Aug 2016 12:50:00 -0500
changeset 315362 344fdfed174a97aef02c99b9738a0b38c5c6fe43
parent 315361 85739737d40d01de29d2f83d9715249900ca58d9
child 315363 f33fe0269dea6849b6543a2f14df841739b3f9ba
push id30748
push usercbook@mozilla.com
push dateWed, 28 Sep 2016 13:53:19 +0000
treeherdermozilla-central@8c84b7618840 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfroydnj
bugs1157323
milestone52.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1157323 - Part 4: Stop modifying mTimeout/mDelay from the TimerThread, plus some simplification. r=froydnj 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