WebKit Bugzilla
Attachment 349260 Details for
Bug 188722
: Many textarea tests leak documents because Document::removeFocusNavigationNodeOfSubtree() can trigger a Document retain cycle
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[patch]
Patch
bug-188722-20180908140626.patch (text/plain), 2.86 KB, created by
Simon Fraser (smfr)
on 2018-09-08 14:06:27 PDT
(
hide
)
Description:
Patch
Filename:
MIME Type:
Creator:
Simon Fraser (smfr)
Created:
2018-09-08 14:06:27 PDT
Size:
2.86 KB
patch
obsolete
>Subversion Revision: 235830 >diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog >index c5a3807f212de07aa798ee53f13972d1117d6035..15d0f9fb14b51e38f7890ba7a050b4999c7a4636 100644 >--- a/Source/WebCore/ChangeLog >+++ b/Source/WebCore/ChangeLog >@@ -1,3 +1,28 @@ >+2018-09-08 Simon Fraser <simon.fraser@apple.com> >+ >+ 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 NOBODY (OOPS!). >+ >+ 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-08 Simon Fraser <simon.fraser@apple.com> > > Clean up code related to Document node removal >diff --git a/Source/WebCore/dom/Document.cpp b/Source/WebCore/dom/Document.cpp >index 609892bdc943ee5eafededc39b5cf3d5ab340ac5..85e94a3bbd03cecfc60630b0892917bd43c20afd 100644 >--- a/Source/WebCore/dom/Document.cpp >+++ b/Source/WebCore/dom/Document.cpp >@@ -4071,6 +4071,7 @@ void Document::setFocusNavigationStartingNode(Node* node) > return; > } > >+ ASSERT(!node || node != this); > m_focusNavigationStartingNode = node; > } > >@@ -4268,7 +4269,10 @@ void Document::adjustFocusNavigationNodeOnNodeRemoval(Node& node, NodeRemoval no > return; > > if (isNodeInSubtree(*m_focusNavigationStartingNode, node, nodeRemoval)) { >- m_focusNavigationStartingNode = (nodeRemoval == NodeRemoval::ChildrenOfNode) ? &node : fallbackFocusNavigationStartingNodeAfterRemoval(node); >+ auto* newNode = (nodeRemoval == NodeRemoval::ChildrenOfNode) ? &node : fallbackFocusNavigationStartingNodeAfterRemoval(node); >+ if (newNode == this) >+ newNode = nullptr; >+ m_focusNavigationStartingNode = newNode; > m_focusNavigationStartingNodeIsRemoved = true; > } > }
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Flags:
rniwa
:
review+
Actions:
View
|
Formatted Diff
|
Diff
Attachments on
bug 188722
: 349260