Diff
Modified: branches/safari-603-branch/LayoutTests/ChangeLog (212597 => 212598)
--- branches/safari-603-branch/LayoutTests/ChangeLog 2017-02-18 00:40:22 UTC (rev 212597)
+++ branches/safari-603-branch/LayoutTests/ChangeLog 2017-02-18 01:12:47 UTC (rev 212598)
@@ -1,5 +1,30 @@
2017-02-17 Matthew Hanson <matthew_han...@apple.com>
+ Merge r212218. rdar://problem/30339793
+
+ 2017-02-12 Ryosuke Niwa <rn...@webkit.org>
+
+ parserRemoveChild should unload subframes
+ https://bugs.webkit.org/show_bug.cgi?id=168151
+
+ Reviewed by Darin Adler.
+
+ Add two W3C-style testharness tests for unloading iframes inside the adoption agency algorithm.
+
+ Also added a test to make sure ContainerNode::takeAllChildrenFrom adjusts the focused element and DOM ranges.
+
+ * fast/css/stylesheet-candidate-nodes-crash-expected.txt: Rebaselined. The difference comes from the fact
+ iframe now is unloaded in parserRemoveChild as expected and then reloaded in parserAppendChild inside
+ insertErrorMessageBlock as opposed to after the parser had completed as if the iframe had never been detached.
+ * fast/parser/adoption-agency-clear-focus-range-expected.txt: Added.
+ * fast/parser/adoption-agency-clear-focus-range.html: Added.
+ * fast/parser/adoption-agency-unload-iframe-1-expected.txt: Added.
+ * fast/parser/adoption-agency-unload-iframe-1.html: Added.
+ * fast/parser/adoption-agency-unload-iframe-2-expected.txt: Added.
+ * fast/parser/adoption-agency-unload-iframe-2.html: Added.
+
+2017-02-17 Matthew Hanson <matthew_han...@apple.com>
+
Merge r212035. rdar://problem/30433204
2017-02-09 Filip Pizlo <fpi...@apple.com>
Modified: branches/safari-603-branch/LayoutTests/fast/css/stylesheet-candidate-nodes-crash-expected.txt (212597 => 212598)
--- branches/safari-603-branch/LayoutTests/fast/css/stylesheet-candidate-nodes-crash-expected.txt 2017-02-18 00:40:22 UTC (rev 212597)
+++ branches/safari-603-branch/LayoutTests/fast/css/stylesheet-candidate-nodes-crash-expected.txt 2017-02-18 01:12:47 UTC (rev 212598)
@@ -1 +1,7 @@
+This page contains the following errors:
+
+error on line 29 at column 9: Double hyphen within comment
+error on line 32 at column 1: Comment not terminated
+Below is a rendering of the page up to the first error.
+
PASS
Added: branches/safari-603-branch/LayoutTests/fast/parser/adoption-agency-clear-focus-range-expected.txt (0 => 212598)
--- branches/safari-603-branch/LayoutTests/fast/parser/adoption-agency-clear-focus-range-expected.txt (rev 0)
+++ branches/safari-603-branch/LayoutTests/fast/parser/adoption-agency-clear-focus-range-expected.txt 2017-02-18 01:12:47 UTC (rev 212598)
@@ -0,0 +1,3 @@
+Active Element must be moved to body: PASS
+range.startContainer must be moved to p: PASS
+
Added: branches/safari-603-branch/LayoutTests/fast/parser/adoption-agency-clear-focus-range.html (0 => 212598)
--- branches/safari-603-branch/LayoutTests/fast/parser/adoption-agency-clear-focus-range.html (rev 0)
+++ branches/safari-603-branch/LayoutTests/fast/parser/adoption-agency-clear-focus-range.html 2017-02-18 01:12:47 UTC (rev 212598)
@@ -0,0 +1,84 @@
+<script>
+
+function runTest() {
+ document.write(`<a><p><span tabindex="0">dummy</span><iframe _onload_="didLoadIframe()"></iframe><script> document.body.innerHTML = ''; <\/script>`);
+}
+
+let callCount = 0;
+let span;
+let range;
+function didLoadIframe() {
+ callCount++;
+ if (callCount == 1)
+ document.write('</a>');
+ else if (callCount == 2) {
+ // Test ContainerNode::takeAllChildrenFrom adjusts focused node and update ranges.
+ span = document.querySelector('span');
+ span.focus();
+ range = new Range;
+ range.selectNode(span.firstChild);
+ } else if (callCount == 3) {
+ setTimeout(() => {
+ let activeElementResult = (document.activeElement == document.body) ? 'PASS' : `FAIL - it was set on ${document.activeElement}`;
+ let rangeStartResult = (range.startContainer == document.querySelector('p')) ? 'PASS' : `FAIL - it was set on ${range.startContainer}`;
+
+ document.documentElement.innerHTML = `<body>
+ Active Element must be moved to body: ${activeElementResult}<br>
+ range.startContainer must be moved to p: ${rangeStartResult}<br>`;
+
+ if (window.testRunner)
+ testRunner.notifyDone();
+ }, 0);
+ }
+}
+
+/*
+
+ 0. The outer document.write creates the following tree:
+ body
+ + a
+ + p
+ + span
+ + iframe
+
+ 1. iframe's load event fires, and invokes the inner document.write,
+ which forces <script> from the outer document.write to be parsed:
+ body
+ + a
+ + p
+ + span
+ + iframe
+ + script
+
+ 1.a. The script runs, and invokes document.body.innerHTML = '':
+ body
+
+ 1.b. The adoption agency algorithm is involved, and p is removed from a
+ while both of them are detached from body.
+
+ 1.c. The adoption agency algorithm inserts p back into body along with iframe.
+ body
+ + p
+ + span
+ + iframe
+ + script
+
+ 2. iframe's load event fires for the second time per step 1.c.
+
+ 2.a. the focus is set on span and a range is created inside span.
+
+ 2.b. The adoption agency algorithm now takes all children of p
+ and inserts them back under "a", and then inserts "p" under body.
+
+3. iframe's load event fires for the third time per step 2.b.
+
+*/
+
+if (window.testRunner) {
+ testRunner.dumpAsText();
+ testRunner.waitUntilDone();
+}
+
+runTest();
+
+</script>
Added: branches/safari-603-branch/LayoutTests/fast/parser/adoption-agency-unload-iframe-1-expected.txt (0 => 212598)
--- branches/safari-603-branch/LayoutTests/fast/parser/adoption-agency-unload-iframe-1-expected.txt (rev 0)
+++ branches/safari-603-branch/LayoutTests/fast/parser/adoption-agency-unload-iframe-1-expected.txt 2017-02-18 01:12:47 UTC (rev 212598)
@@ -0,0 +1,3 @@
+
+PASS An iframe removed by the adoption agency algorithm must be unloaded
+
Added: branches/safari-603-branch/LayoutTests/fast/parser/adoption-agency-unload-iframe-1.html (0 => 212598)
--- branches/safari-603-branch/LayoutTests/fast/parser/adoption-agency-unload-iframe-1.html (rev 0)
+++ branches/safari-603-branch/LayoutTests/fast/parser/adoption-agency-unload-iframe-1.html 2017-02-18 01:12:47 UTC (rev 212598)
@@ -0,0 +1,38 @@
+<!DOCTYPE html>
+<head>
+<script src=""
+<script src=""
+</head>
+<body>
+<table><a><p><script>
+
+let oldBody = document.body;
+oldBody.remove();
+
+let a = oldBody.querySelector('a');
+document.documentElement.appendChild(a);
+/* html
+ + a
+ + p
+ + script */
+
+let iframe = document.createElement('iframe');
+a.firstChild.appendChild(iframe);
+/* html
+ + a
+ + p
+ + script
+ + iframe */
+
+let oldGlobal = iframe.contentWindow;
+
+window._onload_ = () => {
+ document.documentElement.appendChild(document.createElement('body'));
+ let test = async_test('An iframe removed by the adoption agency algorithm must be unloaded');
+ test.step(() => {
+ assert_not_equals(oldGlobal, iframe.contentWindow);
+ });
+ test.done();
+}
+
+</script></a></p>
Added: branches/safari-603-branch/LayoutTests/fast/parser/adoption-agency-unload-iframe-2-expected.txt (0 => 212598)
--- branches/safari-603-branch/LayoutTests/fast/parser/adoption-agency-unload-iframe-2-expected.txt (rev 0)
+++ branches/safari-603-branch/LayoutTests/fast/parser/adoption-agency-unload-iframe-2-expected.txt 2017-02-18 01:12:47 UTC (rev 212598)
@@ -0,0 +1,3 @@
+
+PASS An iframe removed by the adoption agency algorithm must be unloaded
+
Added: branches/safari-603-branch/LayoutTests/fast/parser/adoption-agency-unload-iframe-2.html (0 => 212598)
--- branches/safari-603-branch/LayoutTests/fast/parser/adoption-agency-unload-iframe-2.html (rev 0)
+++ branches/safari-603-branch/LayoutTests/fast/parser/adoption-agency-unload-iframe-2.html 2017-02-18 01:12:47 UTC (rev 212598)
@@ -0,0 +1,38 @@
+<!DOCTYPE html>
+<head>
+<script src=""
+<script src=""
+</head>
+<body>
+<b><p><script>
+
+let oldBody = document.body;
+oldBody.remove();
+
+let b = oldBody.querySelector('b');
+document.documentElement.appendChild(b);
+/* html
+ + b
+ + p
+ + script */
+
+let iframe = document.createElement('iframe');
+b.firstChild.appendChild(iframe);
+/* html
+ + b
+ + p
+ + script
+ + iframe */
+
+let oldGlobal = iframe.contentWindow;
+
+window._onload_ = () => {
+ document.documentElement.appendChild(document.createElement('body'));
+ let test = async_test('An iframe removed by the adoption agency algorithm must be unloaded');
+ test.step(() => {
+ assert_not_equals(oldGlobal, iframe.contentWindow);
+ });
+ test.done();
+}
+
+</script></b>
Modified: branches/safari-603-branch/Source/WebCore/ChangeLog (212597 => 212598)
--- branches/safari-603-branch/Source/WebCore/ChangeLog 2017-02-18 00:40:22 UTC (rev 212597)
+++ branches/safari-603-branch/Source/WebCore/ChangeLog 2017-02-18 01:12:47 UTC (rev 212598)
@@ -1,5 +1,47 @@
2017-02-17 Matthew Hanson <matthew_han...@apple.com>
+ Merge r212218. rdar://problem/30339793
+
+ 2017-02-12 Ryosuke Niwa <rn...@webkit.org>
+
+ parserRemoveChild should unload subframes
+ https://bugs.webkit.org/show_bug.cgi?id=168151
+
+ Reviewed by Darin Adler.
+
+ Fix the bug that the adoption agency algorithm does not unload subframes as it disconnects nodes.
+
+ Also moved calls to nodeWillBeRemoved inside NoEventDispatchAssertion to expand on r211965.
+
+ Tests: fast/parser/adoption-agency-clear-focus-range.html
+ fast/parser/adoption-agency-unload-iframe-1.html
+ fast/parser/adoption-agency-unload-iframe-2.html
+
+ * dom/ContainerNode.cpp:
+ (WebCore::ContainerNode::takeAllChildrenFrom): Rewritten using idioms used in removeChildren and parserAppendChild.
+
+ Disconnect all subframes first since this can synchronously dispatch an unload event. Then update DOM ranges,
+ the focused element, and other states in the document.
+
+ Second, use the regular removeBetween, notifyChildNodeRemoved, childrenChanged sequence of calls to disconnect nodes
+ instead of a single call to removeDetachedChildren to properly disconnect child nodes since those nodes may have
+ already come live due to execution of synchronous scripts prior to the adoption agency algorithm has run, or in
+ response to the unload event we just dispatched.
+
+ Third, append these nodes using parserAppendChild to avoid dispatching mutation events.
+
+ (WebCore::willRemoveChild): Removed the call to nodeWillBeRemoved. It's now called within NoEventDispatchAssertion
+ in each call site of willRemoveChild and willRemoveChildren.
+ (WebCore::willRemoveChildren): Ditto.
+ (WebCore::ContainerNode::removeChild): Call nodeWillBeRemoved inside NoEventDispatchAssertion.
+ (WebCore::ContainerNode::replaceAllChildren): Call nodeWillBeRemoved inside NoEventDispatchAssertion.
+ (WebCore::ContainerNode::parserRemoveChild): Disconnect subframes and update document's states.
+
+ * html/parser/HTMLConstructionSite.cpp:
+ (WebCore::executeTakeAllChildrenAndReparentTask): Add a release assert that new parent does not already have a parent.
+
+2017-02-17 Matthew Hanson <matthew_han...@apple.com>
+
Merge r212035. rdar://problem/30433204
2017-02-09 Filip Pizlo <fpi...@apple.com>
Modified: branches/safari-603-branch/Source/WebCore/dom/ContainerNode.cpp (212597 => 212598)
--- branches/safari-603-branch/Source/WebCore/dom/ContainerNode.cpp 2017-02-18 00:40:22 UTC (rev 212597)
+++ branches/safari-603-branch/Source/WebCore/dom/ContainerNode.cpp 2017-02-18 01:12:47 UTC (rev 212598)
@@ -132,21 +132,27 @@
mutation.willRemoveChild(child);
}
- // FIXME: We need to do notifyMutationObserversNodeWillDetach() for each child,
- // probably inside removeDetachedChildrenInContainer.
+ disconnectSubframesIfNeeded(*oldParent, DescendantsOnly);
+ {
+ NoEventDispatchAssertion assertNoEventDispatch;
- oldParent->removeDetachedChildren();
+ oldParent->document().nodeChildrenWillBeRemoved(*oldParent);
+ WidgetHierarchyUpdatesSuspensionScope suspendWidgetHierarchyUpdates;
+ while (RefPtr<Node> child = oldParent->m_firstChild) {
+ oldParent->removeBetween(nullptr, child->nextSibling(), *child);
+ notifyChildNodeRemoved(*oldParent, *child);
+ }
+ ChildChange change = { AllChildrenRemoved, nullptr, nullptr, ChildChangeSourceParser };
+ childrenChanged(change);
+ }
+
+ // FIXME: assert that we don't dispatch events here since this container node is still disconnected.
for (auto& child : children) {
- destroyRenderTreeIfNeeded(child);
-
- // FIXME: We need a no mutation event version of adoptNode.
- auto adoptedChild = document().adoptNode(child).releaseReturnValue();
- parserAppendChild(adoptedChild);
- // FIXME: Together with adoptNode above, the tree scope might get updated recursively twice
- // (if the document changed or oldParent was in a shadow tree, AND *this is in a shadow tree).
- // Can we do better?
- treeScope().adoptIfNeeded(adoptedChild);
+ RELEASE_ASSERT(!child->parentNode() && &child->treeScope() == &treeScope());
+ ASSERT(!ensurePreInsertionValidity(child, nullptr).hasException());
+ treeScope().adoptIfNeeded(child);
+ parserAppendChild(child);
}
}
@@ -481,11 +487,6 @@
if (is<ContainerNode>(child))
disconnectSubframesIfNeeded(downcast<ContainerNode>(child), RootAndDescendants);
-
- if (child.parentNode() != &container)
- return;
-
- child.document().nodeWillBeRemoved(child); // e.g. mutation event listener can create a new range.
}
static void willRemoveChildren(ContainerNode& container)
@@ -503,9 +504,6 @@
}
disconnectSubframesIfNeeded(container, DescendantsOnly);
-
- container.document().nodeChildrenWillBeRemoved(container);
-
}
void ContainerNode::disconnectDescendantFrames()
@@ -529,7 +527,7 @@
willRemoveChild(*this, child);
- // Mutation events might have moved this child into a different parent.
+ // Mutation events in willRemoveChild might have moved this child into a different parent.
if (child->parentNode() != this)
return Exception { NOT_FOUND_ERR };
@@ -537,6 +535,8 @@
WidgetHierarchyUpdatesSuspensionScope suspendWidgetHierarchyUpdates;
NoEventDispatchAssertion assertNoEventDispatch;
+ document().nodeWillBeRemoved(child);
+
Node* prev = child->previousSibling();
Node* next = child->nextSibling();
removeBetween(prev, next, child);
@@ -592,8 +592,12 @@
void ContainerNode::parserRemoveChild(Node& oldChild)
{
+ disconnectSubframesIfNeeded(*this, DescendantsOnly);
+
NoEventDispatchAssertion assertNoEventDispatch;
+ document().nodeChildrenWillBeRemoved(*this);
+
ASSERT(oldChild.parentNode() == this);
ASSERT(!oldChild.isDocumentFragment());
@@ -600,8 +604,6 @@
Node* prev = oldChild.previousSibling();
Node* next = oldChild.nextSibling();
- oldChild.updateAncestorConnectedSubframeCountForRemoval();
-
ChildListMutationScope(*this).willRemoveChild(oldChild);
oldChild.notifyMutationObserversNodeWillDetach();
@@ -628,6 +630,8 @@
WidgetHierarchyUpdatesSuspensionScope suspendWidgetHierarchyUpdates;
NoEventDispatchAssertion assertNoEventDispatch;
+ document().nodeChildrenWillBeRemoved(*this);
+
while (RefPtr<Node> child = m_firstChild) {
removeBetween(0, child->nextSibling(), *child);
notifyChildNodeRemoved(*this, *child);
Modified: branches/safari-603-branch/Source/WebCore/html/parser/HTMLConstructionSite.cpp (212597 => 212598)
--- branches/safari-603-branch/Source/WebCore/html/parser/HTMLConstructionSite.cpp 2017-02-18 00:40:22 UTC (rev 212597)
+++ branches/safari-603-branch/Source/WebCore/html/parser/HTMLConstructionSite.cpp 2017-02-18 01:12:47 UTC (rev 212598)
@@ -150,6 +150,7 @@
task.parent->takeAllChildrenFrom(task.oldParent());
// Notice that we don't need to manually attach the moved children
// because takeAllChildrenFrom does that work for us.
+ RELEASE_ASSERT(!task.parent->parentNode());
}
static inline void executeTask(HTMLConstructionSiteTask& task)
