It's missing a null pointer check for m_spellChecker. stack trace: Thread 0 *CRASHED* ( EXC_BAD_ACCESS / KERN_INVALID_ADDRESS @ 0x1f0f6602 ) 0x1f0f6602 0x01d36cf5 [Google Chrome Framework - Editor.cpp:443] WebCore::Editor::replaceSelectionWithFragment 0x01d3c299 [Google Chrome Framework - Editor.cpp:448] WebCore::Editor::replaceSelectionWithText 0x01d43381 [Google Chrome Framework - Editor.cpp:197] WebCore::Editor::handleTextEvent 0x01eb038f [Google Chrome Framework - EventHandler.cpp:2762] WebCore::EventHandler::defaultTextInputEventHandler 0x01cd0dcf [Google Chrome Framework - Node.cpp:3054] WebCore::Node::defaultEventHandler 0x01ccc191 [Google Chrome Framework - Node.cpp:2748] WebCore::Node::dispatchGenericEvent 0x01ccc988 [Google Chrome Framework - Node.cpp:2646] WebCore::Node::dispatchEvent 0x01cba659 [Google Chrome Framework - EventTarget.cpp:297] WebCore::EventTarget::dispatchEvent 0x01d3a646 [Google Chrome Framework - Editor.cpp:372] WebCore::Editor::pasteAsPlainText 0x01d3c6ad [Google Chrome Framework - Editor.cpp:393] WebCore::Editor::pasteAsPlainTextWithPasteboard 0x01d3c944 [Google Chrome Framework - Editor.cpp:1299] WebCore::Editor::paste 0x01d485b5 [Google Chrome Framework - EditorCommand.cpp:888] WebCore::executePaste 0x01d4944e [Google Chrome Framework - EditorCommand.cpp:1644] WebCore::Editor::Command::execute 0x01662c9b [Google Chrome Framework - WebFrameImpl.cpp:1199] WebKit::WebFrameImpl::executeCommand 0x007e2fb1 [Google Chrome Framework - render_view.cc:1562] RenderView::OnPaste 0x007e4a83 [Google Chrome Framework - ../base/tuple.h:558] RenderView::OnMessageReceived 0x011ffae2 [Google Chrome Framework - message_router.cc:46] MessageRouter::RouteMessage 0x011ff683 [Google Chrome Framework - message_router.cc:38] MessageRouter::OnMessageReceived 0x011f0881 [Google Chrome Framework - child_thread.cc:167] ChildThread::OnMessageReceived 0x0120f38a [Google Chrome Framework - ../base/tuple.h:551] RunnableMethod<IPC::ChannelProxy::Context,void (IPC::ChannelProxy::Context::*)(const IPC::Message&),Tuple1<IPC::Message> >::Run
Is this reproducible?
(In reply to comment #1) > Is this reproducible? I don't have a reduction but we're getting a lot of crash reports for Chrome 12.
It seems like this is happening inside Chrome extensions. Maybe we don't have a spell checker inside extensions' renderers? Regardless, this is a very easy fix. We just need to do a null pointer check.
Hmm. m_spellChecker should never be null. (It is always instantiated at the constructor.) I'm looking, but simple adding null-check might not be sufficient...
Created attachment 92921 [details] Patch
Comment on attachment 92921 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=92921&action=review > Source/WebCore/ChangeLog:9 > + SpellChecker uses TextCheckerClient, which belongs Page object, > + which is possibly destroyed during SpellChecker's lifetime. Wow! That sounds like a serious issue. We have a raw pointer in SpellChecker.h > Source/WebCore/editing/SpellChecker.cpp:119 > if (!initRequest(node)) > return; > - m_client->requestCheckingOfString(this, m_requestSequence, mask, m_requestText); > + client()->requestCheckingOfString(this, m_requestSequence, mask, m_requestText); We should should have an early exit here as well when the page is gone. > Source/WebCore/editing/SpellChecker.h:59 > TextCheckerClient* m_client; I don't think it's safe to store this pointer.
Created attachment 92924 [details] Patch
Hi Ryosuke, thank you for the lightning review! > > Source/WebCore/editing/SpellChecker.cpp:119 > > if (!initRequest(node)) > > return; > > - m_client->requestCheckingOfString(this, m_requestSequence, mask, m_requestText); > > + client()->requestCheckingOfString(this, m_requestSequence, mask, m_requestText); > > We should should have an early exit here as well when the page is gone. The check is done by caller side using canCheckAsynchronously(). This function also asserts it at the beginning. > > > Source/WebCore/editing/SpellChecker.h:59 > > TextCheckerClient* m_client; > > I don't think it's safe to store this pointer. Makes sense. removed.
Comment on attachment 92924 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=92924&action=review > Source/WebCore/editing/SpellChecker.cpp:116 > if (!initRequest(node)) > return; I see. initRequest asserts that canCheckAsynchronously returns true.
Committed r86132: <http://trac.webkit.org/changeset/86132>
Revision r86132 cherry-picked into qtwebkit-2.2 with commit f38046d <http://gitorious.org/webkit/qtwebkit/commit/f38046d>
(In reply to comment #11) > Revision r86132 cherry-picked into qtwebkit-2.2 with commit f38046d <http://gitorious.org/webkit/qtwebkit/commit/f38046d> Ademar, thanks for caring this.