Rewrite IntersectionObserver list handling to be more robust.

Tag #935.

3 files changed, 40 insertions, 8 deletions

diff --git a/dom/base/DOMIntersectionObserver.cpp b/dom/base/DOMIntersectionObserver.cpp
index 0264a105e..e671b7da9 100644
--- a/dom/base/DOMIntersectionObserver.cpp
+++ b/dom/base/DOMIntersectionObserver.cpp
@@ -162,6 +162,11 @@ DOMIntersectionObserver::Observe(Element& aTarget)
 void
 DOMIntersectionObserver::Unobserve(Element& aTarget)
 {
+  if (!mObservationTargets.Contains(&aTarget)) {
+    // You're not on the list, buddy!
+    return;
+  }
+  
   if (mObservationTargets.Length() == 1) {
     Disconnect();
     return;
@@ -188,7 +193,7 @@ DOMIntersectionObserver::Connect()
   }
   
   mConnected = true;
-  if(mDocument) {
+  if (mDocument) {
     mDocument->AddIntersectionObserver(this);
   }
 }
@@ -293,12 +298,25 @@ DOMIntersectionObserver::Update(nsIDocument* aDocument, DOMHighResTimeStamp time
       if (rootFrame) {
         nsPresContext* presContext = rootFrame->PresContext();
         while (!presContext->IsRootContentDocument()) {
-          presContext = rootFrame->PresContext()->GetParentPresContext();
-          rootFrame = presContext->PresShell()->GetRootScrollFrame();
+          // Walk up the tree
+          presContext = presContext->GetParentPresContext();
+          if (!presContext) {
+            break;
+          }
+          nsIFrame* rootScrollFrame = presContext->PresShell()->GetRootScrollFrame();
+          if (rootScrollFrame) {
+            rootFrame = rootScrollFrame;
+          } else {
+            break;
+          }
         }
         root = rootFrame->GetContent()->AsElement();
         nsIScrollableFrame* scrollFrame = do_QueryFrame(rootFrame);
-        rootRect = scrollFrame->GetScrollPortRect();
+        // If we end up with a null root frame for some reason, we'll proceed
+        // with an empty root intersection rect.
+        if (scrollFrame) {
+          rootRect = scrollFrame->GetScrollPortRect();
+        }
       }
     }
   }
diff --git a/dom/base/DOMIntersectionObserver.h b/dom/base/DOMIntersectionObserver.h
index 414f21482..8674fe25d 100644
--- a/dom/base/DOMIntersectionObserver.h
+++ b/dom/base/DOMIntersectionObserver.h
@@ -109,7 +109,9 @@ protected:
 class DOMIntersectionObserver final : public nsISupports,
                                       public nsWrapperCache
 {
-  virtual ~DOMIntersectionObserver() { }
+  virtual ~DOMIntersectionObserver() {
+    Disconnect();
+  }
 
 public:
   DOMIntersectionObserver(already_AddRefed<nsPIDOMWindowInner>&& aOwner,
diff --git a/dom/base/nsDocument.cpp b/dom/base/nsDocument.cpp
index 14de65cbd..d4d488379 100644
--- a/dom/base/nsDocument.cpp
+++ b/dom/base/nsDocument.cpp
@@ -12313,6 +12313,10 @@ nsDocument::RemoveIntersectionObserver(DOMIntersectionObserver* aObserver)
 void
 nsDocument::UpdateIntersectionObservations()
 {
+  if (mIntersectionObservers.IsEmpty()) {
+    return;
+  }
+
   DOMHighResTimeStamp time = 0;
   if (nsPIDOMWindowInner* window = GetInnerWindow()) {
     Performance* perf = window->GetPerformance();
@@ -12320,9 +12324,15 @@ nsDocument::UpdateIntersectionObservations()
       time = perf->Now();
     }
   }
+  nsTArray<RefPtr<DOMIntersectionObserver>> observers(mIntersectionObservers.Count());
   for (auto iter = mIntersectionObservers.Iter(); !iter.Done(); iter.Next()) {
     DOMIntersectionObserver* observer = iter.Get()->GetKey();
-    observer->Update(this, time);
+    observers.AppendElement(observer);
+  }
+  for (const auto& observer : observers) {
+    if (observer) {
+      observer->Update(this, time);
+    }
   }
 }
 
@@ -12335,7 +12345,7 @@ nsDocument::ScheduleIntersectionObserverNotification()
 
   nsCOMPtr<nsIRunnable> notification = NewRunnableMethod(this,
     &nsDocument::NotifyIntersectionObservers);
-  NS_DispatchToCurrentThread(notification);
+  NS_DispatchToCurrentThread(notification.forget());
 }
 
 void
@@ -12347,7 +12357,9 @@ nsDocument::NotifyIntersectionObservers()
     observers.AppendElement(observer);
   }
   for (const auto& observer : observers) {
-    observer->Notify();
+    if (observer) {
+      observer->Notify();
+    }
   }
 }