Bug 48918 - Crashes in WebCore::DocumentMarkerController::removeMarkersFromMarkerMapVectorPair() when deleting multiple lines of text.
Summary: Crashes in WebCore::DocumentMarkerController::removeMarkersFromMarkerMapVecto...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Macintosh OS X 10.6
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2010-11-03 09:18 PDT by Jia Pu
Modified: 2010-11-03 16:55 PDT (History)
1 user (show)

See Also:


Attachments
Proposed patch (v1) (1.80 KB, patch)
2010-11-03 11:21 PDT, Jia Pu
darin: review+
adele: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jia Pu 2010-11-03 09:18:33 PDT
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
….
Comment 1 Jia Pu 2010-11-03 09:21:35 PDT
<rdar://problem/8620602>
Comment 2 Jia Pu 2010-11-03 09:26:08 PDT
Seems a more reliable reproducible case is:

"abc
abc

abc
abc"
Comment 3 Jia Pu 2010-11-03 09:29:06 PDT
When use TextIterator to iterate multiple lines of text, the returned pointer by TextIterator::node() can be null. Need to guard against this.
Comment 4 Jia Pu 2010-11-03 11:21:55 PDT
Created attachment 72844 [details]
Proposed patch (v1)
Comment 5 Darin Adler 2010-11-03 13:07:32 PDT
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().
Comment 6 Jia Pu 2010-11-03 13:42:51 PDT
(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().
Comment 7 Adele Peterson 2010-11-03 16:53:18 PDT
Comment on attachment 72844 [details]
Proposed patch (v1)

I'll land this manually since the commit queue is taking forever.
Comment 8 Adele Peterson 2010-11-03 16:55:43 PDT
Committed revision 71284.