Bug 188722 - Many textarea tests leak documents because Document::removeFocusNavigationNodeOfSubtree() can trigger a Document retain cycle
Summary: Many textarea tests leak documents because Document::removeFocusNavigationNod...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: HTML Editing (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Simon Fraser (smfr)
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-08-18 12:56 PDT by Simon Fraser (smfr)
Modified: 2018-09-10 14:43 PDT (History)
12 users (show)

See Also:


Attachments
Patch (2.86 KB, patch)
2018-09-08 14:06 PDT, Simon Fraser (smfr)
rniwa: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Fraser (smfr) 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.
Comment 1 Simon Fraser (smfr) 2018-08-18 13:17:06 PDT
Also affects dom/html/level2/html/HTMLAnchorElement14.html and probably others.
Comment 2 Radar WebKit Bug Importer 2018-09-08 08:09:45 PDT
<rdar://problem/44258638>
Comment 3 Simon Fraser (smfr) 2018-09-08 14:06:27 PDT
Created attachment 349260 [details]
Patch
Comment 4 Ryosuke Niwa 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
Comment 5 Simon Fraser (smfr) 2018-09-10 14:43:02 PDT
https://trac.webkit.org/r235863