Diff
Modified: trunk/Source/WebCore/ChangeLog (267174 => 267175)
--- trunk/Source/WebCore/ChangeLog 2020-09-17 00:23:46 UTC (rev 267174)
+++ trunk/Source/WebCore/ChangeLog 2020-09-17 01:53:21 UTC (rev 267175)
@@ -1,3 +1,64 @@
+2020-09-16 Ryosuke Niwa <rn...@webkit.org>
+
+ MutationObserverRegistration should be ref counted
+ https://bugs.webkit.org/show_bug.cgi?id=216528
+
+ Reviewed by Darin Adler.
+
+ This patch makes MutationObserverRegistration ref counted so that the transient registry can store
+ Ref<MutationObserverRegistration> instead of a raw pointer. It also merges NodeMutationObserverData
+ into NodeRareData now that NodeRareData is actually rare after r266714 and r266769.
+
+ Before this patch, MutationObserverRegistration had to be kept alive by the originally observed node
+ using m_nodeKeptAlive whenever a transient observation occurs (i.e. observation of a node which used
+ to be a part of the subtree of the observed node). With this patch, this complexity is eliminated.
+
+ No new tests since there should be no behavioral difference.
+
+ * dom/MutationObserver.cpp:
+ (WebCore::MutationObserver::disconnect): Skip any node for which the observed node had been deleted.
+ This means that the registration is alive for transient nodes (i.e. a node which used to be a part of
+ the subtree of the observed node).
+ (WebCore::MutationObserver::observationStarted): Converted debug assert to a release assert.
+ (WebCore::MutationObserver::observationEnded): Ditto.
+ (WebCore::MutationObserver::deliver): Simplified logic a bit using WTF::map.
+ * dom/MutationObserverRegistration.cpp:
+ (WebCore::MutationObserverRegistration::create): Added.
+ (WebCore::MutationObserverRegistration::MutationObserverRegistration): Create a WeakPtr for m_node.
+ (WebCore::MutationObserverRegistration::observedSubtreeNodeWillDetach): Deleted the code for
+ m_nodeKeptAlive since the observed node is no longer responsible for keeping this
+ MutationObserverRegistration alive while transient observarion takes place.
+ (WebCore::MutationObserverRegistration::takeTransientRegistrations): Ditto.
+ (WebCore::MutationObserverRegistration::shouldReceiveMutationFrom const):
+ (WebCore::MutationObserverRegistration::addRegistrationNodesToSet const): Skip m_node if it has been
+ deleted. This is possible during transient observations.
+ * dom/MutationObserverRegistration.h:
+ (WebCore::MutationObserverRegistration): This class is now ref counted. Also made m_node WeakPtr.
+ (WebCore::MutationObserverRegistration::node):
+ * dom/Node.cpp:
+ (WebCore::Node::clearRareData):
+ (WebCore::Node::moveNodeToNewDocument):
+ (WebCore::Node::mutationObserverRegistry): Deleted.
+ (WebCore::Node::transientMutationObserverRegistry): Deleted.
+ (WebCore::collectMatchingObserversForMutation): Now takes a reference to registry.
+ (WebCore::Node::registeredMutationObservers):
+ (WebCore::Node::registerMutationObserver):
+ (WebCore::Node::unregisterMutationObserver): Converted debug assertions to release assertions. With
+ script execution forbidden during tree mutations, this should never happen now.
+ (WebCore::Node::registerTransientMutationObserver):
+ (WebCore::Node::unregisterTransientMutationObserver): Ditto.
+ (WebCore::Node::notifyMutationObserversNodeWillDetach):
+ * dom/Node.h:
+ * dom/NodeRareData.cpp:
+ * dom/NodeRareData.h:
+ (WebCore::NodeMutationObserverData): Merged into NodeRareData.
+ (WebCore::NodeRareData::mutationObserverData): Deleted.
+ (WebCore::NodeRareData::ensureMutationObserverData): Deleted.
+ (WebCore::NodeRareData::mutationObserverRegistry): Added.
+ (WebCore::NodeRareData::mutationObserverRegistryIfExists): Added.
+ (WebCore::NodeRareData::transientMutationObserverRegistry): Added.
+ (WebCore::NodeRareData::useTypes const):
+
2020-09-16 Sam Weinig <wei...@apple.com>
Remove runtime setting for enabling/disabling CSS shadow parts
Modified: trunk/Source/WebCore/dom/MutationObserver.cpp (267174 => 267175)
--- trunk/Source/WebCore/dom/MutationObserver.cpp 2020-09-17 00:23:46 UTC (rev 267174)
+++ trunk/Source/WebCore/dom/MutationObserver.cpp 2020-09-17 01:53:21 UTC (rev 267175)
@@ -120,21 +120,26 @@
{
m_pendingTargets.clear();
m_records.clear();
- HashSet<MutationObserverRegistration*> registrations(m_registrations);
- for (auto* registration : registrations)
- registration->node().unregisterMutationObserver(*registration);
+ auto registrationAndNodeList = WTF::compactMap(m_registrations, [](auto* registration) -> Optional<std::pair<Ref<Node>, Ref<MutationObserverRegistration>>> {
+ auto node = makeRefPtr(registration->node());
+ if (!node)
+ return WTF::nullopt;
+ return { { node.releaseNonNull(), makeRef(*registration) } };
+ });
+ for (auto& registrationAndNode : registrationAndNodeList)
+ registrationAndNode.first->unregisterMutationObserver(registrationAndNode.second.get());
}
void MutationObserver::observationStarted(MutationObserverRegistration& registration)
{
- ASSERT(!m_registrations.contains(®istration));
- m_registrations.add(®istration);
+ auto result = m_registrations.add(®istration);
+ RELEASE_ASSERT(result.isNewEntry);
}
void MutationObserver::observationEnded(MutationObserverRegistration& registration)
{
- ASSERT(m_registrations.contains(®istration));
- m_registrations.remove(®istration);
+ auto removed = m_registrations.remove(®istration);
+ RELEASE_ASSERT(removed);
}
void MutationObserver::enqueueMutationRecord(Ref<MutationRecord>&& mutation)
@@ -186,18 +191,14 @@
{
ASSERT(canDeliver());
- // Calling takeTransientRegistrations() can modify m_registrations, so it's necessary
- // to make a copy of the transient registrations before operating on them.
- Vector<MutationObserverRegistration*, 1> transientRegistrations;
Vector<std::unique_ptr<HashSet<GCReachableRef<Node>>>, 1> nodesToKeepAlive;
HashSet<GCReachableRef<Node>> pendingTargets;
pendingTargets.swap(m_pendingTargets);
- for (auto* registration : m_registrations) {
+ auto registrations = WTF::map(m_registrations, [](auto* registration) { return makeRef(*registration); });
+ for (auto& registration : registrations) {
if (registration->hasTransientRegistrations())
- transientRegistrations.append(registration);
+ nodesToKeepAlive.append(registration->takeTransientRegistrations());
}
- for (auto& registration : transientRegistrations)
- nodesToKeepAlive.append(registration->takeTransientRegistrations());
if (m_records.isEmpty()) {
ASSERT(m_pendingTargets.isEmpty());
Modified: trunk/Source/WebCore/dom/MutationObserverRegistration.cpp (267174 => 267175)
--- trunk/Source/WebCore/dom/MutationObserverRegistration.cpp 2020-09-17 00:23:46 UTC (rev 267174)
+++ trunk/Source/WebCore/dom/MutationObserverRegistration.cpp 2020-09-17 01:53:21 UTC (rev 267175)
@@ -37,9 +37,14 @@
namespace WebCore {
+Ref<MutationObserverRegistration> MutationObserverRegistration::create(MutationObserver& observer, Node& node, MutationObserverOptions options, const HashSet<AtomString>& attributeFilter)
+{
+ return adoptRef(*new MutationObserverRegistration(observer, node, options, attributeFilter));
+}
+
MutationObserverRegistration::MutationObserverRegistration(MutationObserver& observer, Node& node, MutationObserverOptions options, const HashSet<AtomString>& attributeFilter)
: m_observer(observer)
- , m_node(node)
+ , m_node(makeWeakPtr(node))
, m_options(options)
, m_attributeFilter(attributeFilter)
{
@@ -67,31 +72,21 @@
node.registerTransientMutationObserver(*this);
m_observer->setHasTransientRegistration(node.document());
- if (!m_transientRegistrationNodes) {
+ if (!m_transientRegistrationNodes)
m_transientRegistrationNodes = makeUnique<HashSet<GCReachableRef<Node>>>();
-
- ASSERT(!m_nodeKeptAlive);
- m_nodeKeptAlive = &m_node; // Balanced in takeTransientRegistrations.
- }
m_transientRegistrationNodes->add(node);
}
std::unique_ptr<HashSet<GCReachableRef<Node>>> MutationObserverRegistration::takeTransientRegistrations()
{
- if (!m_transientRegistrationNodes) {
- ASSERT(!m_nodeKeptAlive);
+ if (!m_transientRegistrationNodes)
return nullptr;
- }
- for (auto& node : *m_transientRegistrationNodes)
+ auto transientNodes = WTFMove(m_transientRegistrationNodes);
+ for (auto& node : *transientNodes)
node->unregisterTransientMutationObserver(*this);
- auto returnValue = WTFMove(m_transientRegistrationNodes);
-
- ASSERT(m_nodeKeptAlive);
- m_nodeKeptAlive = nullptr; // Balanced in observeSubtreeNodeWillDetach.
-
- return returnValue;
+ return transientNodes;
}
bool MutationObserverRegistration::shouldReceiveMutationFrom(Node& node, MutationObserver::MutationType type, const QualifiedName* attributeName) const
@@ -100,7 +95,7 @@
if (!(m_options & type))
return false;
- if (&m_node != &node && !isSubtree())
+ if (m_node.get() != &node && !isSubtree())
return false;
if (type != MutationObserver::Attributes || !(m_options & MutationObserver::AttributeFilter))
@@ -114,7 +109,9 @@
void MutationObserverRegistration::addRegistrationNodesToSet(HashSet<Node*>& nodes) const
{
- nodes.add(&m_node);
+ ASSERT(m_node.get() || m_transientRegistrationNodes);
+ if (auto* observedNode = m_node.get())
+ nodes.add(observedNode);
if (!m_transientRegistrationNodes)
return;
for (auto& node : *m_transientRegistrationNodes)
Modified: trunk/Source/WebCore/dom/MutationObserverRegistration.h (267174 => 267175)
--- trunk/Source/WebCore/dom/MutationObserverRegistration.h 2020-09-17 00:23:46 UTC (rev 267174)
+++ trunk/Source/WebCore/dom/MutationObserverRegistration.h 2020-09-17 01:53:21 UTC (rev 267175)
@@ -33,6 +33,7 @@
#include "GCReachableRef.h"
#include "MutationObserver.h"
#include <wtf/HashSet.h>
+#include <wtf/WeakPtr.h>
#include <wtf/text/AtomString.h>
#include <wtf/text/AtomStringHash.h>
@@ -40,10 +41,11 @@
class QualifiedName;
-class MutationObserverRegistration {
+class MutationObserverRegistration : public RefCounted<MutationObserverRegistration> {
WTF_MAKE_FAST_ALLOCATED;
public:
- MutationObserverRegistration(MutationObserver&, Node&, MutationObserverOptions, const HashSet<AtomString>& attributeFilter);
+ static Ref<MutationObserverRegistration> create(MutationObserver&, Node&, MutationObserverOptions, const HashSet<AtomString>& attributeFilter);
+
~MutationObserverRegistration();
void resetObservation(MutationObserverOptions, const HashSet<AtomString>& attributeFilter);
@@ -55,7 +57,7 @@
bool isSubtree() const { return m_options & MutationObserver::Subtree; }
MutationObserver& observer() { return m_observer.get(); }
- Node& node() { return m_node; }
+ Node* node() { return m_node.get(); }
MutationRecordDeliveryOptions deliveryOptions() const { return m_options & (MutationObserver::AttributeOldValue | MutationObserver::CharacterDataOldValue); }
MutationObserverOptions mutationTypes() const { return m_options & MutationObserver::AllMutationTypes; }
@@ -62,9 +64,10 @@
void addRegistrationNodesToSet(HashSet<Node*>&) const;
private:
+ MutationObserverRegistration(MutationObserver&, Node&, MutationObserverOptions, const HashSet<AtomString>& attributeFilter);
+
Ref<MutationObserver> m_observer;
- Node& m_node;
- RefPtr<Node> m_nodeKeptAlive;
+ WeakPtr<Node> m_node;
std::unique_ptr<HashSet<GCReachableRef<Node>>> m_transientRegistrationNodes;
MutationObserverOptions m_options;
HashSet<AtomString> m_attributeFilter;
Modified: trunk/Source/WebCore/dom/Node.cpp (267174 => 267175)
--- trunk/Source/WebCore/dom/Node.cpp 2020-09-17 00:23:46 UTC (rev 267174)
+++ trunk/Source/WebCore/dom/Node.cpp 2020-09-17 01:53:21 UTC (rev 267175)
@@ -429,8 +429,7 @@
void Node::clearRareData()
{
ASSERT(hasRareData());
- ASSERT(!transientMutationObserverRegistry() || transientMutationObserverRegistry()->isEmpty());
-
+ ASSERT(rareData()->transientMutationObserverRegistry().isEmpty());
m_rareDataWithBitfields.setPointer(nullptr);
}
@@ -2035,19 +2034,15 @@
oldDocument.decrementReferencingNodeCount();
if (hasRareData()) {
- if (auto* nodeLists = rareData()->nodeLists())
+ auto* rareData = this->rareData();
+ if (auto* nodeLists = rareData->nodeLists())
nodeLists->adoptDocument(oldDocument, newDocument);
- if (auto* registry = mutationObserverRegistry()) {
+ if (auto* registry = rareData->mutationObserverRegistryIfExists()) {
for (auto& registration : *registry)
newDocument.addMutationObserverTypes(registration->mutationTypes());
}
- if (auto* transientRegistry = transientMutationObserverRegistry()) {
- for (auto& registration : *transientRegistry)
- newDocument.addMutationObserverTypes(registration->mutationTypes());
- }
- } else {
- ASSERT(!mutationObserverRegistry());
- ASSERT(!transientMutationObserverRegistry());
+ for (auto& registration : rareData->transientMutationObserverRegistry())
+ newDocument.addMutationObserverTypes(registration->mutationTypes());
}
oldDocument.moveNodeIteratorsToNewDocument(*this, newDocument);
@@ -2247,32 +2242,9 @@
eventTargetDataMap().remove(this);
}
-Vector<std::unique_ptr<MutationObserverRegistration>>* Node::mutationObserverRegistry()
+template<typename Registry> static inline void collectMatchingObserversForMutation(HashMap<Ref<MutationObserver>, MutationRecordDeliveryOptions>& observers, Registry& registry, Node& target, MutationObserver::MutationType type, const QualifiedName* attributeName)
{
- if (!hasRareData())
- return nullptr;
- auto* data = ""
- if (!data)
- return nullptr;
- return &data->registry;
-}
-
-HashSet<MutationObserverRegistration*>* Node::transientMutationObserverRegistry()
-{
- if (!hasRareData())
- return nullptr;
- auto* data = ""
- if (!data)
- return nullptr;
- return &data->transientRegistry;
-}
-
-template<typename Registry> static inline void collectMatchingObserversForMutation(HashMap<Ref<MutationObserver>, MutationRecordDeliveryOptions>& observers, Registry* registry, Node& target, MutationObserver::MutationType type, const QualifiedName* attributeName)
-{
- if (!registry)
- return;
-
- for (auto& registration : *registry) {
+ for (auto& registration : registry) {
if (registration->shouldReceiveMutationFrom(target, type, attributeName)) {
auto deliveryOptions = registration->deliveryOptions();
auto result = observers.add(registration->observer(), deliveryOptions);
@@ -2286,11 +2258,13 @@
{
HashMap<Ref<MutationObserver>, MutationRecordDeliveryOptions> result;
ASSERT((type == MutationObserver::Attributes && attributeName) || !attributeName);
- collectMatchingObserversForMutation(result, mutationObserverRegistry(), *this, type, attributeName);
- collectMatchingObserversForMutation(result, transientMutationObserverRegistry(), *this, type, attributeName);
- for (Node* node = parentNode(); node; node = node->parentNode()) {
- collectMatchingObserversForMutation(result, node->mutationObserverRegistry(), *this, type, attributeName);
- collectMatchingObserversForMutation(result, node->transientMutationObserverRegistry(), *this, type, attributeName);
+ for (auto node = makeRefPtr(this); node; node = node->parentNode()) {
+ if (!node->hasRareData())
+ continue;
+ auto* rareData = node->rareData();
+ if (auto* registry = rareData->mutationObserverRegistryIfExists())
+ collectMatchingObserversForMutation(result, *registry, *this, type, attributeName);
+ collectMatchingObserversForMutation(result, rareData->transientMutationObserverRegistry(), *this, type, attributeName);
}
return result;
}
@@ -2297,50 +2271,40 @@
void Node::registerMutationObserver(MutationObserver& observer, MutationObserverOptions options, const HashSet<AtomString>& attributeFilter)
{
- MutationObserverRegistration* registration = nullptr;
- auto& registry = ensureRareData().ensureMutationObserverData().registry;
-
- for (auto& candidateRegistration : registry) {
- if (&candidateRegistration->observer() == &observer) {
- registration = candidateRegistration.get();
- registration->resetObservation(options, attributeFilter);
- }
+ auto& registry = ensureRareData().mutationObserverRegistry();
+ auto index = registry.findMatching([&observer](auto& registration) { return ®istration->observer() == &observer; });
+ if (index != notFound) {
+ auto registration = registry[index].copyRef();
+ registration->resetObservation(options, attributeFilter);
+ document().addMutationObserverTypes(registration->mutationTypes());
+ return;
}
-
- if (!registration) {
- registry.append(makeUnique<MutationObserverRegistration>(observer, *this, options, attributeFilter));
- registration = registry.last().get();
- }
-
- document().addMutationObserverTypes(registration->mutationTypes());
+ auto newRegistration = MutationObserverRegistration::create(observer, *this, options, attributeFilter);
+ document().addMutationObserverTypes(newRegistration->mutationTypes());
+ registry.append(WTFMove(newRegistration));
}
void Node::unregisterMutationObserver(MutationObserverRegistration& registration)
{
- auto* registry = mutationObserverRegistry();
+ ASSERT(hasRareData());
+ auto* registry = rareData()->mutationObserverRegistryIfExists();
ASSERT(registry);
- if (!registry)
- return;
-
- registry->removeFirstMatching([®istration] (auto& current) {
- return current.get() == ®istration;
+ auto removed = registry->removeFirstMatching([®istration] (auto& current) {
+ return current.ptr() == ®istration;
});
+ RELEASE_ASSERT(removed);
}
void Node::registerTransientMutationObserver(MutationObserverRegistration& registration)
{
- ensureRareData().ensureMutationObserverData().transientRegistry.add(®istration);
+ ensureRareData().transientMutationObserverRegistry().add(registration);
}
void Node::unregisterTransientMutationObserver(MutationObserverRegistration& registration)
{
- auto* transientRegistry = transientMutationObserverRegistry();
- ASSERT(transientRegistry);
- if (!transientRegistry)
- return;
-
- ASSERT(transientRegistry->contains(®istration));
- transientRegistry->remove(®istration);
+ ASSERT(hasRareData());
+ bool removed = rareData()->transientMutationObserverRegistry().remove(®istration);
+ RELEASE_ASSERT(removed);
}
void Node::notifyMutationObserversNodeWillDetach()
@@ -2349,14 +2313,15 @@
return;
for (Node* node = parentNode(); node; node = node->parentNode()) {
- if (auto* registry = node->mutationObserverRegistry()) {
+ if (!node->hasRareData())
+ continue;
+ auto* rareData = node->rareData();
+ if (auto* registry = rareData->mutationObserverRegistryIfExists()) {
for (auto& registration : *registry)
registration->observedSubtreeNodeWillDetach(*this);
}
- if (auto* transientRegistry = node->transientMutationObserverRegistry()) {
- for (auto* registration : *transientRegistry)
- registration->observedSubtreeNodeWillDetach(*this);
- }
+ for (auto& registration : rareData->transientMutationObserverRegistry())
+ registration->observedSubtreeNodeWillDetach(*this);
}
}
Modified: trunk/Source/WebCore/dom/Node.h (267174 => 267175)
--- trunk/Source/WebCore/dom/Node.h 2020-09-17 00:23:46 UTC (rev 267174)
+++ trunk/Source/WebCore/dom/Node.h 2020-09-17 01:53:21 UTC (rev 267175)
@@ -699,9 +699,6 @@
void trackForDebugging();
void materializeRareData();
- Vector<std::unique_ptr<MutationObserverRegistration>>* mutationObserverRegistry();
- HashSet<MutationObserverRegistration*>* transientMutationObserverRegistry();
-
void adjustStyleValidity(Style::Validity, Style::InvalidationMode);
void* opaqueRootSlow() const;
Modified: trunk/Source/WebCore/dom/NodeRareData.cpp (267174 => 267175)
--- trunk/Source/WebCore/dom/NodeRareData.cpp 2020-09-17 00:23:46 UTC (rev 267174)
+++ trunk/Source/WebCore/dom/NodeRareData.cpp 2020-09-17 01:53:21 UTC (rev 267175)
@@ -38,7 +38,10 @@
struct SameSizeAsNodeRareData {
uint32_t m_tabIndex;
uint32_t m_childIndexAndIsElementRareDataFlag;
- void* m_pointer[2];
+ void* m_pointer[3];
+#if CHECK_HASHTABLE_ITERATORS
+ void* m_hashTableIteratorsAndMutex[2];
+#endif
};
COMPILE_ASSERT(sizeof(NodeRareData) == sizeof(SameSizeAsNodeRareData), NodeRareDataShouldStaySmall);
Modified: trunk/Source/WebCore/dom/NodeRareData.h (267174 => 267175)
--- trunk/Source/WebCore/dom/NodeRareData.h 2020-09-17 00:23:46 UTC (rev 267174)
+++ trunk/Source/WebCore/dom/NodeRareData.h 2020-09-17 01:53:21 UTC (rev 267175)
@@ -229,15 +229,6 @@
CollectionCacheMap m_cachedCollections;
};
-class NodeMutationObserverData {
- WTF_MAKE_NONCOPYABLE(NodeMutationObserverData); WTF_MAKE_FAST_ALLOCATED;
-public:
- Vector<std::unique_ptr<MutationObserverRegistration>> registry;
- HashSet<MutationObserverRegistration*> transientRegistry;
-
- NodeMutationObserverData() { }
-};
-
DECLARE_ALLOCATOR_WITH_HEAP_IDENTIFIER(NodeRareData);
class NodeRareData {
WTF_MAKE_NONCOPYABLE(NodeRareData);
@@ -284,13 +275,22 @@
return *m_nodeLists;
}
- NodeMutationObserverData* mutationObserverData() { return m_mutationObserverData.get(); }
- NodeMutationObserverData& ensureMutationObserverData()
+ using MutationObserverRegistryVector = Vector<Ref<MutationObserverRegistration>, 4>;
+
+ MutationObserverRegistryVector& mutationObserverRegistry()
{
- if (!m_mutationObserverData)
- m_mutationObserverData = makeUnique<NodeMutationObserverData>();
- return *m_mutationObserverData;
+ if (!m_mutationObserverRegistry)
+ m_mutationObserverRegistry = makeUnique<MutationObserverRegistryVector>();
+ return *m_mutationObserverRegistry;
}
+ MutationObserverRegistryVector* mutationObserverRegistryIfExists()
+ {
+ return m_mutationObserverRegistry.get();
+ }
+ HashSet<Ref<MutationObserverRegistration>>& transientMutationObserverRegistry()
+ {
+ return m_transientMutationObserverRegistry;
+ }
#if DUMP_NODE_STATISTICS
OptionSet<UseType> useTypes() const
@@ -298,7 +298,7 @@
OptionSet<UseType> result;
if (m_nodeLists)
result.add(UseType::NodeList);
- if (m_mutationObserverData)
+ if (m_mutationObserverRegistry || m_transientMutationObserverRegistry.capacity())
result.add(UseType::MutationObserver);
return result;
}
@@ -313,7 +313,8 @@
bool m_isElementRareData;
std::unique_ptr<NodeListsNodeData> m_nodeLists;
- std::unique_ptr<NodeMutationObserverData> m_mutationObserverData;
+ std::unique_ptr<MutationObserverRegistryVector> m_mutationObserverRegistry;
+ HashSet<Ref<MutationObserverRegistration>> m_transientMutationObserverRegistry;
};
template<> struct NodeListTypeIdentifier<NameNodeList> {
