Merge r185838 - REGRESSION(r169105) Dangling renderer pointer in Sele…

…ctionSubtreeRoot::SelectionSubtreeData. https://bugs.webkit.org/show_bug.cgi?id=146116 rdar://problem/20959369 Reviewed by Brent Fulgham. This patch ensures that we don't adjust the selection unless the visual selection still matches this subtree root. When multiple selection roots are present we need to ensure that a RenderObject only shows up in one of them. RenderView::splitSelectionBetweenSubtrees(), as the name implies, splits the selection and sets the selection range (start/end) on each selection root. However, SelectionSubtreeRoot::adjustForVisibleSelection() later recomputes the range based on visible selection and that could end up collecting renderers as selection start/end from another selection subtree. RenderObject's holds the last selection state (RenderObject::setSelectionState). If we set a renderer first as "on selection border" and later "inside" using multiple selection roots, we can't clean up selections properly when this object gets destroyed. One of the roots ends up with a dangling RenderObject pointer. Source/WebCore: Test: fast/regions/crash-when-renderer-is-in-multiple-selection-subtrees.html * rendering/SelectionSubtreeRoot.cpp: (WebCore::SelectionSubtreeRoot::adjustForVisibleSelection): LayoutTests: * fast/regions/crash-when-renderer-is-in-multiple-selection-subtrees-expected.txt: Added. * fast/regions/crash-when-renderer-is-in-multiple-selection-subtrees.html: Added.
WebKit · Jul 7, 2015 · dab754b · dab754b
1 parent 3d4c91b
commit dab754b
Show file tree

Hide file tree

Showing 5 changed files with 91 additions and 9 deletions.
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
@@ -1,3 +1,28 @@
+2015-06-22  Zalan Bujtas  <zalan@apple.com>
+
+        REGRESSION(r169105) Dangling renderer pointer in SelectionSubtreeRoot::SelectionSubtreeData.
+        https://bugs.webkit.org/show_bug.cgi?id=146116
+        rdar://problem/20959369
+
+        Reviewed by Brent Fulgham.
+
+        This patch ensures that we don't adjust the selection unless the visual selection still matches this subtree root.
+
+        When multiple selection roots are present we need to ensure that a RenderObject
+        only shows up in one of them.
+        RenderView::splitSelectionBetweenSubtrees(), as the name implies, splits the
+        selection and sets the selection range (start/end) on each selection root.
+        However, SelectionSubtreeRoot::adjustForVisibleSelection() later recomputes the range
+        based on visible selection and that could end up collecting renderers as selection start/end
+        from another selection subtree.
+        RenderObject's holds the last selection state (RenderObject::setSelectionState).
+        If we set a renderer first as "on selection border" and later "inside" using multiple selection roots,
+        we can't clean up selections properly when this object gets destroyed.
+        One of the roots ends up with a dangling RenderObject pointer.
+
+        * fast/regions/crash-when-renderer-is-in-multiple-selection-subtrees-expected.txt: Added.
+        * fast/regions/crash-when-renderer-is-in-multiple-selection-subtrees.html: Added.
+
 2015-06-19  Andy Estes  <aestes@apple.com>
 
         Various assertion failures occur when executing script in the midst of DOM insertion

