Source/WebCore/ChangeLog

 12018-08-14 Ali Juma <ajuma@chromium.org>
 2
 3 [IntersectionObserver] Do not hold a strong reference to the root element
 4 https://bugs.webkit.org/show_bug.cgi?id=188575
 5
 6 Reviewed by NOBODY (OOPS!).
 7
 8 Make IntersectionObserver have only a raw pointer to its root element rather than
 9 a reference, so that an otherwise-unreachable root isn't kept alive. Add logic to
 10 to clear this pointer when the root element gets deleted.
 11
 12 Test: intersection-observer/root-element-deleted.html
 13
 14 * dom/Element.cpp:
 15 (WebCore::Element::~Element):
 16 (WebCore::Element::disconnectFromIntersectionObservers):
 17 (WebCore::Element::ensureIntersectionObserverData):
 18 (WebCore::Element::intersectionObserverData):
 19 * dom/Element.h:
 20 * dom/ElementRareData.cpp:
 21 * dom/ElementRareData.h:
 22 (WebCore::ElementRareData::intersectionObserverData):
 23 (WebCore::ElementRareData::setIntersectionObserverData):
 24 * page/IntersectionObserver.cpp:
 25 (WebCore::IntersectionObserver::create):
 26 (WebCore::IntersectionObserver::IntersectionObserver):
 27 (WebCore::IntersectionObserver::~IntersectionObserver):
 28 (WebCore::IntersectionObserver::rootDestroyed):
 29 * page/IntersectionObserver.h:
 30 (WebCore::IntersectionObserver::root const):
 31
1322018-08-14 Ali Juma <ajuma@chromium.org>
233
334 Follow-up: [IntersectionObserver] Implement rootMargin parsing

Source/WebCore/dom/Element.cpp

@@Element::~Element()
187187 ASSERT(!beforePseudoElement());
188188 ASSERT(!afterPseudoElement());
189189
 190#if ENABLE(INTERSECTION_OBSERVER)
 191 disconnectFromIntersectionObservers();
 192#endif
 193
190194 removeShadowRoot();
191195
192196 if (hasSyntheticAttrChildNodes())

@@void Element::requestPointerLock()
32333237}
32343238#endif
32353239
 3240#if ENABLE(INTERSECTION_OBSERVER)
 3241void Element::disconnectFromIntersectionObservers()
 3242{
 3243 auto* observerData = intersectionObserverData();
 3244 if (!observerData)
 3245 return;
 3246
 3247 for (auto* observer : observerData->observers)
 3248 observer->rootDestroyed();
 3249 observerData->observers.clear();
 3250}
 3251
 3252IntersectionObserverData& Element::ensureIntersectionObserverData()
 3253{
 3254 auto& rareData = ensureElementRareData();
 3255 if (!rareData.intersectionObserverData())
 3256 rareData.setIntersectionObserverData(std::make_unique<IntersectionObserverData>());
 3257 return *rareData.intersectionObserverData();
 3258}
 3259
 3260IntersectionObserverData* Element::intersectionObserverData()
 3261{
 3262 return hasRareData() ? elementRareData()->intersectionObserverData() : nullptr;
 3263}
 3264#endif
 3265
