Changeset 236781 in webkit


Ignore:
Timestamp:
Oct 2, 2018, 5:29:46 PM (7 years ago)
Author:
rniwa@webkit.org
Message:

GC can collect JS wrappers of nodes in the mutation records waiting to be delivered
​https://bugs.webkit.org/show_bug.cgi?id=190115

Reviewed by Geoffrey Garen.

Source/WebCore:

Fixed the bug by retaining JS wrappers of elements in mutation records using GCReachableRef.

This patch deploys GCReachableRef in two places: MutationObserver where each mutation record's
target is kept alive and MutationObserverRegistration where each node which had been removed
from an observed tree is kept alive for a subtree observation.

Test: fast/dom/MutationObserver/mutation-observer-retains-js-wrappers-of-targets-alive.html

  • dom/GCReachableRef.h:

(WebCore::GCReachableRef): Made it work with hash table.
(WebCore::GCReachableRef::operator T& const):
(WebCore::GCReachableRef::GCReachableRef):
(WebCore::GCReachableRef::isHashTableDeletedValue const):
(WebCore::GCReachableRef::isHashTableEmptyValue const):
(WebCore::GCReachableRef::ptrAllowingHashTableEmptyValue const):
(WebCore::GCReachableRef::ptrAllowingHashTableEmptyValue):
(WebCore::GCReachableRef::assignToHashTableEmptyValue):
(WTF::HashTraits<WebCore::GCReachableRef<P>>::emptyValue):
(WTF::HashTraits<WebCore::GCReachableRef<P>>::constructEmptyValue):
(WTF::HashTraits<WebCore::GCReachableRef<P>>::isEmptyValue):
(WTF::HashTraits<WebCore::GCReachableRef<P>>::assignToEmpty):
(WTF::HashTraits<WebCore::GCReachableRef<P>>::peek):
(WTF::HashTraits<WebCore::GCReachableRef<P>>::take):

  • dom/MutationObserver.cpp:

(WebCore::MutationObserver::takeRecords): Don't clear m_pendingTargets because that would allow wrappers
to be collected before elements in mutation records are accessed. We delay until the end of the current
microtask at which point deliver() function is called.
(WebCore::MutationObserver::disconnect):
(WebCore::MutationObserver::enqueueMutationRecord): Add the target to the list of elements to keep alive.
This is needed for a newly inserted node, a node with attribute change, etc...
(WebCore::MutationObserver::deliver): Keep the set of transient registration targets alive until mutation
records are delivered to each observer. These are nodes which had been removed from a tree and whose
subtree had still been obsreved up until this point.

  • dom/MutationObserver.h:
  • dom/MutationObserverRegistration.cpp:

(WebCore::MutationObserverRegistration::observedSubtreeNodeWillDetach):
(WebCore::MutationObserverRegistration::takeTransientRegistrations): Return the hash set of elemenets
that need to be kept alive so that MutationObserver::deliver can keep them alive until the deliver
function had been called.
(WebCore::MutationObserverRegistration::addRegistrationNodesToSet const):

  • dom/MutationObserverRegistration.h:

LayoutTests:

Added a regression test.

  • fast/dom/MutationObserver/mutation-observer-retains-js-wrappers-of-targets-alive-expected.txt: Added.
  • fast/dom/MutationObserver/mutation-observer-retains-js-wrappers-of-targets-alive.html: Added.
