Attachment 349260 Details for Bug 188722 – Patch

[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:

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;
>     }
> }

Flags:

rniwa: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 188722: 349260