Diff
Modified: trunk/LayoutTests/ChangeLog (234883 => 234884)
--- trunk/LayoutTests/ChangeLog 2018-08-15 06:15:23 UTC (rev 234883)
+++ trunk/LayoutTests/ChangeLog 2018-08-15 14:13:06 UTC (rev 234884)
@@ -1,3 +1,13 @@
+2018-08-15 Ali Juma <[email protected]>
+
+ [IntersectionObserver] Do not hold a strong reference to the root element
+ https://bugs.webkit.org/show_bug.cgi?id=188575
+
+ Reviewed by Simon Fraser.
+
+ * intersection-observer/root-element-deleted-expected.txt: Added.
+ * intersection-observer/root-element-deleted.html: Added.
+
2018-08-14 Zalan Bujtas <[email protected]>
[LFC][Floating] Add support for negative clearance.
Added: trunk/LayoutTests/intersection-observer/root-element-deleted-expected.txt (0 => 234884)
--- trunk/LayoutTests/intersection-observer/root-element-deleted-expected.txt (rev 0)
+++ trunk/LayoutTests/intersection-observer/root-element-deleted-expected.txt 2018-08-15 14:13:06 UTC (rev 234884)
@@ -0,0 +1,3 @@
+
+PASS IntersectionObserver doesn't keep unreachable root alive
+
Added: trunk/LayoutTests/intersection-observer/root-element-deleted.html (0 => 234884)
--- trunk/LayoutTests/intersection-observer/root-element-deleted.html (rev 0)
+++ trunk/LayoutTests/intersection-observer/root-element-deleted.html 2018-08-15 14:13:06 UTC (rev 234884)
@@ -0,0 +1,28 @@
+<!DOCTYPE html><!-- webkit-test-runner [ enableIntersectionObserver=true ] -->
+<head>
+<script src=""
+<script src=""
+<body>
+
+<div id="root" style="position:absolute">
+ <div id="target" style="width: 100px; height: 100px; background-color: green"></div>
+</div>
+
+<script>
+ async_test(function(t) {
+ var root = document.getElementById('root');
+ var observer = new IntersectionObserver(function() {}, { root: document.getElementById('root') });;
+ var target = document.getElementById('target');
+ assert_equals(observer.root, root, 'Observer starts out with non-null root');
+ observer.observe(target);
+ root.parentNode.removeChild(root);
+ root = null;
+ target = null;
+ requestAnimationFrame(t.step_func_done(function() {
+ GCController.collect();
+ assert_equals(observer.root, null, 'Observer has null root after root element is destroyed');
+ }));
+ }, "IntersectionObserver doesn't keep unreachable root alive");
+</script>
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (234883 => 234884)
--- trunk/Source/WebCore/ChangeLog 2018-08-15 06:15:23 UTC (rev 234883)
+++ trunk/Source/WebCore/ChangeLog 2018-08-15 14:13:06 UTC (rev 234884)
@@ -1,3 +1,34 @@
+2018-08-15 Ali Juma <[email protected]>
+
+ [IntersectionObserver] Do not hold a strong reference to the root element
+ https://bugs.webkit.org/show_bug.cgi?id=188575
+
+ Reviewed by Simon Fraser.
+
+ Make IntersectionObserver have only a raw pointer to its root element rather than
+ a reference, so that an otherwise-unreachable root isn't kept alive. Add logic to
+ to clear this pointer when the root element gets deleted.
+
+ Test: intersection-observer/root-element-deleted.html
+
+ * dom/Element.cpp:
+ (WebCore::Element::~Element):
+ (WebCore::Element::disconnectFromIntersectionObservers):
+ (WebCore::Element::ensureIntersectionObserverData):
+ (WebCore::Element::intersectionObserverData):
+ * dom/Element.h:
+ * dom/ElementRareData.cpp:
+ * dom/ElementRareData.h:
+ (WebCore::ElementRareData::intersectionObserverData):
+ (WebCore::ElementRareData::setIntersectionObserverData):
+ * page/IntersectionObserver.cpp:
+ (WebCore::IntersectionObserver::create):
+ (WebCore::IntersectionObserver::IntersectionObserver):
+ (WebCore::IntersectionObserver::~IntersectionObserver):
+ (WebCore::IntersectionObserver::rootDestroyed):
+ * page/IntersectionObserver.h:
+ (WebCore::IntersectionObserver::root const):
+
2018-08-14 Zan Dobersek <[email protected]>
[Nicosia] Add Nicosia::BackingStoreTextureMapperImpl
Modified: trunk/Source/WebCore/dom/Element.cpp (234883 => 234884)
--- trunk/Source/WebCore/dom/Element.cpp 2018-08-15 06:15:23 UTC (rev 234883)
+++ trunk/Source/WebCore/dom/Element.cpp 2018-08-15 14:13:06 UTC (rev 234884)
@@ -187,6 +187,10 @@
ASSERT(!beforePseudoElement());
ASSERT(!afterPseudoElement());
+#if ENABLE(INTERSECTION_OBSERVER)
+ disconnectFromIntersectionObservers();
+#endif
+
removeShadowRoot();
if (hasSyntheticAttrChildNodes())
@@ -3233,6 +3237,32 @@
}
#endif
+#if ENABLE(INTERSECTION_OBSERVER)
+void Element::disconnectFromIntersectionObservers()
+{
+ auto* observerData = intersectionObserverData();
+ if (!observerData)
+ return;
+
+ for (auto* observer : observerData->observers)
+ observer->rootDestroyed();
+ observerData->observers.clear();
+}
+
+IntersectionObserverData& Element::ensureIntersectionObserverData()
+{
+ auto& rareData = ensureElementRareData();
+ if (!rareData.intersectionObserverData())
+ rareData.setIntersectionObserverData(std::make_unique<IntersectionObserverData>());
+ return *rareData.intersectionObserverData();
+}
+
+IntersectionObserverData* Element::intersectionObserverData()
+{
+ return hasRareData() ? elementRareData()->intersectionObserverData() : nullptr;
+}
+#endif
+
SpellcheckAttributeState Element::spellcheckAttributeState() const
{
const AtomicString& value = attributeWithoutSynchronization(HTMLNames::spellcheckAttr);
Modified: trunk/Source/WebCore/dom/Element.h (234883 => 234884)
--- trunk/Source/WebCore/dom/Element.h 2018-08-15 06:15:23 UTC (rev 234883)
+++ trunk/Source/WebCore/dom/Element.h 2018-08-15 14:13:06 UTC (rev 234884)
@@ -56,6 +56,10 @@
class WebAnimation;
struct ElementStyle;
+#if ENABLE(INTERSECTION_OBSERVER)
+struct IntersectionObserverData;
+#endif
+
enum SpellcheckAttributeState {
SpellcheckAttributeTrue,
SpellcheckAttributeFalse,
@@ -567,6 +571,11 @@
using ContainerNode::setAttributeEventListener;
void setAttributeEventListener(const AtomicString& eventType, const QualifiedName& attributeName, const AtomicString& value);
+#if ENABLE(INTERSECTION_OBSERVER)
+ IntersectionObserverData& ensureIntersectionObserverData();
+ IntersectionObserverData* intersectionObserverData();
+#endif
+
Element* findAnchorElementForLink(String& outAnchorName);
ExceptionOr<Ref<WebAnimation>> animate(JSC::ExecState&, JSC::Strong<JSC::JSObject>&&, std::optional<Variant<double, KeyframeAnimationOptions>>&&);
@@ -646,6 +655,10 @@
void formatForDebugger(char* buffer, unsigned length) const override;
#endif
+#if ENABLE(INTERSECTION_OBSERVER)
+ void disconnectFromIntersectionObservers();
+#endif
+
// The cloneNode function is private so that non-virtual cloneElementWith/WithoutChildren are used instead.
Ref<Node> cloneNodeInternal(Document&, CloningOperation) override;
virtual Ref<Element> cloneElementWithoutAttributesAndChildren(Document&);
Modified: trunk/Source/WebCore/dom/ElementRareData.cpp (234883 => 234884)
--- trunk/Source/WebCore/dom/ElementRareData.cpp 2018-08-15 06:15:23 UTC (rev 234883)
+++ trunk/Source/WebCore/dom/ElementRareData.cpp 2018-08-15 14:13:06 UTC (rev 234884)
@@ -44,6 +44,10 @@
LayoutSize sizeForResizing;
IntPoint savedLayerScrollPosition;
void* pointers[8];
+#if ENABLE(INTERSECTION_OBSERVER)
+ void* intersectionObserverData;
+#endif
+
};
static_assert(sizeof(ElementRareData) == sizeof(SameSizeAsElementRareData), "ElementRareData should stay small");
Modified: trunk/Source/WebCore/dom/ElementRareData.h (234883 => 234884)
--- trunk/Source/WebCore/dom/ElementRareData.h 2018-08-15 06:15:23 UTC (rev 234883)
+++ trunk/Source/WebCore/dom/ElementRareData.h 2018-08-15 14:13:06 UTC (rev 234884)
@@ -24,6 +24,7 @@
#include "CustomElementReactionQueue.h"
#include "DOMTokenList.h"
#include "DatasetDOMStringMap.h"
+#include "IntersectionObserver.h"
#include "NamedNodeMap.h"
#include "NodeRareData.h"
#include "PseudoElement.h"
@@ -116,6 +117,11 @@
bool hasCSSAnimation() const { return m_hasCSSAnimation; }
void setHasCSSAnimation(bool value) { m_hasCSSAnimation = value; }
+#if ENABLE(INTERSECTION_OBSERVER)
+ IntersectionObserverData* intersectionObserverData() { return m_intersectionObserverData.get(); }
+ void setIntersectionObserverData(std::unique_ptr<IntersectionObserverData>&& data) { m_intersectionObserverData = WTFMove(data); }
+#endif
+
private:
int m_tabIndex;
unsigned short m_childIndex;
@@ -149,6 +155,9 @@
RefPtr<ShadowRoot> m_shadowRoot;
std::unique_ptr<CustomElementReactionQueue> m_customElementReactionQueue;
std::unique_ptr<NamedNodeMap> m_attributeMap;
+#if ENABLE(INTERSECTION_OBSERVER)
+ std::unique_ptr<IntersectionObserverData> m_intersectionObserverData;
+#endif
RefPtr<PseudoElement> m_beforePseudoElement;
RefPtr<PseudoElement> m_afterPseudoElement;
Modified: trunk/Source/WebCore/page/IntersectionObserver.cpp (234883 => 234884)
--- trunk/Source/WebCore/page/IntersectionObserver.cpp 2018-08-15 06:15:23 UTC (rev 234883)
+++ trunk/Source/WebCore/page/IntersectionObserver.cpp 2018-08-15 14:13:06 UTC (rev 234884)
@@ -100,17 +100,27 @@
return Exception { RangeError, "Failed to construct 'IntersectionObserver': all thresholds must lie in the range [0.0, 1.0]." };
}
- return adoptRef(*new IntersectionObserver(WTFMove(callback), WTFMove(init.root), rootMarginOrException.releaseReturnValue(), WTFMove(thresholds)));
+ return adoptRef(*new IntersectionObserver(WTFMove(callback), init.root, rootMarginOrException.releaseReturnValue(), WTFMove(thresholds)));
}
-IntersectionObserver::IntersectionObserver(Ref<IntersectionObserverCallback>&& callback, RefPtr<Element>&& root, LengthBox&& parsedRootMargin, Vector<double>&& thresholds)
- : m_root(WTFMove(root))
+IntersectionObserver::IntersectionObserver(Ref<IntersectionObserverCallback>&& callback, Element* root, LengthBox&& parsedRootMargin, Vector<double>&& thresholds)
+ : m_root(root)
, m_rootMargin(WTFMove(parsedRootMargin))
, m_thresholds(WTFMove(thresholds))
, m_callback(WTFMove(callback))
{
+ if (m_root) {
+ auto& observerData = m_root->ensureIntersectionObserverData();
+ observerData.observers.append(this);
+ }
}
+IntersectionObserver::~IntersectionObserver()
+{
+ if (m_root)
+ m_root->intersectionObserverData()->observers.removeFirst(this);
+}
+
String IntersectionObserver::rootMargin() const
{
StringBuilder stringBuilder;
@@ -145,6 +155,12 @@
return { };
}
+void IntersectionObserver::rootDestroyed()
+{
+ ASSERT(m_root);
+ disconnect();
+ m_root = nullptr;
+}
} // namespace WebCore
Modified: trunk/Source/WebCore/page/IntersectionObserver.h (234883 => 234884)
--- trunk/Source/WebCore/page/IntersectionObserver.h 2018-08-15 06:15:23 UTC (rev 234883)
+++ trunk/Source/WebCore/page/IntersectionObserver.h 2018-08-15 14:13:06 UTC (rev 234884)
@@ -38,17 +38,28 @@
class Element;
+struct IntersectionObserverData {
+ // IntersectionObservers for which the element that owns this IntersectionObserverData is the root.
+ // The IntersectionObservers are owned by _javascript_ wrappers and by IntersectionObserverRegistrations
+ // for each target currently being observed.
+ Vector<IntersectionObserver*> observers;
+
+ // FIXME: Create and track IntersectionObserverRegistrations.
+};
+
class IntersectionObserver : public RefCounted<IntersectionObserver> {
public:
struct Init {
- RefPtr<Element> root;
+ Element* root { nullptr };
String rootMargin;
Variant<double, Vector<double>> threshold;
};
static ExceptionOr<Ref<IntersectionObserver>> create(Ref<IntersectionObserverCallback>&&, Init&&);
-
- Element* root() const { return m_root.get(); }
+
+ ~IntersectionObserver();
+
+ Element* root() const { return m_root; }
String rootMargin() const;
const Vector<double>& thresholds() const { return m_thresholds; }
@@ -58,10 +69,12 @@
Vector<RefPtr<IntersectionObserverEntry>> takeRecords();
+ void rootDestroyed();
+
private:
- IntersectionObserver(Ref<IntersectionObserverCallback>&&, RefPtr<Element>&& root, LengthBox&& parsedRootMargin, Vector<double>&& thresholds);
+ IntersectionObserver(Ref<IntersectionObserverCallback>&&, Element* root, LengthBox&& parsedRootMargin, Vector<double>&& thresholds);
- RefPtr<Element> m_root;
+ Element* m_root;
LengthBox m_rootMargin;
Vector<double> m_thresholds;
Ref<IntersectionObserverCallback> m_callback;
