Bug 188722

Summary: Many textarea tests leak documents because Document::removeFocusNavigationNodeOfSubtree() can trigger a Document retain cycle
Product: WebKit Reporter: Simon Fraser (smfr) <simon.fraser>
Component: HTML EditingAssignee: Simon Fraser (smfr) <simon.fraser>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, cdumez, dbates, esprehn+autocc, ews-watchlist, kangil.han, n_wang, rniwa, sam, simon.fraser, webkit-bug-importer, wenson_hsieh
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch rniwa: review+

Simon Fraser (smfr)
Reported 2018-08-18 12:56:54 PDT
Various textarea tests cause abandoned documents. I looked at fast/forms/textarea-paste-newline.html. Under this stack: * frame #0: 0x000000061788cc74 WebCore`WebCore::Document::removeFocusNavigationNodeOfSubtree(this=0x0000000632f01200, node=0x0000000632f01200, amongChildrenOnly=true) at Document.cpp:4230 frame #1: 0x000000061788c82a WebCore`WebCore::Document::nodeChildrenWillBeRemoved(this=0x0000000632f01200, container=0x0000000632f01200) at Document.cpp:4164 frame #2: 0x00000006178252de WebCore`WebCore::ContainerNode::removeAllChildrenWithScriptAssertion(this=0x0000000632f01200, source=API, deferChildrenChanged=No) at ContainerNode.cpp:107 frame #3: 0x0000000617828d49 WebCore`WebCore::ContainerNode::removeChildren(this=0x0000000632f01200) at ContainerNode.cpp:658 frame #4: 0x0000000617882aef WebCore`WebCore::Document::implicitOpen(this=0x0000000632f01200) at Document.cpp:2691 frame #5: 0x0000000617878043 WebCore`WebCore::Document::open(this=0x0000000632f01200, responsibleDocument=0x0000000632f01200) at Document.cpp:2660 frame #6: 0x0000000617884062 WebCore`WebCore::Document::write(this=0x0000000632f01200, responsibleDocument=0x0000000632f01200, text=0x00007ffee6a2fbb8) at Document.cpp:2984 frame #7: 0x000000061788432b WebCore`WebCore::Document::write(this=0x0000000632f01200, responsibleDocument=0x0000000632f01200, strings={ size = 1, capacity = 1 }) at Document.cpp:2999 frame #8: 0x000000061609d242 WebCore`WebCore::jsDocumentPrototypeFunctionWriteBody(state=0x00007ffee6a2fe30, castedThis=0x000000062d063ea0, throwScope=0x00007ffee6a2fdb8) at JSDocument.cpp:4890 removeFocusNavigationNodeOfSubtree() is called with node == |this|, so the document stores a pointer to itself in a RefPtr, and this is never cleared.
Attachments
Patch (2.86 KB, patch)
2018-09-08 14:06 PDT, Simon Fraser (smfr) 		rniwa: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Simon Fraser (smfr)
Comment 1 2018-08-18 13:17:06 PDT
Also affects dom/html/level2/html/HTMLAnchorElement14.html and probably others.
Radar WebKit Bug Importer
Comment 2 2018-09-08 08:09:45 PDT
<rdar://problem/44258638>
Simon Fraser (smfr)
Comment 3 2018-09-08 14:06:27 PDT
Created attachment 349260 [details] Patch
Ryosuke Niwa
Comment 4 2018-09-08 15:54:09 PDT
Comment on attachment 349260 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=349260&action=review > Source/WebCore/dom/Document.cpp:4275 > + m_focusNavigationStartingNode = newNode; I would have preferred to use a ternary operator here instwad
Simon Fraser (smfr)
Comment 5 2018-09-10 14:43:02 PDT
https://trac.webkit.org/r235863
Note You need to log in before you can comment on or make changes to this bug.