Bug 1415770 - Simplify handling of IdleRequest list. r=smaug, a=gchang
authorAndreas Farre <farre@mozilla.com>
Thu, 30 Nov 2017 08:44:11 -0500
changeset 445053 aeba274613769f5855b0981a9def0efa68b89067
parent 445052 b6d37f26c25b6f3564031382d6c6ec7fde251000
child 445054 e4d5dee669102f6f271a6eebf749d0f9973fde0b
push id1618
push userCallek@gmail.com
push dateThu, 11 Jan 2018 17:45:48 +0000
treeherdermozilla-release@882ca853e05a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug, gchang
bugs1415770
milestone58.0
Bug 1415770 - Simplify handling of IdleRequest list. r=smaug, a=gchang
dom/base/IdleRequest.cpp
dom/base/IdleRequest.h
dom/base/Timeout.cpp
dom/base/nsGlobalWindow.cpp
dom/base/nsGlobalWindow.h
--- a/dom/base/IdleRequest.cpp
+++ b/dom/base/IdleRequest.cpp
@@ -26,23 +26,31 @@ IdleRequest::IdleRequest(IdleRequestCall
 {
   MOZ_DIAGNOSTIC_ASSERT(mCallback);
 }
 
 IdleRequest::~IdleRequest()
 {
 }
 
-NS_IMPL_CYCLE_COLLECTION(IdleRequest, mCallback)
+NS_IMPL_CYCLE_COLLECTION_CLASS(IdleRequest)
 
-NS_IMPL_CYCLE_COLLECTING_ADDREF(IdleRequest)
-NS_IMPL_CYCLE_COLLECTING_RELEASE(IdleRequest)
+NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(IdleRequest)
+  NS_IMPL_CYCLE_COLLECTION_UNLINK(mCallback)
+  if (tmp->isInList()) {
+    tmp->remove();
+  }
+NS_IMPL_CYCLE_COLLECTION_UNLINK_END
 
-NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(IdleRequest)
-NS_INTERFACE_MAP_END
+NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN(IdleRequest)
+  NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mCallback)
+NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
+
+NS_IMPL_CYCLE_COLLECTION_ROOT_NATIVE(IdleRequest, AddRef)
+NS_IMPL_CYCLE_COLLECTION_UNROOT_NATIVE(IdleRequest, Release)
 
 void
 IdleRequest::SetTimeoutHandle(int32_t aHandle)
 {
   mTimeoutHandle = Some(aHandle);
 }
 
 uint32_t
--- a/dom/base/IdleRequest.h
+++ b/dom/base/IdleRequest.h
@@ -4,32 +4,32 @@
  * 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/. */
 
 #ifndef mozilla_dom_idlerequest_h
 #define mozilla_dom_idlerequest_h
 
 #include "mozilla/LinkedList.h"
 #include "mozilla/Maybe.h"
+#include "mozilla/RefPtr.h"
 #include "nsCOMPtr.h"
 #include "nsCycleCollectionParticipant.h"
 #include "nsDOMNavigationTiming.h"
 #include "nsICancelableRunnable.h"
 #include "nsIRunnable.h"
 #include "nsString.h"
 
 class nsPIDOMWindowInner;
 
 namespace mozilla {
 namespace dom {
 
 class IdleRequestCallback;
 
-class IdleRequest final : public nsISupports,
-                          public LinkedListElement<IdleRequest>
+class IdleRequest final : public LinkedListElement<RefPtr<IdleRequest>>
 {
 public:
   IdleRequest(IdleRequestCallback* aCallback, uint32_t aHandle);
 
   nsresult IdleRun(nsPIDOMWindowInner* aWindow,
                    DOMHighResTimeStamp aDeadline,
                    bool aDidTimeout);
 
@@ -37,19 +37,18 @@ public:
   bool HasTimeout() const { return mTimeoutHandle.isSome(); }
   uint32_t GetTimeoutHandle() const;
 
   uint32_t Handle() const
   {
     return mHandle;
   }
 
-  NS_DECL_CYCLE_COLLECTING_ISUPPORTS
-  NS_DECL_CYCLE_COLLECTION_CLASS(IdleRequest)
-
+  NS_DECL_CYCLE_COLLECTION_NATIVE_CLASS(IdleRequest)
+  NS_INLINE_DECL_CYCLE_COLLECTING_NATIVE_REFCOUNTING(IdleRequest)
 private:
   ~IdleRequest();
 
   RefPtr<IdleRequestCallback> mCallback;
   const uint32_t mHandle;
   mozilla::Maybe<int32_t> mTimeoutHandle;
 };
 
--- a/dom/base/Timeout.cpp
+++ b/dom/base/Timeout.cpp
@@ -24,17 +24,19 @@ Timeout::Timeout()
 {
 }
 
 NS_IMPL_CYCLE_COLLECTION_CLASS(Timeout)
 
 NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(Timeout)
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mWindow)
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mScriptHandler)
-  tmp->remove();
+  if (tmp->isInList()) {
+    tmp->remove();
+  }
 NS_IMPL_CYCLE_COLLECTION_UNLINK_END
 
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN(Timeout)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mWindow)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mScriptHandler)
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
 
 NS_IMPL_CYCLE_COLLECTION_ROOT_NATIVE(Timeout, AddRef)
