Bug 211273 - Nullptr crash in CompositeEditCommand::cloneParagraphUnderNewElement when indent and align a paragraph.
Summary: Nullptr crash in CompositeEditCommand::cloneParagraphUnderNewElement when ind...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: HTML Editing (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-04-30 19:55 PDT by Jack
Modified: 2020-05-05 10:18 PDT (History)
7 users (show)

See Also:


Attachments
Patch (4.08 KB, patch)
2020-04-30 21:59 PDT, Jack
no flags Details | Formatted Diff | Diff
Patch for landing (4.10 KB, patch)
2020-05-01 13:17 PDT, Jack
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jack 2020-04-30 19:55:52 PDT
<rdar://61885958>

    #0 0x11d1c4210 in WebCore::Node::parentNode() const (Safari_ASAN_260175_4c99c025e2143b27952e43920016745fa8dcb3f6.app/Contents/Frameworks/WebCore.framework/Versions/A/WebCore:x86_64+0x1dd210)
    #1 0x12051adf5 in WebCore::CompositeEditCommand::cloneParagraphUnderNewElement(WebCore::Position const&, WebCore::Position const&, WebCore::Node*, WebCore::Element*) (Safari_ASAN_260175_4c99c025e2143b27952e43920016745fa8dcb3f6.app/Contents/Frameworks/WebCore.framework/Versions/A/WebCore:x86_64+0x3533df5)
    #2 0x12051bbd2 in WebCore::CompositeEditCommand::moveParagraphWithClones(WebCore::VisiblePosition const&, WebCore::VisiblePosition const&, WebCore::Element*, WebCore::Node*) (Safari_ASAN_260175_4c99c025e2143b27952e43920016745fa8dcb3f6.app/Contents/Frameworks/WebCore.framework/Versions/A/WebCore:x86_64+0x3534bd2)
    #3 0x1205a0a85 in WebCore::IndentOutdentCommand::indentIntoBlockquote(WebCore::Position const&, WebCore::Position const&, WTF::RefPtr<WebCore::Element, WTF::DumbPtrTraits<WebCore::Element> >&) (Safari_ASAN_260175_4c99c025e2143b27952e43920016745fa8dcb3f6.app/Contents/Frameworks/WebCore.framework/Versions/A/WebCore:x86_64+0x35b9a85)
    #4 0x1204f86ee in WebCore::ApplyBlockElementCommand::formatSelection(WebCore::VisiblePosition const&, WebCore::VisiblePosition const&) (Safari_ASAN_260175_4c99c025e2143b27952e43920016745fa8dcb3f6.app/Contents/Frameworks/WebCore.framework/Versions/A/WebCore:x86_64+0x35116ee)
    #5 0x1204f7813 in WebCore::ApplyBlockElementCommand::doApply() (Safari_ASAN_260175_4c99c025e2143b27952e43920016745fa8dcb3f6.app/Contents/Frameworks/WebCore.framework/Versions/A/WebCore:x86_64+0x3510813)
    #6 0x1204f6326 in WebCore::CompositeEditCommand::apply() (Safari_ASAN_260175_4c99c025e2143b27952e43920016745fa8dcb3f6.app/Contents/Frameworks/WebCore.framework/Versions/A/WebCore:x86_64+0x350f326)
    #7 0x1205ac868 in WebCore::executeIndent(WebCore::Frame&, WebCore::Event*, WebCore::EditorCommandSource, WTF::String const&) (Safari_ASAN_260175_4c99c025e2143b27952e43920016745fa8dcb3f6.app/Contents/Frameworks/WebCore.framework/Versions/A/WebCore:x86_64+0x35c5868)
    #8 0x120222757 in WebCore::Document::execCommand(WTF::String const&, bool, WTF::String const&) (Safari_ASAN_260175_4c99c025e2143b27952e43920016745fa8dcb3f6.app/Contents/Frameworks/WebCore.framework/Versions/A/WebCore:x86_64+0x323b757)
Comment 1 Jack 2020-04-30 19:57:27 PDT
Test case:
<script>
    function iframeOnload() {
        document.execCommand("justifyFull", false);
        CANVAS.toBlob(blob);
    }

    function blob() {
        document.execCommand("selectAll", false);
        document.execCommand("indent", false);
    }
</script>
<body contentEditable=true><canvas id=CANVAS hidden="hidden"></canvas><iframe onload="iframeOnload()"></iframe><select></select>

Root cause:

Another crash caused by reentrancy.
1. We are in the middle of processing indent command (frame #99) and just cloned IFRAME and is appending it to BLOCKQUOTE.
2. Second command, justifyFull (frame #21), starts to execute and remove selected range, from IFRAME to SELECT (frame #3).
3. After justifyFull finishes, indent resumes and looks for a proper outerNode in function cloneParagraphUnderNewElement, by walking through parents of BODY.
4. Because now SELECT is dangling, function isDescendantOf() always returns false therefore we exhaust the parent node and deref null outerNode.

Node tree before justifyFull is called.
BODY	0x60c00007f0c0 (renderer 0x61200005a640) 
	#text	0x60800004a1a0 "\n"
	CANVAS	0x6120000559c0 (renderer 0x0) 
		#text	0x60800004a2a0 "d"
	#text	0x60800004a320 "\n"
	BLOCKQUOTE	0x60c0000c6640 (renderer 0x6120000802c0)  STYLE=margin: 0 0 0 40px; border: none; padding: 0px;
	IFRAME	0x61300005c8c0 (renderer 0x61200005a7c0) 
		#text	0x6080000539a0 "El$GJ7vPoBl"
	#text	0x608000053a20 "\n\n"
*	SELECT	0x613000077ac0 (renderer 0x615000085c80) 
		#text	0x608000053aa0 "%3ZqsHz}_JrFJ%'3"

Call stack that removes the SELECT from original node tree:
  * frame #0: 0x0000000161d9461f WebCore`WebCore::Node::setParentNode(this=0x0000613000077ac0, parent=0x0000000000000000) at Node.h:739:18
    frame #1: 0x0000000161d98526 WebCore`WebCore::ContainerNode::removeBetween(this=0x000060c00007f0c0, previousChild=0x000060c0000ca600, nextChild=0x0000000000000000, oldChild=0x0000613000077ac0) at ContainerNode.cpp:615:14
    frame #2: 0x0000000161d979ca WebCore`WebCore::ContainerNode::removeNodeWithScriptAssertion(this=0x000060c00007f0c0, childToRemove=0x0000613000077ac0, source=API) at ContainerNode.cpp:166:9
    frame #3: 0x0000000161d967d9 WebCore`WebCore::ContainerNode::removeChild(this=0x000060c00007f0c0, oldChild=0x0000613000077ac0) at ContainerNode.cpp:577:10
    frame #4: 0x00000001621c69b3 WebCore`WebCore::Node::remove(this=0x0000613000077ac0) at Node.cpp:628:20
    frame #5: 0x00000001625d74cc WebCore`WebCore::RemoveNodeCommand::doApply(this=0x00006110000a70c0) at RemoveNodeCommand.cpp:54:13
    frame #6: 0x000000016248bf2d WebCore`WebCore::CompositeEditCommand::applyCommandToComposite(this=0x000061500009bd00, command=0x00007ffeee757980) at CompositeEditCommand.cpp:463:14
    frame #7: 0x0000000162487b14 WebCore`WebCore::CompositeEditCommand::removeNode(this=0x000061500009bd00, node=0x0000613000077ac0, shouldAssumeContentIsAlwaysEditable=DoNotAssumeContentIsAlwaysEditable) at CompositeEditCommand.cpp:599:5
    frame #8: 0x00000001624ca174 WebCore`WebCore::DeleteSelectionCommand::removeNodeUpdatingStates(this=0x000061500009bd00, node=0x0000613000077ac0, shouldAssumeContentIsAlwaysEditable=DoNotAssumeContentIsAlwaysEditable) at DeleteSelectionCommand.cpp:419:27
    frame #9: 0x00000001624cada7 WebCore`WebCore::DeleteSelectionCommand::removeNode(this=0x000061500009bd00, node=0x0000613000077ac0, shouldAssumeContentIsAlwaysEditable=DoNotAssumeContentIsAlwaysEditable) at DeleteSelectionCommand.cpp:479:5
    frame #10: 0x00000001624cc5fc WebCore`WebCore::DeleteSelectionCommand::handleGeneralDelete(this=0x000061500009bd00) at DeleteSelectionCommand.cpp:622:17
    frame #11: 0x00000001624d1050 WebCore`WebCore::DeleteSelectionCommand::doApply(this=0x000061500009bd00) at DeleteSelectionCommand.cpp:932:5
    frame #12: 0x000000016248bf2d WebCore`WebCore::CompositeEditCommand::applyCommandToComposite(this=0x0000612000082e40, command=0x00007ffeee758840) at CompositeEditCommand.cpp:463:14
    frame #13: 0x0000000162485301 WebCore`WebCore::CompositeEditCommand::deleteSelection(this=0x0000612000082e40, smartDelete=false, mergeBlocksAfterDelete=false, replace=false, expandForSpecialElements=false, sanitizeMarkup=true) at CompositeEditCommand.cpp:829:9
    frame #14: 0x0000000162496868 WebCore`WebCore::CompositeEditCommand::moveParagraphs(this=0x0000612000082e40, startOfParagraphToMove=0x00007ffeee759b80, endOfParagraphToMove=0x00007ffeee759bc0, destination=0x00007ffeee759d60, preserveSelection=false, preserveStyle=true) at CompositeEditCommand.cpp:1475:5
    frame #15: 0x00000001624742a5 WebCore`WebCore::CompositeEditCommand::moveParagraphContentsToNewBlockIfNecessary(this=0x0000612000082e40, pos=0x00007ffeee75a490) at CompositeEditCommand.cpp:1209:5
    frame #16: 0x000000016246ff1e WebCore`WebCore::ApplyStyleCommand::applyBlockStyle(this=0x0000612000082e40, style=0x00006030000f0f40) at ApplyStyleCommand.cpp:267:41
    frame #17: 0x000000016246f045 WebCore`WebCore::ApplyStyleCommand::doApply(this=0x0000612000082e40) at ApplyStyleCommand.cpp:219:9
    frame #18: 0x000000016246616b WebCore`WebCore::CompositeEditCommand::apply(this=0x0000612000082e40) at CompositeEditCommand.cpp:372:9
    frame #19: 0x00000001625159be WebCore`WebCore::Editor::applyParagraphStyle(this=0x0000615000062000, style=0x000060b000074ce0, editingAction=Unspecified) at Editor.cpp:998:135
    frame #20: 0x000000016259820e WebCore`WebCore::executeApplyParagraphStyle(frame={ origin = file://, url = file:///Users/jacklee/browser2/61885958/min-61885958.html, isMainFrame = 1, backForwardCacheState = NotInBackForwardCache }, source=CommandFromDOM, action=Justify, propertyID=CSSPropertyTextAlign, propertyValue={ length = 7, contents = 'justify' }) at EditorCommand.cpp:154:24
    frame #21: 0x000000016258c7b1 WebCore`WebCore::executeJustifyFull(frame={ origin = file://, url = file:///Users/jacklee/browser2/61885958/min-61885958.html, isMainFrame = 1, backForwardCacheState = NotInBackForwardCache }, (null)=0x0000000000000000, source=CommandFromDOM, (null)={ length = 0, contents = '' }) at EditorCommand.cpp:568:12
    frame #22: 0x00000001625232b0 WebCore`WebCore::Editor::Command::execute(this=0x00007ffeee75b1d0, parameter={ length = 0, contents = '' }, triggeringEvent=0x0000000000000000) const at EditorCommand.cpp:1876:12
    frame #23: 0x0000000161e965f7 WebCore`WebCore::Document::execCommand(this={ origin = file://, url = file:///Users/jacklee/browser2/61885958/min-61885958.html, inMainFrame = 1, backForwardCacheState = NotInBackForwardCache }, commandName={ length = 11, contents = 'justifyFull' }, userInterface=false, value={ length = 0, contents = '' }) at Document.cpp:5521:54
    frame #24: 0x000000015cd51652 WebCore`WebCore::jsDocumentPrototypeFunctionExecCommandBody(lexicalGlobalObject=0x000061f000032ce8, callFrame=0x00007ffeee75b950, castedThis=0x000060d00000db38, throwScope=0x00007ffeee75b780) at JSDocument.cpp:6270:57
    frame #25: 0x000000015ca2eace WebCore`long long WebCore::IDLOperation<WebCore::JSDocument>::call<&(lexicalGlobalObject=0x000061f000032ce8, callFrame=0x00007ffeee75b950, operationName="execCommand")), (WebCore::CastedThisErrorBehavior)0>(JSC::JSGlobalObject&, JSC::CallFrame&, char const*) at JSDOMOperation.h:53:16
    frame #26: 0x000000015ca2e624 WebCore`WebCore::jsDocumentPrototypeFunctionExecCommand(lexicalGlobalObject=0x000061f000032ce8, callFrame=0x00007ffeee75b950) at JSDocument.cpp:6276:12
    frame #27: 0x00005d22bc801178
    frame #28: 0x000000018c896b9b JavaScriptCore`llint_entry at LowLevelInterpreter.asm:1045
    frame #29: 0x000000018c896c3e JavaScriptCore`llint_entry at LowLevelInterpreter.asm:1045
    frame #30: 0x000000018c877562 JavaScriptCore`vmEntryToJavaScript at LowLevelInterpreter64.asm:296
    frame #31: 0x000000018ed8fd90 JavaScriptCore`JSC::JITCode::execute(this=0x00006040000e6310, vm=0x000062f00001c400, protoCallFrame=0x00007ffeee75bf60) at JITCodeInlines.h:38:38
    frame #32: 0x000000018ed91107 JavaScriptCore`JSC::Interpreter::executeCall(this=0x00006020000157f0, lexicalGlobalObject=0x000061f000032ce8, function=0x000062d000181260, callData=0x00007ffeee75cea0, thisValue=JSValue @ 0x00007ffeee75bd80, args=0x00007ffeee75d1e0) at Interpreter.cpp:921:31
    frame #33: 0x000000018f724160 JavaScriptCore`JSC::call(globalObject=0x000061f000032ce8, functionObject=JSValue @ 0x00007ffeee75c3c0, callData=0x00007ffeee75cea0, thisValue=JSValue @ 0x00007ffeee75c3e0, args=0x00007ffeee75d1e0) at CallData.cpp:58:28
    frame #34: 0x000000018f724672 JavaScriptCore`JSC::call(globalObject=0x000061f000032ce8, functionObject=JSValue @ 0x00007ffeee75c5c0, callData=0x00007ffeee75cea0, thisValue=JSValue @ 0x00007ffeee75c5e0, args=0x00007ffeee75d1e0, returnedException=0x00007ffeee75d160) at CallData.cpp:65:22
    frame #35: 0x000000018f72531a JavaScriptCore`JSC::profiledCall(globalObject=0x000061f000032ce8, reason=Other, functionObject=JSValue @ 0x00007ffeee75c8c0, callData=0x00007ffeee75cea0, thisValue=JSValue @ 0x00007ffeee75c8e0, args=0x00007ffeee75d1e0, returnedException=0x00007ffeee75d160) at CallData.cpp:86:12
    frame #36: 0x00000001611974be WebCore`WebCore::JSExecState::profiledCall(lexicalGlobalObject=0x000061f000032ce8, reason=Other, functionObject=JSValue @ 0x00007ffeee75cb40, callData=0x00007ffeee75cea0, thisValue=JSValue @ 0x00007ffeee75cb60, args=0x00007ffeee75d1e0, returnedException=0x00007ffeee75d160) at JSExecState.h:73:16
    frame #37: 0x00000001611e626e WebCore`WebCore::JSEventListener::handleEvent(this=0x000060b000073a50, scriptExecutionContext={ origin = file://, url = file:///Users/jacklee/browser2/61885958/min-61885958.html, inMainFrame = 1, backForwardCacheState = NotInBackForwardCache }, event=0x000060700005e2e0) at JSEventListener.cpp:179:22
    frame #38: 0x00000001620db81b WebCore`WebCore::EventTarget::innerInvokeEventListeners(this=0x0000613000082f00, event=0x000060700005e2e0, listeners={ size = 1, capacity = 1 }, phase=Bubbling) at EventTarget.cpp:335:40
    frame #39: 0x00000001620d2004 WebCore`WebCore::EventTarget::fireEventListeners(this=0x0000613000082f00, event=0x000060700005e2e0, phase=Bubbling) at EventTarget.cpp:267:9
    frame #40: 0x00000001621db936 WebCore`WebCore::Node::handleLocalEvents(this=0x0000613000082f00, event=0x000060700005e2e0, phase=Bubbling) at Node.cpp:2371:5
    frame #41: 0x00000001620a891f WebCore`WebCore::EventContext::handleLocalEvents(this=0x0000604000160550, event=0x000060700005e2e0, phase=Bubbling) const at EventContext.cpp:55:17
    frame #42: 0x00000001620a9e0e WebCore`WebCore::dispatchEventInDOM(event=0x000060700005e2e0, path=0x00007ffeee75e000) at EventDispatcher.cpp:100:22
    frame #43: 0x00000001620a964b WebCore`WebCore::EventDispatcher::dispatchEvent(node=0x0000613000082f00, event=0x000060700005e2e0) at EventDispatcher.cpp:154:9
    frame #44: 0x00000001621db98d WebCore`WebCore::Node::dispatchEvent(this=0x0000613000082f00, event=0x000060700005e2e0) at Node.cpp:2381:5
    frame #45: 0x0000000163b77b45 WebCore`WebCore::DOMWindow::dispatchLoadEvent(this=0x000061400004f440) at DOMWindow.cpp:2217:20
    frame #46: 0x0000000161e6f3dd WebCore`WebCore::Document::dispatchWindowLoadEvent(this={ origin = file://, url = about:blank, inMainFrame = 0, backForwardCacheState = NotInBackForwardCache }) at Document.cpp:4776:18
    frame #47: 0x0000000161e6ecc0 WebCore`WebCore::Document::implicitClose(this={ origin = file://, url = about:blank, inMainFrame = 0, backForwardCacheState = NotInBackForwardCache }) at Document.cpp:3062:5
    frame #48: 0x00000001637fc8d0 WebCore`WebCore::FrameLoader::checkCallImplicitClose(this=0x000061400004f240) at FrameLoader.cpp:965:25
    frame #49: 0x00000001637fb9c6 WebCore`WebCore::FrameLoader::checkCompleted(this=0x000061400004f240) at FrameLoader.cpp:906:5
    frame #50: 0x00000001637f670f WebCore`WebCore::FrameLoader::finishedParsing(this=0x000061400004f240) at FrameLoader.cpp:816:5
    frame #51: 0x0000000161e9c36b WebCore`WebCore::Document::finishedParsing(this={ origin = file://, url = about:blank, inMainFrame = 0, backForwardCacheState = NotInBackForwardCache }) at Document.cpp:5840:25
    frame #52: 0x0000000162eab248 WebCore`WebCore::HTMLConstructionSite::finishedParsing(this=0x0000612000082260) at HTMLConstructionSite.cpp:419:16
    frame #53: 0x0000000162f32ca0 WebCore`WebCore::HTMLTreeBuilder::finished(this=0x0000612000082240) at HTMLTreeBuilder.cpp:2843:12
    frame #54: 0x0000000162eb9762 WebCore`WebCore::HTMLDocumentParser::end(this=0x000062500033b900) at HTMLDocumentParser.cpp:449:20
    frame #55: 0x0000000162eb54a2 WebCore`WebCore::HTMLDocumentParser::attemptToRunDeferredScriptsAndEnd(this=0x000062500033b900) at HTMLDocumentParser.cpp:458:5
    frame #56: 0x0000000162eb50f2 WebCore`WebCore::HTMLDocumentParser::prepareToStopParsing(this=0x000062500033b900) at HTMLDocumentParser.cpp:153:5
    frame #57: 0x0000000162eb9881 WebCore`WebCore::HTMLDocumentParser::attemptToEnd(this=0x000062500033b900) at HTMLDocumentParser.cpp:470:5
    frame #58: 0x0000000162eb99b9 WebCore`WebCore::HTMLDocumentParser::finish(this=0x000062500033b900) at HTMLDocumentParser.cpp:498:5
    frame #59: 0x0000000163713ced WebCore`WebCore::DocumentWriter::end(this=0x000061f00004ed10) at DocumentWriter.cpp:288:15
    frame #60: 0x0000000163711adf WebCore`WebCore::DocumentLoader::finishedLoading(this=0x000061f00004ec80) at DocumentLoader.cpp:452:14
    frame #61: 0x000000016372b218 WebCore`WebCore::DocumentLoader::maybeLoadEmpty(this=0x000061f00004ec80) at DocumentLoader.cpp:1798:5
    frame #62: 0x000000016372b868 WebCore`WebCore::DocumentLoader::startLoadingMainResource(this=0x000061f00004ec80) at DocumentLoader.cpp:1812:9
    frame #63: 0x0000000163863c20 WebCore`WebCore::FrameLoader::continueLoadAfterNavigationPolicy(this=0x0000603000022ea8)::$_11::operator()() at FrameLoader.cpp:3492:38
    frame #64: 0x0000000163862d5e WebCore`WTF::Detail::CallableWrapper<WebCore::FrameLoader::continueLoadAfterNavigationPolicy(WebCore::ResourceRequest const&, WebCore::FormState*, WebCore::NavigationPolicyDecision, WebCore::AllowNavigationToInvalidURL)::$_11, void>::call(this=0x0000603000022ea0) at Function.h:52:39
    frame #65: 0x000000015b4bead5 WebCore`WTF::Function<void ()>::operator(this=0x00007ffeee760360)() const at Function.h:84:35
    frame #66: 0x000000015b5916d7 WebCore`WTF::CompletionHandler<void ()>::operator(this=0x00007ffeee760950)() at CompletionHandler.h:62:16
    frame #67: 0x0000000163811ae3 WebCore`WebCore::FrameLoader::continueLoadAfterNavigationPolicy(this=0x000061400004f240, request=0x0000612000081960, formState=0x0000000000000000, navigationPolicyDecision=ContinueLoad, allowNavigationToInvalidURL=Yes) at FrameLoader.cpp:3496:9
    frame #68: 0x000000016385e644 WebCore`WebCore::FrameLoader::loadWithDocumentLoader(this=0x00006040001560d8, request=0x0000612000081960, formState=0x00007ffeee761e50, navigationPolicyDecision=ContinueLoad)>&&)::$_8::operator()(WebCore::ResourceRequest const&, WTF::WeakPtr<WebCore::FormState>&&, WebCore::NavigationPolicyDecision) at FrameLoader.cpp:1647:9
    frame #69: 0x000000016385e2d3 WebCore`WTF::Detail::CallableWrapper<WebCore::FrameLoader::loadWithDocumentLoader(WebCore::DocumentLoader*, WebCore::FrameLoadType, WTF::RefPtr<WebCore::FormState, WTF::DumbPtrTraits<WebCore::FormState> >&&, WebCore::AllowNavigationToInvalidURL, WTF::CompletionHandler<void ()>&&)::$_8, void, WebCore::ResourceRequest&&, WTF::WeakPtr<WebCore::FormState>&&, WebCore::NavigationPolicyDecision>::call(this=0x00006040001560d0, in=0x0000612000081960, in=0x00007ffeee761e50, in=ContinueLoad) at Function.h:52:39
    frame #70: 0x00000001638d88d9 WebCore`WTF::Function<void (WebCore::ResourceRequest&&, WTF::WeakPtr<WebCore::FormState>&&, WebCore::NavigationPolicyDecision)>::operator(this=0x00007ffeee7610d0, in=0x0000612000081960, in=0x00007ffeee761e50, in=ContinueLoad)(WebCore::ResourceRequest&&, WTF::WeakPtr<WebCore::FormState>&&, WebCore::NavigationPolicyDecision) const at Function.h:84:35
    frame #71: 0x00000001638bd359 WebCore`WTF::CompletionHandler<void (WebCore::ResourceRequest&&, WTF::WeakPtr<WebCore::FormState>&&, WebCore::NavigationPolicyDecision)>::operator(this=0x0000612000081950, in=0x0000612000081960, in=0x00007ffeee761e50, in=ContinueLoad)(WebCore::ResourceRequest&&, WTF::WeakPtr<WebCore::FormState>&&, WebCore::NavigationPolicyDecision) at CompletionHandler.h:62:16
    frame #72: 0x00000001638e1065 WebCore`WebCore::PolicyChecker::checkNavigationPolicy(this=0x0000612000081948, policyAction=Use, responseIdentifier=PolicyCheckIdentifier @ 0x00007ffeee761260)>&&, WebCore::PolicyDecisionMode)::$_7::operator()(WebCore::PolicyAction, WebCore::PolicyCheckIdentifier) at PolicyChecker.cpp:237:20
    frame #73: 0x00000001638de0f9 WebCore`WTF::Detail::CallableWrapper<WebCore::PolicyChecker::checkNavigationPolicy(WebCore::ResourceRequest&&, WebCore::ResourceResponse const&, WebCore::DocumentLoader*, WTF::RefPtr<WebCore::FormState, WTF::DumbPtrTraits<WebCore::FormState> >&&, WTF::CompletionHandler<void (WebCore::ResourceRequest&&, WTF::WeakPtr<WebCore::FormState>&&, WebCore::NavigationPolicyDecision)>&&, WebCore::PolicyDecisionMode)::$_7, void, WebCore::PolicyAction, WebCore::PolicyCheckIdentifier>::call(this=0x0000612000081940, in=Use, in=PolicyCheckIdentifier @ 0x00007ffeee7624c0) at Function.h:52:39
    frame #74: 0x00000001637f165c WebCore`WTF::Function<void (WebCore::PolicyAction, WebCore::PolicyCheckIdentifier)>::operator(this=0x00007ffeee7639e0, in=Use, in=PolicyCheckIdentifier @ 0x00007ffeee762640)(WebCore::PolicyAction, WebCore::PolicyCheckIdentifier) const at Function.h:84:35
    frame #75: 0x00000001638bc172 WebCore`WebCore::PolicyChecker::checkNavigationPolicy(this=0x000060b000073b00, request=0x00007ffeee764e60, redirectResponse=0x00007ffeee764f60, loader=0x000061f00004ec80, formState=0x00007ffeee767010, function=0x00007ffeee7650a0, policyDecisionMode=Asynchronous)>&&, WebCore::PolicyDecisionMode) at PolicyChecker.cpp:245:9
    frame #76: 0x000000016380f853 WebCore`WebCore::FrameLoader::loadWithDocumentLoader(this=0x000061400004f240, loader=0x000061f00004ec80, type=RedirectWithLockedBackForwardList, formState=0x00007ffeee767010, allowNavigationToInvalidURL=Yes, completionHandler=0x00007ffeee766510)>&&) at FrameLoader.cpp:1646:21
    frame #77: 0x000000016380b5bb WebCore`WebCore::FrameLoader::loadWithNavigationAction(this=0x000061400004f240, request=0x00007ffeee765cf0, action=0x00007ffeee765df0, type=RedirectWithLockedBackForwardList, formState=0x00007ffeee767010, allowNavigationToInvalidURL=Yes, downloadAttribute={ length = 0, contents = '' }, completionHandler=0x00007ffeee766510)>&&) at FrameLoader.cpp:1513:5
    frame #78: 0x0000000163802904 WebCore`WebCore::FrameLoader::loadURL(this=0x000061400004f240, frameLoadRequest=0x00007ffeee766b50, referrer={ length = 0, contents = '' }, newLoadType=RedirectWithLockedBackForwardList, event=0x0000000000000000, formState=0x00007ffeee767010, adClickAttribution=0x00007ffeee767030, completionHandler=0x00007ffeee7670a0)>&&) at FrameLoader.cpp:1421:5
    frame #79: 0x00000001637fdec3 WebCore`WebCore::FrameLoader::loadURLIntoChildFrame(this=0x0000614000007440, url={ about:blank }, referer={ length = 0, contents = '' }, childFrame={ origin = file://, url = about:blank, isMainFrame = 0, backForwardCacheState = NotInBackForwardCache }) at FrameLoader.cpp:1000:26
    frame #80: 0x000000016390b456 WebCore`WebCore::FrameLoader::SubframeLoader::loadSubframe(this=0x0000602000016fd0, ownerElement=0x0000613000082f00, url={ about:blank }, name={ length = 0, contents = '' }, referrer={ length = 57, contents = 'file:///Users/jacklee/browser2/61885958/min-61885958.html' }) at SubframeLoader.cpp:347:22
    frame #81: 0x0000000163907b1a WebCore`WebCore::FrameLoader::SubframeLoader::loadOrRedirectSubframe(this=0x0000602000016fd0, ownerElement=0x0000613000082f00, requestURL={ about:blank }, frameName={ length = 0, contents = '' }, lockHistory=Yes, lockBackForwardList=Yes) at SubframeLoader.cpp:309:17
    frame #82: 0x0000000163907040 WebCore`WebCore::FrameLoader::SubframeLoader::requestFrame(this=0x0000602000016fd0, ownerElement=0x0000613000082f00, urlString={ length = 11, contents = 'about:blank' }, frameName={ length = 0, contents = '' }, lockHistory=Yes, lockBackForwardList=Yes) at SubframeLoader.cpp:98:20
    frame #83: 0x000000016292107d WebCore`WebCore::HTMLFrameElementBase::openURL(this=0x0000613000082f00, lockHistory=Yes, lockBackForwardList=Yes) at HTMLFrameElementBase.cpp:102:44
    frame #84: 0x00000001629218a2 WebCore`WebCore::HTMLFrameElementBase::didFinishInsertingNode(this=0x0000613000082f00) at HTMLFrameElementBase.cpp:142:5
    frame #85: 0x0000000161d9a77f WebCore`void WebCore::executeNodeInsertionWithScriptAssertion<WebCore::ContainerNode::appendChildWithoutPreInsertionValidityCheck(WebCore::Node&)::$_4>(containerNode=0x000060c0000c6640, child=0x0000613000082f00, source=API, replacedAllChildren=No, doNodeInsertion=(anonymous class) @ 0x00007ffeee767fc0)::$_4) at ContainerNode.cpp:213:17
    frame #86: 0x0000000161d92b6d WebCore`WebCore::ContainerNode::appendChildWithoutPreInsertionValidityCheck(this=0x000060c0000c6640, newChild=0x0000613000082f00) at ContainerNode.cpp:726:9
    frame #87: 0x0000000161d99d3b WebCore`WebCore::ContainerNode::appendChild(this=0x000060c0000c6640, newChild=0x0000613000082f00) at ContainerNode.cpp:692:12
    frame #88: 0x00000001624676d2 WebCore`WebCore::AppendNodeCommand::doApply(this=0x00006110000e6540) at AppendNodeCommand.cpp:51:15
    frame #89: 0x000000016248bf2d WebCore`WebCore::CompositeEditCommand::applyCommandToComposite(this=0x000061200007ffc0, command=0x00007ffeee7689a0) at CompositeEditCommand.cpp:463:14
    frame #90: 0x000000016246aa50 WebCore`WebCore::CompositeEditCommand::appendNode(this=0x000061200007ffc0, node=0x00007ffeee768be0, parent=0x00007ffeee768c00) at CompositeEditCommand.cpp:581:5
    frame #91: 0x000000016249849d WebCore`WebCore::CompositeEditCommand::cloneParagraphUnderNewElement(this=0x000061200007ffc0, start=0x00007ffeee768f60, end=0x00007ffeee768fa0, passedOuterNode=0x000060c00007f0c0, blockElement=0x000060c0000c6640) at CompositeEditCommand.cpp:1258:13
    frame #92: 0x000000016249950a WebCore`WebCore::CompositeEditCommand::moveParagraphWithClones(this=0x000061200007ffc0, startOfParagraphToMove=0x00007ffeee769420, endOfParagraphToMove=0x00007ffeee769580, blockElement=0x000060c0000c6640, outerNode=0x000060c00007f0c0) at CompositeEditCommand.cpp:1361:5
    frame #93: 0x000000016257b479 WebCore`WebCore::IndentOutdentCommand::indentIntoBlockquote(this=0x000061200007ffc0, start=0x00007ffeee7697e0, end=0x00007ffeee769bd0, targetBlockquote=0x00007ffeee769a50) at IndentOutdentCommand.cpp:125:5
    frame #94: 0x000000016257d5c8 WebCore`WebCore::IndentOutdentCommand::formatRange(this=0x000061200007ffc0, start=0x00007ffeee7697e0, end=0x00007ffeee769bd0, (null)=0x00006120000800b0, blockquoteForNextIndent=0x00007ffeee769a50) at IndentOutdentCommand.cpp:253:9
    frame #95: 0x0000000162469d8e WebCore`WebCore::ApplyBlockElementCommand::formatSelection(this=0x000061200007ffc0, startOfSelection=0x00007ffeee76a4e0, endOfSelection=0x00007ffeee76a520) at ApplyBlockElementCommand.cpp:153:9
    frame #96: 0x000000016257d536 WebCore`WebCore::IndentOutdentCommand::formatSelection(this=0x000061200007ffc0, startOfSelection=0x00007ffeee76a4e0, endOfSelection=0x00007ffeee76a520) at IndentOutdentCommand.cpp:243:35
    frame #97: 0x00000001624682b9 WebCore`WebCore::ApplyBlockElementCommand::doApply(this=0x000061200007ffc0) at ApplyBlockElementCommand.cpp:90:5
    frame #98: 0x000000016246616b WebCore`WebCore::CompositeEditCommand::apply(this=0x000061200007ffc0) at CompositeEditCommand.cpp:372:9
    frame #99: 0x000000016258fdd5 WebCore`WebCore::executeIndent(frame={ origin = file://, url = file:///Users/jacklee/browser2/61885958/min-61885958.html, isMainFrame = 1, backForwardCacheState = NotInBackForwardCache }, (null)=0x0000000000000000, (null)=CommandFromDOM, (null)={ length = 0, contents = '' }) at EditorCommand.cpp:450:84
    frame #100: 0x00000001625232b0 WebCore`WebCore::Editor::Command::execute(this=0x00007ffeee76ab70, parameter={ length = 0, contents = '' }, triggeringEvent=0x0000000000000000) const at EditorCommand.cpp:1876:12
    frame #101: 0x0000000161e965f7 WebCore`WebCore::Document::execCommand(this={ origin = file://, url = file:///Users/jacklee/browser2/61885958/min-61885958.html, inMainFrame = 1, backForwardCacheState = NotInBackForwardCache }, commandName={ length = 6, contents = 'indent' }, userInterface=false, value={ length = 0, contents = '' }) at Document.cpp:5521:54
    frame #102: 0x000000015cd51652 WebCore`WebCore::jsDocumentPrototypeFunctionExecCommandBody(lexicalGlobalObject=0x000061f000032ce8, callFrame=0x00007ffeee76b2e0, castedThis=0x000060d00000db38, throwScope=0x00007ffeee76b120) at JSDocument.cpp:6270:57
    frame #103: 0x000000015ca2eace WebCore`long long WebCore::IDLOperation<WebCore::JSDocument>::call<&(lexicalGlobalObject=0x000061f000032ce8, callFrame=0x00007ffeee76b2e0, operationName="execCommand")), (WebCore::CastedThisErrorBehavior)0>(JSC::JSGlobalObject&, JSC::CallFrame&, char const*) at JSDOMOperation.h:53:16
    frame #104: 0x000000015ca2e624 WebCore`WebCore::jsDocumentPrototypeFunctionExecCommand(lexicalGlobalObject=0x000061f000032ce8, callFrame=0x00007ffeee76b2e0) at JSDocument.cpp:6276:12
    frame #105: 0x00005d22bc801178
Comment 2 Jack 2020-04-30 21:59:28 PDT
Created attachment 398159 [details]
Patch
Comment 3 Geoffrey Garen 2020-05-01 12:16:27 PDT
Comment on attachment 398159 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=398159&action=review

r=me

> Source/WebCore/ChangeLog:11
> +        Reentrancy may occur when we clone and append a paragraph. Check if the elements
> +        are removed and bail out.

Instead of saying "reentrancy may occur" here, I would say "a load event can fire". Generally speaking, re-entrancy of some of these functions is OK. What's not OK is that the load event will execute arbitrary JavaScript, which may change the state of the document however it likes (including disconnecting our nodes).

Side note: It is probably a small spec compliance bug that the load event fires synchronously. I believe it is specified to fire async. Still, it's good to add this fix for now.
Comment 4 Jack 2020-05-01 13:17:27 PDT
Created attachment 398224 [details]
Patch for landing
Comment 5 Jack 2020-05-01 13:18:46 PDT
(In reply to Geoffrey Garen from comment #3)
> Comment on attachment 398159 [details]
> Patch
> Instead of saying "reentrancy may occur" here, I would say "a load event can
> fire". Generally speaking, re-entrancy of some of these functions is OK.
> What's not OK is that the load event will execute arbitrary JavaScript,
> which may change the state of the document however it likes (including
> disconnecting our nodes).
Thanks! Will modify the change log and upload with land-safely.
Comment 6 EWS 2020-05-01 13:54:03 PDT
Committed r261019: <https://trac.webkit.org/changeset/261019>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 398224 [details].