Diff
Modified: trunk/LayoutTests/ChangeLog (195262 => 195263)
--- trunk/LayoutTests/ChangeLog 2016-01-19 08:29:54 UTC (rev 195262)
+++ trunk/LayoutTests/ChangeLog 2016-01-19 08:39:44 UTC (rev 195263)
@@ -1,3 +1,17 @@
+2016-01-19 Ryosuke Niwa <[email protected]>
+
+ innerHTML should always add a mutation record for removing all children
+ https://bugs.webkit.org/show_bug.cgi?id=148782
+ <rdar://problem/22571962>
+
+ Reviewed by Antti Koivisto.
+
+ Add a more comprehensive test for replacing a single text node with innerHTML's setter to ensure
+ WebKit's optimization to avoid replacing the node should not be observable by scripts in any way.
+
+ * fast/dom/innerHTML-single-text-node-expected.txt: Added.
+ * fast/dom/innerHTML-single-text-node.html: Added.
+
2016-01-18 Ryosuke Niwa <[email protected]>
createAttribute should lowercase the attribute name in a HTML document
Modified: trunk/LayoutTests/fast/dom/HTMLElement/set-inner-outer-optimization.html (195262 => 195263)
--- trunk/LayoutTests/fast/dom/HTMLElement/set-inner-outer-optimization.html 2016-01-19 08:29:54 UTC (rev 195262)
+++ trunk/LayoutTests/fast/dom/HTMLElement/set-inner-outer-optimization.html 2016-01-19 08:39:44 UTC (rev 195263)
@@ -74,8 +74,8 @@
runTest('', 'innerHTML', '<a></a><b></b>', 'modified');
runTest('text', 'innerHTML', '', 'modified');
- runTest('text', 'innerHTML', 'different text', 'modified, with same first child');
- runTest('text', 'innerHTML', 'text', 'not modified');
+ runTest('text', 'innerHTML', 'different text', 'modified');
+ runTest('text', 'innerHTML', 'text', 'modified');
runTest('text', 'innerHTML', '<a></a>', 'modified');
runTest('text', 'innerHTML', '<a></a><b></b>', 'modified');
Modified: trunk/LayoutTests/fast/dom/adopt-node-crash.html (195262 => 195263)
--- trunk/LayoutTests/fast/dom/adopt-node-crash.html 2016-01-19 08:29:54 UTC (rev 195262)
+++ trunk/LayoutTests/fast/dom/adopt-node-crash.html 2016-01-19 08:39:44 UTC (rev 195263)
@@ -10,8 +10,8 @@
var nodeToAdopt = document.getElementById('node-to-adopt');
var mutationHandler = function() {
+ document.body.removeEventListener('DOMSubtreeModified', mutationHandler, true);
document.getElementById('result').innerHTML = "DOMSubtreeModified fired";
- document.body.removeEventListener('DOMSubtreeModified', mutationHandler, true);
document.body.appendChild(nodeToAdopt);
};
document.body.addEventListener('DOMSubtreeModified', mutationHandler, true);
Added: trunk/LayoutTests/fast/dom/innerHTML-single-text-node-expected.txt (0 => 195263)
--- trunk/LayoutTests/fast/dom/innerHTML-single-text-node-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/dom/innerHTML-single-text-node-expected.txt 2016-01-19 08:39:44 UTC (rev 195263)
@@ -0,0 +1,37 @@
+Tests for replacing exactly one Text node. The optimization to update its data should not be observable.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+Keeping a reference
+container = createElementWithText("old"); textNode = container.firstChild
+PASS container.innerHTML = "new"; container.firstChild is not textNode
+
+Mutation observer
+container = createElementWithText("old"); mutationObserver.observe(container, {childList: true});
+PASS container.innerHTML = "new"; records = mutationObserver.takeRecords(); records.length is 1
+PASS records[0].type is "childList"
+PASS records[0].removedNodes.length is 1
+PASS records[0].removedNodes[0].data is "old"
+PASS records[0].addedNodes.length is 1
+PASS records[0].addedNodes[0].data is "new"
+
+DOMSubtreeModified event
+PASS container = createElementWithText("old"); countEvent(container, "DOMSubtreeModified"); eventCount is 0
+PASS container.innerHTML = "new"; eventCount > 0 is true
+
+DOMNodeInserted event
+PASS container = createElementWithText("old"); countEvent(container, "DOMNodeInserted"); eventCount is 0
+PASS container.innerHTML = "new"; eventCount is 1
+
+DOMNodeRemoved event
+PASS container = createElementWithText("old"); countEvent(container, "DOMNodeRemoved"); eventCount is 0
+PASS container.innerHTML = "new"; eventCount is 1
+
+DOMCharacterDataModified event
+PASS container = createElementWithText("old"); countEvent(container, "DOMCharacterDataModified"); eventCount is 0
+PASS container.innerHTML = "new"; eventCount is 0
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/fast/dom/innerHTML-single-text-node.html (0 => 195263)
--- trunk/LayoutTests/fast/dom/innerHTML-single-text-node.html (rev 0)
+++ trunk/LayoutTests/fast/dom/innerHTML-single-text-node.html 2016-01-19 08:39:44 UTC (rev 195263)
@@ -0,0 +1,64 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script src=""
+<script>
+
+description('Tests for replacing exactly one Text node. The optimization to update its data should not be observable.');
+
+function createElementWithText(text)
+{
+ var iframe = document.createElement('iframe');
+ document.body.appendChild(iframe);
+ var doc = iframe.contentDocument;
+
+ var container = doc.createElement("div");
+ container.textContent = text;
+
+ doc.body.appendChild(container);
+
+ return container;
+}
+
+function eventCounter() { eventCount++; }
+function countEvent(element, name) {
+ eventCount = 0;
+ element.addEventListener(name, eventCounter);
+}
+
+debug('Keeping a reference');
+evalAndLog('container = createElementWithText("old"); textNode = container.firstChild');
+shouldNotBe('container.innerHTML = "new"; container.firstChild', 'textNode');
+
+debug('\nMutation observer');
+var mutationObserver = new MutationObserver(function () {});
+evalAndLog('container = createElementWithText("old"); mutationObserver.observe(container, {childList: true});');
+shouldBe('container.innerHTML = "new"; records = mutationObserver.takeRecords(); records.length', '1');
+shouldBe('records[0].type', '"childList"');
+shouldBe('records[0].removedNodes.length', '1');
+shouldBe('records[0].removedNodes[0].data', '"old"');
+shouldBe('records[0].addedNodes.length', '1');
+shouldBe('records[0].addedNodes[0].data', '"new"');
+
+debug('\nDOMSubtreeModified event');
+shouldBe('container = createElementWithText("old"); countEvent(container, "DOMSubtreeModified"); eventCount', '0');
+shouldBeTrue('container.innerHTML = "new"; eventCount > 0');
+
+debug('\nDOMNodeInserted event');
+shouldBe('container = createElementWithText("old"); countEvent(container, "DOMNodeInserted"); eventCount', '0');
+shouldBe('container.innerHTML = "new"; eventCount', '1');
+
+debug('\nDOMNodeRemoved event');
+shouldBe('container = createElementWithText("old"); countEvent(container, "DOMNodeRemoved"); eventCount', '0');
+shouldBe('container.innerHTML = "new"; eventCount', '1');
+
+debug('\nDOMCharacterDataModified event');
+shouldBe('container = createElementWithText("old"); countEvent(container, "DOMCharacterDataModified"); eventCount', '0');
+shouldBe('container.innerHTML = "new"; eventCount', '0');
+
+// FIXME: Add a test for DOMNodeRemovedFromDocument event.
+
+</script>
+<script src=""
+</body>
+</html>
Modified: trunk/LayoutTests/fast/events/crash-on-mutate-during-drop.html (195262 => 195263)
--- trunk/LayoutTests/fast/events/crash-on-mutate-during-drop.html 2016-01-19 08:29:54 UTC (rev 195262)
+++ trunk/LayoutTests/fast/events/crash-on-mutate-during-drop.html 2016-01-19 08:39:44 UTC (rev 195263)
@@ -1,7 +1,8 @@
<html>
<head>
<script>
-function foo() {
+function onDOMNodeInserted(event) {
+ document.removeEventListener("DOMNodeInserted", onDOMNodeInserted, true);
if (event.type == "DOMNodeInserted" && event.target.nodeType == 3)
document.body.innerHTML = "PASSED";
}
@@ -12,7 +13,7 @@
window.testRunner.dumpAsText();
- document.addEventListener("DOMNodeInserted", function() { foo() }, true);
+ document.addEventListener("DOMNodeInserted", onDOMNodeInserted, true);
// Select the element 'dragSource'.
var selection = window.getSelection();
Modified: trunk/LayoutTests/imported/w3c/ChangeLog (195262 => 195263)
--- trunk/LayoutTests/imported/w3c/ChangeLog 2016-01-19 08:29:54 UTC (rev 195262)
+++ trunk/LayoutTests/imported/w3c/ChangeLog 2016-01-19 08:39:44 UTC (rev 195263)
@@ -1,3 +1,15 @@
+2016-01-19 Ryosuke Niwa <[email protected]>
+
+ innerHTML should always add a mutation record for removing all children
+ https://bugs.webkit.org/show_bug.cgi?id=148782
+ <rdar://problem/22571962>
+
+ Reviewed by Antti Koivisto.
+
+ Rebaseline a test now that all test test cases are passing.
+
+ * web-platform-tests/dom/nodes/MutationObserver-inner-outer-expected.txt:
+
2016-01-18 Ryosuke Niwa <[email protected]>
createAttribute should lowercase the attribute name in a HTML document
Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/dom/nodes/MutationObserver-inner-outer-expected.txt (195262 => 195263)
--- trunk/LayoutTests/imported/w3c/web-platform-tests/dom/nodes/MutationObserver-inner-outer-expected.txt 2016-01-19 08:29:54 UTC (rev 195262)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/dom/nodes/MutationObserver-inner-outer-expected.txt 2016-01-19 08:39:44 UTC (rev 195263)
@@ -1,7 +1,7 @@
MutationObservers: innerHTML, outerHTML mutations
-FAIL innerHTML mutation assert_equals: mutation records must match expected 2 but got 1
+PASS innerHTML mutation
PASS innerHTML with 2 children mutation
PASS outerHTML mutation
Modified: trunk/LayoutTests/platform/mac/fast/events/updateLayoutForHitTest-expected.txt (195262 => 195263)
--- trunk/LayoutTests/platform/mac/fast/events/updateLayoutForHitTest-expected.txt 2016-01-19 08:29:54 UTC (rev 195262)
+++ trunk/LayoutTests/platform/mac/fast/events/updateLayoutForHitTest-expected.txt 2016-01-19 08:39:44 UTC (rev 195263)
@@ -14,5 +14,5 @@
RenderText {#text} at (13,6) size 50x18
text run at (13,6) width 50: " Project"
RenderText {#text} at (0,0) size 0x0
-selection start: position 0 of child 0 {#text} of child 1 {SPAN} of child 3 {DIV} of body
+selection start: position 0 of child 1 {SPAN} of child 3 {DIV} of body
selection end: position 0 of child 1 {#text} of child 3 {DIV} of child 3 {DIV} of body
Modified: trunk/Source/WebCore/ChangeLog (195262 => 195263)
--- trunk/Source/WebCore/ChangeLog 2016-01-19 08:29:54 UTC (rev 195262)
+++ trunk/Source/WebCore/ChangeLog 2016-01-19 08:39:44 UTC (rev 195263)
@@ -1,3 +1,30 @@
+2016-01-19 Ryosuke Niwa <[email protected]>
+
+ innerHTML should always add a mutation record for removing all children
+ https://bugs.webkit.org/show_bug.cgi?id=148782
+ <rdar://problem/22571962>
+
+ Reviewed by Antti Koivisto.
+
+ Fixed the bug by disabling WebKit's optimization to avoid the node replacement when the behavior
+ is observable to scripts by either:
+ - Author scripts has a reference to the node
+ - MutationObserver can be observing this subtree
+ - Mutation events can be observing this subtree
+
+ Note that no caller of this function exposes fragment to author scripts so it couldn't be referenced.
+ It also means that we don't need to check DOMNodeInsertedIntoDocument since it doesn't bubble up
+ (it's only relevant if the text node in fragment has its event listener but that's impossible).
+
+ Test: fast/dom/innerHTML-single-text-node.html
+
+ * dom/ChildListMutationScope.h:
+ (WebCore::ChildListMutationScope::canObserve): Added.
+
+ * editing/markup.cpp:
+ (WebCore::hasMutationEventListeners): Added.
+ (WebCore::replaceChildrenWithFragment):
+
2016-01-18 Ryosuke Niwa <[email protected]>
createAttribute should lowercase the attribute name in a HTML document
Modified: trunk/Source/WebCore/dom/ChildListMutationScope.h (195262 => 195263)
--- trunk/Source/WebCore/dom/ChildListMutationScope.h 2016-01-19 08:29:54 UTC (rev 195262)
+++ trunk/Source/WebCore/dom/ChildListMutationScope.h 2016-01-19 08:39:44 UTC (rev 195263)
@@ -82,6 +82,8 @@
m_accumulator = ChildListMutationAccumulator::getOrCreate(target);
}
+ bool canObserve() const { return m_accumulator; }
+
void childAdded(Node& child)
{
if (m_accumulator && m_accumulator->hasObservers())
Modified: trunk/Source/WebCore/editing/markup.cpp (195262 => 195263)
--- trunk/Source/WebCore/editing/markup.cpp 2016-01-19 08:29:54 UTC (rev 195262)
+++ trunk/Source/WebCore/editing/markup.cpp 2016-01-19 08:39:44 UTC (rev 195263)
@@ -989,6 +989,22 @@
return hasOneChild(node) && node.firstChild()->isTextNode();
}
+static inline bool hasMutationEventListeners(const Document& document)
+{
+ return document.hasListenerType(Document::DOMSUBTREEMODIFIED_LISTENER)
+ || document.hasListenerType(Document::DOMNODEINSERTED_LISTENER)
+ || document.hasListenerType(Document::DOMNODEREMOVED_LISTENER)
+ || document.hasListenerType(Document::DOMNODEREMOVEDFROMDOCUMENT_LISTENER)
+ || document.hasListenerType(Document::DOMCHARACTERDATAMODIFIED_LISTENER);
+}
+
+// We can use setData instead of replacing Text node as long as script can't observe the difference.
+static inline bool canUseSetDataOptimization(const Text& containerChild, const ChildListMutationScope& mutationScope)
+{
+ bool authorScriptMayHaveReference = containerChild.refCount();
+ return !authorScriptMayHaveReference && !mutationScope.canObserve() && !hasMutationEventListeners(containerChild.document());
+}
+
void replaceChildrenWithFragment(ContainerNode& container, Ref<DocumentFragment>&& fragment, ExceptionCode& ec)
{
Ref<ContainerNode> containerNode(container);
@@ -999,13 +1015,15 @@
return;
}
- if (hasOneTextChild(containerNode) && hasOneTextChild(fragment)) {
- downcast<Text>(*containerNode->firstChild()).setData(downcast<Text>(*fragment->firstChild()).data(), ec);
- return;
- }
+ auto* containerChild = containerNode->firstChild();
+ if (containerChild && !containerChild->nextSibling()) {
+ if (is<Text>(*containerChild) && hasOneTextChild(fragment) && canUseSetDataOptimization(downcast<Text>(*containerChild), mutation)) {
+ ASSERT(!fragment->firstChild()->refCount());
+ downcast<Text>(*containerChild).setData(downcast<Text>(*fragment->firstChild()).data(), ec);
+ return;
+ }
- if (hasOneChild(containerNode)) {
- containerNode->replaceChild(WTFMove(fragment), *containerNode->firstChild(), ec);
+ containerNode->replaceChild(WTFMove(fragment), *containerChild, ec);
return;
}
