Skip to content
Permalink
Browse files
Consolidate calls to insertedInto and expand the coverage of NoEventD…
…ispatchAssertion

https://bugs.webkit.org/show_bug.cgi?id=178504

Reviewed by Antti Koivisto.

Consolidated calls to notifyChildNodeInserted, childrenChanged, didFinishInsertingNode, and
dispatchChildInsertionEvents for inserting a node by executeNodeInsertionWithScriptAssertion,
a new templatefunction which takes a closure to do the node insertion to make exactly when
the script becomes runnable clear.

Added an exception to SVGTRefElement::updateReferencedText since this code mutates user agent
shadow root during insertedInto, and turned ChildChangeSource into an enum class.

* dom/CharacterData.cpp:
(WebCore::CharacterData::parserAppendData):
(WebCore::CharacterData::setDataAndUpdate):
* dom/ContainerNode.cpp:
(WebCore::executeNodeInsertionWithScriptAssertion): Extracted.
(WebCore::ContainerNode::takeAllChildrenFrom): Deployed executeNodeInsertionWithScriptAssertion.
(WebCore::ContainerNode::insertBefore): Ditto.
(WebCore::ContainerNode::changeForChildInsertion): Deleted.
(WebCore::ContainerNode::notifyChildInserted): Deleted.
(WebCore::ContainerNode::parserInsertBefore): Deployed executeNodeInsertionWithScriptAssertion.
(WebCore::ContainerNode::replaceChild): Ditto.
(WebCore::ContainerNode::removeChild):
(WebCore::ContainerNode::parserRemoveChild):
(WebCore::ContainerNode::replaceAllChildren): Ditto.
(WebCore::ContainerNode::removeChildren):
(WebCore::ContainerNode::appendChildWithoutPreInsertionValidityCheck): Ditto.
(WebCore::ContainerNode::parserAppendChild): Ditto.
(WebCore::ContainerNode::childrenChanged):
(WebCore::ContainerNode::updateTreeAfterInsertion): Deleted.
* dom/ContainerNode.h:
* dom/Element.cpp:
(WebCore::Element::childrenChanged):
* html/HTMLOutputElement.cpp:
(WebCore::HTMLOutputElement::childrenChanged):
* svg/SVGClipPathElement.cpp:
(WebCore::SVGClipPathElement::childrenChanged):
* svg/SVGElement.cpp:
(WebCore::SVGElement::childrenChanged):
* svg/SVGFELightElement.cpp:
(WebCore::SVGFELightElement::childrenChanged):
* svg/SVGFilterElement.cpp:
(WebCore::SVGFilterElement::childrenChanged):
* svg/SVGFilterPrimitiveStandardAttributes.cpp:
(WebCore::SVGFilterPrimitiveStandardAttributes::childrenChanged):
* svg/SVGGradientElement.cpp:
(WebCore::SVGGradientElement::childrenChanged):
* svg/SVGMarkerElement.cpp:
(WebCore::SVGMarkerElement::childrenChanged):
* svg/SVGMaskElement.cpp:
(WebCore::SVGMaskElement::childrenChanged):
* svg/SVGPatternElement.cpp:
(WebCore::SVGPatternElement::childrenChanged):
* svg/SVGTRefElement.cpp:
(WebCore::SVGTRefElement::updateReferencedText): Allow DOM mutations inside the user agent shadow tree here.


Canonical link: https://commits.webkit.org/194705@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@223687 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
rniwa committed Oct 19, 2017
1 parent ef0f041 commit 34c2ef31e2d069ec715511fee4ed6fdf294d5198
@@ -1,3 +1,63 @@
2017-10-19 Ryosuke Niwa <rniwa@webkit.org>

Consolidate calls to insertedInto and expand the coverage of NoEventDispatchAssertion
https://bugs.webkit.org/show_bug.cgi?id=178504

Reviewed by Antti Koivisto.

Consolidated calls to notifyChildNodeInserted, childrenChanged, didFinishInsertingNode, and
dispatchChildInsertionEvents for inserting a node by executeNodeInsertionWithScriptAssertion,
a new templatefunction which takes a closure to do the node insertion to make exactly when
the script becomes runnable clear.

Added an exception to SVGTRefElement::updateReferencedText since this code mutates user agent
shadow root during insertedInto, and turned ChildChangeSource into an enum class.

