Crashes in WebCore::DocumentMarkerController::removeMarkersFromMarkerMapVectorPair() when deleting multiple lines of text.

• Component: New Bugs
• Status: RESOLVED FIXED

[Jia Pu]

To reproduce:

1. type
     "adfef
      adfef"
2. Select all.
3. Try to delete the selection.

It crashes with following call stack:
>  1 com.apple.WebCore              0x7fff877fea64 WebCore::DocumentMarkerController::removeMarkersFromMarkerMapVectorPair(WebCore::Node*, std::pair<WTF::Vector<WebCore::DocumentMarker, 0ul>, WTF::Vector<WebCore::IntRect, 0ul> >*, WebCore::DocumentMarker::MarkerType) + 0x2a
   2 com.apple.WebCore              0x7fff877fed64 WebCore::DocumentMarkerController::removeMarkers(WebCore::Node*, WebCore::DocumentMarker::MarkerType) + 0x48
   3 com.apple.WebCore              0x7fff8788566b WebCore::Editor::removeSpellAndCorrectionMarkersFromWordsToBeEdited(bool) + 0xe21
   4 com.apple.WebCore              0x7fff875dcdd6 WebCore::TypingCommand::deleteKeyPressed(WebCore::TextGranularity, bool) + 0x36
   5 com.apple.WebCore              0x7fff875d3b32 WebCore::EditCommand::apply() + 0x84
   6 com.apple.WebCore              0x7fff875dcd43 WebCore::TypingCommand::deleteKeyPressed(WebCore::Document*, bool, WebCore::TextGranularity, bool) + 0x167
   7 com.apple.WebCore              0x7fff875dcb78 WebCore::Editor::deleteWithDirection(WebCore::SelectionController::EDirection, WebCore::TextGranularity, bool, bool) + 0x1ec
   8 com.apple.WebCore              0x7fff8789011f WebCore::executeDeleteBackward(WebCore::Frame*, WebCore::Event*, WebCore::EditorCommandSource, WTF::String const&) + 0x1f
   9 com.apple.WebCore              0x7fff8788eb66 WebCore::Editor::Command::execute(WTF::String const&, WebCore::Event*) const + 0x94
  10 com.apple.WebCore              0x7fff875d9757 WebCore::Editor::Command::execute(WebCore::Event*) const + 0x1f
  11 com.apple.WebKit               0x7fff86386fdb -[WebHTMLView(WebNSTextInputSupport) doCommandBySelector:] + 0x25b
  12 com.apple.WebKit               0x7fff863869b8 -[WebHTMLView(WebInternal) _interceptEditingKeyEvent:shouldSaveCommand:] + 0x288
  13 com.apple.WebKit               0x7fff86386abe WebEditorClient::handleKeyboardEvent(WebCore::KeyboardEvent*) + 0x5c
  14 com.apple.WebCore              0x7fff8746fc34 WebCore::EventHandler::defaultKeyboardEventHandler(WebCore::KeyboardEvent*) + 0x42
  15 com.apple.WebCore              0x7fff87415f5d WebCore::Node::defaultEventHandler(WebCore::Event*) + 0x9d
  16 com.apple.WebCore              0x7fff87415b5b WebCore::Node::dispatchGenericEvent(WTF::PassRefPtr<WebCore::Event>) + 0x3a5
  17 com.apple.WebCore              0x7fff874156bb WebCore::Node::dispatchEvent(WTF::PassRefPtr<WebCore::Event>) + 0xe9
  18 com.apple.WebCore              0x7fff8746fb62 WebCore::EventTarget::dispatchEvent(WTF::PassRefPtr<WebCore::Event>, int&) + 0x56
  19 com.apple.WebCore              0x7fff8746f247 WebCore::EventHandler::keyEvent(WebCore::PlatformKeyboardEvent const&) + 0x321
  20 com.apple.WebCore              0x7fff8746ff0d WebCore::EventHandler::keyEvent(NSEvent*) + 0x35
….

[Jia Pu]

<rdar://problem/8620602>

[Jia Pu]

Seems a more reliable reproducible case is:

"abc
abc

abc
abc"

[Jia Pu]

When use TextIterator to iterate multiple lines of text, the returned pointer by TextIterator::node() can be null. Need to guard against this.

[Jia Pu]

Created attachment 72844 [details]
Proposed patch (v1)

[Darin Adler]

Comment on attachment 72844 [details]
Proposed patch (v1)

It’s extremely inefficient to use TextIterator’s node() function on every node in a range you are iterating. The TextIterator::range() function is far more efficient. We should probably have a comment about that in TextIterator.h and perhaps even rename node() to deprecatedNode().

This code change is OK, but I worry that removeSpellAndCorrectionMarkersFromWordsToBeEdited can be pathologically slow with a large selection because of the use of TextIterator::node().

[Jia Pu]

(In reply to comment #5)
> (From update of attachment 72844 [details])
> It’s extremely inefficient to use TextIterator’s node() function on every node in a range you are iterating. The TextIterator::range() function is far more efficient. We should probably have a comment about that in TextIterator.h and perhaps even rename node() to deprecatedNode().
> 
> This code change is OK, but I worry that removeSpellAndCorrectionMarkersFromWordsToBeEdited can be pathologically slow with a large selection because of the use of TextIterator::node().

Thanks for reviewing, Darin. I have create bug #48949 to track replacing Consider replacing TextIterator::node() with TextIterator::range().

[Adele Peterson]

Comment on attachment 72844 [details]
Proposed patch (v1)

I'll land this manually since the commit queue is taking forever.

[Adele Peterson]

Committed revision 71284.