Skip to content

Commit

Permalink
Refactor "had element child" determination logic in ContainerNode::re…
Browse files Browse the repository at this point in the history
…moveAllChildrenWithScriptAssertion

https://bugs.webkit.org/show_bug.cgi?id=244732
rdar://99506128

Reviewed by Chris Dumez.

* Source/WebCore/dom/ContainerNode.cpp:
(WebCore::ContainerNode::removeAllChildrenWithScriptAssertion):

Refactor logic to compute `hadElementChild` in a single place.

* Source/WebCore/dom/ContainerNode.h:

Introduce an `Unknown` value, as `ChildChange` is passed into
`ChildChangeInvalidation` prior to the `hadElementChild` computation. While
the value of `affectsElements` is not read in `ChildChangeInvalidation`,
use the `Unknown` value to make the current behavior explicit.

* Source/WebCore/html/HTMLSelectElement.cpp:
(WebCore::HTMLSelectElement::childrenChanged):

Canonical link: https://commits.webkit.org/254331@main
  • Loading branch information
pxlcoder committed Sep 9, 2022
1 parent aa55950 commit b6660d9
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 9 deletions.
18 changes: 10 additions & 8 deletions Source/WebCore/dom/ContainerNode.cpp
Expand Up @@ -99,11 +99,9 @@ ALWAYS_INLINE auto ContainerNode::removeAllChildrenWithScriptAssertion(ChildChan
return hadElementChild ? DidRemoveElements::Yes : DidRemoveElements::No;
}

bool hadElementChild = false;
if (source == ChildChange::Source::API) {
ChildListMutationScope mutation(*this);
for (auto& child : children) {
hadElementChild |= is<Element>(child);
mutation.willRemoveChild(child.get());
child->notifyMutationObserversNodeWillDetach();
dispatchChildRemovalEvents(child);
Expand All @@ -113,17 +111,16 @@ ALWAYS_INLINE auto ContainerNode::removeAllChildrenWithScriptAssertion(ChildChan
ScriptDisallowedScope::InMainThread scriptDisallowedScope;
if (UNLIKELY(document().hasMutationObserversOfType(MutationObserverOptionType::ChildList))) {
ChildListMutationScope mutation(*this);
for (auto& child : children) {
hadElementChild |= is<Element>(child);
for (auto& child : children)
mutation.willRemoveChild(child.get());
}
} else
hadElementChild = children.containsIf([](auto& child) { return is<Element>(child); });
}
}

disconnectSubframesIfNeeded(*this, DescendantsOnly);

ContainerNode::ChildChange childChange { ChildChange::Type::AllChildrenRemoved, nullptr, nullptr, nullptr, source, hadElementChild ? ContainerNode::ChildChange::AffectsElements::Yes : ContainerNode::ChildChange::AffectsElements::No };
ContainerNode::ChildChange childChange { ChildChange::Type::AllChildrenRemoved, nullptr, nullptr, nullptr, source, ContainerNode::ChildChange::AffectsElements::Unknown };

bool hadElementChild = false;

WidgetHierarchyUpdatesSuspensionScope suspendWidgetHierarchyUpdates;
ScriptDisallowedScope::InMainThread scriptDisallowedScope;
Expand All @@ -136,11 +133,16 @@ ALWAYS_INLINE auto ContainerNode::removeAllChildrenWithScriptAssertion(ChildChan
document().nodeChildrenWillBeRemoved(*this);

while (RefPtr<Node> child = m_firstChild) {
if (is<Element>(*child))
hadElementChild = true;

removeBetween(nullptr, child->nextSibling(), *child);
auto subtreeObservability = notifyChildNodeRemoved(*this, *child);
if (source == ChildChange::Source::API && subtreeObservability == RemovedSubtreeObservability::MaybeObservableByRefPtr)
willCreatePossiblyOrphanedTreeByRemoval(*child);
}

childChange.affectsElements = hadElementChild ? ContainerNode::ChildChange::AffectsElements::Yes : ContainerNode::ChildChange::AffectsElements::No;
}

ASSERT_WITH_SECURITY_IMPLICATION(!document().selection().selection().isOrphan());
Expand Down
2 changes: 1 addition & 1 deletion Source/WebCore/dom/ContainerNode.h
Expand Up @@ -78,7 +78,7 @@ class ContainerNode : public Node {
struct ChildChange {
enum class Type : uint8_t { ElementInserted, ElementRemoved, TextInserted, TextRemoved, TextChanged, AllChildrenRemoved, NonContentsChildRemoved, NonContentsChildInserted, AllChildrenReplaced };
enum class Source : bool { Parser, API };
enum class AffectsElements : bool { No, Yes };
enum class AffectsElements : uint8_t { Unknown, No, Yes };

ChildChange::Type type;
Element* siblingChanged;
Expand Down
2 changes: 2 additions & 0 deletions Source/WebCore/html/HTMLSelectElement.cpp
Expand Up @@ -404,6 +404,8 @@ CompletionHandlerCallingScope HTMLSelectElement::optionToSelectFromChildChangeSc

void HTMLSelectElement::childrenChanged(const ChildChange& change)
{
ASSERT(change.affectsElements != ChildChange::AffectsElements::Unknown);

if (change.affectsElements == ChildChange::AffectsElements::No) {
HTMLFormControlElementWithState::childrenChanged(change);
return;
Expand Down

0 comments on commit b6660d9

Please sign in to comment.