Bug 1059572 - Part 1: Move PostTimerEvent to TimerThread to allow TimerThread's monitor to protect it. r=nfroyd
authorByron Campen [:bwc] <docfaraday@gmail.com>
Wed, 22 Jul 2015 12:39:34 -0500
changeset 286985 7793255f366e01dd38aac596d094c853bc6d94df
parent 286984 88854d602d98f67d4d7e541d41d9647373408d1f
child 286986 80677b385ce0866067fa7bb5c1b3765ac8c67181
push id5067
push userraliiev@mozilla.com
push dateMon, 21 Sep 2015 14:04:52 +0000
treeherdermozilla-beta@14221ffe5b2f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnfroyd
bugs1059572
milestone42.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 1059572 - Part 1: Move PostTimerEvent to TimerThread to allow TimerThread's monitor to protect it. r=nfroyd
xpcom/threads/TimerThread.cpp
xpcom/threads/TimerThread.h
xpcom/threads/nsTimerImpl.cpp
xpcom/threads/nsTimerImpl.h
--- a/xpcom/threads/TimerThread.cpp
+++ b/xpcom/threads/TimerThread.cpp
@@ -3,16 +3,17 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "nsTimerImpl.h"
 #include "TimerThread.h"
 
 #include "nsThreadUtils.h"
+#include "plarena.h"
 #include "pratom.h"
 
 #include "nsIObserverService.h"
 #include "nsIServiceManager.h"
 #include "mozilla/Services.h"
 #include "mozilla/ChaosMode.h"
 #include "mozilla/ArrayUtils.h"
 #include "mozilla/BinarySearch.h"
@@ -75,30 +76,226 @@ TimerObserverRunnable::Run()
     observerService->AddObserver(mObserver, "suspend_process_notification", false);
     observerService->AddObserver(mObserver, "resume_process_notification", false);
   }
   return NS_OK;
 }
 
 } // namespace
 
