Bug 1422092 - Revert the changes made by bug 1399603. r=mrbkap, a=jcristau
authorOlli Pettay <Olli.Pettay@helsinki.fi>
Sun, 10 Dec 2017 14:52:49 -0500
changeset 445297 90dd6724eb2a62a06cbf726ff43979e790465845
parent 445296 49281ca9bc9c07624f3070903aec0deb0e24d058
child 445298 7e553ba7c247bdc9994a924e31f3d0f9b44b7c22
push id1618
push userCallek@gmail.com
push dateThu, 11 Jan 2018 17:45:48 +0000
treeherdermozilla-release@882ca853e05a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmrbkap, jcristau
bugs1422092, 1399603
milestone58.0
Bug 1422092 - Revert the changes made by bug 1399603. r=mrbkap, a=jcristau
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
@@ -43,23 +43,25 @@ 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)
 {
@@ -75,20 +77,17 @@ 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);
 
-  if (aOptions.mRoot) {
-    observer->mRoot = aOptions.mRoot;
-    observer->mRoot->RegisterIntersectionObserver(observer);
-  }
+  observer->mRoot = aOptions.mRoot;
 
   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()) {
@@ -177,23 +176,18 @@ DOMIntersectionObserver::Unobserve(Eleme
     return;
   }
 
   mObservationTargets.RemoveElement(&aTarget);
   aTarget.UnregisterIntersectionObserver(this);
 }
 
 void
-DOMIntersectionObserver::UnlinkElement(Element& aTarget)
+DOMIntersectionObserver::UnlinkTarget(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,21 +111,17 @@ class DOMIntersectionObserver final : pu
 {
   virtual ~DOMIntersectionObserver() {
     Disconnect();
   }
 
 public:
   DOMIntersectionObserver(already_AddRefed<nsPIDOMWindowInner>&& aOwner,
                           mozilla::dom::IntersectionCallback& aCb)
-  : mOwner(aOwner),
-    mDocument(mOwner->GetExtantDoc()),
-    mCallback(&aCb),
-    mRoot(nullptr),
-    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,
@@ -151,17 +147,17 @@ public:
     return mRoot;
   }
 
   void GetRootMargin(mozilla::dom::DOMString& aRetVal);
   void GetThresholds(nsTArray<double>& aRetVal);
   void Observe(Element& aTarget);
   void Unobserve(Element& aTarget);
 
-  void UnlinkElement(Element& aTarget);
+  void UnlinkTarget(Element& aTarget);
   void Disconnect();
 
   void TakeRecords(nsTArray<RefPtr<DOMIntersectionObserverEntry>>& aRetVal);
 
   mozilla::dom::IntersectionCallback* IntersectionCallback() { return mCallback; }
 
   bool SetRootMargin(const nsAString& aString);
 
@@ -175,22 +171,21 @@ 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;
-  // Raw pointer which is explicitly cleared by UnlinkElement().
-  Element*                                        mRoot;
+  RefPtr<Element>                                 mRoot;
   nsCSSRect                                       mRootMargin;
   nsTArray<double>                                mThresholds;
 
-  // Holds raw pointers which are explicitly cleared by UnlinkElement().
+  // Holds raw pointers which are explicitly cleared by UnlinkTarget().
   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
@@ -4310,17 +4310,17 @@ static void
 IntersectionObserverPropertyDtor(void* aObject, nsAtom* aPropertyName,
                                  void* aPropertyValue, void* aData)
 {
   Element* element = static_cast<Element*>(aObject);
   IntersectionObserverList* observers =
     static_cast<IntersectionObserverList*>(aPropertyValue);
   for (auto iter = observers->Iter(); !iter.Done(); iter.Next()) {
     DOMIntersectionObserver* observer = iter.Key();
-    observer->UnlinkElement(*element);
+    observer->UnlinkTarget(*element);
   }
   delete observers;
 }
 
 void
 Element::RegisterIntersectionObserver(DOMIntersectionObserver* aObserver)
 {
   IntersectionObserverList* observers =
@@ -4332,17 +4332,17 @@ Element::RegisterIntersectionObserver(DO
     observers = new IntersectionObserverList();
     observers->Put(aObserver, eUninitialized);
     SetProperty(nsGkAtoms::intersectionobserverlist, observers,
                 IntersectionObserverPropertyDtor, true);
     return;
   }
 
   observers->LookupForAdd(aObserver).OrInsert([]() {
-    // If element is being observed, value can be:
+    // 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;
   });
 }
 
@@ -4365,17 +4365,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->UnlinkElement(*this);
+    observer->UnlinkTarget(*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
@@ -605,17 +605,16 @@ skip-if = toolkit == 'android'
 [test_bug1295852.html]
 [test_bug1307730.html]
 [test_bug1308069.html]
 [test_bug1314032.html]
 [test_bug1318303.html]
 [test_bug1375050.html]
 [test_bug1381710.html]
 [test_bug1384661.html]
-[test_bug1399603.html]
 [test_bug1399605.html]
 [test_bug1404385.html]
 [test_bug1406102.html]
 [test_caretPositionFromPoint.html]
 [test_change_policy.html]
 [test_clearTimeoutIntervalNoArg.html]
 [test_constructor-assignment.html]
 [test_constructor.html]
deleted file mode 100644
--- a/dom/base/test/test_bug1399603.html
+++ /dev/null
@@ -1,63 +0,0 @@
-<!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>