* dom/CharacterData.cpp:
(WebCore::CharacterData::parserAppendData):
(WebCore::CharacterData::setDataAndUpdate):
* dom/ContainerNode.cpp:
(WebCore::executeNodeInsertionWithScriptAssertion): Extracted.
(WebCore::ContainerNode::takeAllChildrenFrom): Deployed executeNodeInsertionWithScriptAssertion.
(WebCore::ContainerNode::insertBefore): Ditto.
(WebCore::ContainerNode::changeForChildInsertion): Deleted.
(WebCore::ContainerNode::notifyChildInserted): Deleted.
(WebCore::ContainerNode::parserInsertBefore): Deployed executeNodeInsertionWithScriptAssertion.
(WebCore::ContainerNode::replaceChild): Ditto.
(WebCore::ContainerNode::removeChild):
(WebCore::ContainerNode::parserRemoveChild):
(WebCore::ContainerNode::replaceAllChildren): Ditto.
(WebCore::ContainerNode::removeChildren):
(WebCore::ContainerNode::appendChildWithoutPreInsertionValidityCheck): Ditto.
(WebCore::ContainerNode::parserAppendChild): Ditto.
(WebCore::ContainerNode::childrenChanged):
(WebCore::ContainerNode::updateTreeAfterInsertion): Deleted.
* dom/ContainerNode.h:
* dom/Element.cpp:
(WebCore::Element::childrenChanged):
* html/HTMLOutputElement.cpp:
(WebCore::HTMLOutputElement::childrenChanged):
* svg/SVGClipPathElement.cpp:
(WebCore::SVGClipPathElement::childrenChanged):
* svg/SVGElement.cpp:
(WebCore::SVGElement::childrenChanged):
* svg/SVGFELightElement.cpp:
(WebCore::SVGFELightElement::childrenChanged):
* svg/SVGFilterElement.cpp:
(WebCore::SVGFilterElement::childrenChanged):
* svg/SVGFilterPrimitiveStandardAttributes.cpp:
(WebCore::SVGFilterPrimitiveStandardAttributes::childrenChanged):
* svg/SVGGradientElement.cpp:
(WebCore::SVGGradientElement::childrenChanged):
* svg/SVGMarkerElement.cpp:
(WebCore::SVGMarkerElement::childrenChanged):
* svg/SVGMaskElement.cpp:
(WebCore::SVGMaskElement::childrenChanged):
* svg/SVGPatternElement.cpp:
(WebCore::SVGPatternElement::childrenChanged):
* svg/SVGTRefElement.cpp:
(WebCore::SVGTRefElement::updateReferencedText): Allow DOM mutations inside the user agent shadow tree here.

2017-10-19 Ryosuke Niwa <rniwa@webkit.org>

Add an argument indicating the type of removal to Node::removedFrom
@@ -103,7 +103,7 @@ unsigned CharacterData::parserAppendData(const String& string, unsigned offset,
if (is<Text>(*this) && parentNode())
downcast<Text>(*this).updateRendererAfterContentChange(oldLength, 0);

notifyParentAfterChange(ContainerNode::ChildChangeSourceParser);
notifyParentAfterChange(ContainerNode::ChildChangeSource::Parser);

return characterLengthLimit;
}
@@ -201,7 +201,7 @@ void CharacterData::setDataAndUpdate(const String& newData, unsigned offsetOfRep
if (document().frame())
document().frame()->selection().textWasReplaced(this, offsetOfReplacedData, oldLength, newLength);

notifyParentAfterChange(ContainerNode::ChildChangeSourceAPI);
notifyParentAfterChange(ContainerNode::ChildChangeSource::API);

dispatchModifiedEvent(oldData);
}
@@ -76,6 +76,40 @@ unsigned NoEventDispatchAssertion::DisableAssertionsInScope::s_existingCount = 0
NoEventDispatchAssertion::EventAllowedScope* NoEventDispatchAssertion::EventAllowedScope::s_currentScope = nullptr;
#endif

enum class ReplacedAllChildren { No, Yes };

