From c9d920e9c630fd8b99b863e034742fd3b38e12d3 Mon Sep 17 00:00:00 2001 From: wolfbeast Date: Sat, 22 Dec 2018 02:15:50 +0100 Subject: Revise lifetime management of IntersectionObservers. Tag #249 --- dom/base/DOMIntersectionObserver.cpp | 18 +++++++++--------- dom/base/DOMIntersectionObserver.h | 6 +++++- dom/base/Element.cpp | 8 ++++---- dom/base/Element.h | 3 ++- dom/base/FragmentOrElement.cpp | 6 ++++++ dom/base/FragmentOrElement.h | 3 ++- dom/base/nsDocument.cpp | 23 +++++++++++++++-------- dom/base/nsDocument.h | 5 +++-- 8 files changed, 46 insertions(+), 26 deletions(-) (limited to 'dom') diff --git a/dom/base/DOMIntersectionObserver.cpp b/dom/base/DOMIntersectionObserver.cpp index e39abf1a6..cb624fb17 100644 --- a/dom/base/DOMIntersectionObserver.cpp +++ b/dom/base/DOMIntersectionObserver.cpp @@ -43,16 +43,18 @@ NS_IMPL_CYCLE_COLLECTION_TRACE_END NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(DOMIntersectionObserver) NS_IMPL_CYCLE_COLLECTION_UNLINK_PRESERVED_WRAPPER + tmp->Disconnect(); NS_IMPL_CYCLE_COLLECTION_UNLINK(mOwner) + NS_IMPL_CYCLE_COLLECTION_UNLINK(mDocument) NS_IMPL_CYCLE_COLLECTION_UNLINK(mCallback) NS_IMPL_CYCLE_COLLECTION_UNLINK(mRoot) NS_IMPL_CYCLE_COLLECTION_UNLINK(mQueuedEntries) - tmp->Disconnect(); NS_IMPL_CYCLE_COLLECTION_UNLINK_END NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN(DOMIntersectionObserver) NS_IMPL_CYCLE_COLLECTION_TRAVERSE_SCRIPT_OBJECTS NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mOwner) + NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mDocument) NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mCallback) NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mRoot) NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mQueuedEntries) @@ -185,10 +187,11 @@ DOMIntersectionObserver::Connect() if (mConnected) { return; } + mConnected = true; - - nsIDocument* document = mOwner->GetExtantDoc(); - document->AddIntersectionObserver(this); + if(mDocument) { + mDocument->AddIntersectionObserver(this); + } } void @@ -202,11 +205,8 @@ DOMIntersectionObserver::Disconnect() target->UnregisterIntersectionObserver(this); } mObservationTargets.Clear(); - if (mOwner) { - nsIDocument* document = mOwner->GetExtantDoc(); - if (document) { - document->RemoveIntersectionObserver(this); - } + if (mDocument) { + mDocument->RemoveIntersectionObserver(this); } mConnected = false; } diff --git a/dom/base/DOMIntersectionObserver.h b/dom/base/DOMIntersectionObserver.h index 8144fc5c5..c290002ea 100644 --- a/dom/base/DOMIntersectionObserver.h +++ b/dom/base/DOMIntersectionObserver.h @@ -106,7 +106,10 @@ class DOMIntersectionObserver final : public nsISupports, public: DOMIntersectionObserver(already_AddRefed&& aOwner, mozilla::dom::IntersectionCallback& aCb) - : mOwner(aOwner), mCallback(&aCb), mConnected(false) + : mOwner(aOwner) + , mDocument(mOwner->GetExtantDoc()) + , mCallback(&aCb) + , mConnected(false) { } NS_DECL_CYCLE_COLLECTING_ISUPPORTS @@ -164,6 +167,7 @@ protected: double aIntersectionRatio); nsCOMPtr mOwner; + RefPtr mDocument; RefPtr mCallback; RefPtr mRoot; nsCSSRect mRootMargin; diff --git a/dom/base/Element.cpp b/dom/base/Element.cpp index 3760dc43f..67759fdb2 100644 --- a/dom/base/Element.cpp +++ b/dom/base/Element.cpp @@ -3947,7 +3947,7 @@ Element::ClearDataset() slots->mDataset = nullptr; } -nsDataHashtable, int32_t>* +nsDataHashtable, int32_t>* Element::RegisteredIntersectionObservers() { nsDOMSlots* slots = DOMSlots(); @@ -3962,7 +3962,7 @@ enum nsPreviousIntersectionThreshold { void Element::RegisterIntersectionObserver(DOMIntersectionObserver* aObserver) { - nsDataHashtable, int32_t>* observers = + nsDataHashtable, int32_t>* observers = RegisteredIntersectionObservers(); if (observers->Contains(aObserver)) { return; @@ -3979,7 +3979,7 @@ Element::RegisterIntersectionObserver(DOMIntersectionObserver* aObserver) void Element::UnregisterIntersectionObserver(DOMIntersectionObserver* aObserver) { - nsDataHashtable, int32_t>* observers = + nsDataHashtable, int32_t>* observers = RegisteredIntersectionObservers(); observers->Remove(aObserver); } @@ -3987,7 +3987,7 @@ Element::UnregisterIntersectionObserver(DOMIntersectionObserver* aObserver) bool Element::UpdateIntersectionObservation(DOMIntersectionObserver* aObserver, int32_t aThreshold) { - nsDataHashtable, int32_t>* observers = + nsDataHashtable, int32_t>* observers = RegisteredIntersectionObservers(); if (!observers->Contains(aObserver)) { return false; diff --git a/dom/base/Element.h b/dom/base/Element.h index c269ab14a..ef57a6466 100644 --- a/dom/base/Element.h +++ b/dom/base/Element.h @@ -1382,7 +1382,8 @@ protected: nsDOMTokenList* GetTokenList(nsIAtom* aAtom, const DOMTokenListSupportedTokenArray aSupportedTokens = nullptr); - nsDataHashtable, int32_t>* RegisteredIntersectionObservers(); + nsDataHashtable, int32_t>* + RegisteredIntersectionObservers(); private: /** diff --git a/dom/base/FragmentOrElement.cpp b/dom/base/FragmentOrElement.cpp index fde983e7c..13ba19c8c 100644 --- a/dom/base/FragmentOrElement.cpp +++ b/dom/base/FragmentOrElement.cpp @@ -585,6 +585,12 @@ FragmentOrElement::nsDOMSlots::Traverse(nsCycleCollectionTraversalCallback &cb, mCustomElementData->mCallbackQueue[i]->Traverse(cb); } } + + for (auto iter = mRegisteredIntersectionObservers.Iter(); !iter.Done(); iter.Next()) { + DOMIntersectionObserver* observer = iter.Key(); + NS_CYCLE_COLLECTION_NOTE_EDGE_NAME(cb, "mSlots->mRegisteredIntersectionObservers[i]"); + cb.NoteXPCOMChild(observer); + } } void diff --git a/dom/base/FragmentOrElement.h b/dom/base/FragmentOrElement.h index 7c74e9cd4..f0cc29f22 100644 --- a/dom/base/FragmentOrElement.h +++ b/dom/base/FragmentOrElement.h @@ -354,7 +354,8 @@ public: /** * Registered Intersection Observers on the element. */ - nsDataHashtable, int32_t> mRegisteredIntersectionObservers; + nsDataHashtable, int32_t> + mRegisteredIntersectionObservers; }; protected: diff --git a/dom/base/nsDocument.cpp b/dom/base/nsDocument.cpp index ac9601caf..e779c060c 100644 --- a/dom/base/nsDocument.cpp +++ b/dom/base/nsDocument.cpp @@ -1736,8 +1736,6 @@ NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN_INTERNAL(nsDocument) NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mOnDemandBuiltInUASheets) NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mPreloadingImages) - NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mIntersectionObservers) - NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mSubImportLinks) for (uint32_t i = 0; i < tmp->mFrameRequestCallbacks.Length(); ++i) { @@ -12375,15 +12373,15 @@ nsDocument::ReportUseCounters() void nsDocument::AddIntersectionObserver(DOMIntersectionObserver* aObserver) { - NS_ASSERTION(mIntersectionObservers.IndexOf(aObserver) == nsTArray::NoIndex, - "Intersection observer already in the list"); - mIntersectionObservers.AppendElement(aObserver); + MOZ_ASSERT(!mIntersectionObservers.Contains(aObserver), + "Intersection observer already in the list"); + mIntersectionObservers.PutEntry(aObserver); } void nsDocument::RemoveIntersectionObserver(DOMIntersectionObserver* aObserver) { - mIntersectionObservers.RemoveElement(aObserver); + mIntersectionObservers.RemoveEntry(aObserver); } void @@ -12396,7 +12394,8 @@ nsDocument::UpdateIntersectionObservations() time = perf->Now(); } } - for (const auto& observer : mIntersectionObservers) { + for (auto iter = mIntersectionObservers.Iter(); !iter.Done(); iter.Next()) { + DOMIntersectionObserver* observer = iter.Get()->GetKey(); observer->Update(this, time); } } @@ -12404,6 +12403,10 @@ nsDocument::UpdateIntersectionObservations() void nsDocument::ScheduleIntersectionObserverNotification() { + if (mIntersectionObservers.IsEmpty()) { + return; + } + nsCOMPtr notification = NewRunnableMethod(this, &nsDocument::NotifyIntersectionObservers); NS_DispatchToCurrentThread(notification); @@ -12412,7 +12415,11 @@ nsDocument::ScheduleIntersectionObserverNotification() void nsDocument::NotifyIntersectionObservers() { - nsTArray> observers(mIntersectionObservers); + nsTArray> observers(mIntersectionObservers.Count()); + for (auto iter = mIntersectionObservers.Iter(); !iter.Done(); iter.Next()) { + DOMIntersectionObserver* observer = iter.Get()->GetKey(); + observers.AppendElement(observer); + } for (const auto& observer : observers) { observer->Notify(); } diff --git a/dom/base/nsDocument.h b/dom/base/nsDocument.h index 3725b3c18..d2f97a33e 100644 --- a/dom/base/nsDocument.h +++ b/dom/base/nsDocument.h @@ -1341,8 +1341,9 @@ protected: // Array of observers nsTObserverArray mObservers; - // Array of intersection observers - nsTArray> mIntersectionObservers; + // Hashtable of intersection observers + nsTHashtable> + mIntersectionObservers; // Tracker for animations that are waiting to start. // nullptr until GetOrCreatePendingAnimationTracker is called. -- cgit v1.2.3