Title: [235863] trunk/Source/WebCore
- Revision
- 235863
- Author
- [email protected]
- Date
- 2018-09-10 14:42:45 -0700 (Mon, 10 Sep 2018)
Log Message
Many textarea tests leak documents because Document::removeFocusNavigationNodeOfSubtree() can trigger a Document retain cycle
https://bugs.webkit.org/show_bug.cgi?id=188722
Reviewed by Ryosuke Niwa.
Fix a retain cycle created when Document::adjustFocusNavigationNodeOnNodeRemoval() sets
m_focusNavigationStartingNode to itself. m_focusNavigationStartingNode is a Node* (not sure why it's not an Element*),
making it possible to assign the Document to it, which creates a reference to the document which prevents
Document::removedLastRef() ever running and doing the necessary cleanup.
Fix by setting m_focusNavigationStartingNode to null if code tries to set it to the Document. This can happen
when an element is focused and the page calls document.write(), which removes all children.
Will be tested by future leak testing. Fixes the document leak in at least the following tests:
fast/forms/append-children-during-form-submission.html
fast/forms/empty-textarea-toggle-disabled.html
fast/forms/textarea-paste-newline.html
fast/forms/textarea-trailing-newline.html
* dom/Document.cpp:
(WebCore::Document::setFocusNavigationStartingNode):
(WebCore::Document::adjustFocusNavigationNodeOnNodeRemoval):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (235862 => 235863)
--- trunk/Source/WebCore/ChangeLog 2018-09-10 21:19:49 UTC (rev 235862)
+++ trunk/Source/WebCore/ChangeLog 2018-09-10 21:42:45 UTC (rev 235863)
@@ -1,5 +1,30 @@
2018-09-10 Simon Fraser <[email protected]>
+ Many textarea tests leak documents because Document::removeFocusNavigationNodeOfSubtree() can trigger a Document retain cycle
+ https://bugs.webkit.org/show_bug.cgi?id=188722
+
+ Reviewed by Ryosuke Niwa.
+
+ Fix a retain cycle created when Document::adjustFocusNavigationNodeOnNodeRemoval() sets
+ m_focusNavigationStartingNode to itself. m_focusNavigationStartingNode is a Node* (not sure why it's not an Element*),
+ making it possible to assign the Document to it, which creates a reference to the document which prevents
+ Document::removedLastRef() ever running and doing the necessary cleanup.
+
+ Fix by setting m_focusNavigationStartingNode to null if code tries to set it to the Document. This can happen
+ when an element is focused and the page calls document.write(), which removes all children.
+
+ Will be tested by future leak testing. Fixes the document leak in at least the following tests:
+ fast/forms/append-children-during-form-submission.html
+ fast/forms/empty-textarea-toggle-disabled.html
+ fast/forms/textarea-paste-newline.html
+ fast/forms/textarea-trailing-newline.html
+
+ * dom/Document.cpp:
+ (WebCore::Document::setFocusNavigationStartingNode):
+ (WebCore::Document::adjustFocusNavigationNodeOnNodeRemoval):
+
+2018-09-10 Simon Fraser <[email protected]>
+
svg/W3C-SVG-1.1/render-groups-03-t.svg and some other SVG tests leak documents
https://bugs.webkit.org/show_bug.cgi?id=189147
Modified: trunk/Source/WebCore/dom/Document.cpp (235862 => 235863)
--- trunk/Source/WebCore/dom/Document.cpp 2018-09-10 21:19:49 UTC (rev 235862)
+++ trunk/Source/WebCore/dom/Document.cpp 2018-09-10 21:42:45 UTC (rev 235863)
@@ -4074,6 +4074,7 @@
return;
}
+ ASSERT(!node || node != this);
m_focusNavigationStartingNode = node;
}
@@ -4271,7 +4272,8 @@
return;
if (isNodeInSubtree(*m_focusNavigationStartingNode, node, nodeRemoval)) {
- m_focusNavigationStartingNode = (nodeRemoval == NodeRemoval::ChildrenOfNode) ? &node : fallbackFocusNavigationStartingNodeAfterRemoval(node);
+ auto* newNode = (nodeRemoval == NodeRemoval::ChildrenOfNode) ? &node : fallbackFocusNavigationStartingNodeAfterRemoval(node);
+ m_focusNavigationStartingNode = (newNode != this) ? newNode : nullptr;
m_focusNavigationStartingNodeIsRemoved = true;
}
}
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
