Bug 8952

Created attachment 8362 [details] Crash log

Confirmed. I think a reduction would be nice.

This is a case of Document::updateRendering() being called under detach(): #0 WebCore::RenderCanvas::setSelection (this=0x260018fc, s=0x0, sp=-1, e=0x0, ep=-1) at WebCore/rendering/RenderCanvas.cpp:327 #1 0x018b459b in WebCore::RenderCanvas::clearSelection (this=0x260018fc) at WebCore/rendering/RenderCanvas.cpp:452 #2 0x018b4eb6 in WebCore::RenderContainer::removeChildNode (this=0x25a7ce2c, oldChild=0x25a7cedc) at WebCore/rendering/RenderContainer.cpp:180 #3 0x018b506f in WebCore::RenderContainer::removeChild (this=0x25a7ce2c, oldChild=0x25a7cedc) at WebCore/rendering/RenderContainer.cpp:213 #4 0x0189c04f in WebCore::RenderBlock::removeChild (this=0x25a7ce2c, oldChild=0x25a7cedc) at WebCore/rendering/RenderBlock.cpp:318 #5 0x018ce4db in WebCore::RenderObject::remove (this=0x25a7cedc) at WebCore/rendering/RenderObject.cpp:2051 #6 0x018ce5e7 in WebCore::RenderObject::destroy (this=0x25a7cedc) at WebCore/rendering/RenderObject.cpp:2067 #7 0x018dcba3 in WebCore::RenderText::destroy (this=0x25a7cedc) at WebCore/rendering/RenderText.cpp:143 #8 0x01979682 in WebCore::Node::detach (this=0x25a7cd80) at WebCore/dom/Node.cpp:712 #9 0x018334e3 in WebCore::ContainerNode::detach (this=0x25a7db30) at WebCore/dom/ContainerNode.cpp:582 #10 0x018334e3 in WebCore::ContainerNode::detach (this=0x25a95940) at WebCore/dom/ContainerNode.cpp:582 #11 0x018334e3 in WebCore::ContainerNode::detach (this=0x25af8830) at WebCore/dom/ContainerNode.cpp:582 #12 0x018334e3 in WebCore::ContainerNode::detach (this=0x26001c50) at WebCore/dom/ContainerNode.cpp:582 #13 0x018334e3 in WebCore::ContainerNode::detach (this=0x25afd640) at WebCore/dom/ContainerNode.cpp:582 #14 0x018334e3 in WebCore::ContainerNode::detach (this=0x25afe4b0) at WebCore/dom/ContainerNode.cpp:582 #15 0x0197f04e in WebCore::Element::recalcStyle (this=0x25afe4b0, change=NoChange) at WebCore/dom/Element.cpp:508 #16 0x0197f250 in WebCore::Element::recalcStyle (this=0x26000190, change=NoChange) at WebCore/dom/Element.cpp:541 #17 0x0197f250 in WebCore::Element::recalcStyle (this=0x26091a60, change=NoChange) at WebCore/dom/Element.cpp:541 #18 0x0197f250 in WebCore::Element::recalcStyle (this=0x25ebf010, change=NoChange) at WebCore/dom/Element.cpp:541 #19 0x0197f250 in WebCore::Element::recalcStyle (this=0x25eb2020, change=NoChange) at WebCore/dom/Element.cpp:541 #20 0x0197f250 in WebCore::Element::recalcStyle (this=0x25ea1810, change=NoChange) at WebCore/dom/Element.cpp:541 #21 0x0197f250 in WebCore::Element::recalcStyle (this=0x25ead450, change=NoChange) at WebCore/dom/Element.cpp:541 #22 0x0197f250 in WebCore::Element::recalcStyle (this=0x260ae9a0, change=NoChange) at WebCore/dom/Element.cpp:541 #23 0x0182c7d2 in WebCore::Document::recalcStyle (this=0x11958200, change=NoChange) at WebCore/dom/Document.cpp:846 #24 0x01825e47 in WebCore::Document::updateRendering (this=0x11958200) at WebCore/dom/Document.cpp:868 #25 0x01739eeb in WebCore::HTMLElement::isContentEditable (this=0x25a7db30) at WebCore/html/HTMLElement.cpp:443 #26 0x01978726 in WebCore::Node::isContentEditable (this=0x25a7cd80) at WebCore/dom/Node.cpp:320 #27 0x0197a294 in WebCore::Node::rootEditableElement (this=0x25a7cd80) at WebCore/dom/Node.cpp:1047 #28 0x0189e399 in WebCore::RenderBlock::isSelectionRoot (this=0x25a7ce2c) at WebCore/rendering/RenderBlock.cpp:1461 #29 0x0189e40e in WebCore::RenderBlock::shouldPaintSelectionGaps (this=0x25a7ce2c) at WebCore/rendering/RenderBlock.cpp:1443 #30 0x018a410d in WebCore::RenderBlock::selectionGapRects (this=0x25a7ce2c) at WebCore/rendering/RenderBlock.cpp:1470 #31 0x01ab49dc in WebCore::RenderBlock::BlockSelectionInfo::BlockSelectionInfo (this=0x260df270, b=0x25a7ce2c) at WebCore/rendering/RenderBlock.h:236 #32 0x018b3581 in WebCore::RenderCanvas::setSelection (this=0x260018fc, s=0x0, sp=-1, e=0x0, ep=-1) at WebCore/rendering/RenderCanvas.cpp:337 #33 0x018b459b in WebCore::RenderCanvas::clearSelection (this=0x260018fc) at WebCore/rendering/RenderCanvas.cpp:452 #34 0x018b4eb6 in WebCore::RenderContainer::removeChildNode (this=0x25a7ce2c, oldChild=0x25a7cedc) at WebCore/rendering/RenderContainer.cpp:180 #35 0x018b506f in WebCore::RenderContainer::removeChild (this=0x25a7ce2c, oldChild=0x25a7cedc) at WebCore/rendering/RenderContainer.cpp:213 #36 0x0189c04f in WebCore::RenderBlock::removeChild (this=0x25a7ce2c, oldChild=0x25a7cedc) at WebCore/rendering/RenderBlock.cpp:318 #37 0x018ce4db in WebCore::RenderObject::remove (this=0x25a7cedc) at WebCore/rendering/RenderObject.cpp:2051 #38 0x018ce5e7 in WebCore::RenderObject::destroy (this=0x25a7cedc) at WebCore/rendering/RenderObject.cpp:2067 #39 0x018dcba3 in WebCore::RenderText::destroy (this=0x25a7cedc) at WebCore/rendering/RenderText.cpp:143 #40 0x01979682 in WebCore::Node::detach (this=0x25a7cd80) at WebCore/dom/Node.cpp:712 #41 0x018334e3 in WebCore::ContainerNode::detach (this=0x25a7db30) at WebCore/dom/ContainerNode.cpp:582 #42 0x018334e3 in WebCore::ContainerNode::detach (this=0x25a95940) at WebCore/dom/ContainerNode.cpp:582 You even end up re-entering setSelection().