diff --git a/LayoutTests/fast/regions/crash-when-renderer-is-in-multiple-selection-subtrees-expected.txt b/LayoutTests/fast/regions/crash-when-renderer-is-in-multiple-selection-subtrees-expected.txt
@@ -0,0 +1,4 @@
+foo
+Pass if no crash or assert in debug.
+foobar
+
diff --git a/LayoutTests/fast/regions/crash-when-renderer-is-in-multiple-selection-subtrees.html b/LayoutTests/fast/regions/crash-when-renderer-is-in-multiple-selection-subtrees.html
@@ -0,0 +1,19 @@
+<html id="webtest0">
+<head>
+<style>
+	:last-child { -webkit-flow-into: foo; }
+</style>
+
+<script>
+    if (window.testRunner)
+        testRunner.dumpAsText();
+    document.write("foo<br>");
+</script>
+</head>
+<body>Pass if no crash or assert in debug.<summary id="webtest5"><mathml><femerge></femerge></mathml>foobar<table></table></summary></body>
+<script>
+document.querySelector("#webtest0").appendChild(document.createElement("canvas"));
+document.execCommand("SelectAll");
+document.getElementById("webtest5").appendChild(document.createElement("feconvolvematrix"));
+</script>
+</html>
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -1,3 +1,30 @@
+2015-06-22  Zalan Bujtas  <zalan@apple.com>
+
+        REGRESSION(r169105) Dangling renderer pointer in SelectionSubtreeRoot::SelectionSubtreeData.
+        https://bugs.webkit.org/show_bug.cgi?id=146116
+        rdar://problem/20959369
+
+        Reviewed by Brent Fulgham.
+
+        This patch ensures that we don't adjust the selection unless the visual selection still matches this subtree root.
+
+        When multiple selection roots are present we need to ensure that a RenderObject
+        only shows up in one of them.
+        RenderView::splitSelectionBetweenSubtrees(), as the name implies, splits the
+        selection and sets the selection range (start/end) on each selection root.
+        However, SelectionSubtreeRoot::adjustForVisibleSelection() later recomputes the range
+        based on visible selection and that could end up collecting renderers as selection start/end
+        from another selection subtree.
+        RenderObject's holds the last selection state (RenderObject::setSelectionState).
+        If we set a renderer first as "on selection border" and later "inside" using multiple selection roots,
+        we can't clean up selections properly when this object gets destroyed.
+        One of the roots ends up with a dangling RenderObject pointer.
+
+        Test: fast/regions/crash-when-renderer-is-in-multiple-selection-subtrees.html
+
+        * rendering/SelectionSubtreeRoot.cpp:
+        (WebCore::SelectionSubtreeRoot::adjustForVisibleSelection):
+
 2015-06-19  Joseph Pecoraro  <pecoraro@apple.com>
 
         Crash under WebCore::PageConsoleClient::addMessage attempting to log insecure content message in ImageDocument

diff --git a/Source/WebCore/rendering/SelectionSubtreeRoot.cpp b/Source/WebCore/rendering/SelectionSubtreeRoot.cpp
@@ -69,15 +69,22 @@ void SelectionSubtreeRoot::adjustForVisibleSelection(Document& document)
 
     m_selectionSubtreeData.clearSelection();
 
-    if (startPos.isNotNull()
-        && endPos.isNotNull()
-        && selection.visibleStart() != selection.visibleEnd()
-        && startPos.deprecatedNode()->renderer()->flowThreadContainingBlock() == endPos.deprecatedNode()->renderer()->flowThreadContainingBlock()) {
-        m_selectionSubtreeData.setSelectionStart(startPos.deprecatedNode()->renderer());
-        m_selectionSubtreeData.setSelectionStartPos(startPos.deprecatedEditingOffset());
-        m_selectionSubtreeData.setSelectionEnd(endPos.deprecatedNode()->renderer());
-        m_selectionSubtreeData.setSelectionEndPos(endPos.deprecatedEditingOffset());
-    }
+    if (startPos.isNull() || endPos.isNull())
+        return;
+
+    if (selection.visibleStart() == selection.visibleEnd())
+        return;
+
+    if (startPos.deprecatedNode()->renderer()->flowThreadContainingBlock() != endPos.deprecatedNode()->renderer()->flowThreadContainingBlock())
+        return;
+
+    if (&startPos.deprecatedNode()->renderer()->selectionRoot() != this)
+        return;
+
+    m_selectionSubtreeData.setSelectionStart(startPos.deprecatedNode()->renderer());
+    m_selectionSubtreeData.setSelectionStartPos(startPos.deprecatedEditingOffset());
+    m_selectionSubtreeData.setSelectionEnd(endPos.deprecatedNode()->renderer());
+    m_selectionSubtreeData.setSelectionEndPos(endPos.deprecatedEditingOffset());
 }
 
 } // namespace WebCore