- Revision
- 110323
- Author
- [email protected]
- Date
- 2012-03-09 13:12:56 -0800 (Fri, 09 Mar 2012)
Log Message
Crash due to accessing removed parent lineboxes when clearing selection.
https://bugs.webkit.org/show_bug.cgi?id=79264
Reviewed by Dave Hyatt.
Source/WebCore:
Test: editing/selection/first-letter-selection-crash.html
* rendering/RenderBoxModelObject.cpp:
(WebCore::RenderBoxModelObject::setSelectionState):
1. No need of checking if we are being set to same selection state.
Now tested by setSelectionStateIfNeeded. Rename 's' with 'state' and
'cb' with 'containingBlock'.
* rendering/RenderListMarker.cpp:
(WebCore::RenderListMarker::setSelectionState):
1. Add check to canUpdateSelectionOnRootLineBoxes to make sure our
root line boxes are still valid before setting them.
2. No need of calling setSelectionState on containing block since our base
class call to RenderBox::setSelectionState covers it. Added a comment to indicate that.
3. Use m_inlineBoxWrapper variable directly to simplify the ifs.
* rendering/RenderObject.cpp:
(WebCore::RenderObject::canUpdateSelectionOnRootLineBoxes):
(WebCore): helper function to tell if we can update selection information
on our root line boxes. This returns false if our containing block is pending layout.
* rendering/RenderObject.h:
(RenderObject):
(WebCore::RenderObject::setSelectionStateIfNeeded):
(WebCore): helper to set selection state only if it is different from our
current selection state.
* rendering/RenderReplaced.cpp:
(WebCore::RenderReplaced::setSelectionState):
1. Rename 's' to 'state', 'line' to 'root' and use m_inlineBoxWrapper directly
to simplify ifs.
2. Add check to canUpdateSelectionOnRootLineBoxes to make sure our
root line boxes are still valid before setting them.
* rendering/RenderText.cpp:
(WebCore::RenderText::setSelectionState):
1. Add check to canUpdateSelectionOnRootLineBoxes to make sure our
root line boxes are still valid before setting them.
2. Rename 'cb' to 'containingBlock', 'line' to 'root', move InlineTextBox
declaration to local.
* rendering/RenderView.cpp:
(WebCore::RenderView::setSelection): Replace all calls to setSelectionState
with setSelectionStateIfNeeded.
* rendering/RenderWidget.cpp:
(WebCore::RenderWidget::setSelectionState):
1. No need of checking if we are being set to same selection state.
Now tested by setSelectionStateIfNeeded.
LayoutTests:
* editing/selection/first-letter-selection-crash-expected.txt: Added.
* editing/selection/first-letter-selection-crash.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (110322 => 110323)
--- trunk/LayoutTests/ChangeLog 2012-03-09 21:04:26 UTC (rev 110322)
+++ trunk/LayoutTests/ChangeLog 2012-03-09 21:12:56 UTC (rev 110323)
@@ -1,3 +1,13 @@
+2012-03-09 Abhishek Arya <[email protected]>
+
+ Crash due to accessing removed parent lineboxes when clearing selection.
+ https://bugs.webkit.org/show_bug.cgi?id=79264
+
+ Reviewed by Dave Hyatt.
+
+ * editing/selection/first-letter-selection-crash-expected.txt: Added.
+ * editing/selection/first-letter-selection-crash.html: Added.
+
2012-03-08 Erik Arvidsson <[email protected]>
[V8] Fix object scope for inline event attribute handlers
Added: trunk/LayoutTests/editing/selection/first-letter-selection-crash-expected.txt (0 => 110323)
--- trunk/LayoutTests/editing/selection/first-letter-selection-crash-expected.txt (rev 0)
+++ trunk/LayoutTests/editing/selection/first-letter-selection-crash-expected.txt 2012-03-09 21:12:56 UTC (rev 110323)
@@ -0,0 +1 @@
+PASS
Added: trunk/LayoutTests/editing/selection/first-letter-selection-crash.html (0 => 110323)
--- trunk/LayoutTests/editing/selection/first-letter-selection-crash.html (rev 0)
+++ trunk/LayoutTests/editing/selection/first-letter-selection-crash.html 2012-03-09 21:12:56 UTC (rev 110323)
@@ -0,0 +1,28 @@
+<!DOCTYPE html>
+<html>
+<body>
+<style>
+#test2:first-letter { display: block; }
+</style>
+<script>
+if (window.layoutTestController)
+ layoutTestController.dumpAsText();
+
+window._onload_ = function() {
+ test1 = document.createElement('div');
+ document.body.appendChild(test1);
+ test2 = document.createElement('div');
+ test2.setAttribute('id', 'test2');
+ test2.appendChild(document.createTextNode('aaa'));
+ test2.style.display = 'inline-block';
+ test1.appendChild(test2);
+ test1.appendChild(document.createTextNode('a'));
+ document.execCommand('selectall');
+ document.body.offsetTop;
+ document.styleSheets[0].insertRule("#test2 { text-transform: uppercase }");
+ document.body.offsetTop;
+ document.body.innerHTML = "PASS";
+}
+</script>
+</body>
+</html>
Property changes on: trunk/LayoutTests/editing/selection/first-letter-selection-crash.html
___________________________________________________________________
Added: svn:executable
Modified: trunk/Source/WebCore/ChangeLog (110322 => 110323)
--- trunk/Source/WebCore/ChangeLog 2012-03-09 21:04:26 UTC (rev 110322)
+++ trunk/Source/WebCore/ChangeLog 2012-03-09 21:12:56 UTC (rev 110323)
@@ -1,3 +1,53 @@
+2012-03-09 Abhishek Arya <[email protected]>
+
+ Crash due to accessing removed parent lineboxes when clearing selection.
+ https://bugs.webkit.org/show_bug.cgi?id=79264
+
+ Reviewed by Dave Hyatt.
+
+ Test: editing/selection/first-letter-selection-crash.html
+
+ * rendering/RenderBoxModelObject.cpp:
+ (WebCore::RenderBoxModelObject::setSelectionState):
+ 1. No need of checking if we are being set to same selection state.
+ Now tested by setSelectionStateIfNeeded. Rename 's' with 'state' and
+ 'cb' with 'containingBlock'.
+ * rendering/RenderListMarker.cpp:
+ (WebCore::RenderListMarker::setSelectionState):
+ 1. Add check to canUpdateSelectionOnRootLineBoxes to make sure our
+ root line boxes are still valid before setting them.
+ 2. No need of calling setSelectionState on containing block since our base
+ class call to RenderBox::setSelectionState covers it. Added a comment to indicate that.
+ 3. Use m_inlineBoxWrapper variable directly to simplify the ifs.
+ * rendering/RenderObject.cpp:
+ (WebCore::RenderObject::canUpdateSelectionOnRootLineBoxes):
+ (WebCore): helper function to tell if we can update selection information
+ on our root line boxes. This returns false if our containing block is pending layout.
+ * rendering/RenderObject.h:
+ (RenderObject):
+ (WebCore::RenderObject::setSelectionStateIfNeeded):
+ (WebCore): helper to set selection state only if it is different from our
+ current selection state.
+ * rendering/RenderReplaced.cpp:
+ (WebCore::RenderReplaced::setSelectionState):
+ 1. Rename 's' to 'state', 'line' to 'root' and use m_inlineBoxWrapper directly
+ to simplify ifs.
+ 2. Add check to canUpdateSelectionOnRootLineBoxes to make sure our
+ root line boxes are still valid before setting them.
+ * rendering/RenderText.cpp:
+ (WebCore::RenderText::setSelectionState):
+ 1. Add check to canUpdateSelectionOnRootLineBoxes to make sure our
+ root line boxes are still valid before setting them.
+ 2. Rename 'cb' to 'containingBlock', 'line' to 'root', move InlineTextBox
+ declaration to local.
+ * rendering/RenderView.cpp:
+ (WebCore::RenderView::setSelection): Replace all calls to setSelectionState
+ with setSelectionStateIfNeeded.
+ * rendering/RenderWidget.cpp:
+ (WebCore::RenderWidget::setSelectionState):
+ 1. No need of checking if we are being set to same selection state.
+ Now tested by setSelectionStateIfNeeded.
+
2012-03-09 Levi Weintraub <[email protected]>
Move TransformationMatrix and TransformState to LayoutUnits.
Modified: trunk/Source/WebCore/rendering/RenderBoxModelObject.cpp (110322 => 110323)
--- trunk/Source/WebCore/rendering/RenderBoxModelObject.cpp 2012-03-09 21:04:26 UTC (rev 110322)
+++ trunk/Source/WebCore/rendering/RenderBoxModelObject.cpp 2012-03-09 21:12:56 UTC (rev 110323)
@@ -221,26 +221,23 @@
return gImageQualityController;
}
-void RenderBoxModelObject::setSelectionState(SelectionState s)
+void RenderBoxModelObject::setSelectionState(SelectionState state)
{
- if (selectionState() == s)
+ if (state == SelectionInside && selectionState() != SelectionNone)
return;
-
- if (s == SelectionInside && selectionState() != SelectionNone)
- return;
- if ((s == SelectionStart && selectionState() == SelectionEnd)
- || (s == SelectionEnd && selectionState() == SelectionStart))
+ if ((state == SelectionStart && selectionState() == SelectionEnd)
+ || (state == SelectionEnd && selectionState() == SelectionStart))
RenderObject::setSelectionState(SelectionBoth);
else
- RenderObject::setSelectionState(s);
-
- // FIXME:
- // We should consider whether it is OK propagating to ancestor RenderInlines.
+ RenderObject::setSelectionState(state);
+
+ // FIXME: We should consider whether it is OK propagating to ancestor RenderInlines.
// This is a workaround for http://webkit.org/b/32123
- RenderBlock* cb = containingBlock();
- if (cb && !cb->isRenderView())
- cb->setSelectionState(s);
+ // The containing block can be null in case of an orphaned tree.
+ RenderBlock* containingBlock = this->containingBlock();
+ if (containingBlock && !containingBlock->isRenderView())
+ containingBlock->setSelectionState(state);
}
bool RenderBoxModelObject::shouldPaintAtLowQuality(GraphicsContext* context, Image* image, const void* layer, const LayoutSize& size)
Modified: trunk/Source/WebCore/rendering/RenderListMarker.cpp (110322 => 110323)
--- trunk/Source/WebCore/rendering/RenderListMarker.cpp 2012-03-09 21:04:26 UTC (rev 110322)
+++ trunk/Source/WebCore/rendering/RenderListMarker.cpp 2012-03-09 21:12:56 UTC (rev 110323)
@@ -1694,11 +1694,12 @@
void RenderListMarker::setSelectionState(SelectionState state)
{
+ // The selection state for our containing block hierarchy is updated by the base class call.
RenderBox::setSelectionState(state);
- if (InlineBox* box = inlineBoxWrapper())
- if (RootInlineBox* root = box->root())
+
+ if (m_inlineBoxWrapper && canUpdateSelectionOnRootLineBoxes())
+ if (RootInlineBox* root = m_inlineBoxWrapper->root())
root->setHasSelectedChildren(state != SelectionNone);
- containingBlock()->setSelectionState(state);
}
LayoutRect RenderListMarker::selectionRectForRepaint(RenderBoxModelObject* repaintContainer, bool clipToVisibleContent)
Modified: trunk/Source/WebCore/rendering/RenderObject.cpp (110322 => 110323)
--- trunk/Source/WebCore/rendering/RenderObject.cpp 2012-03-09 21:04:26 UTC (rev 110322)
+++ trunk/Source/WebCore/rendering/RenderObject.cpp 2012-03-09 21:12:56 UTC (rev 110323)
@@ -2826,6 +2826,12 @@
return SetCursorBasedOnStyle;
}
+bool RenderObject::canUpdateSelectionOnRootLineBoxes()
+{
+ RenderBlock* containingBlock = this->containingBlock();
+ return containingBlock ? !containingBlock->needsLayout() : true;
+}
+
#if ENABLE(SVG)
RenderSVGResourceContainer* RenderObject::toRenderSVGResourceContainer()
Modified: trunk/Source/WebCore/rendering/RenderObject.h (110322 => 110323)
--- trunk/Source/WebCore/rendering/RenderObject.h 2012-03-09 21:04:26 UTC (rev 110322)
+++ trunk/Source/WebCore/rendering/RenderObject.h 2012-03-09 21:12:56 UTC (rev 110323)
@@ -758,6 +758,8 @@
// descendants (as described above in the SelectionState enum declaration).
SelectionState selectionState() const { return m_bitfields.selectionState(); }
virtual void setSelectionState(SelectionState state) { m_bitfields.setSelectionState(state); }
+ inline void setSelectionStateIfNeeded(SelectionState);
+ bool canUpdateSelectionOnRootLineBoxes();
// A single rectangle that encompasses all of the selected objects within this object. Used to determine the tightest
// possible bounding box for the selection.
@@ -1114,6 +1116,14 @@
return true;
}
+inline void RenderObject::setSelectionStateIfNeeded(SelectionState state)
+{
+ if (selectionState() == state)
+ return;
+
+ setSelectionState(state);
+}
+
inline void makeMatrixRenderable(TransformationMatrix& matrix, bool has3DRendering)
{
#if !ENABLE(3D_RENDERING)
Modified: trunk/Source/WebCore/rendering/RenderReplaced.cpp (110322 => 110323)
--- trunk/Source/WebCore/rendering/RenderReplaced.cpp 2012-03-09 21:04:26 UTC (rev 110322)
+++ trunk/Source/WebCore/rendering/RenderReplaced.cpp 2012-03-09 21:12:56 UTC (rev 110323)
@@ -504,14 +504,14 @@
return IntRect(newLogicalTop, 0, root->selectionHeight(), height());
}
-void RenderReplaced::setSelectionState(SelectionState s)
+void RenderReplaced::setSelectionState(SelectionState state)
{
- RenderBox::setSelectionState(s); // The selection state for our containing block hierarchy is updated by the base class call.
- if (m_inlineBoxWrapper) {
- RootInlineBox* line = m_inlineBoxWrapper->root();
- if (line)
- line->setHasSelectedChildren(isSelected());
- }
+ // The selection state for our containing block hierarchy is updated by the base class call.
+ RenderBox::setSelectionState(state);
+
+ if (m_inlineBoxWrapper && canUpdateSelectionOnRootLineBoxes())
+ if (RootInlineBox* root = m_inlineBoxWrapper->root())
+ root->setHasSelectedChildren(isSelected());
}
bool RenderReplaced::isSelected() const
Modified: trunk/Source/WebCore/rendering/RenderText.cpp (110322 => 110323)
--- trunk/Source/WebCore/rendering/RenderText.cpp 2012-03-09 21:04:26 UTC (rev 110322)
+++ trunk/Source/WebCore/rendering/RenderText.cpp 2012-03-09 21:12:56 UTC (rev 110323)
@@ -1126,39 +1126,41 @@
void RenderText::setSelectionState(SelectionState state)
{
- InlineTextBox* box;
-
RenderObject::setSelectionState(state);
- if (state == SelectionStart || state == SelectionEnd || state == SelectionBoth) {
- int startPos, endPos;
- selectionStartEnd(startPos, endPos);
- if (selectionState() == SelectionStart) {
- endPos = textLength();
- // to handle selection from end of text to end of line
- if (startPos != 0 && startPos == endPos)
- startPos = endPos - 1;
- } else if (selectionState() == SelectionEnd)
- startPos = 0;
+ if (canUpdateSelectionOnRootLineBoxes()) {
+ if (state == SelectionStart || state == SelectionEnd || state == SelectionBoth) {
+ int startPos, endPos;
+ selectionStartEnd(startPos, endPos);
+ if (selectionState() == SelectionStart) {
+ endPos = textLength();
- for (box = firstTextBox(); box; box = box->nextTextBox()) {
- if (box->isSelected(startPos, endPos)) {
- RootInlineBox* line = box->root();
- if (line)
- line->setHasSelectedChildren(true);
+ // to handle selection from end of text to end of line
+ if (startPos && startPos == endPos)
+ startPos = endPos - 1;
+ } else if (selectionState() == SelectionEnd)
+ startPos = 0;
+
+ for (InlineTextBox* box = firstTextBox(); box; box = box->nextTextBox()) {
+ if (box->isSelected(startPos, endPos)) {
+ RootInlineBox* root = box->root();
+ if (root)
+ root->setHasSelectedChildren(true);
+ }
}
+ } else {
+ for (InlineTextBox* box = firstTextBox(); box; box = box->nextTextBox()) {
+ RootInlineBox* root = box->root();
+ if (root)
+ root->setHasSelectedChildren(state == SelectionInside);
+ }
}
- } else {
- for (box = firstTextBox(); box; box = box->nextTextBox()) {
- RootInlineBox* line = box->root();
- if (line)
- line->setHasSelectedChildren(state == SelectionInside);
- }
}
- // The returned value can be null in case of an orphaned tree.
- if (RenderBlock* cb = containingBlock())
- cb->setSelectionState(state);
+ // The containing block can be null in case of an orphaned tree.
+ RenderBlock* containingBlock = this->containingBlock();
+ if (containingBlock && !containingBlock->isRenderView())
+ containingBlock->setSelectionState(state);
}
void RenderText::setTextWithOffset(PassRefPtr<StringImpl> text, unsigned offset, unsigned len, bool force)
Modified: trunk/Source/WebCore/rendering/RenderView.cpp (110322 => 110323)
--- trunk/Source/WebCore/rendering/RenderView.cpp 2012-03-09 21:04:26 UTC (rev 110322)
+++ trunk/Source/WebCore/rendering/RenderView.cpp 2012-03-09 21:12:56 UTC (rev 110323)
@@ -501,7 +501,7 @@
// Now clear the selection.
SelectedObjectMap::iterator oldObjectsEnd = oldSelectedObjects.end();
for (SelectedObjectMap::iterator i = oldSelectedObjects.begin(); i != oldObjectsEnd; ++i)
- i->first->setSelectionState(SelectionNone);
+ i->first->setSelectionStateIfNeeded(SelectionNone);
// set selection start and end
m_selectionStart = start;
@@ -511,12 +511,12 @@
// Update the selection status of all objects between m_selectionStart and m_selectionEnd
if (start && start == end)
- start->setSelectionState(SelectionBoth);
+ start->setSelectionStateIfNeeded(SelectionBoth);
else {
if (start)
- start->setSelectionState(SelectionStart);
+ start->setSelectionStateIfNeeded(SelectionStart);
if (end)
- end->setSelectionState(SelectionEnd);
+ end->setSelectionStateIfNeeded(SelectionEnd);
}
RenderObject* o = start;
@@ -524,7 +524,7 @@
while (o && o != stop) {
if (o != start && o != end && o->canBeSelectionLeaf())
- o->setSelectionState(SelectionInside);
+ o->setSelectionStateIfNeeded(SelectionInside);
o = o->nextInPreOrder();
}
Modified: trunk/Source/WebCore/rendering/RenderWidget.cpp (110322 => 110323)
--- trunk/Source/WebCore/rendering/RenderWidget.cpp 2012-03-09 21:04:26 UTC (rev 110322)
+++ trunk/Source/WebCore/rendering/RenderWidget.cpp 2012-03-09 21:12:56 UTC (rev 110323)
@@ -359,11 +359,11 @@
void RenderWidget::setSelectionState(SelectionState state)
{
- if (selectionState() != state) {
- RenderReplaced::setSelectionState(state);
- if (m_widget)
- m_widget->setIsSelected(isSelected());
- }
+ // The selection state for our containing block hierarchy is updated by the base class call.
+ RenderReplaced::setSelectionState(state);
+
+ if (m_widget)
+ m_widget->setIsSelected(isSelected());
}
void RenderWidget::clearWidget()
