Diff
Modified: branches/safari-603-branch/LayoutTests/ChangeLog (212720 => 212721)
--- branches/safari-603-branch/LayoutTests/ChangeLog 2017-02-21 18:21:22 UTC (rev 212720)
+++ branches/safari-603-branch/LayoutTests/ChangeLog 2017-02-21 18:21:28 UTC (rev 212721)
@@ -1,3 +1,20 @@
+2017-02-18 Ryosuke Niwa <[email protected]>
+
+ REGRESSION(r212218): Assertion failures in and after parserRemoveChild
+ https://bugs.webkit.org/show_bug.cgi?id=168458
+
+ Reviewed by Antti Koivisto.
+
+ Add tests to make sure parserAppendChild aren't called when a node removed by parserRemoveChild
+ had already been been inserted elsewhere by scripts.
+
+ * fast/parser/adoption-agency-unload-iframe-3-expected.txt: Added.
+ * fast/parser/adoption-agency-unload-iframe-3.html: Added.
+ * fast/parser/adoption-agency-unload-iframe-4-expected.txt: Added.
+ * fast/parser/adoption-agency-unload-iframe-4.html: Added.
+ * fast/parser/xml-error-unload-iframe-expected.txt: Added.
+ * fast/parser/xml-error-unload-iframe.html: Added.
+
2017-02-20 Matthew Hanson <[email protected]>
Rollout r212647. rdar://problem/30563318
Added: branches/safari-603-branch/LayoutTests/fast/parser/adoption-agency-unload-iframe-3-expected.txt (0 => 212721)
--- branches/safari-603-branch/LayoutTests/fast/parser/adoption-agency-unload-iframe-3-expected.txt (rev 0)
+++ branches/safari-603-branch/LayoutTests/fast/parser/adoption-agency-unload-iframe-3-expected.txt 2017-02-21 18:21:28 UTC (rev 212721)
@@ -0,0 +1,3 @@
+
+PASS An element removed by the adoption agency algorithm must not be inserted if it had been inserted elsewhere
+
Added: branches/safari-603-branch/LayoutTests/fast/parser/adoption-agency-unload-iframe-3.html (0 => 212721)
--- branches/safari-603-branch/LayoutTests/fast/parser/adoption-agency-unload-iframe-3.html (rev 0)
+++ branches/safari-603-branch/LayoutTests/fast/parser/adoption-agency-unload-iframe-3.html 2017-02-21 18:21:28 UTC (rev 212721)
@@ -0,0 +1,41 @@
+<!DOCTYPE html>
+<head>
+<script src=""
+<script src=""
+<script>
+
+function runTest() {
+ const section = document.querySelector('section');
+ const container = document.querySelector('div');
+ const p = document.querySelector('p');
+ const iframe = document.createElement('iframe');
+ document.querySelector('b').appendChild(iframe);
+ /* div
+ + b
+ + p
+ + script
+ + iframe */
+ iframe.contentWindow._onunload_ = () => {
+ section.appendChild(p);
+ container.remove();
+ /* body
+ + p
+ + script
+ + iframe */
+ }
+
+ window._onload_ = () => {
+ let test = async_test('An element removed by the adoption agency algorithm must not be inserted if it had been inserted elsewhere');
+ test.step(() => {
+ assert_not_equals(p.parentNode, container);
+ assert_equals(p.parentNode, section);
+ });
+ test.done();
+ }
+}
+
+</script>
+</head>
+<body>
+<section><div><b><p><script>runTest();</script></b></p></div></section>
+</body>
Added: branches/safari-603-branch/LayoutTests/fast/parser/adoption-agency-unload-iframe-4-expected.txt (0 => 212721)
--- branches/safari-603-branch/LayoutTests/fast/parser/adoption-agency-unload-iframe-4-expected.txt (rev 0)
+++ branches/safari-603-branch/LayoutTests/fast/parser/adoption-agency-unload-iframe-4-expected.txt 2017-02-21 18:21:28 UTC (rev 212721)
@@ -0,0 +1,3 @@
+
+PASS An element removed by the adoption agency algorithm must not be inserted if it had been inserted elsewhere
+
Added: branches/safari-603-branch/LayoutTests/fast/parser/adoption-agency-unload-iframe-4.html (0 => 212721)
--- branches/safari-603-branch/LayoutTests/fast/parser/adoption-agency-unload-iframe-4.html (rev 0)
+++ branches/safari-603-branch/LayoutTests/fast/parser/adoption-agency-unload-iframe-4.html 2017-02-21 18:21:28 UTC (rev 212721)
@@ -0,0 +1,47 @@
+<html>
+<head>
+<script src=""
+<script src=""
+</head>
+<body>
+<script>
+
+let test = async_test('An element removed by the adoption agency algorithm must not be inserted if it had been inserted elsewhere');
+
+var p;
+test.step(() => {
+ const iframe = document.createElement('iframe');
+ document.body.appendChild(iframe);
+
+ let doc = iframe.contentDocument;
+ doc.write(`<body><a id="target" href=""
+
+ const target = doc.querySelector('a');
+ target._onfocus_ = () => {
+ target._onfocus_ = null;
+
+ test.step(() => {
+ let container = doc.querySelector('div');
+ container.remove();
+ doc.body.appendChild(p);
+ });
+
+ setTimeout(() => {
+ test.step(() => {
+ assert_equals(p.parentNode, doc.body);
+ });
+ test.done();
+ iframe.remove();
+ }, 0);
+ }
+
+ doc.write(`<div><b><p><script>
+ parent.p = document.querySelector('p');
+ document.write('<link rel="stylesheet" href=""
+ location.hash = 'target';
+ <\/script></b></p></div></body>`);
+});
+
+</script>
+</body>
+</html>
Added: branches/safari-603-branch/LayoutTests/fast/parser/xml-error-unload-iframe-expected.txt (0 => 212721)
--- branches/safari-603-branch/LayoutTests/fast/parser/xml-error-unload-iframe-expected.txt (rev 0)
+++ branches/safari-603-branch/LayoutTests/fast/parser/xml-error-unload-iframe-expected.txt 2017-02-21 18:21:28 UTC (rev 212721)
@@ -0,0 +1,3 @@
+
+PASS An element removed by the adoption agency algorithm must not be inserted if it had been inserted elsewhere
+
Added: branches/safari-603-branch/LayoutTests/fast/parser/xml-error-unload-iframe.html (0 => 212721)
--- branches/safari-603-branch/LayoutTests/fast/parser/xml-error-unload-iframe.html (rev 0)
+++ branches/safari-603-branch/LayoutTests/fast/parser/xml-error-unload-iframe.html 2017-02-21 18:21:28 UTC (rev 212721)
@@ -0,0 +1,36 @@
+<!DOCTYPE html>
+<head>
+<script src=""
+<script src=""
+</head>
+<body>
+<script>
+
+let svgElement;
+function moveIframe(svgDoc) {
+ if (svgElement)
+ return;
+ svgElement = svgDoc.documentElement;
+ const iframe = document.createElement('iframe');
+ svgDoc.documentElement.appendChild(iframe);
+ iframe.contentWindow._onunload_ = () => {
+ document.body.appendChild(svgElement);
+ }
+}
+
+const content = `<svg xmlns="http://www.w3.org/2000/svg"><script>parent.moveIframe(document);<\/script><element a="1" a="2"/></svg>`;
+const iframe = document.createElement('iframe');
+iframe.src = "" Blob([content], {type: 'text/xml'}));
+document.documentElement.appendChild(iframe);
+
+window._onload_ = () => {
+ let test = async_test('An element removed by the adoption agency algorithm must not be inserted if it had been inserted elsewhere');
+ test.step(() => {
+ assert_equals(svgElement.parentNode, document.body);
+ svgElement.remove();
+ });
+ test.done();
+}
+
+</script>
+</body>
Modified: branches/safari-603-branch/Source/WebCore/ChangeLog (212720 => 212721)
--- branches/safari-603-branch/Source/WebCore/ChangeLog 2017-02-21 18:21:22 UTC (rev 212720)
+++ branches/safari-603-branch/Source/WebCore/ChangeLog 2017-02-21 18:21:28 UTC (rev 212721)
@@ -1,3 +1,39 @@
+2017-02-18 Ryosuke Niwa <[email protected]>
+
+ REGRESSION(r212218): Assertion failures in and after parserRemoveChild
+ https://bugs.webkit.org/show_bug.cgi?id=168458
+
+ Reviewed by Antti Koivisto.
+
+ The bug was caused by parserRemoveChild not preceeding to remove oldChild even when
+ oldChild had been inserted elsewhere during unload evnets of the disconnected frames.
+ Fixed the bug by checking this condition and exiting early.
+
+ Also fixed various callers of parserRemoveChild to not call parserAppendChild when
+ the removed node had already been inserted elsewhere by scripts.
+
+ Tests: fast/parser/adoption-agency-unload-iframe-3.html
+ fast/parser/adoption-agency-unload-iframe-4.html
+ fast/parser/xml-error-unload-iframe.html
+
+ * dom/ContainerNode.cpp:
+ (WebCore::ContainerNode::parserRemoveChild): Exit early when the node had been
+ inserted elsewhere while firing unload events. Also moved the call to
+ notifyRemovePendingSheetIfNeeded outside NoEventDispatchAssertion since it can
+ synchrnously fire a focus event.
+ (WebCore::ContainerNode::parserAppendChild): Moved adoptNode call to inside
+ NoEventDispatchAssertion since adoptNode call here should never mutate DOM.
+ * html/parser/HTMLConstructionSite.cpp:
+ (WebCore::executeReparentTask): Added an early exit when the node had already been
+ inserted elsewhere.
+ (WebCore::executeInsertAlreadyParsedChildTask): Ditto.
+ * xml/XMLErrors.cpp:
+ (WebCore::XMLErrors::insertErrorMessageBlock): Ditto.
+ * xml/parser/XMLDocumentParser.cpp:
+ (WebCore::XMLDocumentParser::end): Fixed a crash unveiled by one of the test cases.
+ Exit early when insertErrorMessageBlock detached the parser (by author scripts).
+ (WebCore::XMLDocumentParser::finish): Keep the parser alive until we exit.
+
2017-02-20 Ryosuke Niwa <[email protected]>
HTMLConstructionSiteTask::Insert should never be called on a node with a parent
Modified: branches/safari-603-branch/Source/WebCore/dom/ContainerNode.cpp (212720 => 212721)
--- branches/safari-603-branch/Source/WebCore/dom/ContainerNode.cpp 2017-02-21 18:21:22 UTC (rev 212720)
+++ branches/safari-603-branch/Source/WebCore/dom/ContainerNode.cpp 2017-02-21 18:21:28 UTC (rev 212721)
@@ -598,23 +598,27 @@
void ContainerNode::parserRemoveChild(Node& oldChild)
{
disconnectSubframesIfNeeded(*this, DescendantsOnly);
+ if (oldChild.parentNode() != this)
+ return;
- NoEventDispatchAssertion assertNoEventDispatch;
+ {
+ NoEventDispatchAssertion assertNoEventDispatch;
- document().nodeChildrenWillBeRemoved(*this);
+ document().nodeChildrenWillBeRemoved(*this);
- ASSERT(oldChild.parentNode() == this);
- ASSERT(!oldChild.isDocumentFragment());
+ ASSERT(oldChild.parentNode() == this);
+ ASSERT(!oldChild.isDocumentFragment());
- Node* prev = oldChild.previousSibling();
- Node* next = oldChild.nextSibling();
+ Node* prev = oldChild.previousSibling();
+ Node* next = oldChild.nextSibling();
- ChildListMutationScope(*this).willRemoveChild(oldChild);
- oldChild.notifyMutationObserversNodeWillDetach();
+ ChildListMutationScope(*this).willRemoveChild(oldChild);
+ oldChild.notifyMutationObserversNodeWillDetach();
- removeBetween(prev, next, oldChild);
+ removeBetween(prev, next, oldChild);
- notifyChildRemoved(oldChild, prev, next, ChildChangeSourceParser);
+ notifyChildRemoved(oldChild, prev, next, ChildChangeSourceParser);
+ }
document().notifyRemovePendingSheetIfNeeded();
}
@@ -719,11 +723,12 @@
ASSERT(!newChild.isDocumentFragment());
ASSERT(!hasTagName(HTMLNames::templateTag));
- if (&document() != &newChild.document())
- document().adoptNode(newChild);
-
{
NoEventDispatchAssertion assertNoEventDispatch;
+
+ if (&document() != &newChild.document())
+ document().adoptNode(newChild);
+
appendChildCommon(newChild);
treeScope().adoptIfNeeded(newChild);
}
Modified: branches/safari-603-branch/Source/WebCore/html/parser/HTMLConstructionSite.cpp (212720 => 212721)
--- branches/safari-603-branch/Source/WebCore/html/parser/HTMLConstructionSite.cpp 2017-02-21 18:21:22 UTC (rev 212720)
+++ branches/safari-603-branch/Source/WebCore/html/parser/HTMLConstructionSite.cpp 2017-02-21 18:21:28 UTC (rev 212721)
@@ -131,6 +131,9 @@
if (auto* parent = task.child->parentNode())
parent->parserRemoveChild(*task.child);
+ if (task.child->parentNode())
+ return;
+
task.parent->parserAppendChild(*task.child);
}
@@ -140,6 +143,10 @@
if (ContainerNode* parent = task.child->parentNode())
parent->parserRemoveChild(*task.child);
+
+ if (task.child->parentNode())
+ return;
+
insert(task);
}
Modified: branches/safari-603-branch/Source/WebCore/xml/XMLErrors.cpp (212720 => 212721)
--- branches/safari-603-branch/Source/WebCore/xml/XMLErrors.cpp 2017-02-21 18:21:22 UTC (rev 212720)
+++ branches/safari-603-branch/Source/WebCore/xml/XMLErrors.cpp 2017-02-21 18:21:28 UTC (rev 212721)
@@ -140,8 +140,9 @@
rootElement->parserAppendChild(body);
m_document.parserRemoveChild(*documentElement);
+ if (!documentElement->parentNode())
+ body->parserAppendChild(*documentElement);
- body->parserAppendChild(*documentElement);
m_document.parserAppendChild(rootElement);
documentElement = WTFMove(body);
Modified: branches/safari-603-branch/Source/WebCore/xml/parser/XMLDocumentParser.cpp (212720 => 212721)
--- branches/safari-603-branch/Source/WebCore/xml/parser/XMLDocumentParser.cpp 2017-02-21 18:21:22 UTC (rev 212720)
+++ branches/safari-603-branch/Source/WebCore/xml/parser/XMLDocumentParser.cpp 2017-02-21 18:21:28 UTC (rev 212721)
@@ -195,9 +195,11 @@
if (m_parserPaused)
return;
- if (m_sawError)
+ if (m_sawError) {
insertErrorMessageBlock();
- else {
+ if (isDetached()) // Inserting an error message may have ran arbitrary scripts.
+ return;
+ } else {
updateLeafTextNode();
document()->styleScope().didChangeStyleSheetEnvironment();
}
@@ -215,6 +217,8 @@
// makes sense to call any methods on DocumentParser once it's been stopped.
// However, FrameLoader::stop calls DocumentParser::finish unconditionally.
+ Ref<XMLDocumentParser> protectedThis(*this);
+
if (m_parserPaused)
m_finishCalled = true;
else
