Bug 1337936 - (intersection-observer) Revise lifetime management. r=smaug
☠☠ backed out by 6e1c2cb7b5ba ☠ ☠
authorTobias Schneider <schneider@jancona.com>
Tue, 21 Feb 2017 03:13:39 -0800
changeset 373261 5f93d62d9229e1736486f1e3e41624caf7f2b2af
parent 373173 8a6084bc234ceb6036af6c660c5162cde5eaf047
child 373262 139294baffa2bf634f8f91d77b6b14112ec3f331
push id10863
push userjlorenzo@mozilla.com
push dateMon, 06 Mar 2017 23:02:23 +0000
treeherdermozilla-aurora@0931190cd725 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1337936
milestone54.0a1
Bug 1337936 - (intersection-observer) Revise lifetime management. r=smaug MozReview-Commit-ID: AvdDJaRELXm
dom/base/DOMIntersectionObserver.cpp
dom/base/DOMIntersectionObserver.h
dom/base/Element.cpp
dom/base/Element.h
dom/base/FragmentOrElement.cpp
dom/base/FragmentOrElement.h
dom/base/nsDocument.cpp
dom/base/nsDocument.h
--- a/dom/base/DOMIntersectionObserver.cpp
+++ b/dom/base/DOMIntersectionObserver.cpp
@@ -38,25 +38,27 @@ NS_IMPL_CYCLE_COLLECTING_RELEASE(DOMInte
 NS_IMPL_CYCLE_COLLECTION_CLASS(DOMIntersectionObserver)
 
 NS_IMPL_CYCLE_COLLECTION_TRACE_BEGIN(DOMIntersectionObserver)
   NS_IMPL_CYCLE_COLLECTION_TRACE_PRESERVED_WRAPPER
 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(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)
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
 
 already_AddRefed<DOMIntersectionObserver>
 DOMIntersectionObserver::Constructor(const mozilla::dom::GlobalObject& aGlobal,
                                      mozilla::dom::IntersectionCallback& aCb,
@@ -184,38 +186,36 @@ DOMIntersectionObserver::UnlinkTarget(El
 void
 DOMIntersectionObserver::Connect()
 {
   if (mConnected) {
     return;
   }
 
   mConnected = true;
-  nsIDocument* document = mOwner->GetExtantDoc();
-  document->AddIntersectionObserver(this);
+  if (mDocument) {
+    mDocument->AddIntersectionObserver(this);
+  }
 }
 
 void
 DOMIntersectionObserver::Disconnect()
 {
   if (!mConnected) {
     return;
   }
 
   mConnected = false;
   for (auto iter = mObservationTargets.Iter(); !iter.Done(); iter.Next()) {
     Element* target = iter.Get()->GetKey();
     target->UnregisterIntersectionObserver(this);
   }
   mObservationTargets.Clear();
-  if (mOwner) {
-    nsIDocument* document = mOwner->GetExtantDoc();
-    if (document) {
-      document->RemoveIntersectionObserver(this);
-    }
+  if (mDocument) {
+    mDocument->RemoveIntersectionObserver(this);
   }
 }
 
 void
 DOMIntersectionObserver::TakeRecords(nsTArray<RefPtr<DOMIntersectionObserverEntry>>& aRetVal)
 {
   aRetVal.SwapElements(mQueuedEntries);
   mQueuedEntries.Clear();
--- a/dom/base/DOMIntersectionObserver.h
+++ b/dom/base/DOMIntersectionObserver.h
@@ -103,17 +103,17 @@ class DOMIntersectionObserver final : pu
 {
   virtual ~DOMIntersectionObserver() {
     Disconnect();
   }
 
 public:
   DOMIntersectionObserver(already_AddRefed<nsPIDOMWindowInner>&& 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
   NS_DECL_CYCLE_COLLECTION_SCRIPT_HOLDER_CLASS(DOMIntersectionObserver)
   NS_DECLARE_STATIC_IID_ACCESSOR(NS_DOM_INTERSECTION_OBSERVER_IID)
 
   static already_AddRefed<DOMIntersectionObserver>
   Constructor(const mozilla::dom::GlobalObject& aGlobal,
@@ -161,16 +161,17 @@ protected:
   void QueueIntersectionObserverEntry(Element* aTarget,
                                       DOMHighResTimeStamp time,
                                       const Maybe<nsRect>& aRootRect,
                                       const nsRect& aTargetRect,
                                       const Maybe<nsRect>& aIntersectionRect,
                                       double aIntersectionRatio);
 
   nsCOMPtr<nsPIDOMWindowInner>                    mOwner;
+  RefPtr<nsIDocument>                             mDocument;
   RefPtr<mozilla::dom::IntersectionCallback>      mCallback;
   RefPtr<Element>                                 mRoot;
   nsCSSRect                                       mRootMargin;
   nsTArray<double>                                mThresholds;
   nsTHashtable<nsPtrHashKey<Element>>             mObservationTargets;
   nsTArray<RefPtr<DOMIntersectionObserverEntry>>  mQueuedEntries;
   bool                                            mConnected;
 };
--- a/dom/base/Element.cpp
+++ b/dom/base/Element.cpp
@@ -3846,46 +3846,46 @@ Element::ClearDataset()
 {
   nsDOMSlots *slots = GetExistingDOMSlots();
 
   MOZ_ASSERT(slots && slots->mDataset,
              "Slots should exist and dataset should not be null.");
   slots->mDataset = nullptr;
 }
 
-nsDataHashtable<nsPtrHashKey<DOMIntersectionObserver>, int32_t>*
+nsDataHashtable<nsRefPtrHashKey<DOMIntersectionObserver>, int32_t>*
 Element::RegisteredIntersectionObservers()
 {
   nsDOMSlots* slots = DOMSlots();
   return &slots->mRegisteredIntersectionObservers;
 }
 
 void
 Element::RegisterIntersectionObserver(DOMIntersectionObserver* aObserver)
 {
-  nsDataHashtable<nsPtrHashKey<DOMIntersectionObserver>, int32_t>* observers =
+  nsDataHashtable<nsRefPtrHashKey<DOMIntersectionObserver>, int32_t>* observers =
     RegisteredIntersectionObservers();
   if (observers->Contains(aObserver)) {
     return;
   }
   RegisteredIntersectionObservers()->Put(aObserver, -1);
 }
 
 void
 Element::UnregisterIntersectionObserver(DOMIntersectionObserver* aObserver)
 {
-  nsDataHashtable<nsPtrHashKey<DOMIntersectionObserver>, int32_t>* observers =
+  nsDataHashtable<nsRefPtrHashKey<DOMIntersectionObserver>, int32_t>* observers =
     RegisteredIntersectionObservers();
   observers->Remove(aObserver);
 }
 
 bool
 Element::UpdateIntersectionObservation(DOMIntersectionObserver* aObserver, int32_t aThreshold)
 {
-  nsDataHashtable<nsPtrHashKey<DOMIntersectionObserver>, int32_t>* observers =
+  nsDataHashtable<nsRefPtrHashKey<DOMIntersectionObserver>, int32_t>* observers =
     RegisteredIntersectionObservers();
   if (!observers->Contains(aObserver)) {
     return false;
   }
   int32_t previousThreshold = observers->Get(aObserver);
   if (previousThreshold != aThreshold) {
     observers->Put(aObserver, aThreshold);
     return true;
--- a/dom/base/Element.h
+++ b/dom/base/Element.h
@@ -1461,17 +1461,18 @@ protected:
    * the value of xlink:show, converted to a suitably equivalent named target
    * (e.g. _blank).
    */
   virtual void GetLinkTarget(nsAString& aTarget);
 
   nsDOMTokenList* GetTokenList(nsIAtom* aAtom,
                                const DOMTokenListSupportedTokenArray aSupportedTokens = nullptr);
 
-  nsDataHashtable<nsPtrHashKey<DOMIntersectionObserver>, int32_t>* RegisteredIntersectionObservers();
+  nsDataHashtable<nsRefPtrHashKey<DOMIntersectionObserver>, int32_t>*
+    RegisteredIntersectionObservers();
 
 private:
   /**
    * Hook for implementing GetClasses.  This is guaranteed to only be
    * called if the NODE_MAY_HAVE_CLASS flag is set.
    */
   const nsAttrValue* DoGetClasses() const;
 
--- a/dom/base/FragmentOrElement.cpp
+++ b/dom/base/FragmentOrElement.cpp
@@ -621,16 +621,22 @@ FragmentOrElement::nsDOMSlots::Traverse(
   NS_CYCLE_COLLECTION_NOTE_EDGE_NAME(cb, "mSlots->mClassList");
   cb.NoteXPCOMChild(mClassList.get());
 
   if (mCustomElementData) {
     for (uint32_t i = 0; i < mCustomElementData->mCallbackQueue.Length(); i++) {
       mCustomElementData->mCallbackQueue[i]->Traverse(cb);
     }
   }
+
+  for (auto iter = mRegisteredIntersectionObservers.Iter(); !iter.Done(); iter.Next()) {
+    RefPtr<DOMIntersectionObserver> observer = iter.Key();
+    NS_CYCLE_COLLECTION_NOTE_EDGE_NAME(cb, "mSlots->mRegisteredIntersectionObservers[i]");
+    cb.NoteXPCOMChild(observer.get());
+  }
 }
 
 void
 FragmentOrElement::nsDOMSlots::Unlink(bool aIsXUL)
 {
   mStyle = nullptr;
   mSMILOverrideStyle = nullptr;
   if (mAttributeMap) {
--- a/dom/base/FragmentOrElement.h
+++ b/dom/base/FragmentOrElement.h
@@ -343,17 +343,18 @@ public:
     /**
      * Web components custom element data.
      */
     RefPtr<CustomElementData> mCustomElementData;
 
     /**
      * Registered Intersection Observers on the element.
      */
-    nsDataHashtable<nsPtrHashKey<DOMIntersectionObserver>, int32_t> mRegisteredIntersectionObservers;
+    nsDataHashtable<nsRefPtrHashKey<DOMIntersectionObserver>, int32_t>
+      mRegisteredIntersectionObservers;
   };
 
 protected:
   void GetMarkup(bool aIncludeSelf, nsAString& aMarkup);
   void SetInnerHTMLInternal(const nsAString& aInnerHTML, ErrorResult& aError);
 
   // Override from nsINode
   virtual nsINode::nsSlots* CreateSlots() override;
--- a/dom/base/nsDocument.cpp
+++ b/dom/base/nsDocument.cpp
@@ -1827,18 +1827,16 @@ NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN_
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mChildrenCollection)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mAnonymousContents)
 
   // Traverse all our nsCOMArrays.
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mStyleSheets)
   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) {
     NS_CYCLE_COLLECTION_NOTE_EDGE_NAME(cb, "mFrameRequestCallbacks[i]");
     cb.NoteXPCOMChild(tmp->mFrameRequestCallbacks[i].mCallback);
   }
 
   // Traverse animation components
@@ -12589,64 +12587,68 @@ nsDocument::ReportUseCounters(UseCounter
       }
     }
   }
 }
 
 void
 nsDocument::AddIntersectionObserver(DOMIntersectionObserver* aObserver)
 {
-  NS_ASSERTION(mIntersectionObservers.IndexOf(aObserver) == nsTArray<int>::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
 nsDocument::UpdateIntersectionObservations()
 {
   if (mIntersectionObservers.IsEmpty()) {
     return;
   }
 
   DOMHighResTimeStamp time = 0;
   if (nsPIDOMWindowInner* window = GetInnerWindow()) {
     Performance* perf = window->GetPerformance();
     if (perf) {
       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);
   }
 }
 
 void
 nsDocument::ScheduleIntersectionObserverNotification()
 {
   if (mIntersectionObservers.IsEmpty()) {
     return;
   }
-
   MOZ_RELEASE_ASSERT(NS_IsMainThread());
   nsCOMPtr<nsIRunnable> notification =
     NewRunnableMethod(this, &nsDocument::NotifyIntersectionObservers);
   Dispatch("nsDocument::IntersectionObserverNotification", TaskCategory::Other,
            notification.forget());
 }
 
 void
 nsDocument::NotifyIntersectionObservers()
 {
-  nsTArray<RefPtr<DOMIntersectionObserver>> observers(mIntersectionObservers);
+  nsTArray<RefPtr<DOMIntersectionObserver>> 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();
   }
 }
 
 static bool
 NotifyLayerManagerRecreatedCallback(nsIDocument* aDocument, void* aData)
 {
--- a/dom/base/nsDocument.h
+++ b/dom/base/nsDocument.h
@@ -1354,17 +1354,18 @@ protected:
   nsTArray<RefPtr<mozilla::StyleSheet>> mStyleSheets;
   nsTArray<RefPtr<mozilla::StyleSheet>> mOnDemandBuiltInUASheets;
   nsTArray<RefPtr<mozilla::StyleSheet>> mAdditionalSheets[AdditionalSheetTypeCount];
 
   // Array of observers
   nsTObserverArray<nsIDocumentObserver*> mObservers;
 
   // Array of intersection observers
-  nsTArray<RefPtr<mozilla::dom::DOMIntersectionObserver>> mIntersectionObservers;
+  nsTHashtable<nsPtrHashKey<mozilla::dom::DOMIntersectionObserver>>
+    mIntersectionObservers;
 
   // Tracker for animations that are waiting to start.
   // nullptr until GetOrCreatePendingAnimationTracker is called.
   RefPtr<mozilla::PendingAnimationTracker> mPendingAnimationTracker;
 
   // Weak reference to the scope object (aka the script global object)
   // that, unlike mScriptGlobalObject, is never unset once set. This
   // is a weak reference to avoid leaks due to circular references.