48918 2010-11-03 09:18:33 -0700 Crashes in WebCore::DocumentMarkerController::removeMarkersFromMarkerMapVectorPair() when deleting multiple lines of text. 2010-11-03 16:55:43 -0700 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Mac OS X 10.6 RESOLVED FIXED InRadar P2 Normal --- 1 jiapu.mail webkit-unassigned adele oldest_to_newest 304054 0 jiapu.mail 2010-11-03 09:18:33 -0700 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 …. 304058 1 jiapu.mail 2010-11-03 09:21:35 -0700 <rdar://problem/8620602> 304061 2 jiapu.mail 2010-11-03 09:26:08 -0700 Seems a more reliable reproducible case is: "abc abc abc abc" 304063 3 jiapu.mail 2010-11-03 09:29:06 -0700 When use TextIterator to iterate multiple lines of text, the returned pointer by TextIterator::node() can be null. Need to guard against this. 304138 4 72844 jiapu.mail 2010-11-03 11:21:55 -0700 Created attachment 72844 Proposed patch (v1) 304229 5 72844 darin 2010-11-03 13:07:32 -0700 Comment on attachment 72844 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(). 304256 6 jiapu.mail 2010-11-03 13:42:51 -0700 (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(). 304356 7 72844 adele 2010-11-03 16:53:18 -0700 Comment on attachment 72844 Proposed patch (v1) I'll land this manually since the commit queue is taking forever. 304358 8 adele 2010-11-03 16:55:43 -0700 Committed revision 71284. 72844 2010-11-03 11:21:55 -0700 2010-11-03 16:53:18 -0700 Proposed patch (v1) webkit_bug48918.patch text/plain 1847 jiapu.mail SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiA3MTI1MSkKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMTggQEAKKzIwMTAtMTEtMDMgIEppYSBQdSAgPGpwdUBhcHBsZS5jb20+CisKKyAg ICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgQ3Jhc2hlcyBpbiBX ZWJDb3JlOjpEb2N1bWVudE1hcmtlckNvbnRyb2xsZXI6OnJlbW92ZU1hcmtlcnNGcm9tTWFya2Vy TWFwVmVjdG9yUGFpcigpIHdoZW4gZGVsZXRpbmcgbXVsdGlwbGUgbGluZXMgb2YgdGV4dC4KKyAg ICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTQ4OTE4CisgICAg ICAgIDxyZGFyOi8vcHJvYmxlbS84NjIwNjAyPgorCisgICAgICAgIEkgaGF2ZW4ndCBiZWVuIGFi bGUgdG8gZm91bmQgYSByZWFsaWFibGUgd2F5IHRvIHJlcHJvZHVjZSB0aGUgYnVnLiBIb3dldmVy LCB3aGVuZXZlciBpdCBoYXBwZW5zLAorICAgICAgICB0aGUgY3Jhc2ggaXMgY2F1c2VkIGJ5IGEg bnVsbCBub2RlIHBvaW50ZXIgcmV0dXJuZWQgYnkgVGV4dEl0ZXJhdG9yLiBTbyBpdCBzZWVtcyB0 byBiZSBhIAorICAgICAgICBzYWZlIGZpeCB0byBndWFyZCBhZ2FpbnN0IHRoYXQuIAorCisgICAg ICAgICogZWRpdGluZy9FZGl0b3IuY3BwOgorICAgICAgICAoV2ViQ29yZTo6RWRpdG9yOjpyZW1v dmVTcGVsbEFuZENvcnJlY3Rpb25NYXJrZXJzRnJvbVdvcmRzVG9CZUVkaXRlZCk6CisKIDIwMTAt MTEtMDMgIERhbiBCZXJuc3RlaW4gIDxtaXR6QGFwcGxlLmNvbT4KIAogICAgICAgICBSZXZpZXdl ZCBieSBEYXZlIEh5YXR0LgpJbmRleDogV2ViQ29yZS9lZGl0aW5nL0VkaXRvci5jcHAKPT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PQotLS0gV2ViQ29yZS9lZGl0aW5nL0VkaXRvci5jcHAJKHJldmlzaW9uIDcxMjUxKQorKysg V2ViQ29yZS9lZGl0aW5nL0VkaXRvci5jcHAJKHdvcmtpbmcgY29weSkKQEAgLTI1MzcsNiArMjUz Nyw4IEBAIHZvaWQgRWRpdG9yOjpyZW1vdmVTcGVsbEFuZENvcnJlY3Rpb25NYXIKICAgICBWZWN0 b3I8UmFuZ2VNYXJrZXJQYWlyLCAxNj4gbWFya2Vyc1RvUmVtb3ZlOwogICAgIGZvciAoVGV4dEl0 ZXJhdG9yIHRleHRJdGVyYXRvcih3b3JkUmFuZ2UuZ2V0KCkpOyAhdGV4dEl0ZXJhdG9yLmF0RW5k KCk7IHRleHRJdGVyYXRvci5hZHZhbmNlKCkpIHsKICAgICAgICAgTm9kZSogbm9kZSA9IHRleHRJ dGVyYXRvci5ub2RlKCk7CisgICAgICAgIGlmICghbm9kZSkKKyAgICAgICAgICAgIGNvbnRpbnVl OwogICAgICAgICBpZiAobm9kZSA9PSBzdGFydE9mRmlyc3RXb3JkLmRlZXBFcXVpdmFsZW50KCku Y29udGFpbmVyTm9kZSgpIHx8IG5vZGUgPT0gZW5kT2ZMYXN0V29yZC5kZWVwRXF1aXZhbGVudCgp LmNvbnRhaW5lck5vZGUoKSkgewogICAgICAgICAgICAgLy8gRmlyc3Qgd29yZCBhbmQgbGFzdCB3 b3JkIGNhbiBiZWxvbmcgdG8gdGhlIHNhbWUgbm9kZQogICAgICAgICAgICAgYm9vbCBwcm9jZXNz Rmlyc3RXb3JkID0gbm9kZSA9PSBzdGFydE9mRmlyc3RXb3JkLmRlZXBFcXVpdmFsZW50KCkuY29u dGFpbmVyTm9kZSgpICYmIGRvY3VtZW50LT5tYXJrZXJzKCktPmhhc01hcmtlcnMocmFuZ2VPZkZp cnN0V29yZC5nZXQoKSwgRG9jdW1lbnRNYXJrZXI6OlNwZWxsaW5nIHwgRG9jdW1lbnRNYXJrZXI6 OkNvcnJlY3Rpb25JbmRpY2F0b3IpOwo=