32363266SpellcheckAttributeState Element::spellcheckAttributeState() const
32373267{
32383268 const AtomicString& value = attributeWithoutSynchronization(HTMLNames::spellcheckAttr);

Source/WebCore/dom/Element.h

@@class RenderTreePosition;
5656class WebAnimation;
5757struct ElementStyle;
5858
 59#if ENABLE(INTERSECTION_OBSERVER)
 60struct IntersectionObserverData;
 61#endif
 62
5963enum SpellcheckAttributeState {
6064 SpellcheckAttributeTrue,
6165 SpellcheckAttributeFalse,

@@public:
567571 using ContainerNode::setAttributeEventListener;
568572 void setAttributeEventListener(const AtomicString& eventType, const QualifiedName& attributeName, const AtomicString& value);
569573
 574#if ENABLE(INTERSECTION_OBSERVER)
 575 IntersectionObserverData& ensureIntersectionObserverData();
 576 IntersectionObserverData* intersectionObserverData();
 577#endif
 578
570579 Element* findAnchorElementForLink(String& outAnchorName);
571580
572581 ExceptionOr<Ref<WebAnimation>> animate(JSC::ExecState&, JSC::Strong<JSC::JSObject>&&, std::optional<Variant<double, KeyframeAnimationOptions>>&&);

@@private:
646655 void formatForDebugger(char* buffer, unsigned length) const override;
647656#endif
648657
 658#if ENABLE(INTERSECTION_OBSERVER)
 659 void disconnectFromIntersectionObservers();
 660#endif
 661
649662 // The cloneNode function is private so that non-virtual cloneElementWith/WithoutChildren are used instead.
650663 Ref<Node> cloneNodeInternal(Document&, CloningOperation) override;
651664 virtual Ref<Element> cloneElementWithoutAttributesAndChildren(Document&);

Source/WebCore/dom/ElementRareData.cpp

@@struct SameSizeAsElementRareData : NodeRareData {
4444 LayoutSize sizeForResizing;
4545 IntPoint savedLayerScrollPosition;
4646 void* pointers[8];
 47#if ENABLE(INTERSECTION_OBSERVER)
 48 void* intersectionObserverData;
 49#endif
 50
4751};
4852
4953static_assert(sizeof(ElementRareData) == sizeof(SameSizeAsElementRareData), "ElementRareData should stay small");

Source/WebCore/dom/ElementRareData.h

2424#include "CustomElementReactionQueue.h"
2525#include "DOMTokenList.h"
2626#include "DatasetDOMStringMap.h"
 27#include "IntersectionObserver.h"
2728#include "NamedNodeMap.h"
2829#include "NodeRareData.h"
2930#include "PseudoElement.h"

@@public:
116117 bool hasCSSAnimation() const { return m_hasCSSAnimation; }
117118 void setHasCSSAnimation(bool value) { m_hasCSSAnimation = value; }
118119
 120#if ENABLE(INTERSECTION_OBSERVER)
 121 IntersectionObserverData* intersectionObserverData() { return m_intersectionObserverData.get(); }
 122 void setIntersectionObserverData(std::unique_ptr<IntersectionObserverData>&& data) { m_intersectionObserverData = WTFMove(data); }
 123#endif
 124
119125private:
120126 int m_tabIndex;
121127 unsigned short m_childIndex;

@@private:
149155 RefPtr<ShadowRoot> m_shadowRoot;
150156 std::unique_ptr<CustomElementReactionQueue> m_customElementReactionQueue;
151157 std::unique_ptr<NamedNodeMap> m_attributeMap;
 158#if ENABLE(INTERSECTION_OBSERVER)
 159 std::unique_ptr<IntersectionObserverData> m_intersectionObserverData;
 160#endif
152161
153162 RefPtr<PseudoElement> m_beforePseudoElement;
154163 RefPtr<PseudoElement> m_afterPseudoElement;

Source/WebCore/page/IntersectionObserver.cpp

@@ExceptionOr<Ref<IntersectionObserver>> IntersectionObserver::create(Ref<Intersec
100100 return Exception { RangeError, "Failed to construct 'IntersectionObserver': all thresholds must lie in the range [0.0, 1.0]." };
101101 }
102102
103  return adoptRef(*new IntersectionObserver(WTFMove(callback), WTFMove(init.root), rootMarginOrException.releaseReturnValue(), WTFMove(thresholds)));
 103 return adoptRef(*new IntersectionObserver(WTFMove(callback), init.root, rootMarginOrException.releaseReturnValue(), WTFMove(thresholds)));
104104}
105105
106 IntersectionObserver::IntersectionObserver(Ref<IntersectionObserverCallback>&& callback, RefPtr<Element>&& root, LengthBox&& parsedRootMargin, Vector<double>&& thresholds)
107  : m_root(WTFMove(root))
 106IntersectionObserver::IntersectionObserver(Ref<IntersectionObserverCallback>&& callback, Element* root, LengthBox&& parsedRootMargin, Vector<double>&& thresholds)
 107 : m_root(root)
108108 , m_rootMargin(WTFMove(parsedRootMargin))
109109 , m_thresholds(WTFMove(thresholds))
110110 , m_callback(WTFMove(callback))
111111{
 112 if (m_root) {
 113 auto& observerData = m_root->ensureIntersectionObserverData();
 114 observerData.observers.append(this);
 115 }
 116}
 117
 118IntersectionObserver::~IntersectionObserver()
 119{
 120 if (m_root) {
 121 auto& elementObservers = m_root->intersectionObserverData()->observers;
 122 for (size_t i = 0; i < elementObservers.size(); ++i) {
 123 if (elementObservers.at(i) == this) {
 124 elementObservers.remove(i);
 125 break;
 126 }
 127 }
 128 }
112129}
113130
114131String IntersectionObserver::rootMargin() const

@@Vector<RefPtr<IntersectionObserverEntry>> IntersectionObserver::takeRecords()
145162 return { };
146163}
147164
 165void IntersectionObserver::rootDestroyed()
 166{
 167 ASSERT(m_root);
 168 disconnect();
 169 m_root = nullptr;
 170}
