Diff
Modified: trunk/LayoutTests/ChangeLog (218082 => 218083)
--- trunk/LayoutTests/ChangeLog 2017-06-12 02:40:43 UTC (rev 218082)
+++ trunk/LayoutTests/ChangeLog 2017-06-12 03:46:47 UTC (rev 218083)
@@ -1,3 +1,16 @@
+2017-06-09 Ryosuke Niwa <[email protected]>
+
+ didMoveToNewDocument doesn't get called on an Attr inside a shadow tree
+ https://bugs.webkit.org/show_bug.cgi?id=173133
+
+ Reviewed by Antti Koivisto.
+
+ Added a regression test for moving a shadow tree with an Attr node across a document.
+ The test hits an assertion in a debug build and fails in a release build without the fix.
+
+ * fast/dom/adopt-attr-with-shadow-tree-expected.txt: Added.
+ * fast/dom/adopt-attr-with-shadow-tree.html: Added.
+
2017-06-11 Keith Miller <[email protected]>
TypedArray constructor with string shouldn't throw
Added: trunk/LayoutTests/fast/dom/adopt-attr-with-shadow-tree-expected.txt (0 => 218083)
--- trunk/LayoutTests/fast/dom/adopt-attr-with-shadow-tree-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/dom/adopt-attr-with-shadow-tree-expected.txt 2017-06-12 03:46:47 UTC (rev 218083)
@@ -0,0 +1,10 @@
+This tests adopting a shadow tree with an Attr node.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS referenceCountInDestination is referenceCountInSource
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/fast/dom/adopt-attr-with-shadow-tree.html (0 => 218083)
--- trunk/LayoutTests/fast/dom/adopt-attr-with-shadow-tree.html (rev 0)
+++ trunk/LayoutTests/fast/dom/adopt-attr-with-shadow-tree.html 2017-06-12 03:46:47 UTC (rev 218083)
@@ -0,0 +1,50 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script src=""
+<script>
+
+description('This tests adopting a shadow tree with an Attr node.');
+
+var referenceCountInSource;
+var referenceCountInDestination;
+function runTest() {
+ let startCount = internals.referencingNodeCount(document);
+
+ const outerHost = document.createElement('shadow-host');
+ document.body.appendChild(outerHost);
+ const outerRoot = outerHost.attachShadow({mode: 'closed'});
+ outerRoot.innerHTML = '<span title="foo"></span>';
+
+ const innerHost = outerRoot.firstChild;
+ const innerRoot = innerHost.attachShadow({mode: 'closed'});
+ innerRoot.innerHTML = '<div lang="en"></div>';
+
+ const outerAttr = innerHost.attributes[0];
+ const outerAttrNodeList = outerAttr.childNodes;
+ const innerAttr = innerRoot.firstChild.attributes[0];
+ const innerAttrNodeList = innerAttr.childNodes;
+
+ referenceCountInSource = internals.referencingNodeCount(document) - startCount;
+
+ const iframe = document.createElement('iframe');
+ document.body.appendChild(iframe);
+
+ startCount = internals.referencingNodeCount(iframe.contentDocument);
+ iframe.contentDocument.body.appendChild(outerHost);
+ referenceCountInDestination = internals.referencingNodeCount(iframe.contentDocument) - startCount;
+
+ iframe.remove();
+}
+
+if (!window.GCController || !window.internals)
+ testFailed('This test requires testRunner, internals, and GCController objects.');
+else {
+ runTest();
+ GCController.collect();
+ shouldBe('referenceCountInDestination', 'referenceCountInSource');
+}
+
+</script>
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (218082 => 218083)
--- trunk/Source/WebCore/ChangeLog 2017-06-12 02:40:43 UTC (rev 218082)
+++ trunk/Source/WebCore/ChangeLog 2017-06-12 03:46:47 UTC (rev 218083)
@@ -1,3 +1,32 @@
+2017-06-09 Ryosuke Niwa <[email protected]>
+
+ didMoveToNewDocument doesn't get called on an Attr inside a shadow tree
+ https://bugs.webkit.org/show_bug.cgi?id=173133
+
+ Reviewed by Antti Koivisto.
+
+ The bug was caused by moveShadowTreeToNewDocument never calling didMoveToNewDocument on Attr nodes.
+ Fixed the bug by using the same traversal code as moveTreeToNewScope in moveShadowTreeToNewDocument
+ by extracting the traversal code as a templatized traverseSubtreeToUpdateTreeScope.
+
+ Also removed the code to increment the DOM tree version in moveTreeToNewScope. This code was there
+ to invalidate the HTML collection caches which used to clear the cache whenever the DOM tree version
+ changed before r122621 (five years ago! by me). Since we now eagerly invalidate each node list and
+ HTML collection's caches via NodeListsNodeData::adoptTreeScope and NodeListsNodeData::adoptDocument.
+
+ Test: fast/dom/adopt-attr-with-shadow-tree.html
+
+ * dom/Node.cpp:
+ (WebCore::moveNodeToNewDocument): Assert that the node had been adopted to a new document.
+ (WebCore::traverseSubtreeToUpdateTreeScope): Extracted from moveTreeToNewScope.
+ (WebCore::moveShadowTreeToNewDocument): Use traverseSubtreeToUpdateTreeScope to adopt each node
+ to the new document.
+ (WebCore::Node::moveTreeToNewScope): See above.
+ * testing/Internals.cpp:
+ (WebCore::Internals::referencingNodeCount): Added. Used in the newly added regression test.
+ * testing/Internals.h:
+ * testing/Internals.idl:
+
2017-06-11 Dan Bernstein <[email protected]>
[Mac] Unaligned pointers in static CMBufferCallbacks structs defined in WebCoreDecompressionSession.mm
Modified: trunk/Source/WebCore/dom/Node.cpp (218082 => 218083)
--- trunk/Source/WebCore/dom/Node.cpp 2017-06-12 02:40:43 UTC (rev 218082)
+++ trunk/Source/WebCore/dom/Node.cpp 2017-06-12 03:46:47 UTC (rev 218083)
@@ -1975,65 +1975,73 @@
ASSERT(!node.isConnected() || &oldDocument != &newDocument);
DidMoveToNewDocumentAssertionScope scope(node, oldDocument, newDocument);
node.didMoveToNewDocument(oldDocument, newDocument);
+ ASSERT_WITH_SECURITY_IMPLICATION(&node.document() == &newDocument);
}
-static void moveShadowTreeToNewDocument(ShadowRoot& shadowRoot, Document& oldDocument, Document& newDocument)
+template <typename MoveNodeFunction, typename MoveShadowRootFunction>
+static void traverseSubtreeToUpdateTreeScope(Node& root, MoveNodeFunction moveNode, MoveShadowRootFunction moveShadowRoot)
{
- for (Node* node = &shadowRoot; node; node = NodeTraversal::next(*node, &shadowRoot)) {
- moveNodeToNewDocument(*node, oldDocument, newDocument);
- if (auto* shadow = node->shadowRoot())
- moveShadowTreeToNewDocument(*shadow, oldDocument, newDocument);
+ for (Node* node = &root; node; node = NodeTraversal::next(*node, &root)) {
+ moveNode(*node);
+
+ if (!is<Element>(*node))
+ continue;
+ Element& element = downcast<Element>(*node);
+
+ if (element.hasSyntheticAttrChildNodes()) {
+ for (auto& attr : element.attrNodeList())
+ moveNode(*attr);
+ }
+
+ if (auto* shadow = element.shadowRoot())
+ moveShadowRoot(*shadow);
}
}
+static void moveShadowTreeToNewDocument(ShadowRoot& shadowRoot, Document& oldDocument, Document& newDocument)
+{
+ traverseSubtreeToUpdateTreeScope(shadowRoot, [&oldDocument, &newDocument](Node& node) {
+ moveNodeToNewDocument(node, oldDocument, newDocument);
+ }, [&oldDocument, &newDocument](ShadowRoot& innerShadowRoot) {
+ ASSERT_WITH_SECURITY_IMPLICATION(&innerShadowRoot.document() == &oldDocument);
+ moveShadowTreeToNewDocument(innerShadowRoot, oldDocument, newDocument);
+ });
+}
+
void Node::moveTreeToNewScope(Node& root, TreeScope& oldScope, TreeScope& newScope)
{
ASSERT(&oldScope != &newScope);
ASSERT_WITH_SECURITY_IMPLICATION(&root.treeScope() == &oldScope);
- // If an element is moved from a document and then eventually back again the collection cache for
- // that element may contain stale data as changes made to it will have updated the DOMTreeVersion
- // of the document it was moved to. By increasing the DOMTreeVersion of the donating document here
- // we ensure that the collection cache will be invalidated as needed when the element is moved back.
Document& oldDocument = oldScope.documentScope();
Document& newDocument = newScope.documentScope();
- bool shouldUpdateDocumentScope = &oldDocument != &newDocument;
- if (shouldUpdateDocumentScope) {
+ if (&oldDocument != &newDocument) {
oldDocument.incrementReferencingNodeCount();
- oldDocument.incDOMTreeVersion();
- }
-
- for (Node* node = &root; node; node = NodeTraversal::next(*node, &root)) {
- ASSERT(!node->isTreeScope());
- ASSERT(&node->treeScope() == &oldScope);
- node->setTreeScope(newScope);
-
- if (shouldUpdateDocumentScope)
- moveNodeToNewDocument(*node, oldDocument, newDocument);
- else if (node->hasRareData()) {
- if (auto* nodeLists = node->rareData()->nodeLists())
+ traverseSubtreeToUpdateTreeScope(root, [&](Node& node) {
+ ASSERT(!node.isTreeScope());
+ ASSERT_WITH_SECURITY_IMPLICATION(&node.treeScope() == &oldScope);
+ ASSERT_WITH_SECURITY_IMPLICATION(&node.document() == &oldDocument);
+ node.setTreeScope(newScope);
+ moveNodeToNewDocument(node, oldDocument, newDocument);
+ }, [&](ShadowRoot& shadowRoot) {
+ ASSERT_WITH_SECURITY_IMPLICATION(&shadowRoot.document() == &oldDocument);
+ shadowRoot.setParentTreeScope(newScope);
+ moveShadowTreeToNewDocument(shadowRoot, oldDocument, newDocument);
+ });
+ oldDocument.decrementReferencingNodeCount();
+ } else {
+ traverseSubtreeToUpdateTreeScope(root, [&](Node& node) {
+ ASSERT(!node.isTreeScope());
+ ASSERT_WITH_SECURITY_IMPLICATION(&node.treeScope() == &oldScope);
+ node.setTreeScope(newScope);
+ if (!node.hasRareData())
+ return;
+ if (auto* nodeLists = node.rareData()->nodeLists())
nodeLists->adoptTreeScope();
- }
-
- if (!is<Element>(*node))
- continue;
- Element& element = downcast<Element>(*node);
-
- if (element.hasSyntheticAttrChildNodes()) {
- for (auto& attr : element.attrNodeList())
- moveTreeToNewScope(*attr, oldScope, newScope);
- }
-
- if (auto* shadow = element.shadowRoot()) {
- ASSERT_WITH_SECURITY_IMPLICATION(&shadow->document() == &oldDocument);
- shadow->setParentTreeScope(newScope);
- if (shouldUpdateDocumentScope)
- moveShadowTreeToNewDocument(*shadow, oldDocument, newDocument);
- }
+ }, [&newScope](ShadowRoot& shadowRoot) {
+ shadowRoot.setParentTreeScope(newScope);
+ });
}
-
- if (shouldUpdateDocumentScope)
- oldDocument.decrementReferencingNodeCount();
}
void Node::didMoveToNewDocument(Document& oldDocument, Document& newDocument)
Modified: trunk/Source/WebCore/testing/Internals.cpp (218082 => 218083)
--- trunk/Source/WebCore/testing/Internals.cpp 2017-06-12 02:40:43 UTC (rev 218082)
+++ trunk/Source/WebCore/testing/Internals.cpp 2017-06-12 03:46:47 UTC (rev 218083)
@@ -2195,6 +2195,11 @@
return Document::allDocuments().size();
}
+unsigned Internals::referencingNodeCount(const Document& document) const
+{
+ return document.referencingNodeCount();
+}
+
RefPtr<DOMWindow> Internals::openDummyInspectorFrontend(const String& url)
{
auto* inspectedPage = contextDocument()->frame()->page();
Modified: trunk/Source/WebCore/testing/Internals.h (218082 => 218083)
--- trunk/Source/WebCore/testing/Internals.h 2017-06-12 02:40:43 UTC (rev 218082)
+++ trunk/Source/WebCore/testing/Internals.h 2017-06-12 03:46:47 UTC (rev 218083)
@@ -325,6 +325,7 @@
unsigned numberOfLiveNodes() const;
unsigned numberOfLiveDocuments() const;
+ unsigned referencingNodeCount(const Document&) const;
RefPtr<DOMWindow> openDummyInspectorFrontend(const String& url);
void closeDummyInspectorFrontend();
Modified: trunk/Source/WebCore/testing/Internals.idl (218082 => 218083)
--- trunk/Source/WebCore/testing/Internals.idl 2017-06-12 02:40:43 UTC (rev 218082)
+++ trunk/Source/WebCore/testing/Internals.idl 2017-06-12 03:46:47 UTC (rev 218083)
@@ -305,6 +305,7 @@
unsigned long numberOfLiveNodes();
unsigned long numberOfLiveDocuments();
+ unsigned long referencingNodeCount(Document document);
DOMWindow? openDummyInspectorFrontend(DOMString url);
void closeDummyInspectorFrontend();
[MayThrowException] void setInspectorIsUnderTest(boolean isUnderTest);