Location:
trunk
Files:
2 added
7 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r236779 r236781  
     12018-10-01  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        GC can collect JS wrappers of nodes in the mutation records waiting to be delivered
     4        https://bugs.webkit.org/show_bug.cgi?id=190115
     5
     6        Reviewed by Geoffrey Garen.
     7
     8        Added a regression test.
     9
     10        * fast/dom/MutationObserver/mutation-observer-retains-js-wrappers-of-targets-alive-expected.txt: Added.
     11        * fast/dom/MutationObserver/mutation-observer-retains-js-wrappers-of-targets-alive.html: Added.
     12
    1132018-10-02  Chris Dumez  <cdumez@apple.com>
    214

  • trunk/Source/WebCore/ChangeLog

    r236779 r236781  
     12018-10-01  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        GC can collect JS wrappers of nodes in the mutation records waiting to be delivered
     4        https://bugs.webkit.org/show_bug.cgi?id=190115
     5
     6        Reviewed by Geoffrey Garen.
     7
     8        Fixed the bug by retaining JS wrappers of elements in mutation records using GCReachableRef.
     9
     10        This patch deploys GCReachableRef in two places: MutationObserver where each mutation record's
     11        target is kept alive and MutationObserverRegistration where each node which had been removed
     12        from an observed tree is kept alive for a subtree observation.
     13
     14        Test: fast/dom/MutationObserver/mutation-observer-retains-js-wrappers-of-targets-alive.html
     15
     16        * dom/GCReachableRef.h:
     17        (WebCore::GCReachableRef): Made it work with hash table.
     18        (WebCore::GCReachableRef::operator T& const):
     19        (WebCore::GCReachableRef::GCReachableRef):
     20        (WebCore::GCReachableRef::isHashTableDeletedValue const):
     21        (WebCore::GCReachableRef::isHashTableEmptyValue const):
     22        (WebCore::GCReachableRef::ptrAllowingHashTableEmptyValue const):
     23        (WebCore::GCReachableRef::ptrAllowingHashTableEmptyValue):
     24        (WebCore::GCReachableRef::assignToHashTableEmptyValue):
     25        (WTF::HashTraits<WebCore::GCReachableRef<P>>::emptyValue):
     26        (WTF::HashTraits<WebCore::GCReachableRef<P>>::constructEmptyValue):
     27        (WTF::HashTraits<WebCore::GCReachableRef<P>>::isEmptyValue):
     28        (WTF::HashTraits<WebCore::GCReachableRef<P>>::assignToEmpty):
     29        (WTF::HashTraits<WebCore::GCReachableRef<P>>::peek):
     30        (WTF::HashTraits<WebCore::GCReachableRef<P>>::take):
     31        * dom/MutationObserver.cpp:
     32        (WebCore::MutationObserver::takeRecords): Don't clear m_pendingTargets because that would allow wrappers
     33        to be collected before elements in mutation records are accessed. We delay until the end of the current
     34        microtask at which point deliver() function is called.
     35        (WebCore::MutationObserver::disconnect):
     36        (WebCore::MutationObserver::enqueueMutationRecord): Add the target to the list of elements to keep alive.
     37        This is needed for a newly inserted node, a node with attribute change, etc...
     38        (WebCore::MutationObserver::deliver): Keep the set of transient registration targets alive until mutation
     39        records are delivered to each observer. These are nodes which had been removed from a tree and whose
     40        subtree had still been obsreved up until this point.
     41        * dom/MutationObserver.h:
     42        * dom/MutationObserverRegistration.cpp:
     43        (WebCore::MutationObserverRegistration::observedSubtreeNodeWillDetach):
     44        (WebCore::MutationObserverRegistration::takeTransientRegistrations): Return the hash set of elemenets
     45        that need to be kept alive so that MutationObserver::deliver can keep them alive until the deliver
     46        function had been called.
     47        (WebCore::MutationObserverRegistration::addRegistrationNodesToSet const):
     48        * dom/MutationObserverRegistration.h:
     49
    1502018-10-02  Chris Dumez  <cdumez@apple.com>
    251

  • trunk/Source/WebCore/dom/GCReachableRef.h

    r236693 r236781  
    2828#include <wtf/DumbPtrTraits.h>
    2929#include <wtf/HashCountedSet.h>
    30 #include <wtf/Ref.h>
     30#include <wtf/RefPtr.h>
    3131
    3232namespace WebCore {
    … …  
    7070    T* ptr() const RETURNS_NONNULL { return &get(); }
    7171    T& get() const { ASSERT(m_ptr); return *m_ptr; }
    72     operator T&() const { ASSERT(m_ptr); return *m_ptr; }
     72    operator T&() const { return get(); }
    7373    bool operator!() const { return !get(); }
    7474
     75    // Hash table deleted values, which are only constructed and never copied or destroyed.
     76    GCReachableRef(WTF::HashTableDeletedValueType)
     77        : m_ptr(RefPtr<T>::hashTableDeletedValue())
     78    { }
     79    bool isHashTableDeletedValue() const { return m_ptr.isHashTableDeletedValue(); }
     80
     81    GCReachableRef(WTF::HashTableEmptyValueType)
     82        : m_ptr(nullptr)
     83    { }
     84    bool isHashTableEmptyValue() const { return !m_ptr; }
     85
     86    const T* ptrAllowingHashTableEmptyValue() const { ASSERT(m_ptr || isHashTableEmptyValue()); return m_ptr.get(); }
     87    T* ptrAllowingHashTableEmptyValue() { ASSERT(m_ptr || isHashTableEmptyValue()); return m_ptr.get(); }
     88
     89    void assignToHashTableEmptyValue(GCReachableRef&& reference)
     90    {
     91        ASSERT(!m_ptr);
     92        m_ptr = WTFMove(reference.m_ptr);
     93        ASSERT(m_ptr);
     94    }
     95
    7596private:
    76 
    7797    RefPtr<T> m_ptr;
    7898};
    7999
    80 }
     100} // namespace WebCore
     101
     102namespace WTF {
     103
     104template<typename P> struct HashTraits<WebCore::GCReachableRef<P>> : SimpleClassHashTraits<WebCore::GCReachableRef<P>> {
     105    static const bool emptyValueIsZero = true;
     106    static WebCore::GCReachableRef<P> emptyValue() { return HashTableEmptyValue; }
     107
     108    template <typename>
     109    static void constructEmptyValue(WebCore::GCReachableRef<P>& slot)
     110    {
     111        new (NotNull, std::addressof(slot)) WebCore::GCReachableRef<P>(HashTableEmptyValue);
     112    }
     113
     114    static const bool hasIsEmptyValueFunction = true;
     115    static bool isEmptyValue(const WebCore::GCReachableRef<P>& value) { return value.isHashTableEmptyValue(); }
     116
     117    static void assignToEmpty(WebCore::GCReachableRef<P>& emptyValue, WebCore::GCReachableRef<P>&& newValue)
     118    {
     119        ASSERT(isEmptyValue(emptyValue));
     120        emptyValue.assignToHashTableEmptyValue(WTFMove(newValue));
     121    }
     122
     123    typedef P* PeekType;
     124    static PeekType peek(const Ref<P>& value) { return const_cast<PeekType>(value.ptrAllowingHashTableEmptyValue()); }
     125    static PeekType peek(P* value) { return value; }
     126
     127    typedef std::optional<Ref<P>> TakeType;
     128    static TakeType take(Ref<P>&& value) { return isEmptyValue(value) ? std::nullopt : std::optional<Ref<P>>(WTFMove(value)); }
     129};
     130
     131template <typename T, typename U>
     132struct GetPtrHelper<WebCore::GCReachableRef<T, U>> {
     133    typedef T* PtrType;
     134    static T* getPtr(const WebCore::GCReachableRef<T, U>& reference) { return const_cast<T*>(reference.ptr()); }
     135};
     136
     137template <typename T, typename U>
     138struct IsSmartPtr<WebCore::GCReachableRef<T, U>> {
     139    static const bool value = true;
     140};
     141
     142template<typename P> struct PtrHash<WebCore::GCReachableRef<P>> : PtrHashBase<WebCore::GCReachableRef<P>, IsSmartPtr<WebCore::GCReachableRef<P>>::value> {
     143    static const bool safeToCompareToEmptyOrDeleted = false;
     144};
     145
     146template<typename P> struct DefaultHash<WebCore::GCReachableRef<P>> {
     147    typedef PtrHash<WebCore::GCReachableRef<P>> Hash;
     148};
     149
     150} // namespace WTF
     151

  • trunk/Source/WebCore/dom/MutationObserver.cpp

    r236440 r236781  
    115115    Vector<Ref<MutationRecord>> records;
    116116    records.swap(m_records);
     117    // Don't clear m_pendingTargets here because we can collect JS wrappers
     118    // between the time takeRecords is called and nodes in records are accesssed.
    117119    return records;
    118120}
    … …  
    120122void MutationObserver::disconnect()
    121123{
     124    m_pendingTargets.clear();
    122125    m_records.clear();
    123126    HashSet<MutationObserverRegistration*> registrations(m_registrations);
    … …  
    182185{
    183186    ASSERT(isMainThread());
     187    ASSERT(mutation->target());
     188    m_pendingTargets.add(*mutation->target());
    184189    m_records.append(WTFMove(mutation));
    185190    activeMutationObservers().add(this);
    … …  
    222227    ASSERT(canDeliver());
    223228
    224     // Calling clearTransientRegistrations() can modify m_registrations, so it's necessary
     229    // Calling takeTransientRegistrations() can modify m_registrations, so it's necessary
    225230    // to make a copy of the transient registrations before operating on them.
    226231    Vector<MutationObserverRegistration*, 1> transientRegistrations;
     232    Vector<std::unique_ptr<HashSet<GCReachableRef<Node>>>, 1> nodesToKeepAlive;
     233    HashSet<GCReachableRef<Node>> pendingTargets;
     234    pendingTargets.swap(m_pendingTargets);
    227235    for (auto* registration : m_registrations) {
    228236        if (registration->hasTransientRegistrations())
    … …  
    230238    }
    231239    for (auto& registration : transientRegistrations)
    232         registration->clearTransientRegistrations();
     240        nodesToKeepAlive.append(registration->takeTransientRegistrations());
    233241
    234242    if (m_records.isEmpty())

  • trunk/Source/WebCore/dom/MutationObserver.h

    r230695 r236781  
    3333
    3434#include "ExceptionOr.h"
     35#include "GCReachableRef.h"
    3536#include <wtf/Forward.h>
    3637#include <wtf/HashSet.h>
    … …  
    110111    Ref<MutationCallback> m_callback;
    111112    Vector<Ref<MutationRecord>> m_records;
     113    HashSet<GCReachableRef<Node>> m_pendingTargets;
    112114    HashSet<MutationObserverRegistration*> m_registrations;
    113115    unsigned m_priority;

  • trunk/Source/WebCore/dom/MutationObserverRegistration.cpp

    r210216 r236781  
    4949MutationObserverRegistration::~MutationObserverRegistration()
    5050{
    51     clearTransientRegistrations();
     51    takeTransientRegistrations();
    5252    m_observer->observationEnded(*this);
    5353}
    … …  
    5555void MutationObserverRegistration::resetObservation(MutationObserverOptions options, const HashSet<AtomicString>& attributeFilter)
    5656{
    57     clearTransientRegistrations();
     57    takeTransientRegistrations();
    5858    m_options = options;
    5959    m_attributeFilter = attributeFilter;
    … …  
    6969
    7070    if (!m_transientRegistrationNodes) {
    71         m_transientRegistrationNodes = std::make_unique<HashSet<RefPtr<Node>>>();
     71        m_transientRegistrationNodes = std::make_unique<HashSet<GCReachableRef<Node>>>();
    7272
    7373        ASSERT(!m_nodeKeptAlive);
    74         m_nodeKeptAlive = &m_node; // Balanced in clearTransientRegistrations.
     74        m_nodeKeptAlive = &m_node; // Balanced in takeTransientRegistrations.
    7575    }
    76     m_transientRegistrationNodes->add(&node);
     76    m_transientRegistrationNodes->add(node);
    7777}
    7878
    79 void MutationObserverRegistration::clearTransientRegistrations()
     79std::unique_ptr<HashSet<GCReachableRef<Node>>> MutationObserverRegistration::takeTransientRegistrations()
    8080{
    8181    if (!m_transientRegistrationNodes) {
    8282        ASSERT(!m_nodeKeptAlive);
    83         return;
     83        return nullptr;
    8484    }
    8585
    … …  
    8787        node->unregisterTransientMutationObserver(*this);
    8888
    89     m_transientRegistrationNodes = nullptr;
     89    auto returnValue = WTFMove(m_transientRegistrationNodes);
    9090
    9191    ASSERT(m_nodeKeptAlive);
    9292    m_nodeKeptAlive = nullptr; // Balanced in observeSubtreeNodeWillDetach.
     93
     94    return returnValue;
    9395}
    9496
    … …  
    117119        return;
    118120    for (auto& node : *m_transientRegistrationNodes)
    119         nodes.add(node.get());
     121        nodes.add(node.ptr());
    120122}
    121123

  • trunk/Source/WebCore/dom/MutationObserverRegistration.h

    r210216 r236781  
    3131#pragma once
    3232
     33#include "GCReachableRef.h"
    3334#include "MutationObserver.h"
    3435#include <wtf/HashSet.h>
    … …  
    4849    void resetObservation(MutationObserverOptions, const HashSet<AtomicString>& attributeFilter);
    4950    void observedSubtreeNodeWillDetach(Node&);
    50     void clearTransientRegistrations();
     51    std::unique_ptr<HashSet<GCReachableRef<Node>>> takeTransientRegistrations();
    5152    bool hasTransientRegistrations() const { return m_transientRegistrationNodes && !m_transientRegistrationNodes->isEmpty(); }
    5253
    … …  
    6566    Node& m_node;
    6667    RefPtr<Node> m_nodeKeptAlive;
    67     std::unique_ptr<HashSet<RefPtr<Node>>> m_transientRegistrationNodes;
     68    std::unique_ptr<HashSet<GCReachableRef<Node>>> m_transientRegistrationNodes;
    6869    MutationObserverOptions m_options;
    6970    HashSet<AtomicString> m_attributeFilter;
Note: See TracChangeset for help on using the changeset viewer.