The key to this bug is ** highlighted modules ** being dragged. The problem does not happen if the modules are not highlighted.

Created attachment 8417 [details] Reduction (will crash)

SelectionController::nodeWillBeRemoved does take care of this same issue, so the clearSelection call in RenderContainer::removeChildNode could perhaps be removed entirely to fix this bug.

<rdar://problem/4575185>

*** Bug 9425 has been marked as a duplicate of this bug. ***

Adding GoogleBug keyword in one big change.

Created attachment 9423 [details] patch that fixes the bug -- needs change log, layout test, etc.

Regressed the problem with WebKit 15403 on 10.4.6. Problem continues with the same steps to reproduce as well as the attached regression. Please see attached log. It's slightly different than the original crash log. I'll also update this problem with a Safari 2.0.4 regression on 10.4.7.

Created attachment 9445 [details] patch with detailed change log and a layout test

Now we'd blow away the selection if a node inside it is removed, instead of just invalidating the rect(s) that it paints in. Is that necessary to fix the crash?

(In reply to comment #13) > Now we'd blow away the selection if a node inside it is removed, instead of > just invalidating the rect(s) that it paints in. Is that necessary to fix the > crash? I think you're misreading the patch. I didn't change that.

(In reply to comment #13) > Now we'd blow away the selection if a node inside it is removed, instead of > just invalidating the rect(s) that it paints in. Is that necessary to fix the > crash? Here's the old code that does it: - } else if (Range::compareBoundaryPoints(m_sel.start(), Position(node, 0)) == -1 && - Range::compareBoundaryPoints(m_sel.end(), Position(node, 0)) == 1) { - // If we did nothing here, when this node's renderer was destroyed, the rect that it - // occupied would be invalidated, but, selection gaps that change as a result of - // the removal wouldn't be invalidated. - // FIXME: Don't do so much unnecessary invalidation. - static_cast<RenderView*>(start->document()->renderer())->clearSelection(); My new code does the same thing in this case.

Comment on attachment 9445 [details] patch with detailed change log and a layout test r=me

Committed revision 15459.

Regression testing caught one small thing wrong with the change. Changed logic back to what it was before, but retained the bug fix. Committed revision 15461.