template<typename DOMInsertionWork>
static ALWAYS_INLINE void executeNodeInsertionWithScriptAssertion(ContainerNode& containerNode, Node& child,
ContainerNode::ChildChangeSource source, ReplacedAllChildren replacedAllChildren, DOMInsertionWork doNodeInsertion)
{
NodeVector postInsertionNotificationTargets;
{
NoEventDispatchAssertion assertNoEventDispatch;
doNodeInsertion();
ChildListMutationScope(containerNode).childAdded(child);
postInsertionNotificationTargets = notifyChildNodeInserted(containerNode, child);
}

// FIXME: Move childrenChanged into NoEventDispatchAssertion block.
if (replacedAllChildren == ReplacedAllChildren::Yes)
containerNode.childrenChanged(ContainerNode::ChildChange { ContainerNode::AllChildrenReplaced, nullptr, nullptr, source });
else {
containerNode.childrenChanged(ContainerNode::ChildChange {
child.isElementNode() ? ContainerNode::ElementInserted : (child.isTextNode() ? ContainerNode::TextInserted : ContainerNode::NonContentsChildInserted),
ElementTraversal::previousSibling(child),
ElementTraversal::nextSibling(child),
source
});
}

ASSERT(NoEventDispatchAssertion::isEventDispatchAllowedInSubtree(child));
for (auto& target : postInsertionNotificationTargets)
target->didFinishInsertingNode();

if (source == ContainerNode::ChildChangeSource::API)
dispatchChildInsertionEvents(child);
}

static ExceptionOr<void> collectChildrenAndRemoveFromOldParent(Node& node, NodeVector& nodes)
{
if (!is<DocumentFragment>(node)) {
@@ -140,7 +174,7 @@ void ContainerNode::takeAllChildrenFrom(ContainerNode* oldParent)
oldParent->removeBetween(nullptr, child->nextSibling(), *child);
notifyChildNodeRemoved(*oldParent, *child);
}
ChildChange change = { AllChildrenRemoved, nullptr, nullptr, ChildChangeSourceParser };
ChildChange change = { AllChildrenRemoved, nullptr, nullptr, ChildChangeSource::Parser };
childrenChanged(change);
}

@@ -285,14 +319,10 @@ ExceptionOr<void> ContainerNode::insertBefore(Node& newChild, Node* refChild)
if (child->parentNode())
break;

{
NoEventDispatchAssertion assertNoEventDispatch;

executeNodeInsertionWithScriptAssertion(*this, child.get(), ChildChangeSource::API, ReplacedAllChildren::No, [&] {
child->setTreeScopeRecursively(treeScope());
insertBeforeCommon(next, child);
}

updateTreeAfterInsertion(child);
});
}

dispatchSubtreeModifiedEvent();
@@ -339,31 +369,6 @@ void ContainerNode::appendChildCommon(Node& child)
m_lastChild = &child;
}

inline auto ContainerNode::changeForChildInsertion(Node& child, ChildChangeSource source, ReplacedAllChildren replacedAllChildren) -> ChildChange
{
if (replacedAllChildren == ReplacedAllChildren::Yes)
return { AllChildrenReplaced, nullptr, nullptr, source };

return {
child.isElementNode() ? ElementInserted : child.isTextNode() ? TextInserted : NonContentsChildInserted,
ElementTraversal::previousSibling(child),
ElementTraversal::nextSibling(child),
source
};
}

void ContainerNode::notifyChildInserted(Node& child, const ChildChange& change)
{
ChildListMutationScope(*this).childAdded(child);

auto postInsertionNotificationTargets = notifyChildNodeInserted(*this, child);

childrenChanged(change);

for (auto& target : postInsertionNotificationTargets)
target->didFinishInsertingNode();
}

void ContainerNode::notifyChildRemoved(Node& child, Node* previousSibling, Node* nextSibling, ChildChangeSource source)
{
NoEventDispatchAssertion assertNoEventDispatch;
@@ -387,14 +392,14 @@ void ContainerNode::parserInsertBefore(Node& newChild, Node& nextChild)
if (nextChild.previousSibling() == &newChild || &nextChild == &newChild) // nothing to do
return;

if (&document() != &newChild.document())
document().adoptNode(newChild);

insertBeforeCommon(nextChild, newChild);
executeNodeInsertionWithScriptAssertion(*this, newChild, ChildChangeSource::Parser, ReplacedAllChildren::No, [&] {
if (&document() != &newChild.document())
document().adoptNode(newChild);

newChild.updateAncestorConnectedSubframeCountForInsertion();
insertBeforeCommon(nextChild, newChild);

notifyChildInserted(newChild, changeForChildInsertion(newChild, ChildChangeSourceParser));
newChild.updateAncestorConnectedSubframeCountForInsertion();
});
}

