Bug 1399603 - [intersection-observer] Stop observing a target when intersection root is deleted. r=mrbkap
authorTobias Schneider <schneider@jancona.com>
Fri, 06 Oct 2017 07:39:54 -0700
changeset 676118 f70abea8b810cc5a9604d0fb2db77eb46f485000
parent 676117 ce5016d6b5dfd2c1c53b6178d84fe279a0990e13
child 676119 85a4fbfc88efcbe88c2d7cd5a76c7916a80624b7
push id83398
push userbmo:rail@mozilla.com
push dateFri, 06 Oct 2017 17:12:44 +0000
reviewersmrbkap
bugs1399603
milestone58.0a1
Bug 1399603 - [intersection-observer] Stop observing a target when intersection root is deleted. r=mrbkap
dom/base/DOMIntersectionObserver.cpp
dom/base/DOMIntersectionObserver.h
dom/base/Element.cpp
dom/base/test/mochitest.ini
dom/base/test/test_bug1399603.html
--- a/dom/base/DOMIntersectionObserver.cpp
+++ b/dom/base/DOMIntersectionObserver.cpp
@@ -42,25 +42,23 @@ NS_IMPL_CYCLE_COLLECTION_TRACE_BEGIN(DOM
 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)
 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,
                                      mozilla::ErrorResult& aRv)
 {
@@ -76,17 +74,20 @@ DOMIntersectionObserver::Constructor(con
   nsCOMPtr<nsPIDOMWindowInner> window = do_QueryInterface(aGlobal.GetAsSupports());
   if (!window) {
     aRv.Throw(NS_ERROR_FAILURE);
     return nullptr;
   }
   RefPtr<DOMIntersectionObserver> observer =
     new DOMIntersectionObserver(window.forget(), aCb);
 
-  observer->mRoot = aOptions.mRoot;
+  if (aOptions.mRoot) {
+    observer->mRoot = aOptions.mRoot;
+    observer->mRoot->RegisterIntersectionObserver(observer);
+  }
 
   if (!observer->SetRootMargin(aOptions.mRootMargin)) {
     aRv.ThrowDOMException(NS_ERROR_DOM_SYNTAX_ERR,
       NS_LITERAL_CSTRING("rootMargin must be specified in pixels or percent."));
     return nullptr;
   }
 
   if (aOptions.mThreshold.IsDoubleSequence()) {
@@ -170,18 +171,23 @@ DOMIntersectionObserver::Unobserve(Eleme
     return;
   }
 
   mObservationTargets.RemoveElement(&aTarget);
   aTarget.UnregisterIntersectionObserver(this);
 }
 
 void
-DOMIntersectionObserver::UnlinkTarget(Element& aTarget)
+DOMIntersectionObserver::UnlinkElement(Element& aTarget)
 {
+  if (mRoot && mRoot == &aTarget) {
+    mRoot = nullptr;
+    Disconnect();
+    return;
+  }
   mObservationTargets.RemoveElement(&aTarget);
   if (mObservationTargets.Length() == 0) {
     Disconnect();
   }
 }
 
 void
 DOMIntersectionObserver::Connect()
--- a/dom/base/DOMIntersectionObserver.h
+++ b/dom/base/DOMIntersectionObserver.h
@@ -111,17 +111,21 @@ class DOMIntersectionObserver final : pu
 {
   virtual ~DOMIntersectionObserver() {
     Disconnect();
   }
 
 public:
   DOMIntersectionObserver(already_AddRefed<nsPIDOMWindowInner>&& aOwner,
                           mozilla::dom::IntersectionCallback& aCb)
-  : mOwner(aOwner), mDocument(mOwner->GetExtantDoc()), mCallback(&aCb), mConnected(false)
+  : mOwner(aOwner),
+    mDocument(mOwner->GetExtantDoc()),
+    mCallback(&aCb),
+    mRoot(nullptr),
+    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,
@@ -147,17 +151,17 @@ public:
     return mRoot;
   }
 
   void GetRootMargin(mozilla::dom::DOMString& aRetVal);
   void GetThresholds(nsTArray<double>& aRetVal);
   void Observe(Element& aTarget);
   void Unobserve(Element& aTarget);
 
-  void UnlinkTarget(Element& aTarget);
+  void UnlinkElement(Element& aTarget);
   void Disconnect();
 
   void TakeRecords(nsTArray<RefPtr<DOMIntersectionObserverEntry>>& aRetVal);
 
   mozilla::dom::IntersectionCallback* IntersectionCallback() { return mCallback; }
 
   bool SetRootMargin(const nsAString& aString);
 
@@ -171,21 +175,22 @@ protected:
                                       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;
+  // Raw pointer which is explicitly cleared by UnlinkElement().
+  Element*                                        mRoot;
   nsCSSRect                                       mRootMargin;
   nsTArray<double>                                mThresholds;
 
-  // Holds raw pointers which are explicitly cleared by UnlinkTarget().
+  // Holds raw pointers which are explicitly cleared by UnlinkElement().
   nsTArray<Element*>                              mObservationTargets;
 
   nsTArray<RefPtr<DOMIntersectionObserverEntry>>  mQueuedEntries;
   bool                                            mConnected;
 };
 
 NS_DEFINE_STATIC_IID_ACCESSOR(DOMIntersectionObserver, NS_DOM_INTERSECTION_OBSERVER_IID)
 
--- a/dom/base/Element.cpp
+++ b/dom/base/Element.cpp
@@ -4211,17 +4211,17 @@ Element::RegisterIntersectionObserver(DO
     observers = new IntersectionObserverList();
     observers->Put(aObserver, eUninitialized);
     SetProperty(nsGkAtoms::intersectionobserverlist, observers,
                 nsINode::DeleteProperty<IntersectionObserverList>);
     return;
   }
 
   observers->LookupForAdd(aObserver).OrInsert([]() {
-    // Value can be:
+    // If element is being observed, value can be:
     //   -2:   Makes sure next calculated threshold always differs, leading to a
     //         notification task being scheduled.
     //   -1:   Non-intersecting.
     //   >= 0: Intersecting, valid index of aObserver->mThresholds.
     return eUninitialized;
   });
 }
 