+namespace {
+
+// TimerEventAllocator is a thread-safe allocator used only for nsTimerEvents.
+// It's needed to avoid contention over the default allocator lock when
+// firing timer events (see bug 733277).  The thread-safety is required because
+// nsTimerEvent objects are allocated on the timer thread, and freed on another
+// thread.  Because TimerEventAllocator has its own lock, contention over that
+// lock is limited to the allocation and deallocation of nsTimerEvent objects.
+//
+// Because this allocator is layered over PLArenaPool, it never shrinks -- even
+// "freed" nsTimerEvents aren't truly freed, they're just put onto a free-list
+// for later recycling.  So the amount of memory consumed will always be equal
+// to the high-water mark consumption.  But nsTimerEvents are small and it's
+// unusual to have more than a few hundred of them, so this shouldn't be a
+// problem in practice.
+
+class TimerEventAllocator
+{
+private:
+  struct FreeEntry
+  {
+    FreeEntry* mNext;
+  };
+
+  PLArenaPool mPool;
+  FreeEntry* mFirstFree;
+  mozilla::Monitor mMonitor;
+
+public:
+  TimerEventAllocator()
+    : mFirstFree(nullptr)
+    , mMonitor("TimerEventAllocator")
+  {
+    PL_InitArenaPool(&mPool, "TimerEventPool", 4096, /* align = */ 0);
+  }
+
+  ~TimerEventAllocator()
+  {
+    PL_FinishArenaPool(&mPool);
+  }
+
+  void* Alloc(size_t aSize);
+  void Free(void* aPtr);
+};
+
+} // namespace
+
+class nsTimerEvent : public nsRunnable
+{
+public:
+  NS_IMETHOD Run();
+
+  nsTimerEvent()
+    : mTimer()
+    , mGeneration(0)
+  {
+    MOZ_COUNT_CTOR(nsTimerEvent);
+
+    // Note: We override operator new for this class, and the override is
+    // fallible!
+    sAllocatorUsers++;
+  }
+
+  TimeStamp mInitTime;
+
+  static void Init();
+  static void Shutdown();
+  static void DeleteAllocatorIfNeeded();
+
+  static void* operator new(size_t aSize) CPP_THROW_NEW
+  {
+    return sAllocator->Alloc(aSize);
+  }
+  void operator delete(void* aPtr)
+  {
+    sAllocator->Free(aPtr);
+    DeleteAllocatorIfNeeded();
+  }
+
+  already_AddRefed<nsTimerImpl> ForgetTimer()
+  {
+    return mTimer.forget();
+  }
+
+  void SetTimer(already_AddRefed<nsTimerImpl> aTimer)
+  {
+    mTimer = aTimer;
+    mGeneration = mTimer->GetGeneration();
+  }
+
+private:
+  ~nsTimerEvent()
+  {
+    MOZ_COUNT_DTOR(nsTimerEvent);
+
+    MOZ_ASSERT(!sCanDeleteAllocator || sAllocatorUsers > 0,
+               "This will result in us attempting to deallocate the nsTimerEvent allocator twice");
+    sAllocatorUsers--;
+  }
+
+  nsRefPtr<nsTimerImpl> mTimer;
+  int32_t      mGeneration;
+
+  static TimerEventAllocator* sAllocator;
+  static Atomic<int32_t> sAllocatorUsers;
+  static bool sCanDeleteAllocator;
+};
+
+TimerEventAllocator* nsTimerEvent::sAllocator = nullptr;
+Atomic<int32_t> nsTimerEvent::sAllocatorUsers;
+bool nsTimerEvent::sCanDeleteAllocator = false;
+
+namespace {
+
+void*
+TimerEventAllocator::Alloc(size_t aSize)
+{
+  MOZ_ASSERT(aSize == sizeof(nsTimerEvent));
+
+  mozilla::MonitorAutoLock lock(mMonitor);
+
+  void* p;
+  if (mFirstFree) {
+    p = mFirstFree;
+    mFirstFree = mFirstFree->mNext;
+  } else {
+    PL_ARENA_ALLOCATE(p, &mPool, aSize);
+    if (!p) {
+      return nullptr;
+    }
+  }
+
+  return p;
+}
+
+void
+TimerEventAllocator::Free(void* aPtr)
+{
+  mozilla::MonitorAutoLock lock(mMonitor);
+
+  FreeEntry* entry = reinterpret_cast<FreeEntry*>(aPtr);
+
+  entry->mNext = mFirstFree;
+  mFirstFree = entry;
+}
+
+} // namespace
+
+void
+nsTimerEvent::Init()
+{
+  sAllocator = new TimerEventAllocator();
+}
+
+void
+nsTimerEvent::Shutdown()
+{
+  sCanDeleteAllocator = true;
+  DeleteAllocatorIfNeeded();
+}
+
+void
+nsTimerEvent::DeleteAllocatorIfNeeded()
+{
+  if (sCanDeleteAllocator && sAllocatorUsers == 0) {
+    delete sAllocator;
+    sAllocator = nullptr;
+  }
+}
+
+NS_IMETHODIMP
+nsTimerEvent::Run()
+{
+  if (mGeneration != mTimer->GetGeneration()) {
+    return NS_OK;
+  }
+
+  if (MOZ_LOG_TEST(GetTimerLog(), LogLevel::Debug)) {
+    TimeStamp now = TimeStamp::Now();
+    MOZ_LOG(GetTimerLog(), LogLevel::Debug,
+           ("[this=%p] time between PostTimerEvent() and Fire(): %fms\n",
+            this, (now - mInitTime).ToMilliseconds()));
+  }
+
+  mTimer->Fire();
+  // Since nsTimerImpl is not thread-safe, we should release |mTimer|
+  // here in the target thread to avoid race condition. Otherwise,
+  // ~nsTimerEvent() which calls nsTimerImpl::Release() could run in the
+  // timer thread and result in race condition.
+  mTimer = nullptr;
+
+  return NS_OK;
+}
+
 nsresult
 TimerThread::Init()
 {
   MOZ_LOG(GetTimerLog(), LogLevel::Debug,
          ("TimerThread::Init [%d]\n", mInitialized));
 
   if (mInitialized) {
     if (!mThread) {
       return NS_ERROR_FAILURE;
     }
 
     return NS_OK;
   }
 
+  nsTimerEvent::Init();
+
   if (mInitInProgress.exchange(true) == false) {
     // We hold on to mThread to keep the thread alive.
     nsresult rv = NS_NewThread(getter_AddRefs(mThread), this);
     if (NS_FAILED(rv)) {
       mThread = nullptr;
     } else {
       nsRefPtr<TimerObserverRunnable> r = new TimerObserverRunnable(this);
       if (NS_IsMainThread()) {
@@ -163,16 +360,18 @@ TimerThread::Shutdown()
   for (uint32_t i = 0; i < timersCount; i++) {
     nsTimerImpl* timer = timers[i];
     timer->ReleaseCallback();
     ReleaseTimerInternal(timer);
   }
 
   mThread->Shutdown();    // wait for the thread to die
 
+  nsTimerEvent::Shutdown();
+
   MOZ_LOG(GetTimerLog(), LogLevel::Debug, ("TimerThread::Shutdown end\n"));
   return NS_OK;
 }
 
 #ifdef MOZ_NUWA_PROCESS
 #include "ipc/Nuwa.h"
 #endif
 
@@ -260,25 +459,20 @@ TimerThread::Run()
           nsRefPtr<nsTimerImpl> timerRef(timer);
           RemoveTimerInternal(timer);
           timer = nullptr;
 
           MOZ_LOG(GetTimerLog(), LogLevel::Debug,
                  ("Timer thread woke up %fms from when it was supposed to\n",
                   fabs((now - timerRef->mTimeout).ToMilliseconds())));
 
-          {
-            // We release mMonitor around the Fire call to avoid deadlock.
-            MonitorAutoUnlock unlock(mMonitor);
-
-            // We are going to let the call to PostTimerEvent here handle the
-            // release of the timer so that we don't end up releasing the timer
-            // on the TimerThread instead of on the thread it targets.
-            timerRef = nsTimerImpl::PostTimerEvent(timerRef.forget());
-          }
+          // We are going to let the call to PostTimerEvent here handle the
+          // release of the timer so that we don't end up releasing the timer
+          // on the TimerThread instead of on the thread it targets.
+          timerRef = PostTimerEvent(timerRef.forget());
 
           if (timerRef) {
             // We got our reference back due to an error.
             // Unhook the nsRefPtr, and release manually so we can get the
             // refcount.
             nsrefcnt rc = timerRef.forget().take()->Release();
             (void)rc;
 
@@ -482,16 +676,85 @@ TimerThread::ReleaseTimerInternal(nsTime
     // copied to a local array before releasing in shutdown
     mMonitor.AssertCurrentThreadOwns();
   }
   // Order is crucial here -- see nsTimerImpl::Release.
   aTimer->mArmed = false;
   NS_RELEASE(aTimer);
 }
 
+already_AddRefed<nsTimerImpl>
+TimerThread::PostTimerEvent(already_AddRefed<nsTimerImpl> aTimerRef)
+{
+  mMonitor.AssertCurrentThreadOwns();
+
+  nsRefPtr<nsTimerImpl> timer(aTimerRef);
+  if (!timer->mEventTarget) {
+    NS_ERROR("Attempt to post timer event to NULL event target");
+    return timer.forget();
+  }
+
+  // XXX we may want to reuse this nsTimerEvent in the case of repeating timers.
+
+  // Since we already addref'd 'timer', we don't need to addref here.
+  // We will release either in ~nsTimerEvent(), or pass the reference back to
+  // the caller. We need to copy the generation number from this timer into the
+  // event, so we can avoid firing a timer that was re-initialized after being
+  // canceled.
+
+  nsRefPtr<nsTimerEvent> event = new nsTimerEvent;
+  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.
+  if (timer->IsRepeatingPrecisely()) {
+    timer->SetDelayInternal(timer->mDelay);
+
+    // But only re-arm REPEATING_PRECISE timers.
+    if (timer->mType == nsTimerImpl::TYPE_REPEATING_PRECISE) {
+      if (AddTimerInternal(timer) == -1) {
+        return timer.forget();
+      }
+    }
+  }
+
+#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
+
+  nsIEventTarget* target = timer->mEventTarget;
+  event->SetTimer(timer.forget());
+
+  nsresult rv;
+  {
+    // We release mMonitor around the Dispatch because if this timer is targeted
+    // at the TimerThread we'll deadlock.
+    MonitorAutoUnlock unlock(mMonitor);
+    rv = target->Dispatch(event, NS_DISPATCH_NORMAL);
+  }
+
+  if (NS_FAILED(rv)) {
+    timer = event->ForgetTimer();
+    RemoveTimerInternal(timer);
+    return timer.forget();
+  }
+
+  return nullptr;
+}
+
 void
 TimerThread::DoBeforeSleep()
 {
   // Mainthread
   MonitorAutoLock lock(mMonitor);
   mLastTimerEventLoopRun = TimeStamp::Now();
   mSleeping = true;
 }
--- a/xpcom/threads/TimerThread.h
+++ b/xpcom/threads/TimerThread.h
@@ -63,16 +63,18 @@ private:
 
   // These two internal helper methods must be called while mMonitor is held.
   // AddTimerInternal returns the position where the timer was added in the
   // list, or -1 if it failed.
   int32_t AddTimerInternal(nsTimerImpl* aTimer);
   bool    RemoveTimerInternal(nsTimerImpl* aTimer);
   void    ReleaseTimerInternal(nsTimerImpl* aTimer);
 
+  already_AddRefed<nsTimerImpl> PostTimerEvent(already_AddRefed<nsTimerImpl> aTimerRef);
+
   nsCOMPtr<nsIThread> mThread;
   Monitor mMonitor;
 
   bool mShutdown;
   bool mWaiting;
   bool mNotified;
   bool mSleeping;
   TimeStamp mLastTimerEventLoopRun;
--- a/xpcom/threads/nsTimerImpl.cpp
+++ b/xpcom/threads/nsTimerImpl.cpp
@@ -4,17 +4,16 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "nsTimerImpl.h"
 #include "TimerThread.h"
 #include "nsAutoPtr.h"
 #include "nsThreadManager.h"
 #include "nsThreadUtils.h"
-#include "plarena.h"
 #include "pratom.h"
 #include "GeckoProfiler.h"
 #include "mozilla/Atomics.h"
 #include "mozilla/Logging.h"
 #ifdef MOZ_NUWA_PROCESS
 #include "ipc/Nuwa.h"
 #endif
 #ifdef MOZ_TASK_TRACER
@@ -61,165 +60,16 @@ myNS_MeanAndStdDev(double n, double sumO
     }
     // for some reason, Windows says sqrt(0.0) is "-1.#J" (?!) so do this:
     stdDev = var != 0.0 ? sqrt(var) : 0.0;
   }
   *meanResult = mean;
   *stdDevResult = stdDev;
 }
 
-namespace {
-
-// TimerEventAllocator is a thread-safe allocator used only for nsTimerEvents.
-// It's needed to avoid contention over the default allocator lock when
-// firing timer events (see bug 733277).  The thread-safety is required because
-// nsTimerEvent objects are allocated on the timer thread, and freed on another
-// thread.  Because TimerEventAllocator has its own lock, contention over that
-// lock is limited to the allocation and deallocation of nsTimerEvent objects.
-//
-// Because this allocator is layered over PLArenaPool, it never shrinks -- even
-// "freed" nsTimerEvents aren't truly freed, they're just put onto a free-list
-// for later recycling.  So the amount of memory consumed will always be equal
-// to the high-water mark consumption.  But nsTimerEvents are small and it's
-// unusual to have more than a few hundred of them, so this shouldn't be a
-// problem in practice.
-
-class TimerEventAllocator
-{
-private:
-  struct FreeEntry
-  {
-    FreeEntry* mNext;
-  };
-
-  PLArenaPool mPool;
-  FreeEntry* mFirstFree;
-  mozilla::Monitor mMonitor;
-
-public:
-  TimerEventAllocator()
-    : mFirstFree(nullptr)
-    , mMonitor("TimerEventAllocator")
-  {
-    PL_InitArenaPool(&mPool, "TimerEventPool", 4096, /* align = */ 0);
-  }
-
-  ~TimerEventAllocator()
-  {
-    PL_FinishArenaPool(&mPool);
-  }
-
-  void* Alloc(size_t aSize);
-  void Free(void* aPtr);
-};
-
-} // namespace
-
-class nsTimerEvent : public nsRunnable
-{
-public:
-  NS_IMETHOD Run();
-
-  nsTimerEvent()
-    : mTimer()
-    , mGeneration(0)
-  {
-    MOZ_COUNT_CTOR(nsTimerEvent);
-
-    MOZ_ASSERT(gThread->IsOnTimerThread(),
-               "nsTimer must always be allocated on the timer thread");
-
-    sAllocatorUsers++;
-  }
-
-  TimeStamp mInitTime;
-
-  static void Init();
-  static void Shutdown();
-  static void DeleteAllocatorIfNeeded();
-
-  static void* operator new(size_t aSize) CPP_THROW_NEW
-  {
-    return sAllocator->Alloc(aSize);
-  }
-  void operator delete(void* aPtr)
-  {
-    sAllocator->Free(aPtr);
-    DeleteAllocatorIfNeeded();
-  }
-
-  already_AddRefed<nsTimerImpl> ForgetTimer()
-  {
-    return mTimer.forget();
-  }
-
-  void SetTimer(already_AddRefed<nsTimerImpl> aTimer)
-  {
-    mTimer = aTimer;
-    mGeneration = mTimer->GetGeneration();
-  }
-
-private:
-  ~nsTimerEvent()
-  {
-    MOZ_COUNT_DTOR(nsTimerEvent);
-
-    MOZ_ASSERT(!sCanDeleteAllocator || sAllocatorUsers > 0,
-               "This will result in us attempting to deallocate the nsTimerEvent allocator twice");
-    sAllocatorUsers--;
-  }
-
-  nsRefPtr<nsTimerImpl> mTimer;
-  int32_t      mGeneration;
-
-  static TimerEventAllocator* sAllocator;
-  static Atomic<int32_t> sAllocatorUsers;
-  static bool sCanDeleteAllocator;
-};
-
-TimerEventAllocator* nsTimerEvent::sAllocator = nullptr;
-Atomic<int32_t> nsTimerEvent::sAllocatorUsers;
-bool nsTimerEvent::sCanDeleteAllocator = false;
-
-namespace {
-
-void*
-TimerEventAllocator::Alloc(size_t aSize)
-{
-  MOZ_ASSERT(aSize == sizeof(nsTimerEvent));
-
-  mozilla::MonitorAutoLock lock(mMonitor);
-
-  void* p;
-  if (mFirstFree) {
-    p = mFirstFree;
-    mFirstFree = mFirstFree->mNext;
-  } else {
-    PL_ARENA_ALLOCATE(p, &mPool, aSize);
-    if (!p) {
-      return nullptr;
-    }
-  }
-
-  return p;
-}
-
-void
-TimerEventAllocator::Free(void* aPtr)
-{
-  mozilla::MonitorAutoLock lock(mMonitor);
-
-  FreeEntry* entry = reinterpret_cast<FreeEntry*>(aPtr);
-
-  entry->mNext = mFirstFree;
-  mFirstFree = entry;
-}
-
-} // namespace
-
 NS_IMPL_QUERY_INTERFACE(nsTimerImpl, nsITimer)
 NS_IMPL_ADDREF(nsTimerImpl)
 
 NS_IMETHODIMP_(MozExternalRefCountType)
 nsTimerImpl::Release(void)
 {
   nsrefcnt count;
 
@@ -297,18 +147,16 @@ nsTimerImpl::~nsTimerImpl()
 }
 
 //static
 nsresult
 nsTimerImpl::Startup()
 {
   nsresult rv;
 
-  nsTimerEvent::Init();
-
   gThread = new TimerThread();
 
   NS_ADDREF(gThread);
   rv = gThread->InitLocks();
 
   if (NS_FAILED(rv)) {
     NS_RELEASE(gThread);
   }
@@ -331,18 +179,16 @@ nsTimerImpl::Shutdown()
   }
 
   if (!gThread) {
     return;
   }
 
   gThread->Shutdown();
   NS_RELEASE(gThread);
-
-  nsTimerEvent::Shutdown();
 }
 
 
 nsresult
 nsTimerImpl::InitCommon(uint32_t aType, uint32_t aDelay)
 {
   nsresult rv;
 
@@ -657,127 +503,16 @@ nsTimerImpl::Fire()
     // already happened.
     if (gThread) {
       gThread->AddTimer(this);
     }
   }
 }
 
 void
