Changeset 234884 in webkit

Timestamp:

Aug 15, 2018 7:13:06 AM (6 years ago)

Author:

ajuma@chromium.org

Message:

[IntersectionObserver] Do not hold a strong reference to the root element
​https://bugs.webkit.org/show_bug.cgi?id=188575

Reviewed by Simon Fraser.

Source/WebCore:

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):

LayoutTests:

• intersection-observer/root-element-deleted-expected.txt: Added.
• intersection-observer/root-element-deleted.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/intersection-observer/root-element-deleted-expected.txt (added)
• LayoutTests/intersection-observer/root-element-deleted.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/dom/Element.cpp (modified) (2 diffs)
• Source/WebCore/dom/Element.h (modified) (3 diffs)
• Source/WebCore/dom/ElementRareData.cpp (modified) (1 diff)
• Source/WebCore/dom/ElementRareData.h (modified) (3 diffs)
• Source/WebCore/page/IntersectionObserver.cpp (modified) (2 diffs)
• Source/WebCore/page/IntersectionObserver.h (modified) (3 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2018-08-15  Ali Juma  <ajuma@chromium.org>
+
+        [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  <zalan@apple.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2018-08-15  Ali Juma  <ajuma@chromium.org>
+
+        [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  <zdobersek@igalia.com>
 

trunk/Source/WebCore/dom/Element.cpp

@@ -188 +188 @@
     ASSERT(!afterPseudoElement());
 
+#if ENABLE(INTERSECTION_OBSERVER)
+    disconnectFromIntersectionObservers();
+#endif
+
     removeShadowRoot();
 
@@ -3234 +3238 @@
 #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
 {

trunk/Source/WebCore/dom/Element.h

@@ -57 +57 @@
 struct ElementStyle;
 
+#if ENABLE(INTERSECTION_OBSERVER)
+struct IntersectionObserverData;
+#endif
+
 enum SpellcheckAttributeState {
     SpellcheckAttributeTrue,
@@ -568 +572 @@
     void setAttributeEventListener(const AtomicString& eventType, const QualifiedName& attributeName, const AtomicString& value);
 
+#if ENABLE(INTERSECTION_OBSERVER)
+    IntersectionObserverData& ensureIntersectionObserverData();
+    IntersectionObserverData* intersectionObserverData();
+#endif
+
     Element* findAnchorElementForLink(String& outAnchorName);
 
@@ -645 +654 @@
 #if ENABLE(TREE_DEBUGGING)
     void formatForDebugger(char* buffer, unsigned length) const override;
+#endif
+
+#if ENABLE(INTERSECTION_OBSERVER)
+    void disconnectFromIntersectionObservers();
 #endif
 

trunk/Source/WebCore/dom/ElementRareData.cpp

@@ -45 +45 @@
     IntPoint savedLayerScrollPosition;
     void* pointers[8];
+#if ENABLE(INTERSECTION_OBSERVER)
+    void* intersectionObserverData;
+#endif
+
 };
 

trunk/Source/WebCore/dom/ElementRareData.h

@@ -25 +25 @@
 #include "DOMTokenList.h"
 #include "DatasetDOMStringMap.h"
+#include "IntersectionObserver.h"
 #include "NamedNodeMap.h"
 #include "NodeRareData.h"
@@ -116 +117 @@
     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:
@@ -150 +156 @@
     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;

trunk/Source/WebCore/page/IntersectionObserver.cpp

@@ -101 +101 @@
     }
 
-    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);
 }
 
@@ -146 +156 @@
 }
 
+void IntersectionObserver::rootDestroyed()
+{
+    ASSERT(m_root);
+    disconnect();
+    m_root = nullptr;
+}
 
 } // namespace WebCore

trunk/Source/WebCore/page/IntersectionObserver.h

@@ -39 +39 @@
 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;
@@ -48 +57 @@
 
     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; }
@@ -59 +70 @@
     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;