Skip to content

GitLab

T
tangled
Commit e15391d9 authored by tkent's avatar tkent Committed by Commit bot
Browse files
Options

Change the timing of event dispatching and <script> execution in... 

Change the timing of event dispatching and <script> execution in Node::appendChild, insertBefore, and replaceChild.

When we insert a DocumentFragment including multiple children into another node,

* WebKit and Blink: DOMNodeInserted is dispatched just after each of
  DocumentFragment children was inserted.

* Firefox and Edge: DOMNodeInserted is dispatched after all of DocumentFragment
  children were inserted.

<script> execution and <iframe> load event have the same incompatibility.

This CL makes Blink same as Firefox and Edge in order to improve interoperability
and avoid additional hierarchy check for crbug.com/523157.

Implementation:
Fold updateTreeAfterInsertion() and notifyNodeInserted() into insertNodeVector(),
and move childrenChange(), didNotifySubtreeInsertionsToDocument(), and
dispatchChildInsertionEvents() out from the loop.

Test changes:
* fast/dom/MutationObserver/removed-out-of-order.html
  DOMNodeInserted timing change.  New result matches to Firefox and Edge.

BUG=523157

Review-Url: https://codereview.chromium.org/2306323002
Cr-Commit-Position: refs/heads/master@{#417209}
parent 30d9858f
Show whitespace changes
Inline Side-by-side
Showing with 73 additions and 24 deletions
third_party/WebKit/LayoutTests/fast/dom/MutationObserver/removed-out-of-order-expected.txt
View file @ e15391d9
...@@ -4,13 +4,13 @@ On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE ...@@ -4,13 +4,13 @@ On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE
PASS mutations.length is 2 PASS mutations.length is 2
PASS mutations[0].addedNodes.length is 1 PASS mutations[0].addedNodes.length is 2
PASS mutations[0].removedNodes.length is 0 PASS mutations[0].removedNodes.length is 0
PASS mutations[0].addedNodes[0].tagName is 'B' PASS mutations[0].addedNodes[0].tagName is 'B'
PASS mutations[1].addedNodes.length is 1 PASS mutations[1].addedNodes.length is 0
PASS mutations[1].removedNodes.length is 1 PASS mutations[1].removedNodes.length is 1
PASS mutations[1].removedNodes[0].tagName is 'B' PASS mutations[1].removedNodes[0].tagName is 'B'
PASS mutations[1].addedNodes[0].tagName is 'I' PASS mutations[1].addedNodes[0] is undefined.
PASS successfullyParsed is true PASS successfullyParsed is true
TEST COMPLETE TEST COMPLETE
......
third_party/WebKit/LayoutTests/fast/dom/MutationObserver/removed-out-of-order.html
View file @ e15391d9
...@@ -17,12 +17,12 @@ sandbox.innerHTML = '<b></b><i></i>'; ...@@ -17,12 +17,12 @@ sandbox.innerHTML = '<b></b><i></i>';
var mutations = observer.takeRecords(); var mutations = observer.takeRecords();
shouldBe("mutations.length", "2"); shouldBe("mutations.length", "2");
shouldBe("mutations[0].addedNodes.length", "1"); shouldBe("mutations[0].addedNodes.length", "2");
shouldBe("mutations[0].removedNodes.length", "0"); shouldBe("mutations[0].removedNodes.length", "0");
shouldBe("mutations[0].addedNodes[0].tagName", "'B'"); shouldBe("mutations[0].addedNodes[0].tagName", "'B'");
shouldBe("mutations[1].addedNodes.length", "1"); shouldBe("mutations[1].addedNodes.length", "0");
shouldBe("mutations[1].removedNodes.length", "1"); shouldBe("mutations[1].removedNodes.length", "1");
shouldBe("mutations[1].removedNodes[0].tagName", "'B'"); shouldBe("mutations[1].removedNodes[0].tagName", "'B'");
shouldBe("mutations[1].addedNodes[0].tagName", "'I'"); shouldBeUndefined("mutations[1].addedNodes[0]");
</script> </script>
</script> </script>
third_party/WebKit/LayoutTests/fast/dom/mutation-event-for-fragment-insertion.html 0 → 100644
View file @ e15391d9
<!DOCTYPE html>
<body>
<script src="../../resources/testharness.js"></script>
<script src="../../resources/testharnessreport.js"></script>
<script>
function createContainerNode(test, expectedLength) {
var div = document.createElement('div');
var oldChild = document.createElement('span');
div.appendChild(oldChild);
document.body.appendChild(div);
div.dispatchCount = 0;
div.addEventListener('DOMNodeInserted', () => {
assert_equals(div.childNodes.length, expectedLength);
if (++div.dispatchCount == 2)
test.done();
}, false);
return div;
}
function createFragment() {
var fragment = document.createDocumentFragment();
fragment.appendChild(document.createElement('span'));
fragment.appendChild(document.createElement('span'));
return fragment;
}
var test1 = async_test('appendChild: DOMNodeInserted event should be dispatched after all nodes in a DocumentFragment were inserted.');
test1.step(() => {
createContainerNode(test1, 3).appendChild(createFragment());
});
var test2 = async_test('insertBefore: DOMNodeInserted event should be dispatched after all nodes in a DocumentFragment were inserted.');
test2.step(() => {
createContainerNode(test2, 3).insertBefore(createFragment(), null);
});
var test3 = async_test('replaceChild: DOMNodeInserted event should be dispatched after all nodes in a DocumentFragment were inserted.');
test3.step(() => {
var div = createContainerNode(test3, 2);
div.replaceChild(createFragment(), div.firstChild);
});
</script>
</body>
third_party/WebKit/Source/core/dom/ContainerNode.cpp
View file @ e15391d9
...@@ -154,16 +154,32 @@ template <typename Functor> ...@@ -154,16 +154,32 @@ template <typename Functor>
void ContainerNode::insertNodeVector(const NodeVector& targets, Node* next, const Functor& mutator) void ContainerNode::insertNodeVector(const NodeVector& targets, Node* next, const Functor& mutator)
{ {
InspectorInstrumentation::willInsertDOMNode(this); InspectorInstrumentation::willInsertDOMNode(this);
for (const auto& targetNode : targets) { NodeVector postInsertionNotificationTargets;
DCHECK(targetNode);
{ {
EventDispatchForbiddenScope assertNoEventDispatch; EventDispatchForbiddenScope assertNoEventDispatch;
ScriptForbiddenScope forbidScript; ScriptForbiddenScope forbidScript;
for (const auto& targetNode : targets) {
if (!mutator(*this, *targetNode, next)) DCHECK(targetNode);
Node& child = *targetNode;
// TODO(tkent): mutator never returns false because scripts don't run in the loop.
if (!mutator(*this, child, next))
break; break;
ChildListMutationScope(*this).childAdded(child);
if (document().containsV1ShadowTree())
child.checkSlotChangeAfterInserted();
InspectorInstrumentation::didInsertDOMNode(&child);
notifyNodeInsertedInternal(child, postInsertionNotificationTargets);
}
} }
updateTreeAfterInsertion(*targetNode); for (const auto& targetNode : targets)
childrenChanged(ChildrenChange::forInsertion(*targetNode, ChildrenChangeSourceAPI));
for (const auto& descendant : postInsertionNotificationTargets) {
if (descendant->isConnected())
descendant->didNotifySubtreeInsertionsToDocument();
}
for (const auto& targetNode : targets) {
if (targetNode->parentNode() == this)
dispatchChildInsertionEvents(*targetNode);
} }
dispatchSubtreeModifiedEvent(); dispatchSubtreeModifiedEvent();
} }
...@@ -1170,15 +1186,6 @@ static void dispatchChildRemovalEvents(Node& child) ...@@ -1170,15 +1186,6 @@ static void dispatchChildRemovalEvents(Node& child)
} }
} }
void ContainerNode::updateTreeAfterInsertion(Node& child)
{
ChildListMutationScope(*this).childAdded(child);
notifyNodeInserted(child);
dispatchChildInsertionEvents(child);
}
bool ContainerNode::hasRestyleFlagInternal(DynamicRestyleFlags mask) const bool ContainerNode::hasRestyleFlagInternal(DynamicRestyleFlags mask) const
{ {
return rareData()->hasRestyleFlag(mask); return rareData()->hasRestyleFlag(mask);
......
third_party/WebKit/Source/core/dom/ContainerNode.h
View file @ e15391d9
...@@ -249,7 +249,6 @@ private: ...@@ -249,7 +249,6 @@ private:
friend class AdoptAndAppendChild; friend class AdoptAndAppendChild;
void insertBeforeCommon(Node& nextChild, Node& newChild); void insertBeforeCommon(Node& nextChild, Node& newChild);
void appendChildCommon(Node& child); void appendChildCommon(Node& child);
void updateTreeAfterInsertion(Node& child);
void willRemoveChildren(); void willRemoveChildren();
void willRemoveChild(Node& child); void willRemoveChild(Node& child);
void removeDetachedChildrenInContainer(ContainerNode&); void removeDetachedChildrenInContainer(ContainerNode&);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment