diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog index cc939e140777..7a7c11361f21 100644 --- a/LayoutTests/ChangeLog +++ b/LayoutTests/ChangeLog @@ -1,3 +1,28 @@ +2015-06-22 Zalan Bujtas + + 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 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 new file mode 100644 index 000000000000..4ee3fc88af2b --- /dev/null +++ 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 new file mode 100644 index 000000000000..cf6c3997d684 --- /dev/null +++ b/LayoutTests/fast/regions/crash-when-renderer-is-in-multiple-selection-subtrees.html @@ -0,0 +1,19 @@ + + + + + + +Pass if no crash or assert in debug.foobar
 + + diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog index 63966fc5036d..27814b626848 100644 --- a/Source/WebCore/ChangeLog +++ b/Source/WebCore/ChangeLog @@ -1,3 +1,30 @@ +2015-06-22 Zalan Bujtas + + 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 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 index 8b6b2e611232..6ef069647278 100644 --- 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