-nsTimerEvent::Init()
-{
-  sAllocator = new TimerEventAllocator();
-}
-
-void
-nsTimerEvent::Shutdown()
-{
-  sCanDeleteAllocator = true;
-  DeleteAllocatorIfNeeded();
-}
-
-void
-nsTimerEvent::DeleteAllocatorIfNeeded()
-{
-  if (sCanDeleteAllocator && sAllocatorUsers == 0) {
-    delete sAllocator;
-    sAllocator = nullptr;
-  }
-}
-
-NS_IMETHODIMP
-nsTimerEvent::Run()
-{
-  if (mGeneration != mTimer->GetGeneration()) {
-    return NS_OK;
-  }
-
-  if (MOZ_LOG_TEST(GetTimerLog(), LogLevel::Debug)) {
-    TimeStamp now = TimeStamp::Now();
-    MOZ_LOG(GetTimerLog(), LogLevel::Debug,
-           ("[this=%p] time between PostTimerEvent() and Fire(): %fms\n",
-            this, (now - mInitTime).ToMilliseconds()));
-  }
-
-  mTimer->Fire();
-  // Since nsTimerImpl is not thread-safe, we should release |mTimer|
-  // here in the target thread to avoid race condition. Otherwise,
-  // ~nsTimerEvent() which calls nsTimerImpl::Release() could run in the
-  // timer thread and result in race condition.
-  mTimer = nullptr;
-
-  return NS_OK;
-}
-
-already_AddRefed<nsTimerImpl>
-nsTimerImpl::PostTimerEvent(already_AddRefed<nsTimerImpl> aTimerRef)
-{
-  nsRefPtr<nsTimerImpl> timer(aTimerRef);
-  if (!timer->mEventTarget) {
-    NS_ERROR("Attempt to post timer event to NULL event target");
-    return timer.forget();
-  }
-
-  // XXX we may want to reuse this nsTimerEvent in the case of repeating timers.
-
-  // Since TimerThread addref'd 'timer' for us, we don't need to addref here.
-  // We will release either in ~nsTimerEvent(), or pass the reference back to
-  // the caller. We need to copy the generation number from this timer into the
-  // event, so we can avoid firing a timer that was re-initialized after being
-  // canceled.
-
-  // Note: We override operator new for this class, and the override is
-  // fallible!
-  nsRefPtr<nsTimerEvent> event = new nsTimerEvent;
-  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.
-  if (timer->IsRepeatingPrecisely()) {
-    timer->SetDelayInternal(timer->mDelay);
-
-    // But only re-arm REPEATING_PRECISE timers.
-    if (gThread && timer->mType == TYPE_REPEATING_PRECISE) {
-      nsresult rv = gThread->AddTimer(timer);
-      if (NS_FAILED(rv)) {
-        return timer.forget();
-      }
-    }
-  }
-
-#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
-
-  nsIEventTarget* target = timer->mEventTarget;
-  event->SetTimer(timer.forget());
-
-  nsresult rv = target->Dispatch(event, NS_DISPATCH_NORMAL);
-  if (NS_FAILED(rv)) {
-    timer = event->ForgetTimer();
-    if (gThread) {
-      gThread->RemoveTimer(timer);
-    }
-    return timer.forget();
-  }
-
-  return nullptr;
-}
-
-void
 nsTimerImpl::SetDelayInternal(uint32_t aDelay)
 {
   TimeDuration delayInterval = TimeDuration::FromMilliseconds(aDelay);
 
   mDelay = aDelay;
 
   TimeStamp now = TimeStamp::Now();
   if (mTimeout.IsNull() || mType != TYPE_REPEATING_PRECISE) {
--- a/xpcom/threads/nsTimerImpl.h
+++ b/xpcom/threads/nsTimerImpl.h
@@ -37,18 +37,18 @@ public:
   typedef mozilla::TimeStamp TimeStamp;
 
   nsTimerImpl();
 
   static nsresult Startup();
   static void Shutdown();
 
   friend class TimerThread;
+  friend class nsTimerEvent;
   friend struct TimerAdditionComparator;
-  friend class nsTimerEvent;
 
   NS_DECL_THREADSAFE_ISUPPORTS
   NS_DECL_NSITIMER
 
   virtual size_t SizeOfIncludingThis(mozilla::MallocSizeOf aMallocSizeOf) const override;
 
 private:
   void SetDelayInternal(uint32_t aDelay);