From ed65a2bfc8a4a3548a242cf8f65fa02b11b3b58d Mon Sep 17 00:00:00 2001 From: Olli Pettay Date: Fri, 4 May 2018 19:25:05 +0300 Subject: [PATCH] bug 1457867, store DOMEventTargetHelper objects in global object as a linked list, r=bkelly --HG-- extra : rebase_source : 415c03249bae1edc4313bcf15f91716a748565f9 --- dom/animation/AnimationTimeline.cpp | 4 ++-- dom/animation/DocumentTimeline.cpp | 3 ++- dom/base/nsDocument.cpp | 6 ++++-- dom/base/nsIGlobalObject.cpp | 23 ++++++++++++----------- dom/base/nsIGlobalObject.h | 7 +++---- dom/events/DOMEventTargetHelper.h | 6 ++++-- layout/base/nsPresContext.cpp | 3 ++- layout/style/MediaQueryList.cpp | 2 +- 8 files changed, 30 insertions(+), 24 deletions(-) diff --git a/dom/animation/AnimationTimeline.cpp b/dom/animation/AnimationTimeline.cpp index e99ae54cce13..acfa88b84105 100644 --- a/dom/animation/AnimationTimeline.cpp +++ b/dom/animation/AnimationTimeline.cpp @@ -48,8 +48,8 @@ void AnimationTimeline::RemoveAnimation(Animation* aAnimation) { MOZ_ASSERT(!aAnimation->GetTimeline() || aAnimation->GetTimeline() == this); - if (aAnimation->isInList()) { - aAnimation->remove(); + if (static_cast*>(aAnimation)->isInList()) { + static_cast*>(aAnimation)->remove(); } mAnimations.RemoveEntry(aAnimation); } diff --git a/dom/animation/DocumentTimeline.cpp b/dom/animation/DocumentTimeline.cpp index 2d729ec6c6a2..42d0a603d1bd 100644 --- a/dom/animation/DocumentTimeline.cpp +++ b/dom/animation/DocumentTimeline.cpp @@ -167,7 +167,8 @@ DocumentTimeline::WillRefresh(mozilla::TimeStamp aTime) nsAutoAnimationMutationBatch mb(mDocument); for (Animation* animation = mAnimationOrder.getFirst(); animation; - animation = animation->getNext()) { + animation = + static_cast*>(animation)->getNext()) { // Skip any animations that are longer need associated with this timeline. if (animation->GetTimeline() != this) { // If animation has some other timeline, it better not be also in the diff --git a/dom/base/nsDocument.cpp b/dom/base/nsDocument.cpp index 17fcd8ac1b0f..cd559c364be8 100644 --- a/dom/base/nsDocument.cpp +++ b/dom/base/nsDocument.cpp @@ -1959,7 +1959,8 @@ NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN_INTERNAL(nsDocument) // We own only the items in mDOMMediaQueryLists that have listeners; // this reference is managed by their AddListener and RemoveListener // methods. - for (auto mql : tmp->mDOMMediaQueryLists) { + for (MediaQueryList* mql = tmp->mDOMMediaQueryLists.getFirst(); mql; + mql = static_cast*>(mql)->getNext()) { if (mql->HasListeners()) { NS_CYCLE_COLLECTION_NOTE_EDGE_NAME(cb, "mDOMMediaQueryLists item"); cb.NoteXPCOMChild(mql); @@ -2079,7 +2080,8 @@ NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(nsDocument) // this reference is managed by their AddListener and RemoveListener // methods. for (MediaQueryList* mql = tmp->mDOMMediaQueryLists.getFirst(); mql;) { - MediaQueryList* next = mql->getNext(); + MediaQueryList* next = + static_cast*>(mql)->getNext(); mql->Disconnect(); mql = next; } diff --git a/dom/base/nsIGlobalObject.cpp b/dom/base/nsIGlobalObject.cpp index 53abc02386ff..7d2125d4d391 100644 --- a/dom/base/nsIGlobalObject.cpp +++ b/dom/base/nsIGlobalObject.cpp @@ -25,7 +25,7 @@ nsIGlobalObject::~nsIGlobalObject() { UnlinkHostObjectURIs(); DisconnectEventTargetObjects(); - MOZ_DIAGNOSTIC_ASSERT(mEventTargetObjects.IsEmpty()); + MOZ_DIAGNOSTIC_ASSERT(mEventTargetObjects.isEmpty()); } nsIPrincipal* @@ -131,27 +131,29 @@ void nsIGlobalObject::AddEventTargetObject(DOMEventTargetHelper* aObject) { MOZ_DIAGNOSTIC_ASSERT(aObject); - MOZ_ASSERT(!mEventTargetObjects.Contains(aObject)); - mEventTargetObjects.PutEntry(aObject); + MOZ_ASSERT(!aObject->isInList()); + mEventTargetObjects.insertBack(aObject); } void nsIGlobalObject::RemoveEventTargetObject(DOMEventTargetHelper* aObject) { MOZ_DIAGNOSTIC_ASSERT(aObject); - MOZ_ASSERT(mEventTargetObjects.Contains(aObject)); - mEventTargetObjects.RemoveEntry(aObject); + MOZ_ASSERT(aObject->isInList()); + MOZ_ASSERT(aObject->GetOwnerGlobal() == this); + aObject->remove(); } void nsIGlobalObject::ForEachEventTargetObject(const std::function& aFunc) const { - // Protect against the function call triggering a mutation of the hash table + // Protect against the function call triggering a mutation of the list // while we are iterating by copying the DETH references to a temporary // list. AutoTArray targetList; - for (auto iter = mEventTargetObjects.ConstIter(); !iter.Done(); iter.Next()) { - targetList.AppendElement(iter.Get()->GetKey()); + for (const DOMEventTargetHelper* deth = mEventTargetObjects.getFirst(); + deth; deth = deth->getNext()) { + targetList.AppendElement(const_cast(deth)); } // Iterate the target list and call the function on each one. @@ -159,7 +161,7 @@ nsIGlobalObject::ForEachEventTargetObject(const std::functionGetOwnerGlobal() != this) { continue; } aFunc(target, &done); @@ -177,7 +179,7 @@ nsIGlobalObject::DisconnectEventTargetObjects() // Calling DisconnectFromOwner() should result in // RemoveEventTargetObject() being called. - MOZ_DIAGNOSTIC_ASSERT(!mEventTargetObjects.Contains(aTarget)); + MOZ_DIAGNOSTIC_ASSERT(aTarget->GetOwnerGlobal() != this); }); } @@ -215,6 +217,5 @@ size_t nsIGlobalObject::ShallowSizeOfExcludingThis(MallocSizeOf aSizeOf) const { size_t rtn = mHostObjectURIs.ShallowSizeOfExcludingThis(aSizeOf); - rtn += mEventTargetObjects.ShallowSizeOfExcludingThis(aSizeOf); return rtn; } diff --git a/dom/base/nsIGlobalObject.h b/dom/base/nsIGlobalObject.h index f4aca14af7d2..bd0f3e428792 100644 --- a/dom/base/nsIGlobalObject.h +++ b/dom/base/nsIGlobalObject.h @@ -7,6 +7,7 @@ #ifndef nsIGlobalObject_h__ #define nsIGlobalObject_h__ +#include "mozilla/LinkedList.h" #include "mozilla/Maybe.h" #include "mozilla/dom/ClientInfo.h" #include "mozilla/dom/DispatcherTrait.h" @@ -41,10 +42,8 @@ class nsIGlobalObject : public nsISupports, nsTArray mHostObjectURIs; // Raw pointers to bound DETH objects. These are added by - // AddEventTargetObject(). The DETH object must call - // RemoveEventTargetObject() before its destroyed to clear - // its raw pointer here. - nsTHashtable> mEventTargetObjects; + // AddEventTargetObject(). + mozilla::LinkedList mEventTargetObjects; bool mIsDying; diff --git a/dom/events/DOMEventTargetHelper.h b/dom/events/DOMEventTargetHelper.h index 0dd6e2c8dacd..cf154ed0fde4 100644 --- a/dom/events/DOMEventTargetHelper.h +++ b/dom/events/DOMEventTargetHelper.h @@ -17,6 +17,7 @@ #include "MainThreadUtils.h" #include "mozilla/Attributes.h" #include "mozilla/EventListenerManager.h" +#include "mozilla/LinkedList.h" #include "mozilla/dom/EventTarget.h" struct JSCompartment; @@ -34,7 +35,8 @@ class Event; { 0xa28385c6, 0x9451, 0x4d7e, \ { 0xa3, 0xdd, 0xf4, 0xb6, 0x87, 0x2f, 0xa4, 0x76 } } -class DOMEventTargetHelper : public dom::EventTarget +class DOMEventTargetHelper : public dom::EventTarget, + public LinkedListElement { public: DOMEventTargetHelper() @@ -172,7 +174,7 @@ public: virtual void DisconnectFromOwner(); using EventTarget::GetParentObject; - virtual nsIGlobalObject* GetOwnerGlobal() const override + virtual nsIGlobalObject* GetOwnerGlobal() const final { return mParentObject; } diff --git a/layout/base/nsPresContext.cpp b/layout/base/nsPresContext.cpp index e79583de0c33..54a19a629c2f 100644 --- a/layout/base/nsPresContext.cpp +++ b/layout/base/nsPresContext.cpp @@ -2084,7 +2084,8 @@ nsPresContext::FlushPendingMediaFeatureValuesChanged() // Copy pointers to all the lists into a new array, in case one of our // notifications modifies the list. nsTArray> localMediaQueryLists; - for (auto* mql : mDocument->MediaQueryLists()) { + for (MediaQueryList* mql = mDocument->MediaQueryLists().getFirst(); mql; + mql = static_cast*>(mql)->getNext()) { localMediaQueryLists.AppendElement(mql); } diff --git a/layout/style/MediaQueryList.cpp b/layout/style/MediaQueryList.cpp index dc7a4c7e9e5f..d2eaab49f18f 100644 --- a/layout/style/MediaQueryList.cpp +++ b/layout/style/MediaQueryList.cpp @@ -45,7 +45,7 @@ NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN_INHERITED(MediaQueryList, DOMEventTargetHelper) if (tmp->mDocument) { - tmp->remove(); + static_cast*>(tmp)->remove(); NS_IMPL_CYCLE_COLLECTION_UNLINK(mDocument) } tmp->Disconnect();