Bug 149292 – Null dereference loading Blink layout test editing/execCommand/indent-no-visible-contents-crash.html

• Status: RESOLVED
• Resolution: FIXED
• Component: HTML Editing

[Jon Honeycutt]

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

[Radar WebKit Bug Importer]

<rdar://problem/22746530>

[Jon Honeycutt]

Created attachment 261427 [details]
crashing test

[Jon Honeycutt]

*** Bug 149293 has been marked as a duplicate of this bug. ***

[Jiewen Tan]

Created attachment 263629 [details]
Patch

[Jiewen Tan]

Created attachment 263630 [details]
Patch

[Alex Christensen]

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?

[Jiewen Tan]

(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()?

[Alex Christensen]

Comment on attachment 263630 [details]
Patch

I think a better fix would be to add an early return in CompositeEditCommand::cloneParagraphUnderNewElement if !passedOuterNode.

[Jiewen Tan]

Created attachment 264055 [details]
Patch

[Alex Christensen]

This is a more generic fix than the blink fix because it catches all calls to moveParagraphWithClones

[WebKit Commit Bot]

Comment on attachment 264055 [details]
Patch

Clearing flags on attachment: 264055

Committed r191596: <http://trac.webkit.org/changeset/191596>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.