ExceptionOr<void> ContainerNode::replaceChild(Node& newChild, Node& oldChild)
@@ -461,16 +466,13 @@ ExceptionOr<void> ContainerNode::replaceChild(Node& newChild, Node& oldChild)
if (child->parentNode())
break;

{
NoEventDispatchAssertion assertNoEventDispatch;
executeNodeInsertionWithScriptAssertion(*this, child.get(), ChildChangeSource::API, ReplacedAllChildren::No, [&] {
child->setTreeScopeRecursively(treeScope());
if (refChild)
insertBeforeCommon(*refChild, child.get());
else
appendChildCommon(child);
}

updateTreeAfterInsertion(child.get());
});
}

dispatchSubtreeModifiedEvent();
@@ -544,7 +546,7 @@ ExceptionOr<void> ContainerNode::removeChild(Node& oldChild)
Node* next = child->nextSibling();
removeBetween(prev, next, child);

notifyChildRemoved(child, prev, next, ChildChangeSourceAPI);
notifyChildRemoved(child, prev, next, ChildChangeSource::API);
}

rebuildSVGExtensionsElementsIfNecessary();
@@ -609,7 +611,7 @@ void ContainerNode::parserRemoveChild(Node& oldChild)

removeBetween(prev, next, oldChild);

notifyChildRemoved(oldChild, prev, next, ChildChangeSourceParser);
notifyChildRemoved(oldChild, prev, next, ChildChangeSource::Parser);
}
}

@@ -643,9 +645,7 @@ void ContainerNode::replaceAllChildren(Ref<Node>&& node)

{
WidgetHierarchyUpdatesSuspensionScope suspendWidgetHierarchyUpdates;
{
NoEventDispatchAssertion assertNoEventDispatch;

executeNodeInsertionWithScriptAssertion(*this, node.get(), ChildChangeSource::API, ReplacedAllChildren::Yes, [&] {
document().nodeChildrenWillBeRemoved(*this);

while (RefPtr<Node> child = m_firstChild) {
@@ -658,9 +658,7 @@ void ContainerNode::replaceAllChildren(Ref<Node>&& node)
InspectorInstrumentation::willInsertDOMNode(document(), *this);

appendChildCommon(node);
}

updateTreeAfterInsertion(node, ReplacedAllChildren::Yes);
});
}

rebuildSVGExtensionsElementsIfNecessary();
@@ -698,8 +696,7 @@ void ContainerNode::removeChildren()
notifyChildNodeRemoved(*this, *child);
}

ChildChange change = { AllChildrenRemoved, nullptr, nullptr, ChildChangeSourceAPI };
childrenChanged(change);
childrenChanged(ChildChange { AllChildrenRemoved, nullptr, nullptr, ChildChangeSource::API });
}

rebuildSVGExtensionsElementsIfNecessary();
@@ -749,13 +746,10 @@ ExceptionOr<void> ContainerNode::appendChildWithoutPreInsertionValidityCheck(Nod
break;

// Append child to the end of the list
{
NoEventDispatchAssertion assertNoEventDispatch;
executeNodeInsertionWithScriptAssertion(*this, child.get(), ChildChangeSource::API, ReplacedAllChildren::No, [&] {
child->setTreeScopeRecursively(treeScope());
appendChildCommon(child);
}

updateTreeAfterInsertion(child.get());
});
}

dispatchSubtreeModifiedEvent();
@@ -768,25 +762,20 @@ void ContainerNode::parserAppendChild(Node& newChild)
ASSERT(!newChild.isDocumentFragment());
ASSERT(!hasTagName(HTMLNames::templateTag));

{
NoEventDispatchAssertion assertNoEventDispatch;

executeNodeInsertionWithScriptAssertion(*this, newChild, ChildChangeSource::Parser, ReplacedAllChildren::No, [&] {
if (&document() != &newChild.document())
document().adoptNode(newChild);

appendChildCommon(newChild);
newChild.setTreeScopeRecursively(treeScope());
}

newChild.updateAncestorConnectedSubframeCountForInsertion();

notifyChildInserted(newChild, changeForChildInsertion(newChild, ChildChangeSourceParser));
newChild.updateAncestorConnectedSubframeCountForInsertion();
});
}

