Revert part 5 also. It does depends on part 4.

Revert part 5 also. It does depends on part 4.

xpcom/threads/TimerThread.cpp

@@ -359,7 +359,7 @@ TimerThread::Shutdown()
 
   uint32_t timersCount = timers.Length();
   for (uint32_t i = 0; i < timersCount; i++) {
-    RefPtr<nsTimerImpl> timer = timers[i].Take();
+    RefPtr<nsTimerImpl> timer = timers[i].mTimerImpl.forget();
     if (timer) {
       timer->Cancel();
     }
@@ -433,11 +433,14 @@ TimerThread::Run()
     } else {
       waitFor = PR_INTERVAL_NO_TIMEOUT;
       TimeStamp now = TimeStamp::Now();
+	  nsTimerImpl* timer = nullptr;
 
       RemoveLeadingCanceledTimersInternal();
 
       if (!mTimers.IsEmpty()) {
-        if (now >= mTimers[0].Value()->mTimeout || forceRunThisTimer) {
+        timer = mTimers[0].mTimerImpl;
+
+        if (now >= timer->mTimeout || forceRunThisTimer) {
     next:
           // NB: AddRef before the Release under RemoveTimerInternal to avoid
           // mRefCnt passing through zero, in case all other refs than the one
@@ -445,8 +448,9 @@ TimerThread::Run()
           // must be racing with us, blocked in gThread->RemoveTimer waiting
           // for TimerThread::mMonitor, under nsTimerImpl::Release.
 
-          RefPtr<nsTimerImpl> timerRef(mTimers[0].Take());
+          RefPtr<nsTimerImpl> timerRef(timer);
           RemoveFirstTimerInternal();
+		  timer = nullptr;
 
           MOZ_LOG(GetTimerLog(), LogLevel::Debug,
                  ("Timer thread woke up %fms from when it was supposed to\n",
@@ -491,7 +495,9 @@ TimerThread::Run()
       RemoveLeadingCanceledTimersInternal();
 
       if (!mTimers.IsEmpty()) {
-        TimeStamp timeout = mTimers[0].Value()->mTimeout;
+        timer = mTimers[0].mTimerImpl;
+
+        TimeStamp timeout = timer->mTimeout;
 
         // Don't wait at all (even for PR_INTERVAL_NO_WAIT) if the next timer
         // is due now or overdue.
@@ -562,7 +568,7 @@ TimerThread::AddTimer(nsTimerImpl* aTimer)
   }
 
   // Awaken the timer thread.
-  if (mWaiting && mTimers[0].Value() == aTimer) {
+  if (mWaiting && mTimers[0].mTimerImpl == aTimer) {
     mNotified = true;
     mMonitor.Notify();
   }
@@ -669,11 +675,13 @@ TimerThread::AddTimerInternal(nsTimerImpl* aTimer)
 bool
 TimerThread::RemoveTimerInternal(nsTimerImpl* aTimer)
 {
-  if (!aTimer || !aTimer->mHolder) {
-    return false;
+  for (uint32_t i = 0; i < mTimers.Length(); ++i) {
+    if (mTimers[i].mTimerImpl == aTimer) {
+      mTimers[i].mTimerImpl = nullptr;
+      return true;
+    }
   }
-  aTimer->mHolder->Forget(aTimer);
-  return true;
+  return false;
 }
 
 void
@@ -686,7 +694,7 @@ TimerThread::RemoveLeadingCanceledTimersInternal()
   // without actually removing them from the list so we can
   // modify the nsTArray in a single bulk operation.
   auto sortedEnd = mTimers.end();
-  while (sortedEnd != mTimers.begin() && !mTimers[0].Value()) {
+  while (sortedEnd != mTimers.begin() && !mTimers[0].mTimerImpl) {
     std::pop_heap(mTimers.begin(), sortedEnd);
     --sortedEnd;
   }

xpcom/threads/TimerThread.h

@@ -83,32 +83,16 @@ private:
   bool mNotified;
   bool mSleeping;
 
-  class Entry final : public nsTimerImplHolder
+  struct Entry
   {
     TimeStamp mTimeout;
+	RefPtr<nsTimerImpl> mTimerImpl;
 
-  public:
     Entry(const TimeStamp& aMinTimeout, const TimeStamp& aTimeout,
           nsTimerImpl* aTimerImpl)
-      : nsTimerImplHolder(aTimerImpl)
-      , mTimeout(std::max(aMinTimeout, aTimeout))
-    {
-    }
-
-    nsTimerImpl*
-    Value() const
-    {
-      return mTimerImpl;
-    }
-
-    already_AddRefed<nsTimerImpl>
-    Take()
-    {
-      if (mTimerImpl) {
-        mTimerImpl->SetHolder(nullptr);
-      }
-      return mTimerImpl.forget();
-    }
+      : mTimeout(std::max(aMinTimeout, aTimeout)),
+      mTimerImpl(aTimerImpl)
+    { }
 
     Entry(Entry&& aRight) = default;
     Entry& operator=(Entry&& aRight) = default;

xpcom/threads/nsTimerImpl.cpp

@@ -142,7 +142,6 @@ nsTimer::Release(void)
 }
 
 nsTimerImpl::nsTimerImpl(nsITimer* aTimer) :
-  mHolder(nullptr),
   mGeneration(0),
   mDelay(0),
   mITimer(aTimer),
@@ -648,12 +647,6 @@ nsTimerImpl::LogFiring(const Callback& aCallback, uint8_t aType, uint32_t aDelay
   }
 }
 
-void
-nsTimerImpl::SetHolder(nsTimerImplHolder* aHolder)
-{
-  mHolder = aHolder;
-}
-
 nsTimer::~nsTimer()
 {
 }

xpcom/threads/nsTimerImpl.h

@@ -32,17 +32,12 @@ extern mozilla::LogModule* GetTimerLog();
     {0x84, 0x27, 0xfb, 0xab, 0x44, 0xf2, 0x9b, 0xc8} \
 }
 
-class nsTimerImplHolder;
-
 // TimerThread, nsTimerEvent, and nsTimer have references to these. nsTimer has
 // a separate lifecycle so we can Cancel() the underlying timer when the user of
 // the nsTimer has let go of its last reference.
 class nsTimerImpl
 {
-  ~nsTimerImpl()
-  {
-    MOZ_ASSERT(!mHolder);
-  }
+  ~nsTimerImpl() {}
 
 public:
   typedef mozilla::TimeStamp TimeStamp;
@@ -164,8 +159,6 @@ public:
            mType == nsITimer::TYPE_REPEATING_SLACK_LOW_PRIORITY;
   }
 
-  void SetHolder(nsTimerImplHolder* aHolder);
-
   nsCOMPtr<nsIEventTarget> mEventTarget;
 
   void LogFiring(const Callback& aCallback, uint8_t aType, uint32_t aDelay);
@@ -176,10 +169,6 @@ public:
                                       uint32_t aType,
                                       Callback::Name aName);
 
-  // This weak reference must be cleared by the nsTimerImplHolder by calling
-  // SetHolder(nullptr) before the holder is destroyed.
-  nsTimerImplHolder*    mHolder;
-
   // These members are set by the initiating thread, when the timer's type is
   // changed and during the period where it fires on that thread.
   uint8_t               mType;
@@ -231,39 +220,4 @@ private:
   RefPtr<nsTimerImpl> mImpl;
 };
 
-// A class that holds on to an nsTimerImpl.  This lets the nsTimerImpl object
-// directly instruct its holder to forget the timer, avoiding list lookups.
-class nsTimerImplHolder
-{
-public:
-  explicit nsTimerImplHolder(nsTimerImpl* aTimerImpl)
-    : mTimerImpl(aTimerImpl)
-  {
-    if (mTimerImpl) {
-      mTimerImpl->SetHolder(this);
-    }
-  }
-
-  ~nsTimerImplHolder()
-  {
-    if (mTimerImpl) {
-      mTimerImpl->SetHolder(nullptr);
-    }
-  }
-
-  void
-  Forget(nsTimerImpl* aTimerImpl)
-  {
-    if (MOZ_UNLIKELY(!mTimerImpl)) {
-      return;
-    }
-    MOZ_ASSERT(aTimerImpl == mTimerImpl);
-    mTimerImpl->SetHolder(nullptr);
-    mTimerImpl = nullptr;
-  }
-
-protected:
-  RefPtr<nsTimerImpl> mTimerImpl;
-};
-
 #endif /* nsTimerImpl_h___ */