@@ -4244,17 +4244,17 @@ Element::UnlinkIntersectionObservers()
     static_cast<IntersectionObserverList*>(
       GetProperty(nsGkAtoms::intersectionobserverlist)
     );
   if (!observers) {
     return;
   }
   for (auto iter = observers->Iter(); !iter.Done(); iter.Next()) {
     DOMIntersectionObserver* observer = iter.Key();
-    observer->UnlinkTarget(*this);
+    observer->UnlinkElement(*this);
   }
   observers->Clear();
 }
 
 bool
 Element::UpdateIntersectionObservation(DOMIntersectionObserver* aObserver, int32_t aThreshold)
 {
   IntersectionObserverList* observers =
--- a/dom/base/test/mochitest.ini
+++ b/dom/base/test/mochitest.ini
@@ -619,16 +619,17 @@ skip-if = toolkit == 'android'
 [test_bug1307730.html]
 [test_bug1308069.html]
 [test_bug1314032.html]
 [test_bug1318303.html]
 [test_bug1375050.html]
 [test_bug1381710.html]
 [test_bug1384658.html]
 skip-if = toolkit == 'android'
+[test_bug1399603.html]
 [test_bug1399605.html]
 [test_caretPositionFromPoint.html]
 [test_change_policy.html]
 [test_clearTimeoutIntervalNoArg.html]
 [test_constructor-assignment.html]
 [test_constructor.html]
 [test_copyimage.html]
 subsuite = clipboard
new file mode 100644
--- /dev/null
+++ b/dom/base/test/test_bug1399603.html
@@ -0,0 +1,63 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+  <meta charset="utf-8">
+  <title>Test for Bug 1399603</title>
+  <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1399603">Mozilla Bug 1399603</a>
+<p id="display"></p>
+<div id="content">
+  <div id="root">
+    <div id="target"></div>
+  </div>
+</div>
+<pre id="test">
+<script type="application/javascript">
+
+  function waitForNotification(f) {
+    requestAnimationFrame(function() {
+      setTimeout(function() { setTimeout(f); });
+    });
+  }
+
+  function forceGC() {
+    SpecialPowers.gc();
+    SpecialPowers.forceShrinkingGC();
+    SpecialPowers.forceCC();
+    SpecialPowers.gc();
+    SpecialPowers.forceShrinkingGC();
+    SpecialPowers.forceCC();
+  }
+
+  let content = document.getElementById('content');
+  let root = document.getElementById('root');
+  let target = document.getElementById('target');
+  let entries = [];
+  let observer = new IntersectionObserver(function(changes) {
+    entries = entries.concat(changes);
+  }, { root: root });
+  observer.observe(target);
+
+  waitForNotification(function () {
+    is(entries.length, 1, "Initial notification.");
+    root.removeChild(target);
+    content.removeChild(root);
+    root = null;
+    forceGC();
+    waitForNotification(function () {
+      is(entries.length, 1, "No new notifications.");
+      SimpleTest.finish();
+    });
+  });
+
+  SimpleTest.waitForExplicitFinish();
+
+</script>
+</pre>
+<div id="log">
+</div>
+</body>
+</html>