--- a/dom/base/nsGlobalWindow.cpp
+++ b/dom/base/nsGlobalWindow.cpp
@@ -800,35 +800,26 @@ void
 nsGlobalWindow::ResumeIdleRequests()
 {
   MOZ_ASSERT(!mIdleRequestExecutor);
 
   ScheduleIdleRequestDispatch();
 }
 
 void
-nsGlobalWindow::InsertIdleCallback(IdleRequest* aRequest)
-{
-  AssertIsOnMainThread();
-  mIdleRequestCallbacks.insertBack(aRequest);
-  aRequest->AddRef();
-}
-
-void
 nsGlobalWindow::RemoveIdleCallback(mozilla::dom::IdleRequest* aRequest)
 {
   AssertIsOnMainThread();
 
   if (aRequest->HasTimeout()) {
     mTimeoutManager->ClearTimeout(aRequest->GetTimeoutHandle(),
                                   Timeout::Reason::eIdleCallbackTimeout);
   }
 
   aRequest->removeFrom(mIdleRequestCallbacks);
-  aRequest->Release();
 }
 
 nsresult
 nsGlobalWindow::RunIdleRequest(IdleRequest* aRequest,
                                DOMHighResTimeStamp aDeadline,
                                bool aDidTimeout)
 {
   AssertIsOnMainThread();
@@ -937,18 +928,17 @@ nsGlobalWindow::RequestIdleCallback(JSCo
 
     if (NS_WARN_IF(NS_FAILED(rv))) {
       return 0;
     }
 
     request->SetTimeoutHandle(timeoutHandle);
   }
 
-  // mIdleRequestCallbacks now owns request
-  InsertIdleCallback(request);
+  mIdleRequestCallbacks.insertBack(request);
 
   if (!IsSuspended()) {
     ScheduleIdleRequestDispatch();
   }
 
   return handle;
 }
 
@@ -2362,16 +2352,20 @@ NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(ns
     NS_IMPL_CYCLE_COLLECTION_UNLINK(mOuterWindow)
   }
 
   if (tmp->mListenerManager) {
     tmp->mListenerManager->Disconnect();
     NS_IMPL_CYCLE_COLLECTION_UNLINK(mListenerManager)
   }
 
+  // Here the Timeouts list would've been unlinked, but we rely on
+  // that Timeout objects have been traced and will remove themselves
+  // while unlinking.
+
   tmp->UpdateTopInnerWindow();
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mTopInnerWindow)
 
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mLocation)
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mHistory)
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mCustomElements)
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mLocalStorage)
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mSessionStorage)
@@ -2414,17 +2408,20 @@ NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(ns
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mMozSelfSupport)
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mIntlUtils)
 
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mDocShell)
 
   tmp->UnlinkHostObjectURIs();
 
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mIdleRequestExecutor)
-  tmp->DisableIdleCallbackRequests();
+
+  // Here the IdleRequest list would've been unlinked, but we rely on
+  // that IdleRequest objects have been traced and will remove
+  // themselves while unlinking.
 
   if (tmp->IsChromeWindow()) {
     NS_IMPL_CYCLE_COLLECTION_UNLINK(mChromeFields.mBrowserDOMWindow)
     if (tmp->mChromeFields.mMessageManager) {
       static_cast<nsFrameMessageManager*>(
         tmp->mChromeFields.mMessageManager.get())->Disconnect();
       NS_IMPL_CYCLE_COLLECTION_UNLINK(mChromeFields.mMessageManager)
     }
--- a/dom/base/nsGlobalWindow.h
+++ b/dom/base/nsGlobalWindow.h
@@ -1740,19 +1740,17 @@ public:
   uint32_t LastIdleRequestHandle() const { return mIdleRequestCallbackCounter - 1; }
   nsresult RunIdleRequest(mozilla::dom::IdleRequest* aRequest,
                           DOMHighResTimeStamp aDeadline, bool aDidTimeout);
   nsresult ExecuteIdleRequest(TimeStamp aDeadline);
   void ScheduleIdleRequestDispatch();
   void SuspendIdleRequests();
   void ResumeIdleRequests();
 
-  typedef mozilla::LinkedList<mozilla::dom::IdleRequest> IdleRequests;
-  void InsertIdleCallback(mozilla::dom::IdleRequest* aRequest);
-
+  typedef mozilla::LinkedList<RefPtr<mozilla::dom::IdleRequest>> IdleRequests;
   void RemoveIdleCallback(mozilla::dom::IdleRequest* aRequest);
 
 protected:
   // These members are only used on outer window objects. Make sure
   // you never set any of these on an inner object!
   bool                          mFullScreen : 1;
   bool                          mFullscreenMode : 1;
   bool                          mIsClosed : 1;