Stabilize and align Intersection Observers

- Fixes several crashes
- Aligns the feature with the W3C WD spec

Tag #249
This commit is contained in:
wolfbeast 2018-06-27 16:00:53 +02:00 committed by Roy Tam
commit 5af4107852
9 changed files with 111 additions and 64 deletions

View file

@ -47,6 +47,7 @@ NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(DOMIntersectionObserver)
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)
@ -184,9 +185,10 @@ DOMIntersectionObserver::Connect()
if (mConnected) {
return;
}
mConnected = true;
nsIDocument* document = mOwner->GetExtantDoc();
document->AddIntersectionObserver(this);
mConnected = true;
}
void
@ -202,7 +204,9 @@ DOMIntersectionObserver::Disconnect()
mObservationTargets.Clear();
if (mOwner) {
nsIDocument* document = mOwner->GetExtantDoc();
document->RemoveIntersectionObserver(this);
if (document) {
document->RemoveIntersectionObserver(this);
}
}
mConnected = false;
}
@ -248,6 +252,12 @@ EdgeInclusiveIntersection(const nsRect& aRect, const nsRect& aOtherRect)
return Some(nsRect(left, top, right - left, bottom - top));
}
enum class BrowsingContextInfo {
SimilarOriginBrowsingContext,
DifferentOriginBrowsingContext,
UnknownBrowsingContext
};
void
DOMIntersectionObserver::Update(nsIDocument* aDocument, DOMHighResTimeStamp time)
{
@ -359,11 +369,22 @@ DOMIntersectionObserver::Update(nsIDocument* aDocument, DOMHighResTimeStamp time
}
}
nsRect rootIntersectionRect = rootRect;
bool isInSimilarOriginBrowsingContext = rootFrame && targetFrame &&
CheckSimilarOrigin(root, target);
nsRect rootIntersectionRect;
BrowsingContextInfo isInSimilarOriginBrowsingContext =
BrowsingContextInfo::UnknownBrowsingContext;
if (isInSimilarOriginBrowsingContext) {
if (rootFrame && targetFrame) {
rootIntersectionRect = rootRect;
}
if (root && target) {
isInSimilarOriginBrowsingContext = CheckSimilarOrigin(root, target) ?
BrowsingContextInfo::SimilarOriginBrowsingContext :
BrowsingContextInfo::DifferentOriginBrowsingContext;
}
if (isInSimilarOriginBrowsingContext ==
BrowsingContextInfo::SimilarOriginBrowsingContext) {
rootIntersectionRect.Inflate(rootMargin);
}
@ -413,7 +434,9 @@ DOMIntersectionObserver::Update(nsIDocument* aDocument, DOMHighResTimeStamp time
if (target->UpdateIntersectionObservation(this, threshold)) {
QueueIntersectionObserverEntry(
target, time,
isInSimilarOriginBrowsingContext ? Some(rootIntersectionRect) : Nothing(),
isInSimilarOriginBrowsingContext ==
BrowsingContextInfo::DifferentOriginBrowsingContext ?
Nothing() : Some(rootIntersectionRect),
targetRect, intersectionRect, intersectionRatio
);
}

View file

@ -101,9 +101,7 @@ protected:
class DOMIntersectionObserver final : public nsISupports,
public nsWrapperCache
{
virtual ~DOMIntersectionObserver() {
Disconnect();
}
virtual ~DOMIntersectionObserver() { }
public:
DOMIntersectionObserver(already_AddRefed<nsPIDOMWindowInner>&& aOwner,

View file

@ -3912,44 +3912,55 @@ Element::ClearDataset()
slots->mDataset = nullptr;
}
nsTArray<Element::nsDOMSlots::IntersectionObserverRegistration>*
nsDataHashtable<nsPtrHashKey<DOMIntersectionObserver>, int32_t>*
Element::RegisteredIntersectionObservers()
{
nsDOMSlots* slots = DOMSlots();
return &slots->mRegisteredIntersectionObservers;
}
enum nsPreviousIntersectionThreshold {
eUninitialized = -2,
eNonIntersecting = -1
};
void
Element::RegisterIntersectionObserver(DOMIntersectionObserver* aObserver)
{
RegisteredIntersectionObservers()->AppendElement(
nsDOMSlots::IntersectionObserverRegistration { aObserver, -1 });
nsDataHashtable<nsPtrHashKey<DOMIntersectionObserver>, int32_t>* observers =
RegisteredIntersectionObservers();
if (observers->Contains(aObserver)) {
return;
}
// 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.
RegisteredIntersectionObservers()->Put(aObserver, eUninitialized);
}
void
Element::UnregisterIntersectionObserver(DOMIntersectionObserver* aObserver)
{
nsTArray<nsDOMSlots::IntersectionObserverRegistration>* observers =
nsDataHashtable<nsPtrHashKey<DOMIntersectionObserver>, int32_t>* observers =
RegisteredIntersectionObservers();
for (uint32_t i = 0; i < observers->Length(); ++i) {
nsDOMSlots::IntersectionObserverRegistration reg = observers->ElementAt(i);
if (reg.observer == aObserver) {
observers->RemoveElementAt(i);
break;
}
}
observers->Remove(aObserver);
}
bool
Element::UpdateIntersectionObservation(DOMIntersectionObserver* aObserver, int32_t aThreshold)
{
nsTArray<nsDOMSlots::IntersectionObserverRegistration>* observers =
nsDataHashtable<nsPtrHashKey<DOMIntersectionObserver>, int32_t>* observers =
RegisteredIntersectionObservers();
for (auto& reg : *observers) {
if (reg.observer == aObserver && reg.previousThreshold != aThreshold) {
reg.previousThreshold = aThreshold;
return true;
}
if (!observers->Contains(aObserver)) {
return false;
}
int32_t previousThreshold = observers->Get(aObserver);
if (previousThreshold != aThreshold) {
observers->Put(aObserver, aThreshold);
return true;
}
return false;
}

View file

@ -1380,7 +1380,7 @@ protected:
nsDOMTokenList* GetTokenList(nsIAtom* aAtom,
const DOMTokenListSupportedTokenArray aSupportedTokens = nullptr);
nsTArray<nsDOMSlots::IntersectionObserverRegistration>* RegisteredIntersectionObservers();
nsDataHashtable<nsPtrHashKey<DOMIntersectionObserver>, int32_t>* RegisteredIntersectionObservers();
private:
/**

View file

@ -608,6 +608,7 @@ FragmentOrElement::nsDOMSlots::Unlink(bool aIsXUL)
mLabelsList = nullptr;
mCustomElementData = nullptr;
mClassList = nullptr;
mRegisteredIntersectionObservers.Clear();
}
size_t
@ -1359,6 +1360,13 @@ NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(FragmentOrElement)
{
nsDOMSlots *slots = tmp->GetExistingDOMSlots();
if (slots) {
if (tmp->IsElement()) {
Element* elem = tmp->AsElement();
for (auto iter = slots->mRegisteredIntersectionObservers.Iter(); !iter.Done(); iter.Next()) {
DOMIntersectionObserver* observer = iter.Key();
observer->UnlinkTarget(*elem);
}
}
slots->Unlink(tmp->IsXULElement());
}
}

View file

@ -21,6 +21,7 @@
#include "nsIWeakReference.h" // base class
#include "nsNodeUtils.h" // class member nsNodeUtils::CloneNodeImpl
#include "nsIHTMLCollection.h"
#include "nsDataHashtable.h"
class ContentUnbinder;
class nsContentList;
@ -353,12 +354,7 @@ public:
/**
* Registered Intersection Observers on the element.
*/
struct IntersectionObserverRegistration {
DOMIntersectionObserver* observer;
int32_t previousThreshold;
};
nsTArray<IntersectionObserverRegistration> mRegisteredIntersectionObservers;
nsDataHashtable<nsPtrHashKey<DOMIntersectionObserver>, int32_t> mRegisteredIntersectionObservers;
};
protected:

View file

@ -12507,7 +12507,8 @@ nsDocument::ScheduleIntersectionObserverNotification()
void
nsDocument::NotifyIntersectionObservers()
{
for (const auto& observer : mIntersectionObservers) {
nsTArray<RefPtr<DOMIntersectionObserver>> observers(mIntersectionObservers);
for (const auto& observer : observers) {
observer->Notify();
}
}

View file

@ -297,15 +297,6 @@ nsNodeUtils::LastRelease(nsINode* aNode)
NodeWillBeDestroyed, (aNode));
}
if (aNode->IsElement()) {
Element* elem = aNode->AsElement();
FragmentOrElement::nsDOMSlots* domSlots =
static_cast<FragmentOrElement::nsDOMSlots*>(slots);
for (auto& reg : domSlots->mRegisteredIntersectionObservers) {
reg.observer->UnlinkTarget(*elem);
}
}
delete slots;
aNode->mSlots = nullptr;
}

View file

@ -325,7 +325,7 @@ limitations under the License.
});
it('does not trigger if target does not intersect when observing begins',
it('does trigger if target does not intersect when observing begins',
function(done) {
var spy = sinon.spy();
@ -334,7 +334,7 @@ limitations under the License.
targetEl2.style.top = '-40px';
io.observe(targetEl2);
callDelayed(function() {
expect(spy.callCount).to.be(0);
expect(spy.callCount).to.be(1);
done();
}, ASYNC_TIMEOUT);
});
@ -528,7 +528,7 @@ limitations under the License.
spy.waitForNotification(function() {
expect(spy.callCount).to.be(1);
var records = sortRecords(spy.lastCall.args[0]);
expect(records.length).to.be(2);
expect(records.length).to.be(3);
expect(records[0].target).to.be(targetEl1);
expect(records[0].intersectionRatio).to.be(0.25);
expect(records[1].target).to.be(targetEl2);
@ -636,10 +636,10 @@ limitations under the License.
expect(records.length).to.be(3);
expect(records[0].target).to.be(targetEl1);
expect(records[0].intersectionRatio).to.be(0.5);
expect(records[1].target).to.be(targetEl3);
expect(records[1].intersectionRatio).to.be(0.5);
expect(records[2].target).to.be(targetEl4);
expect(records[2].target).to.be(targetEl3);
expect(records[2].intersectionRatio).to.be(0.5);
expect(records[3].target).to.be(targetEl4);
expect(records[3].intersectionRatio).to.be(0.5);
io.disconnect();
done();
}, {root: rootEl, rootMargin: '-10px 10%'});
@ -652,11 +652,11 @@ limitations under the License.
function(done) {
io = new IntersectionObserver(function(records) {
records = sortRecords(records);
expect(records.length).to.be(2);
expect(records.length).to.be(4);
expect(records[0].target).to.be(targetEl1);
expect(records[0].intersectionRatio).to.be(0.5);
expect(records[1].target).to.be(targetEl4);
expect(records[1].intersectionRatio).to.be(0.5);
expect(records[3].target).to.be(targetEl4);
expect(records[3].intersectionRatio).to.be(0.5);
io.disconnect();
done();
}, {root: rootEl, rootMargin: '-5% -2.5% 0px'});
@ -669,13 +669,13 @@ limitations under the License.
function(done) {
io = new IntersectionObserver(function(records) {
records = sortRecords(records);
expect(records.length).to.be(3);
expect(records.length).to.be(4);
expect(records[0].target).to.be(targetEl1);
expect(records[0].intersectionRatio).to.be(0.5);
expect(records[1].target).to.be(targetEl2);
expect(records[1].intersectionRatio).to.be(0.5);
expect(records[2].target).to.be(targetEl4);
expect(records[2].intersectionRatio).to.be(0.25);
expect(records[3].target).to.be(targetEl4);
expect(records[3].intersectionRatio).to.be(0.25);
io.disconnect();
done();
}, {root: rootEl, rootMargin: '5% -2.5% -10px -190px'});
@ -705,9 +705,9 @@ limitations under the License.
spy.waitForNotification(function() {
expect(spy.callCount).to.be(1);
var records = sortRecords(spy.lastCall.args[0]);
expect(records.length).to.be(1);
expect(records[0].intersectionRatio).to.be(0);
expect(records[0].target).to.be(targetEl2);
expect(records.length).to.be(2);
expect(records[1].intersectionRatio).to.be(0);
expect(records[1].target).to.be(targetEl2);
done();
}, ASYNC_TIMEOUT);
},
@ -797,14 +797,14 @@ limitations under the License.
function(done) {
document.getElementById('fixtures').appendChild(rootEl);
callDelayed(function() {
expect(spy.callCount).to.be(0);
expect(spy.callCount).to.be(1);
done();
}, ASYNC_TIMEOUT);
},
function(done) {
parentEl.insertBefore(targetEl1, targetEl2);
spy.waitForNotification(function() {
expect(spy.callCount).to.be(1);
expect(spy.callCount).to.be(2);
var records = sortRecords(spy.lastCall.args[0]);
expect(records.length).to.be(1);
expect(records[0].intersectionRatio).to.be(1);
@ -815,7 +815,7 @@ limitations under the License.
function(done) {
grandParentEl.parentNode.removeChild(grandParentEl);
spy.waitForNotification(function() {
expect(spy.callCount).to.be(2);
expect(spy.callCount).to.be(3);
var records = sortRecords(spy.lastCall.args[0]);
expect(records.length).to.be(1);
expect(records[0].intersectionRatio).to.be(0);
@ -826,7 +826,7 @@ limitations under the License.
function(done) {
rootEl.appendChild(targetEl1);
spy.waitForNotification(function() {
expect(spy.callCount).to.be(3);
expect(spy.callCount).to.be(4);
var records = sortRecords(spy.lastCall.args[0]);
expect(records.length).to.be(1);
expect(records[0].intersectionRatio).to.be(1);
@ -837,7 +837,7 @@ limitations under the License.
function(done) {
rootEl.parentNode.removeChild(rootEl);
spy.waitForNotification(function() {
expect(spy.callCount).to.be(4);
expect(spy.callCount).to.be(5);
var records = sortRecords(spy.lastCall.args[0]);
expect(records.length).to.be(1);
expect(records[0].intersectionRatio).to.be(0);
@ -867,8 +867,14 @@ limitations under the License.
targetEl1.style.top = '220px';
targetEl1.style.left = '220px';
var callCount = 0;
io = new IntersectionObserver(function(records) {
callCount++;
if (callCount <= 1) {
return;
}
expect(records.length).to.be(1);
expect(records[0].intersectionRatio).to.be(1);
done();
@ -891,6 +897,19 @@ limitations under the License.
var win = window.open("intersectionobserver_window.html");
});
it('triggers only once if observed multiple times (and does not crash when collected)', function(done) {
var spy = sinon.spy();
io = new IntersectionObserver(spy, {root: rootEl});
io.observe(targetEl1);
io.observe(targetEl1);
io.observe(targetEl1);
callDelayed(function () {
expect(spy.callCount).to.be(1);
done();
}, ASYNC_TIMEOUT);
});
});
describe('observe subframe', function () {