Changeset 185838 in webkit

Timestamp:

Jun 22, 2015, 12:35:19 PM (11 years ago)

Author:

Alan Bujtas

Message:

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.

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.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/regions/crash-when-renderer-is-in-multiple-selection-subtrees-expected.txt (added)
• LayoutTests/fast/regions/crash-when-renderer-is-in-multiple-selection-subtrees.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/rendering/SelectionSubtreeRoot.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+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-22  Daniel Bates  <dabates@apple.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+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-22  Jeremy Jones  <jeremyj@apple.com>
 

trunk/Source/WebCore/rendering/SelectionSubtreeRoot.cpp

@@ -71 +71 @@
     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());
 }