Diff
Modified: branches/safari-600.1.4.17-branch/LayoutTests/ChangeLog (186515 => 186516)
--- branches/safari-600.1.4.17-branch/LayoutTests/ChangeLog 2015-07-08 17:58:35 UTC (rev 186515)
+++ branches/safari-600.1.4.17-branch/LayoutTests/ChangeLog 2015-07-08 18:34:52 UTC (rev 186516)
@@ -1,3 +1,23 @@
+2015-06-10 Chris Dumez <cdu...@apple.com>
+
+ ASSERT_WITH_SECURITY_IMPLICATION in WebCore::DocumentOrderedMap::getElementById
+ https://bugs.webkit.org/show_bug.cgi?id=145857
+ <rdar://problem/16798440>
+
+ Reviewed by Darin Adler.
+
+ Add layout tests covering different crashes caused by the same bug.
+
+ * fast/dom/script-getElementById-during-insertion-expected.txt: Added.
+ * fast/dom/script-getElementById-during-insertion.html: Added.
+
+ Reduction test case for <rdar://problem/16798440>.
+
+ * fast/dom/script-remove-child-id-map-expected.txt: Added.
+ * fast/dom/script-remove-child-id-map.html: Added.
+
+ Test imported from Blink r178976.
+
2015-07-08 Matthew Hanson <matthew_han...@apple.com>
Merge r185848. rdar://problem/21708274
Added: branches/safari-600.1.4.17-branch/LayoutTests/fast/dom/script-getElementById-during-insertion-expected.txt (0 => 186516)
--- branches/safari-600.1.4.17-branch/LayoutTests/fast/dom/script-getElementById-during-insertion-expected.txt (rev 0)
+++ branches/safari-600.1.4.17-branch/LayoutTests/fast/dom/script-getElementById-during-insertion-expected.txt 2015-07-08 18:34:52 UTC (rev 186516)
@@ -0,0 +1,2 @@
+PASS
+
Added: branches/safari-600.1.4.17-branch/LayoutTests/fast/dom/script-getElementById-during-insertion.html (0 => 186516)
--- branches/safari-600.1.4.17-branch/LayoutTests/fast/dom/script-getElementById-during-insertion.html (rev 0)
+++ branches/safari-600.1.4.17-branch/LayoutTests/fast/dom/script-getElementById-during-insertion.html 2015-07-08 18:34:52 UTC (rev 186516)
@@ -0,0 +1,31 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script>
+// Tests that we don't crash if a script is being executed as a result of appending a child to it.</p>
+executedScript = false;
+if (window.testRunner)
+ testRunner.dumpAsText();
+</script>
+</head>
+<body>
+<div id="test"></div>
+<script>
+var elem = document.getElementById("test");
+if (!executedScript) {
+ executedScript = true;
+
+ document.documentElement.appendChild(elem.cloneNode(true));
+
+ var range = document.createRange();
+ range.setStartBefore(document.body);
+ range.setEndAfter(document.body);
+ range.surroundContents(document.head.appendChild(document.createElement("script")));
+} else {
+ var span = document.createElement("span");
+ document.documentElement.appendChild(span);
+ span.innerHTML = 'PASS<br/>';
+}
+</script>
+</body>
+</html>
Added: branches/safari-600.1.4.17-branch/LayoutTests/fast/dom/script-remove-child-id-map-expected.txt (0 => 186516)
--- branches/safari-600.1.4.17-branch/LayoutTests/fast/dom/script-remove-child-id-map-expected.txt (rev 0)
+++ branches/safari-600.1.4.17-branch/LayoutTests/fast/dom/script-remove-child-id-map-expected.txt 2015-07-08 18:34:52 UTC (rev 186516)
@@ -0,0 +1,10 @@
+Passes if it doesn't crash and the child is not in the id map
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS document.getElementById('child') is null
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: branches/safari-600.1.4.17-branch/LayoutTests/fast/dom/script-remove-child-id-map.html (0 => 186516)
--- branches/safari-600.1.4.17-branch/LayoutTests/fast/dom/script-remove-child-id-map.html (rev 0)
+++ branches/safari-600.1.4.17-branch/LayoutTests/fast/dom/script-remove-child-id-map.html 2015-07-08 18:34:52 UTC (rev 186516)
@@ -0,0 +1,30 @@
+<!DOCTYPE html>
+
+<script src=""
+
+<script>
+description("Passes if it doesn't crash and the child is not in the id map");
+
+var script = document.createElement("script");
+script.type = "dont-execute";
+script.textContent = "script.remove()";
+child = document.createElement("div");
+child.id = "child";
+script.appendChild(child);
+
+// The script won't execute here because the type is invalid, but it also won't
+// get marked as being already run, so changing the children later will run it.
+document.documentElement.appendChild(script);
+
+// Per the spec setting the type has no effect
+script.type = "";
+
+// but changing the children *will* execute the script now that the type is
+// is valid.
+child.remove();
+
+child = null;
+gc();
+
+shouldBeNull("document.getElementById('child')");
+</script>
Modified: branches/safari-600.1.4.17-branch/Source/WebCore/ChangeLog (186515 => 186516)
--- branches/safari-600.1.4.17-branch/Source/WebCore/ChangeLog 2015-07-08 17:58:35 UTC (rev 186515)
+++ branches/safari-600.1.4.17-branch/Source/WebCore/ChangeLog 2015-07-08 18:34:52 UTC (rev 186516)
@@ -1,3 +1,56 @@
+2015-06-10 Chris Dumez <cdu...@apple.com>
+
+ ASSERT_WITH_SECURITY_IMPLICATION in WebCore::DocumentOrderedMap::getElementById
+ https://bugs.webkit.org/show_bug.cgi?id=145857
+ <rdar://problem/16798440>
+
+ Reviewed by Darin Adler.
+
+ Make sure Node::insertedInto() gets called on the inserted node and its
+ descendants after its insertion into the tree but *before*
+ ContainerNode::childrenChanged() is called on the parent node. This is
+ needed so that the descendants know they've been inserted into the tree
+ (and their InDocumentFlag flag gets set) before the parent node does
+ anything with them in childrenChanged().
+
+ In the case of <rdar://problem/16798440>, executing HTMLScriptElement's
+ childrenChanged() after appending a child to a script element was causing
+ the script to be executed. The script would call getElementBy() which
+ would traverse the DOM tree and find a matching Element in the newly
+ inserted subtree. However, the matching Element's InDocumentFlag flag was
+ not set yet because the element's insertedInto() method has not been called
+ yet at this point. This would cause us to hit an assertion as
+ DocumentOrderedMap::getElementById() is only supposed to return elements
+ that are in a Document.
+
+ This patch is based on Blink r178976 by <espr...@chromium.org>:
+ https://src.chromium.org/viewvc/blink?view=rev&revision=178976
+
+ Tests: fast/dom/script-getElementById-during-insertion.html
+ fast/dom/script-remove-child-id-map.html
+
+ * dom/ContainerNode.cpp:
+ (WebCore::ContainerNode::notifyChildInserted):
+ (WebCore::ContainerNode::notifyChildRemoved):
+ (WebCore::ContainerNode::removeChildren):
+ (WebCore::ContainerNode::parserInsertBefore): Deleted.
+ (WebCore::ContainerNode::removeChild): Deleted.
+ (WebCore::ContainerNode::parserRemoveChild): Deleted.
+ (WebCore::ContainerNode::parserAppendChild): Deleted.
+ (WebCore::ContainerNode::childrenChanged): Deleted.
+ (WebCore::ContainerNode::setAttributeEventListener): Deleted.
+ (WebCore::ContainerNode::querySelector): Deleted.
+ * dom/ContainerNodeAlgorithms.cpp:
+ (WebCore::ChildNodeInsertionNotifier::notifyDescendantInsertedIntoDocument):
+ (WebCore::ChildNodeInsertionNotifier::notifyDescendantInsertedIntoTree):
+ * dom/ContainerNodeAlgorithms.h:
+ (WebCore::ChildNodeInsertionNotifier::notifyNodeInsertedIntoDocument):
+ (WebCore::ChildNodeInsertionNotifier::notifyNodeInsertedIntoTree):
+ (WebCore::ChildNodeInsertionNotifier::notify):
+ (WebCore::ChildNodeRemovalNotifier::notifyNodeRemovedFromDocument): Deleted.
+ * dom/Element.cpp:
+ (WebCore::Element::addShadowRoot):
+
2015-07-08 Matthew Hanson <matthew_han...@apple.com>
Merge r186389. rdar://problem/21708243
Modified: branches/safari-600.1.4.17-branch/Source/WebCore/dom/ContainerNode.cpp (186515 => 186516)
--- branches/safari-600.1.4.17-branch/Source/WebCore/dom/ContainerNode.cpp 2015-07-08 17:58:35 UTC (rev 186515)
+++ branches/safari-600.1.4.17-branch/Source/WebCore/dom/ContainerNode.cpp 2015-07-08 18:34:52 UTC (rev 186516)
@@ -335,6 +335,11 @@
void ContainerNode::notifyChildInserted(Node& child, ChildChangeSource source)
{
+ ChildListMutationScope(*this).childAdded(child);
+
+ NodeVector postInsertionNotificationTargets;
+ ChildNodeInsertionNotifier(*this).notify(child, postInsertionNotificationTargets);
+
ChildChange change;
change.type = child.isElementNode() ? ElementInserted : child.isTextNode() ? TextInserted : NonContentsChildChanged;
change.previousSiblingElement = ElementTraversal::previousSibling(&child);
@@ -342,10 +347,15 @@
change.source = source;
childrenChanged(change);
+
+ for (auto& target : postInsertionNotificationTargets)
+ target->didNotifySubtreeInsertions(this);
}
void ContainerNode::notifyChildRemoved(Node& child, Node* previousSibling, Node* nextSibling, ChildChangeSource source)
{
+ ChildNodeRemovalNotifier(*this).notify(child);
+
ChildChange change;
change.type = child.isElementNode() ? ElementRemoved : child.isTextNode() ? TextRemoved : NonContentsChildChanged;
change.previousSiblingElement = (!previousSibling || previousSibling->isElementNode()) ? toElement(previousSibling) : ElementTraversal::previousSibling(previousSibling);
@@ -375,12 +385,8 @@
newChild->updateAncestorConnectedSubframeCountForInsertion();
- ChildListMutationScope(*this).childAdded(*newChild);
-
notifyChildInserted(*newChild, ChildChangeSourceParser);
- ChildNodeInsertionNotifier(*this).notify(*newChild);
-
newChild->setNeedsStyleRecalc(ReconstructRenderTree);
}
@@ -565,8 +571,6 @@
removeBetween(prev, next, child.get());
notifyChildRemoved(child.get(), prev, next, ChildChangeSourceAPI);
-
- ChildNodeRemovalNotifier(*this).notify(child.get());
}
@@ -623,8 +627,6 @@
removeBetween(prev, next, oldChild);
notifyChildRemoved(oldChild, prev, next, ChildChangeSourceParser);
-
- ChildNodeRemovalNotifier(*this).notify(oldChild);
}
// this differs from other remove functions because it forcibly removes all the children,
@@ -648,23 +650,18 @@
// and remove... e.g. stop loading frames, fire unload events.
willRemoveChildren(*this);
- NodeVector removedChildren;
{
WidgetHierarchyUpdatesSuspensionScope suspendWidgetHierarchyUpdates;
{
NoEventDispatchAssertion assertNoEventDispatch;
- removedChildren.reserveInitialCapacity(childNodeCount());
while (RefPtr<Node> n = m_firstChild) {
- removedChildren.append(*m_firstChild);
removeBetween(0, m_firstChild->nextSibling(), *m_firstChild);
+ ChildNodeRemovalNotifier(*this).notify(*n);
}
}
ChildChange change = { AllChildrenRemoved, nullptr, nullptr, ChildChangeSourceAPI };
childrenChanged(change);
-
- for (size_t i = 0; i < removedChildren.size(); ++i)
- ChildNodeRemovalNotifier(*this).notify(removedChildren[i].get());
}
if (document().svgExtensions()) {
@@ -754,12 +751,8 @@
newChild->updateAncestorConnectedSubframeCountForInsertion();
- ChildListMutationScope(*this).childAdded(*newChild);
-
notifyChildInserted(*newChild, ChildChangeSourceParser);
- ChildNodeInsertionNotifier(*this).notify(*newChild);
-
newChild->setNeedsStyleRecalc(ReconstructRenderTree);
}
@@ -871,12 +864,8 @@
{
ASSERT(child.refCount());
- ChildListMutationScope(*this).childAdded(child);
-
notifyChildInserted(child, ChildChangeSourceAPI);
- ChildNodeInsertionNotifier(*this).notify(child);
-
child.setNeedsStyleRecalc(ReconstructRenderTree);
dispatchChildInsertionEvents(child);
Modified: branches/safari-600.1.4.17-branch/Source/WebCore/dom/ContainerNodeAlgorithms.cpp (186515 => 186516)
--- branches/safari-600.1.4.17-branch/Source/WebCore/dom/ContainerNodeAlgorithms.cpp 2015-07-08 17:58:35 UTC (rev 186515)
+++ branches/safari-600.1.4.17-branch/Source/WebCore/dom/ContainerNodeAlgorithms.cpp 2015-07-08 18:34:52 UTC (rev 186516)
@@ -29,7 +29,7 @@
namespace WebCore {
-void ChildNodeInsertionNotifier::notifyDescendantInsertedIntoDocument(ContainerNode& node)
+void ChildNodeInsertionNotifier::notifyDescendantInsertedIntoDocument(ContainerNode& node, NodeVector& postInsertionNotificationTargets)
{
ChildNodesLazySnapshot snapshot(node);
while (RefPtr<Node> child = snapshot.nextNode()) {
@@ -37,7 +37,7 @@
// we don't want to tell the rest of our children that they've been
// inserted into the document because they haven't.
if (node.inDocument() && child->parentNode() == &node)
- notifyNodeInsertedIntoDocument(*child.get());
+ notifyNodeInsertedIntoDocument(*child.get(), postInsertionNotificationTargets);
}
if (!node.isElementNode())
@@ -45,19 +45,19 @@
if (RefPtr<ShadowRoot> root = toElement(node).shadowRoot()) {
if (node.inDocument() && root->hostElement() == &node)
- notifyNodeInsertedIntoDocument(*root.get());
+ notifyNodeInsertedIntoDocument(*root.get(), postInsertionNotificationTargets);
}
}
-void ChildNodeInsertionNotifier::notifyDescendantInsertedIntoTree(ContainerNode& node)
+void ChildNodeInsertionNotifier::notifyDescendantInsertedIntoTree(ContainerNode& node, NodeVector& postInsertionNotificationTargets)
{
for (Node* child = node.firstChild(); child; child = child->nextSibling()) {
if (child->isContainerNode())
- notifyNodeInsertedIntoTree(*toContainerNode(child));
+ notifyNodeInsertedIntoTree(*toContainerNode(child), postInsertionNotificationTargets);
}
if (ShadowRoot* root = node.shadowRoot())
- notifyNodeInsertedIntoTree(*root);
+ notifyNodeInsertedIntoTree(*root, postInsertionNotificationTargets);
}
void ChildNodeRemovalNotifier::notifyDescendantRemovedFromDocument(ContainerNode& node)
Modified: branches/safari-600.1.4.17-branch/Source/WebCore/dom/ContainerNodeAlgorithms.h (186515 => 186516)
--- branches/safari-600.1.4.17-branch/Source/WebCore/dom/ContainerNodeAlgorithms.h 2015-07-08 17:58:35 UTC (rev 186515)
+++ branches/safari-600.1.4.17-branch/Source/WebCore/dom/ContainerNodeAlgorithms.h 2015-07-08 18:34:52 UTC (rev 186516)
@@ -41,16 +41,15 @@
{
}
- void notify(Node&);
+ void notify(Node&, NodeVector& postInsertionNotificationTargets);
private:
- void notifyDescendantInsertedIntoDocument(ContainerNode&);
- void notifyDescendantInsertedIntoTree(ContainerNode&);
- void notifyNodeInsertedIntoDocument(Node&);
- void notifyNodeInsertedIntoTree(ContainerNode&);
+ void notifyDescendantInsertedIntoDocument(ContainerNode&, NodeVector& postInsertionNotificationTargets);
+ void notifyDescendantInsertedIntoTree(ContainerNode&, NodeVector& postInsertionNotificationTargets);
+ void notifyNodeInsertedIntoDocument(Node&, NodeVector& postInsertionNotificationTargets);
+ void notifyNodeInsertedIntoTree(ContainerNode&, NodeVector& postInsertionNotificationTargets);
ContainerNode& m_insertionPoint;
- Vector<Ref<Node>> m_postInsertionNotificationTargets;
};
class ChildNodeRemovalNotifier {
@@ -194,26 +193,26 @@
} // namespace Private
-inline void ChildNodeInsertionNotifier::notifyNodeInsertedIntoDocument(Node& node)
+inline void ChildNodeInsertionNotifier::notifyNodeInsertedIntoDocument(Node& node, NodeVector& postInsertionNotificationTargets)
{
ASSERT(m_insertionPoint.inDocument());
if (Node::InsertionShouldCallDidNotifySubtreeInsertions == node.insertedInto(m_insertionPoint))
- m_postInsertionNotificationTargets.append(node);
+ postInsertionNotificationTargets.append(node);
if (node.isContainerNode())
- notifyDescendantInsertedIntoDocument(toContainerNode(node));
+ notifyDescendantInsertedIntoDocument(toContainerNode(node), postInsertionNotificationTargets);
}
-inline void ChildNodeInsertionNotifier::notifyNodeInsertedIntoTree(ContainerNode& node)
+inline void ChildNodeInsertionNotifier::notifyNodeInsertedIntoTree(ContainerNode& node, NodeVector& postInsertionNotificationTargets)
{
NoEventDispatchAssertion assertNoEventDispatch;
ASSERT(!m_insertionPoint.inDocument());
if (Node::InsertionShouldCallDidNotifySubtreeInsertions == node.insertedInto(m_insertionPoint))
- m_postInsertionNotificationTargets.append(node);
- notifyDescendantInsertedIntoTree(node);
+ postInsertionNotificationTargets.append(node);
+ notifyDescendantInsertedIntoTree(node, postInsertionNotificationTargets);
}
-inline void ChildNodeInsertionNotifier::notify(Node& node)
+inline void ChildNodeInsertionNotifier::notify(Node& node, NodeVector& postInsertionNotificationTargets)
{
ASSERT(!NoEventDispatchAssertion::isEventDispatchForbidden());
@@ -225,12 +224,9 @@
Ref<Node> protectNode(node);
if (m_insertionPoint.inDocument())
- notifyNodeInsertedIntoDocument(node);
+ notifyNodeInsertedIntoDocument(node, postInsertionNotificationTargets);
else if (node.isContainerNode())
- notifyNodeInsertedIntoTree(toContainerNode(node));
-
- for (size_t i = 0; i < m_postInsertionNotificationTargets.size(); ++i)
- m_postInsertionNotificationTargets[i]->didNotifySubtreeInsertions(&m_insertionPoint);
+ notifyNodeInsertedIntoTree(toContainerNode(node), postInsertionNotificationTargets);
}
Modified: branches/safari-600.1.4.17-branch/Source/WebCore/dom/Element.cpp (186515 => 186516)
--- branches/safari-600.1.4.17-branch/Source/WebCore/dom/Element.cpp 2015-07-08 17:58:35 UTC (rev 186515)
+++ branches/safari-600.1.4.17-branch/Source/WebCore/dom/Element.cpp 2015-07-08 18:34:52 UTC (rev 186516)
@@ -1483,8 +1483,12 @@
shadowRoot->setParentTreeScope(&treeScope());
shadowRoot->distributor().didShadowBoundaryChange(this);
- ChildNodeInsertionNotifier(*this).notify(*shadowRoot);
+ NodeVector postInsertionNotificationTargets;
+ ChildNodeInsertionNotifier(*this).notify(*shadowRoot, postInsertionNotificationTargets);
+ for (auto& target : postInsertionNotificationTargets)
+ target->didNotifySubtreeInsertions(this);
+
resetNeedsNodeRenderingTraversalSlowPath();
setNeedsStyleRecalc(ReconstructRenderTree);
