| Summary: | Null dereference loading Blink layout test editing/execCommand/indent-no-visible-contents-crash.html | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Jon Honeycutt <jhoneycutt> | ||||||||||
| Component: | HTML Editing | Assignee: | Jiewen Tan <jiewen_tan> | ||||||||||
| Status: | RESOLVED FIXED | ||||||||||||
| Severity: | Normal | CC: | achristensen, commit-queue, jiewen_tan, webkit-bug-importer | ||||||||||
| Priority: | P2 | Keywords: | BlinkMergeCandidate, HasReduction, InRadar | ||||||||||
| Version: | WebKit Nightly Build | ||||||||||||
| Hardware: | Unspecified | ||||||||||||
| OS: | Unspecified | ||||||||||||
| Attachments: |
|
||||||||||||
Created attachment 261427 [details]
crashing test
*** Bug 149293 has been marked as a duplicate of this bug. *** Created attachment 263629 [details]
Patch
Created attachment 263630 [details]
Patch
Comment on attachment 263630 [details]
Patch
Why does this fix the crash? Why don't we add checks before all the other calls to moveParagraphWithClones?
(In reply to comment #6) > Comment on attachment 263630 [details] > Patch > > Why does this fix the crash? Why don't we add checks before all the other > calls to moveParagraphWithClones? Quote from Blink review: "Don't try to move non visible contents in "Indent" command This patch makes "Indent" command not to try move non visible contents into block quote. The root cause of issue 343958 is "Indent" command calls |moveParagraphWithClones()| with null |VisiblePosition| for start and end of contents to move. This patch checks them before calling |moveParagraphWithClones()|." I don't know whether other functions/methods who call moveParagraphWithClones() will have the same problem. Certainly, this test case doesn't reveal all other possibilities. I think it will be good to keep this change now. We can add the check later on if any new bug reports come in for different function. Maybe I should add assertions at the beginning of moveParagraphWithClones()? Comment on attachment 263630 [details]
Patch
I think a better fix would be to add an early return in CompositeEditCommand::cloneParagraphUnderNewElement if !passedOuterNode.
Created attachment 264055 [details]
Patch
This is a more generic fix than the blink fix because it catches all calls to moveParagraphWithClones Comment on attachment 264055 [details] Patch Clearing flags on attachment: 264055 Committed r191596: <http://trac.webkit.org/changeset/191596> All reviewed patches have been landed. Closing bug. |
Null dereference loading Blink layout test editing/execCommand/indent-no-visible-contents-crash.html. Stack trace: Crashed Thread: 0 Dispatch queue: com.apple.main-thread Exception Type: EXC_BAD_ACCESS (SIGSEGV) Exception Codes: KERN_INVALID_ADDRESS at 0x0000000000000014 Exception Note: EXC_CORPSE_NOTIFY VM Regions Near 0x14: --> __TEXT 000000010c635000-000000010c637000 [ 8K] r-x/rwx SM=COW /Users/USER/*/WebKit.framework/Versions/A/XPCServices/com.apple.WebKit.WebContent.Development.xpc/Contents/MacOS/com.apple.WebKit.WebContent.Development Application Specific Information: CRASHING TEST: temp-tests/editing/execCommand/indent-no-visible-contents-crash.html Global Trace Buffer (reverse chronological seconds): 58.426716 CFNetwork 0x00007fff88d43b97 Explicitly setting CF cookie storage singleton 58.427067 CFNetwork 0x00007fff88d8f211 Explicitly setting cookie storage singleton Thread 0 Crashed:: Dispatch queue: com.apple.main-thread 0 com.apple.WebCore 0x0000000114e55560 WebCore::Node::isDescendantOf(WebCore::Node const*) const + 32 (Node.h:638) 1 com.apple.WebCore 0x00000001144df07e WebCore::CompositeEditCommand::cloneParagraphUnderNewElement(WebCore::Position const&, WebCore::Position const&, WebCore::Node*, WebCore::Element*) + 318 (CompositeEditCommand.cpp:1067) 2 com.apple.WebCore 0x00000001144dfe2d WebCore::CompositeEditCommand::moveParagraphWithClones(WebCore::VisiblePosition const&, WebCore::VisiblePosition const&, WebCore::Element*, WebCore::Node*) + 621 (CompositeEditCommand.cpp:1176) 3 com.apple.WebCore 0x000000011493b755 WebCore::IndentOutdentCommand::indentIntoBlockquote(WebCore::Position const&, WebCore::Position const&, WTF::RefPtr<WebCore::Element>&) + 901 (StdLibExtras.h:366) 4 com.apple.WebCore 0x0000000114421605 WebCore::ApplyBlockElementCommand::formatSelection(WebCore::VisiblePosition const&, WebCore::VisiblePosition const&) + 4677 (ApplyBlockElementCommand.cpp:145) 5 com.apple.WebCore 0x000000011442004d WebCore::ApplyBlockElementCommand::doApply() + 1037 (Ref.h:123) 6 com.apple.WebCore 0x00000001144d8216 WebCore::CompositeEditCommand::apply() + 102 (ScopedEventQueue.h:71) 7 com.apple.WebCore 0x000000011470bc4b WebCore::executeIndent(WebCore::Frame&, WebCore::Event*, WebCore::EditorCommandSource, WTF::String const&) + 59 (StdLibExtras.h:366) 8 com.apple.WebCore 0x000000011470a876 WebCore::Editor::Command::execute(WTF::String const&, WebCore::Event*) const + 182 (EditorCommand.cpp:1704) 9 com.apple.WebCore 0x0000000114643c36 WebCore::Document::execCommand(WTF::String const&, bool, WTF::String const&) + 214 (Document.cpp:4666) 10 com.apple.WebCore 0x0000000114a5a074 WebCore::jsDocumentPrototypeFunctionExecCommand(JSC::ExecState*) + 420 (JSCJSValue.h:499) 11 ??? 0x0000445e06801028 0 + 75170626670632 12 com.apple.JavaScriptCore 0x0000000113db676f llint_entry + 22696 13 com.apple.JavaScriptCore 0x0000000113db0ce4 vmEntryToJavaScript + 299 14 com.apple.JavaScriptCore 0x0000000113c712d9 JSC::JITCode::execute(JSC::VM*, JSC::ProtoCallFrame*) + 169 (JITCode.cpp:82) 15 com.apple.JavaScriptCore 0x0000000113c57a10 JSC::Interpreter::execute(JSC::ProgramExecutable*, JSC::ExecState*, JSC::JSObject*) + 10448 (Interpreter.cpp:945) 16 com.apple.JavaScriptCore 0x000000011396a4c5 JSC::evaluate(JSC::ExecState*, JSC::SourceCode const&, JSC::JSValue, WTF::NakedPtr<JSC::Exception>&) + 469 (Completion.cpp:104) 17 com.apple.WebCore 0x00000001150cd8ec WebCore::ScriptController::evaluateInWorld(WebCore::ScriptSourceCode const&, WebCore::DOMWrapperWorld&) + 284 (JSMainThreadExecState.h:62) 18 com.apple.WebCore 0x00000001150cdb29 WebCore::ScriptController::evaluate(WebCore::ScriptSourceCode const&) + 41 (ScriptController.cpp:180) 19 com.apple.WebCore 0x00000001150d3aac WebCore::ScriptElement::executeScript(WebCore::ScriptSourceCode const&) + 316 (ScriptElement.cpp:309) 20 com.apple.WebCore 0x00000001150d2756 WebCore::ScriptElement::prepareScript(WTF::TextPosition const&, WebCore::ScriptElement::LegacyTypeSupport) + 1046 (StdLibExtras.h:366) 21 com.apple.WebCore 0x00000001148cf5eb WebCore::HTMLScriptRunner::runScript(WebCore::Element*, WTF::TextPosition const&) + 347 (ScriptElement.h:58) 22 com.apple.WebCore 0x00000001148cf440 WebCore::HTMLScriptRunner::execute(WTF::PassRefPtr<WebCore::Element>, WTF::TextPosition const&) + 48 (HTMLScriptRunner.cpp:191) 23 com.apple.WebCore 0x0000000114872466 WebCore::HTMLDocumentParser::runScriptsForPausedTreeBuilder() + 86 (StdLibExtras.h:366) 24 com.apple.WebCore 0x000000011487252d WebCore::HTMLDocumentParser::canTakeNextToken(WebCore::HTMLDocumentParser::SynchronousMode, WebCore::PumpSession&) + 93 (HTMLDocumentParser.cpp:214) 25 com.apple.WebCore 0x00000001148720c3 WebCore::HTMLDocumentParser::pumpTokenizer(WebCore::HTMLDocumentParser::SynchronousMode) + 595 (HTMLDocumentParser.cpp:259) 26 com.apple.WebCore 0x0000000114872ddd WebCore::HTMLDocumentParser::append(WTF::PassRefPtr<WTF::StringImpl>) + 669 (DocumentParser.h:71) 27 com.apple.WebCore 0x000000011461561c WebCore::DecodedDataDocumentParser::flush(WebCore::DocumentWriter&) + 92 (StdLibExtras.h:366) 28 com.apple.WebCore 0x000000011467568b WebCore::DocumentWriter::end() + 43 (RefPtr.h:71) 29 com.apple.WebCore 0x000000011465d9ec WebCore::DocumentLoader::finishedLoading(double) + 268 (ResourceErrorBase.h:42) 30 com.apple.WebCore 0x000000011448e179 WebCore::CachedResource::checkNotify() + 153 (CachedResourceClientWalker.h:51) 31 com.apple.WebCore 0x000000011448a433 WebCore::CachedRawResource::finishLoading(WebCore::SharedBuffer*) + 227 (CachedRawResource.cpp:104) 32 com.apple.WebCore 0x0000000115205501 WebCore::SubresourceLoader::didFinishLoading(double) + 1153 (ResourceLoader.h:154) 33 com.apple.WebKit 0x0000000112d4b98d WebKit::WebResourceLoader::didReceiveWebResourceLoaderMessage(IPC::Connection&, IPC::MessageDecoder&) + 561 (HandleMessage.h:16) 34 com.apple.WebKit 0x0000000112b251f1 IPC::Connection::dispatchMessage(std::__1::unique_ptr<IPC::MessageDecoder, std::__1::default_delete<IPC::MessageDecoder> >) + 127 (memory:2636) 35 com.apple.WebKit 0x0000000112b27b4a IPC::Connection::dispatchOneMessage() + 126 (memory:2656) 36 com.apple.JavaScriptCore 0x0000000113f69985 WTF::RunLoop::performWork() + 437 (functional:1742) 37 com.apple.JavaScriptCore 0x0000000113f69d32 WTF::RunLoop::performWork(void*) + 34 (RunLoopCF.cpp:39) 38 com.apple.CoreFoundation 0x00007fff949e2c01 __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ + 17 39 com.apple.CoreFoundation 0x00007fff949d4b1c __CFRunLoopDoSources0 + 556 40 com.apple.CoreFoundation 0x00007fff949d403f __CFRunLoopRun + 927 41 com.apple.CoreFoundation 0x00007fff949d3a38 CFRunLoopRunSpecific + 296 42 com.apple.HIToolbox 0x00007fff88e673bd RunCurrentEventLoopInMode + 235 43 com.apple.HIToolbox 0x00007fff88e67153 ReceiveNextEventCommon + 432 44 com.apple.HIToolbox 0x00007fff88e66f93 _BlockUntilNextEventMatchingListInModeWithFilter + 71 45 com.apple.AppKit 0x00007fff870b81e7 _DPSNextEvent + 1076 46 com.apple.AppKit 0x00007fff8748490d -[NSApplication _nextEventMatchingEventMask:untilDate:inMode:dequeue:] + 454 47 com.apple.AppKit 0x00007fff870ae0b8 -[NSApplication run] + 682 48 com.apple.AppKit 0x00007fff87030396 NSApplicationMain + 1176 49 libxpc.dylib 0x00007fff8c70ff70 _xpc_objc_main + 793 50 libxpc.dylib 0x00007fff8c7116bf xpc_main + 494 51 com.apple.WebKit.WebContent.Development 0x000000010c636424 main + 409 (XPCServiceMain.Development.mm:187) 52 libdyld.dylib 0x00007fff93aa15ad start + 1