void ContainerNode::childrenChanged(const ChildChange& change)
{
document().incDOMTreeVersion();
if (change.source == ChildChangeSourceAPI && change.type != TextChanged)
if (change.source == ChildChangeSource::API && change.type != TextChanged)
document().updateRangesAfterChildrenChanged(*this);
invalidateNodeListAndCollectionCachesInAncestors();
}
@@ -863,15 +852,6 @@ static void dispatchChildRemovalEvents(Node& child)
}
}

void ContainerNode::updateTreeAfterInsertion(Node& child, ReplacedAllChildren replacedAllChildren)
{
ASSERT(child.refCount());

notifyChildInserted(child, changeForChildInsertion(child, ChildChangeSourceAPI, replacedAllChildren));

dispatchChildInsertionEvents(child);
}

ExceptionOr<Element*> ContainerNode::querySelector(const String& selectors)
{
auto query = document().selectorQueryForString(selectors);
@@ -70,7 +70,7 @@ class ContainerNode : public Node {
void cloneChildNodes(ContainerNode& clone);

enum ChildChangeType { ElementInserted, ElementRemoved, TextInserted, TextRemoved, TextChanged, AllChildrenRemoved, NonContentsChildRemoved, NonContentsChildInserted, AllChildrenReplaced };
enum ChildChangeSource { ChildChangeSourceParser, ChildChangeSourceAPI };
enum class ChildChangeSource { Parser, API };
struct ChildChange {
ChildChangeType type;
Element* previousSiblingElement;
@@ -142,12 +142,8 @@ class ContainerNode : public Node {
void insertBeforeCommon(Node& nextChild, Node& oldChild);
void appendChildCommon(Node&);

void notifyChildInserted(Node& child, const ChildChange&);
void notifyChildRemoved(Node& child, Node* previousSibling, Node* nextSibling, ChildChangeSource);

enum class ReplacedAllChildren { No, Yes };
void updateTreeAfterInsertion(Node& child, ReplacedAllChildren = ReplacedAllChildren::No);
static ChildChange changeForChildInsertion(Node& child, ChildChangeSource, ReplacedAllChildren = ReplacedAllChildren::No);
void rebuildSVGExtensionsElementsIfNecessary();

bool isContainerNode() const = delete;
@@ -2044,7 +2044,7 @@ static void checkForSiblingStyleChanges(Element& parent, SiblingCheckType checkT
void Element::childrenChanged(const ChildChange& change)
{
ContainerNode::childrenChanged(change);
if (change.source == ChildChangeSourceParser)
if (change.source == ChildChangeSource::Parser)
checkForEmptyStyleChange(*this);
else {
SiblingCheckType checkType = change.type == ElementRemoved ? SiblingElementRemoved : Other;
@@ -76,7 +76,7 @@ void HTMLOutputElement::childrenChanged(const ChildChange& change)
{
HTMLFormControlElement::childrenChanged(change);

if (change.source == ChildChangeSourceParser || m_isSetTextContentInProgress) {
if (change.source == ChildChangeSource::Parser || m_isSetTextContentInProgress) {
m_isSetTextContentInProgress = false;
return;
}
@@ -95,7 +95,7 @@ void SVGClipPathElement::childrenChanged(const ChildChange& change)
{
SVGGraphicsElement::childrenChanged(change);

if (change.source == ChildChangeSourceParser)
if (change.source == ChildChangeSource::Parser)
return;

if (RenderObject* object = renderer())
@@ -1016,7 +1016,7 @@ void SVGElement::childrenChanged(const ChildChange& change)
{
StyledElement::childrenChanged(change);

if (change.source == ChildChangeSourceParser)
if (change.source == ChildChangeSource::Parser)
return;
invalidateInstances();
}
@@ -173,7 +173,7 @@ void SVGFELightElement::childrenChanged(const ChildChange& change)
{
SVGElement::childrenChanged(change);

if (change.source == ChildChangeSourceParser)
if (change.source == ChildChangeSource::Parser)
return;
ContainerNode* parent = parentNode();
if (!parent)

0 comments on commit 34c2ef3

Please sign in to comment.