148171
149172} // namespace WebCore
150173

Source/WebCore/page/IntersectionObserver.h

@@namespace WebCore {
3838
3939class Element;
4040
 41struct IntersectionObserverData {
 42 // IntersectionObservers for which the element that owns this IntersectionObserverData is the root.
 43 Vector<IntersectionObserver*> observers;
 44
 45 // FIXME: Also track IntersectionObservers for which the element that owns this IntersectionObserverData is the target.
 46};
 47
4148class IntersectionObserver : public RefCounted<IntersectionObserver> {
4249public:
4350 struct Init {
44  RefPtr<Element> root;
 51 Element* root { nullptr };
4552 String rootMargin;
4653 Variant<double, Vector<double>> threshold;
4754 };
4855
4956 static ExceptionOr<Ref<IntersectionObserver>> create(Ref<IntersectionObserverCallback>&&, Init&&);
50 
51  Element* root() const { return m_root.get(); }
 57
 58 ~IntersectionObserver();
 59
 60 Element* root() const { return m_root; }
5261 String rootMargin() const;
5362 const Vector<double>& thresholds() const { return m_thresholds; }
5463

@@public:
5867
5968 Vector<RefPtr<IntersectionObserverEntry>> takeRecords();
6069
 70 void rootDestroyed();
 71
6172private:
62  IntersectionObserver(Ref<IntersectionObserverCallback>&&, RefPtr<Element>&& root, LengthBox&& parsedRootMargin, Vector<double>&& thresholds);
 73 IntersectionObserver(Ref<IntersectionObserverCallback>&&, Element* root, LengthBox&& parsedRootMargin, Vector<double>&& thresholds);
6374
64  RefPtr<Element> m_root;
 75 Element* m_root;
6576 LengthBox m_rootMargin;
6677 Vector<double> m_thresholds;
6778 Ref<IntersectionObserverCallback> m_callback;

LayoutTests/ChangeLog

 12018-08-14 Ali Juma <ajuma@chromium.org>
 2
 3 [IntersectionObserver] Do not hold a strong reference to the root element
 4 https://bugs.webkit.org/show_bug.cgi?id=188575
 5
 6 Reviewed by NOBODY (OOPS!).
 7
 8 * intersection-observer/root-element-deleted-expected.txt: Added.
 9 * intersection-observer/root-element-deleted.html: Added.
 10
1112018-08-14 Antoine Quint <graouts@apple.com>
212
313 [Web Animations] Crash under AnimationTimeline::cancelOrRemoveDeclarativeAnimation()

LayoutTests/intersection-observer/root-element-deleted-expected.txt

 1
 2PASS IntersectionObserver doesn't keep unreachable root alive
 3

LayoutTests/intersection-observer/root-element-deleted.html

 1<!DOCTYPE html>
 2<head>
 3<script src="../resources/testharness.js"></script>
 4<script src="../resources/testharnessreport.js"></script>
 5<body>
 6
 7<div id="root" style="position:absolute">
 8 <div id="target" style="width: 100px; height: 100px; background-color: green"></div>
 9</div>
 10
 11<script>
 12 async_test(function(t) {
 13 var root = document.getElementById('root');
 14 var observer = new IntersectionObserver(function() {}, { root: document.getElementById('root') });;
 15 var target = document.getElementById('target');
 16 assert_equals(observer.root, root, 'Observer starts out with non-null root');
 17 observer.observe(target);
 18 root.parentNode.removeChild(root);
 19 root = null;
 20 target = null;
 21 requestAnimationFrame(t.step_func_done(function() {
 22 GCController.collect();
 23 assert_equals(observer.root, null, 'Observer has null root after root element is destroyed');
 24 }));
 25 }, "IntersectionObserver doesn't keep unreachable root alive");
 26</script>
 27</body>
 28</html>