224977 – Crash in InsertParagraphSeparatorCommand::doApply

Bug 224977 - Crash in InsertParagraphSeparatorCommand::doApply

Summary: Crash in InsertParagraphSeparatorCommand::doApply

Status:	RESOLVED FIXED

Alias:	None

Product:	WebKit
Classification:	Unclassified
Component:	HTML Editing (show other bugs)
Version:	WebKit Nightly Build
Hardware:	Unspecified Unspecified

Importance:	P2 Normal
Assignee:	Frédéric Wang (:fredw)

URL:
Keywords:	InRadar

Depends on:	226527
Blocks:
	Show dependency tree / graph

Reported:	2021-04-23 07:49 PDT by Ali Juma
Modified:	2021-07-26 13:47 PDT (History)
CC List:	18 users (show)

See Also:	225267

Attachments
Minimal test case (3.97 KB, text/html) 2021-04-23 07:49 PDT, Ali Juma	no flags	Details
Reduction (ASSERTION FAILED: refNode) -- fixed in bug 226527 (361 bytes, text/html) 2021-05-18 17:45 PDT, Ryosuke Niwa	no flags	Details
tentative patch for attachment 429009 (2.69 KB, patch) 2021-05-24 08:31 PDT, Frédéric Wang (:fredw)	ews-feeder: commit-queue-	Details \| Formatted Diff \| Diff
Reduce a bit the second issue (2.10 KB, text/html) 2021-05-25 10:24 PDT, Frédéric Wang (:fredw)	no flags	Details
Reduction (ASSERTION FAILED: startBlock->firstChild()) (1.81 KB, text/html) 2021-05-26 03:31 PDT, Frédéric Wang (:fredw)	no flags	Details
Add an assert for Position's offset (535 bytes, patch) 2021-05-27 04:36 PDT, Frédéric Wang (:fredw)	ews-feeder: commit-queue-	Details \| Formatted Diff \| Diff
Add an assert for Position's offset (537 bytes, patch) 2021-05-27 05:56 PDT, Frédéric Wang (:fredw)	no flags	Details \| Formatted Diff \| Diff
Patch for ASSERTION FAILED: startBlock->firstChild() (8.74 KB, patch) 2021-06-02 06:31 PDT, Frédéric Wang (:fredw)	ews-feeder: commit-queue-	Details \| Formatted Diff \| Diff
Patch for ASSERTION FAILED: refNode (4.57 KB, patch) 2021-06-02 06:47 PDT, Frédéric Wang (:fredw)	rniwa: review+	Details \| Formatted Diff \| Diff
Patch for ASSERTION FAILED: startBlock->firstChild() (11.75 KB, patch) 2021-06-09 05:47 PDT, Frédéric Wang (:fredw)	ews-feeder: commit-queue-	Details \| Formatted Diff \| Diff
Patch (fix offset) (7.67 KB, patch) 2021-06-15 02:41 PDT, Frédéric Wang (:fredw)	ews-feeder: commit-queue-	Details \| Formatted Diff \| Diff
Tentative patch (961 bytes, patch) 2021-06-15 06:12 PDT, Frédéric Wang (:fredw)	no flags	Details \| Formatted Diff \| Diff
Patch (6.13 KB, patch) 2021-06-16 06:53 PDT, Frédéric Wang (:fredw)	no flags	Details \| Formatted Diff \| Diff
Patch (attempt to rebuild the dom tree and selection before crash) (3.46 KB, patch) 2021-07-03 06:12 PDT, Frédéric Wang (:fredw)	no flags	Details \| Formatted Diff \| Diff
Show Obsolete (8) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.

Description Ali Juma 2021-04-23 07:49:17 PDT

Created attachment 426906 [details]
Minimal test case

Filing this as a security bug since it was found using a fuzzer; there's no disclosure deadline for this bug.

This reproduces in an ASan build of WebKitTestRunner, and also in STP 123.

Stack:
=================================================================
==35165==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000020 (pc 0x0006500bb361 bp 0x7ffee22d3cf0 sp 0x7ffee22d3cf0 T0)
==35165==The signal is caused by a READ memory access.
==35165==Hint: address points to the zero page.
    #0 0x6500bb360 in WebCore::Node::parentNode() const (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0x18d360)
    #1 0x653784827 in WebCore::CompositeEditCommand::insertNodeBefore(WTF::Ref<WebCore::Node, WTF::RawPtrTraits<WebCore::Node> >&&, WebCore::Node&, WebCore::ShouldAssumeContentIsAlwaysEditable) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0x3856827)
    #2 0x6538413ae in WebCore::InsertParagraphSeparatorCommand::doApply() (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0x39133ae)
    #3 0x65378c29a in WebCore::CompositeEditCommand::applyCommandToComposite(WTF::Ref<WebCore::EditCommand, WTF::RawPtrTraits<WebCore::EditCommand> >&&) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0x385e29a)
    #4 0x6538a5c25 in WebCore::TypingCommand::insertParagraphSeparator() (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0x3977c25)
    #5 0x6538a3b07 in WebCore::TypingCommand::insertParagraphSeparatorAndNotifyAccessibility() (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0x3975b07)
    #6 0x65376dba4 in WebCore::CompositeEditCommand::apply() (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0x383fba4)
    #7 0x6538a397a in WebCore::TypingCommand::insertParagraphSeparator(WebCore::Document&, unsigned int) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0x397597a)
    #8 0x6538267a2 in WebCore::executeInsertParagraph(WebCore::Frame&, WebCore::Event*, WebCore::EditorCommandSource, WTF::String const&) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0x38f87a2)
    #9 0x65348cfa3 in WebCore::Document::execCommand(WTF::String const&, bool, WTF::String const&) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0x355efa3)
    #10 0x6509104aa in WebCore::jsDocumentPrototypeFunction_execCommandBody(JSC::JSGlobalObject*, JSC::CallFrame*, WebCore::JSDocument*) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0x9e24aa)
    #11 0x65090ff6b in long long WebCore::IDLOperation<WebCore::JSDocument>::call<&(WebCore::jsDocumentPrototypeFunction_execCommandBody(JSC::JSGlobalObject*, JSC::CallFrame*, WebCore::JSDocument*)), (WebCore::CastedThisErrorBehavior)0>(JSC::JSGlobalObject&, JSC::CallFrame&, char const*) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0x9e1f6b)
    #12 0x26ee116011d7  (<unknown module>)
    #13 0x66c9f826a in llint_entry (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/JavaScriptCore.framework/Versions/A/JavaScriptCore:x86_64+0xb5026a)
    #14 0x66c9dd308 in vmEntryToJavaScript (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/JavaScriptCore.framework/Versions/A/JavaScriptCore:x86_64+0xb35308)
    #15 0x66e1822fd in JSC::Interpreter::executeCall(JSC::JSGlobalObject*, JSC::JSObject*, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/JavaScriptCore.framework/Versions/A/JavaScriptCore:x86_64+0x22da2fd)
    #16 0x66e825d8f in JSC::call(JSC::JSGlobalObject*, JSC::JSValue, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&, WTF::NakedPtr<JSC::Exception>&) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/JavaScriptCore.framework/Versions/A/JavaScriptCore:x86_64+0x297dd8f)
    #17 0x66e82614b in JSC::profiledCall(JSC::JSGlobalObject*, JSC::ProfilingReason, JSC::JSValue, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&, WTF::NakedPtr<JSC::Exception>&) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/JavaScriptCore.framework/Versions/A/JavaScriptCore:x86_64+0x297e14b)
    #18 0x652ce28b8 in WebCore::JSExecState::profiledCall(JSC::JSGlobalObject*, JSC::ProfilingReason, JSC::JSValue, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&, WTF::NakedPtr<JSC::Exception>&) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0x2db48b8)
    #19 0x652d0b9d7 in WebCore::JSEventListener::handleEvent(WebCore::ScriptExecutionContext&, WebCore::Event&) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0x2ddd9d7)
    #20 0x6535b0a5f in WebCore::EventTarget::innerInvokeEventListeners(WebCore::Event&, WTF::Vector<WTF::RefPtr<WebCore::RegisteredEventListener, WTF::RawPtrTraits<WebCore::RegisteredEventListener>, WTF::DefaultRefDerefTraits<WebCore::RegisteredEventListener> >, 1ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc>, WebCore::EventTarget::EventInvokePhase) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0x3682a5f)
    #21 0x6535b0302 in WebCore::EventTarget::fireEventListeners(WebCore::Event&, WebCore::EventTarget::EventInvokePhase) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0x3682302)
    #22 0x65357f349 in WebCore::EventContext::handleLocalEvents(WebCore::Event&, WebCore::EventTarget::EventInvokePhase) const (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0x3651349)
    #23 0x6535807ac in WebCore::dispatchEventInDOM(WebCore::Event&, WebCore::EventPath const&) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0x36527ac)
    #24 0x65357fdb3 in WebCore::EventDispatcher::dispatchEvent(WebCore::Node&, WebCore::Event&) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0x3651db3)
    #25 0x65368adf7 in WebCore::ScopedEventQueue::enqueueEvent(WTF::Ref<WebCore::Event, WTF::RawPtrTraits<WebCore::Event> >&&) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0x375cdf7)
    #26 0x65357f678 in WebCore::EventDispatcher::dispatchScopedEvent(WebCore::Node&, WebCore::Event&) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0x3651678)
    #27 0x65340cd9e in WebCore::dispatchChildRemovalEvents(WTF::Ref<WebCore::Node, WTF::RawPtrTraits<WebCore::Node> >&) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0x34ded9e)
    #28 0x6533f9b65 in WebCore::ContainerNode::removeChild(WebCore::Node&) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0x34cbb65)
    #29 0x6533fe07a in WebCore::ContainerNode::replaceChild(WebCore::Node&, WebCore::Node&) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0x34d007a)
    #30 0x65357073e in WebCore::Element::setOuterHTML(WTF::String const&) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0x364273e)
    #31 0x6509d1746 in WebCore::setJSElement_outerHTMLSetter(JSC::JSGlobalObject&, WebCore::JSElement&, JSC::JSValue)::'lambda'()::operator()() const (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0xaa3746)
    #32 0x6509d15e6 in void WebCore::invokeFunctorPropagatingExceptionIfNecessary<WebCore::setJSElement_outerHTMLSetter(JSC::JSGlobalObject&, WebCore::JSElement&, JSC::JSValue)::'lambda'()>(JSC::JSGlobalObject&, JSC::ThrowScope&, WebCore::setJSElement_outerHTMLSetter(JSC::JSGlobalObject&, WebCore::JSElement&, JSC::JSValue)::'lambda'()&&) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0xaa35e6)
    #33 0x6509d1340 in WebCore::setJSElement_outerHTMLSetter(JSC::JSGlobalObject&, WebCore::JSElement&, JSC::JSValue) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0xaa3340)
    #34 0x65088d0d6 in bool WebCore::IDLAttribute<WebCore::JSElement>::set<&(WebCore::setJSElement_outerHTMLSetter(JSC::JSGlobalObject&, WebCore::JSElement&, JSC::JSValue)), (WebCore::CastedThisErrorBehavior)0>(JSC::JSGlobalObject&, long long, long long, JSC::PropertyName) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0x95f0d6)
    #35 0x66e8d4afa in JSC::callCustomSetter(JSC::JSGlobalObject*, bool (*)(JSC::JSGlobalObject*, long long, long long, JSC::PropertyName), bool, JSC::JSObject*, JSC::JSValue, JSC::JSValue, JSC::PropertyName) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/JavaScriptCore.framework/Versions/A/JavaScriptCore:x86_64+0x2a2cafa)
    #36 0x66eb4d5d6 in JSC::JSObject::putInlineSlow(JSC::JSGlobalObject*, JSC::PropertyName, JSC::JSValue, JSC::PutPropertySlot&) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/JavaScriptCore.framework/Versions/A/JavaScriptCore:x86_64+0x2ca55d6)
    #37 0x66e45364c in llint_slow_path_put_by_id (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/JavaScriptCore.framework/Versions/A/JavaScriptCore:x86_64+0x25ab64c)
    #38 0x66c9e752f in llint_entry (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/JavaScriptCore.framework/Versions/A/JavaScriptCore:x86_64+0xb3f52f)
    #39 0x66c9dd308 in vmEntryToJavaScript (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/JavaScriptCore.framework/Versions/A/JavaScriptCore:x86_64+0xb35308)
    #40 0x66e1822fd in JSC::Interpreter::executeCall(JSC::JSGlobalObject*, JSC::JSObject*, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/JavaScriptCore.framework/Versions/A/JavaScriptCore:x86_64+0x22da2fd)
    #41 0x66e825d8f in JSC::call(JSC::JSGlobalObject*, JSC::JSValue, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&, WTF::NakedPtr<JSC::Exception>&) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/JavaScriptCore.framework/Versions/A/JavaScriptCore:x86_64+0x297dd8f)
    #42 0x66e82614b in JSC::profiledCall(JSC::JSGlobalObject*, JSC::ProfilingReason, JSC::JSValue, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&, WTF::NakedPtr<JSC::Exception>&) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/JavaScriptCore.framework/Versions/A/JavaScriptCore:x86_64+0x297e14b)
    #43 0x652ce28b8 in WebCore::JSExecState::profiledCall(JSC::JSGlobalObject*, JSC::ProfilingReason, JSC::JSValue, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&, WTF::NakedPtr<JSC::Exception>&) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0x2db48b8)
    #44 0x652dba67d in WebCore::ScheduledAction::executeFunctionInContext(JSC::JSGlobalObject*, JSC::JSValue, WebCore::ScriptExecutionContext&) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0x2e8c67d)
    #45 0x652dba05a in WebCore::ScheduledAction::execute(WebCore::Document&) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0x2e8c05a)
    #46 0x6544bf035 in WebCore::DOMTimer::fired() (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0x4591035)
    #47 0x65483bcb4 in WebCore::ThreadTimers::sharedTimerFiredInternal() (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0x490dcb4)
    #48 0x6548c5939 in WebCore::timerFired(__CFRunLoopTimer*, void*) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0x4997939)
    #49 0x7fff30b00467 in __CFRUNLOOP_IS_CALLING_OUT_TO_A_TIMER_CALLBACK_FUNCTION__ (/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation:x86_64+0x9f467)
    #50 0x7fff30afffcd in __CFRunLoopDoTimer (/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation:x86_64+0x9efcd)
    #51 0x7fff30affab8 in __CFRunLoopDoTimers (/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation:x86_64+0x9eab8)
    #52 0x7fff30ae470c in __CFRunLoopRun (/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation:x86_64+0x8370c)
    #53 0x7fff30ae3952 in CFRunLoopRunSpecific (/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation:x86_64+0x82952)
    #54 0x7fff331a11c7 in -[NSRunLoop(NSRunLoop) runMode:beforeDate:] (/System/Library/Frameworks/Foundation.framework/Versions/C/Foundation:x86_64+0x601c7)
    #55 0x7fff33253c6e in -[NSRunLoop(NSRunLoop) run] (/System/Library/Frameworks/Foundation.framework/Versions/C/Foundation:x86_64+0x112c6e)
    #56 0x7fff6acc04e9 in _xpc_objc_main.cold.4 (/usr/lib/system/libxpc.dylib:x86_64+0x164e9)
    #57 0x7fff6acc042f in _xpc_objc_main (/usr/lib/system/libxpc.dylib:x86_64+0x1642f)
    #58 0x7fff6acbff62 in xpc_main (/usr/lib/system/libxpc.dylib:x86_64+0x15f62)
    #59 0x640f03a83 in WebKit::XPCServiceMain(int, char const**) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebKit.framework/Versions/A/WebKit:x86_64+0xf03a83)
    #60 0x7fff6aa6ecc8 in start (/usr/lib/system/libdyld.dylib:x86_64+0x1acc8)
==35165==Register values:
rax = 0x0000000000000004  rbx = 0x00007ffee22d3d80  rcx = 0x0000100000000004  rdx = 0x0000000000000000
rdi = 0x0000000000000020  rsi = 0x00007ffee22d4370  rbp = 0x00007ffee22d3cf0  rsp = 0x00007ffee22d3cf0
 r8 = 0x0000000000000001   r9 = 0x0000000000000000  r10 = 0xffffffffffffffff  r11 = 0xfffffffffffffaf0
r12 = 0x00007ffee22d3d20  r13 = 0x00007ffee22d3d00  r14 = 0x00001fffdc45a7a0  r15 = 0x0000000000000000
========================
Clusterfuzz-id: 5684221868572672

Comment 1 Radar WebKit Bug Importer 2021-04-29 10:05:21 PDT

<rdar://problem/77327339>

Comment 2 Ryosuke Niwa 2021-05-18 17:44:51 PDT

<rdar://77270374>

Comment 3 Ryosuke Niwa 2021-05-18 17:45:35 PDT

Created attachment 429009 [details]
Reduction (ASSERTION FAILED: refNode) -- fixed in bug 226527

Comment 4 Frédéric Wang (:fredw) 2021-05-22 12:29:11 PDT

Quick reverse-debugging shows we are in a situation where a <body> has display: table and a single <img> child. Then ContainerNode::traverseToChildAt with child index = 1 returns nullptr.

Although the code involved is different from bug 225267, this issue is also prevented by attachment 428919 [details].

ASSERTION FAILED: refNode
at https://webkit-search.igalia.com/webkit/rev/61241a69457b1d5d1c68c82b673f489e8e3caf81/Source/WebCore/editing/InsertParagraphSeparatorCommand.cpp#278
(rr) reverse-f
(rr) reverse-f
(rr) reverse-f
(rr) p refNode
$1 = (WebCore::Node *) 0x0
(rr) rn
(rr) rn
https://webkit-search.igalia.com/webkit/rev/61241a69457b1d5d1c68c82b673f489e8e3caf81/Source/WebCore/editing/InsertParagraphSeparatorCommand.cpp#277
(rr) p startBlock
(rr) p showTree((Node*) startBlock)
*BODY	0x7f92618707b0 (renderer 0x7f9261870a80)  STYLE=display: table
	IMG	0x7f9261870840 (renderer 0x7f9261870c50)  STYLE=content: ''
$2 = void
(rr) p insertionPosition.computeOffsetInContainerNode()
$3 = 1

Comment 5 Frédéric Wang (:fredw) 2021-05-24 02:26:47 PDT

(In reply to Ryosuke Niwa from comment #3)
> Created attachment 429009 [details]
> Reduction

I'm no longer able to reproduce the crash with this reduction. I'm also getting a new debug assert with the original testcase:

ASSERTION FAILED: startBlock->firstChild()
../../Source/WebCore/editing/InsertParagraphSeparatorCommand.cpp(275) : virtual void WebCore::InsertParagraphSeparatorCommand::doApply()

Comment 6 Frédéric Wang (:fredw) 2021-05-24 08:31:25 PDT

Created attachment 429534 [details]
tentative patch for attachment 429009 [details]

Comment 7 Frédéric Wang (:fredw) 2021-05-24 08:41:27 PDT

(In reply to Frédéric Wang (:fredw) from comment #5)
> (In reply to Ryosuke Niwa from comment #3)
> > Created attachment 429009 [details]
> > Reduction
> 
> I'm no longer able to reproduce the crash with this reduction.

So there was some subtle thing happening. My editor adds a new line at the end of the tescase, which makes it no longer crash. Anyway, I uploaded a tentative patch to fix attachment 429009 [details], which is essentially the same I had tried for bug 225267  and bug 221387 (tweaking createFragmentFromText does not seem enough here).

> I'm also getting a new debug assert with the original testcase:
> 
> ASSERTION FAILED: startBlock->firstChild()
> ../../Source/WebCore/editing/InsertParagraphSeparatorCommand.cpp(275) :
> virtual void WebCore::InsertParagraphSeparatorCommand::doApply()

This still happens with attachment 429534 [details] applied and we have a AddressSanitizer: SEGV. Still need to debug this one...

Comment 8 Frédéric Wang (:fredw) 2021-05-25 10:24:01 PDT

Created attachment 429664 [details]
Reduce a bit the second issue

Reducing a bit the issue from comment 7.

Comment 9 Frédéric Wang (:fredw) 2021-05-26 03:31:58 PDT

Created attachment 429744 [details]
Reduction (ASSERTION FAILED: startBlock->firstChild())

Comment 10 Frédéric Wang (:fredw) 2021-05-26 10:53:10 PDT

(In reply to Frédéric Wang (:fredw) from comment #5)
> (In reply to Ryosuke Niwa from comment #3)
> > Created attachment 429009 [details]
> > Reduction
> 
> I'm no longer able to reproduce the crash with this reduction. I'm also
> getting a new debug assert with the original testcase:
> 
> ASSERTION FAILED: startBlock->firstChild()
> ../../Source/WebCore/editing/InsertParagraphSeparatorCommand.cpp(275) :
> virtual void WebCore::InsertParagraphSeparatorCommand::doApply()

Preliminary debugging. At https://webkit-search.igalia.com/webkit/rev/59144827bcba50f5e287f957c38137c3bbe4a98e/Source/WebCore/editing/InsertParagraphSeparatorCommand.cpp#201 the state looks like this:

(rr) p showTree(visiblePos)
#document	0x61f00001dc80 (renderer 0x6160003ced80) 
	html	0x60c0002a6a00 (renderer (nil))  (needs style recalc)
	HTML	0x60c0002a6ac0 (renderer 0x61200007da40) 
		HTML	0x60c0002ab5c0 (renderer 0x61200006c4c0) 
*		#text	0x60b0000e91f0 "0"
		DIV	0x60c0002aacc0 (renderer 0x61200006c640) 
		HTML	0x60c0002aab40 (renderer 0x61200006c7c0) 
		HTML	0x60c0002b1d40 (renderer 0x6120000a1440) 
legacy, offset, offset:0
$1 = void
(rr) p isFirstInBlock
$2 = true
(rr) p isLastInBlock
$3 = false

When we hit the assert, startBlock is the inner (empty) HTML element, and so we are not following the comment saying "startBlock should always have children, otherwise isLastInBlock would be true and it's handled above.".

The reason why isFirstInBlock is true is because PositionIterator::isCandidate() skips HTML element. Here is the backtrace:

#0  WebCore::PositionIterator::isCandidate() const (this=0x7ffd3ab7fe60)
    at ../../Source/WebCore/dom/PositionIterator.cpp:176
#1  0x00007fc13932439e in WebCore::nextCandidate(WebCore::Position const&)
    (position=...) at ../../Source/WebCore/editing/Editing.cpp:198
#2  0x00007fc1394b5856 in WebCore::VisiblePosition::canonicalPosition(WebCore::Position const&) (passedPosition=...)
    at ../../Source/WebCore/editing/VisiblePosition.cpp:558
#3  0x00007fc1394aef6f in WebCore::VisiblePosition::VisiblePosition(WebCore::Position const&, WebCore::Affinity)
    (this=0x7ffd3ab80280, position=..., affinity=WebCore::Affinity::Downstream)
    at ../../Source/WebCore/editing/VisiblePosition.cpp:58
#4  0x00007fc1394da026 in WebCore::startOfBlock(WebCore::VisiblePosition const&, WebCore::EditingBoundaryCrossingRule)
    (visiblePosition=..., rule=WebCore::CanCrossEditingBoundary)
    at ../../Source/WebCore/editing/VisibleUnits.cpp:1338
#5  0x00007fc1394da7e2 in WebCore::isStartOfBlock(WebCore::VisiblePosition const&) (pos=...) at ../../Source/WebCore/editing/VisibleUnits.cpp:1357
#6  0x00007fc1393f6c63 in WebCore::InsertParagraphSeparatorCommand::doApply()
    (this=0x6120000a1740)
    at ../../Source/WebCore/editing/InsertParagraphSeparatorCommand.cpp:200

Comment 11 Frédéric Wang (:fredw) 2021-05-27 04:36:11 PDT

Created attachment 429867 [details]
Add an assert for Position's offset

Comment 12 Frédéric Wang (:fredw) 2021-05-27 05:56:06 PDT

Created attachment 429872 [details]
Add an assert for Position's offset

for EWS

Comment 13 Frédéric Wang (:fredw) 2021-05-27 06:56:34 PDT

(In reply to Frédéric Wang (:fredw) from comment #10)
> (rr) p showTree(visiblePos)
> #document	0x61f00001dc80 (renderer 0x6160003ced80) 
> 	html	0x60c0002a6a00 (renderer (nil))  (needs style recalc)
> 	HTML	0x60c0002a6ac0 (renderer 0x61200007da40) 
> 		HTML	0x60c0002ab5c0 (renderer 0x61200006c4c0) 
> *		#text	0x60b0000e91f0 "0"
> 		DIV	0x60c0002aacc0 (renderer 0x61200006c640) 
> 		HTML	0x60c0002aab40 (renderer 0x61200006c7c0) 
> 		HTML	0x60c0002b1d40 (renderer 0x6120000a1440) 

So the fact that the startBlock is the inner HTML but the selection visiblePos on the text sibling is quite confusing. Debugging a bit more (see below), endingSelection() seems wrong to when entering InsertParagraphSeparatorCommand::doApply() : the anchor is the empty inner HTML but the associated OffsetInAnchor is 2. executing backward, this setup happened when the surroundContents function is called. It removes children of the inner HTML element and informs the doc about node removals, but for some reason the selection is not updated properly.

I attached a patch to ASSERT that the m_offset is valid, but this is causing failures.


ASSERTION FAILED: startBlock->firstChild()
../../Source/WebCore/editing/InsertParagraphSeparatorCommand.cpp(275) : virtual void WebCore::InsertParagraphSeparatorCommand::doApply()
(rr)  b InsertParagraphSeparatorCommand::doApply()
(rr) rc
(rr) 
(rr) delete
(rr)  p showTree(endingSelection().m_start)
#document	0x61f00001dc80 (renderer 0x6160003ce480) 
	html	0x60c0002a6940 (renderer (nil))  (needs style recalc)
	HTML	0x60c0002a6a00 (renderer 0x61200007d740) 
*		HTML	0x60c0002ab500 (renderer 0x61200006c1c0) 
		#text	0x60b0000e92a0 "0"
		DIV	0x60c0002aac00 (renderer 0x61200006c340) 
		HTML	0x60c0002aaa80 (renderer 0x61200006c4c0) 
		HTML	0x60c0002b1c80 (renderer 0x6120000a1140) 
offset, offset:2
$1 = void
(rr) watch -l endingSelection().m_start
Hardware watchpoint 3: -location endingSelection().m_start
(rr) rc
(rr) delete
(rr) reverse-f
(rr) 
(rr) bt
#0  0x00007f65de74ea03 in WebCore::EditCommand::EditCommand(WebCore::Document&, WebCore::EditAction) (this=0x6120000a1440, document=..., editingAction=WebCore::EditAction::TypingInsertParagraph)
    at ../../Source/WebCore/editing/EditCommand.cpp:128
#1  0x00007f65e29d508c in WebCore::CompositeEditCommand::CompositeEditCommand(WebCore::Document&, WebCore::EditAction) (this=0x6120000a1440, document=..., editingAction=WebCore::EditAction::TypingInsertParagraph)
    at ../../Source/WebCore/editing/CompositeEditCommand.cpp:335
#2  0x00007f65de820feb in WebCore::InsertParagraphSeparatorCommand::InsertParagraphSeparatorCommand(WebCore::Document&, bool, bool, WebCore::EditAction)
    (this=0x6120000a1440, document=..., mustUseDefaultParagraphElement=false, pasteBlockqutoeIntoUnquotedArea=false, editingAction=WebCore::EditAction::TypingInsertParagraph)
    at ../../Source/WebCore/editing/InsertParagraphSeparatorCommand.cpp:65
#3  0x00007f65de7737c0 in WebCore::InsertParagraphSeparatorCommand::create(WebCore::Document&, bool, bool, WebCore::EditAction)
    (document=..., useDefaultParagraphElement=false, pasteBlockqutoeIntoUnquotedArea=false, editingAction=WebCore::EditAction::TypingInsertParagraph) at ../../Source/WebCore/editing/InsertParagraphSeparatorCommand.h:38
(rr) p showTree(m_document->selection().selection())
#document	0x61f00001dc80 (renderer 0x6160003ce480) 
	html	0x60c0002a6940 (renderer (nil))  (needs style recalc)
	HTML	0x60c0002a6a00 (renderer 0x61200007d740) 
SE		HTML	0x60c0002ab500 (renderer 0x61200006c1c0) 
		#text	0x60b0000e92a0 "0"
		DIV	0x60c0002aac00 (renderer 0x61200006c340) 
		HTML	0x60c0002aaa80 (renderer 0x61200006c4c0) 
		HTML	0x60c0002b1c80 (renderer 0x6120000a1140) 
start: offset, offset:2
end: offset, offset:2
$2 = void
(rr) watch -l m_document->selection().selection().m_start
(rr) rc
(rr) delete
(rr) bt
#0  0x00007f65dd5bb208 in WebCore::Position::operator=(WebCore::Position const&) (this=0x613000082bf8) at ../../Source/WebCore/dom/Position.h:54
#1  0x00007f65de8ec09c in WebCore::VisibleSelection::setWithoutValidation(WebCore::Position const&, WebCore::Position const&) (this=0x613000082bb8, anchor=..., focus=...) at ../../Source/WebCore/editing/VisibleSelection.cpp:467
#2  0x00007f65de7d5985 in WebCore::FrameSelection::respondToNodeModification(WebCore::Node&, bool, bool, bool, bool) (this=0x613000082b80, node=warning: RTTI symbol not found for class 'WebCore::Text'
..., baseRemoved=true, extentRemoved=true, startRemoved=true, endRemoved=true)
    at ../../Source/WebCore/editing/FrameSelection.cpp:560
#3  0x00007f65de7d55cf in WebCore::FrameSelection::nodeWillBeRemoved(WebCore::Node&) (this=0x613000082b80, node=warning: RTTI symbol not found for class 'WebCore::Text'
...) at ../../Source/WebCore/editing/FrameSelection.cpp:541
#4  0x00007f65de27be09 in WebCore::Document::nodeChildrenWillBeRemoved(WebCore::ContainerNode&) (this=0x61f00001dc80, container=...) at ../../Source/WebCore/dom/Document.cpp:4816
#5  0x00007f65de1bc2bc in WebCore::ContainerNode::removeAllChildrenWithScriptAssertion(WebCore::ContainerNode::ChildChange::Source, WebCore::ContainerNode::DeferChildrenChanged)
    (this=0x60c0002ab500, source=WebCore::ContainerNode::ChildChange::Source::API, deferChildrenChanged=WebCore::ContainerNode::DeferChildrenChanged::No) at ../../Source/WebCore/dom/ContainerNode.cpp:120
#6  0x00007f65de1af59c in WebCore::ContainerNode::removeChildren() (this=0x60c0002ab500) at ../../Source/WebCore/dom/ContainerNode.cpp:715
#7  0x00007f65de1aef4f in WebCore::ContainerNode::replaceAll(WebCore::Node*) (this=0x60c0002ab500, node=0x0) at ../../Source/WebCore/dom/ContainerNode.cpp:670
#8  0x00007f65de5a6c0b in WebCore::Range::surroundContents(WebCore::Node&) (this=0x607000103100, newParent=...) at ../../Source/WebCore/dom/Range.cpp:827
(rr) reverse-f
(rr) 
(rr) 
(rr) 
(rr) 
(rr) 
(rr) 
(rr) 
(rr) p showTree(newParent.document().selection().selection())
#document	0x61f00001dc80 (renderer 0x6160003ce480) 
	html	0x60c0002a6940 (renderer (nil))  (needs style recalc)
	HTML	0x60c0002a6a00 (renderer 0x61200007d740) 
		HTML	0x60c0002ab500 (renderer 0x61200006c1c0) 
			svg	0x61300008a0c0 (renderer 0x615000258d00) 
			svg	0x61300008a280 (renderer 0x615000258f80) 
SE			#text	0x60b0000e7a90 "["
		DIV	0x60c0002aac00 (renderer 0x61200006c340) 
		HTML	0x60c0002aaa80 (renderer 0x61200006c4c0) 
start: legacy, offset, offset:0
end: legacy, offset, offset:0
$3 = void
(rr) n
(rr) p showTree(newParent.document().selection().selection())
#document	0x61f00001dc80 (renderer 0x6160003ce480) 
	html	0x60c0002a6940 (renderer (nil))  (needs style recalc)
	HTML	0x60c0002a6a00 (renderer 0x61200007d740) 
SE		HTML	0x60c0002ab500 (renderer 0x61200006c1c0) 
		DIV	0x60c0002aac00 (renderer 0x61200006c340) 
		HTML	0x60c0002aaa80 (renderer 0x61200006c4c0) 
start: offset, offset:2
end: offset, offset:2
$4 = void

Comment 14 Frédéric Wang (:fredw) 2021-06-02 06:31:53 PDT

Created attachment 430341 [details]
Patch for ASSERTION FAILED: startBlock->firstChild()

Comment 15 Frédéric Wang (:fredw) 2021-06-02 06:33:45 PDT

(In reply to Frédéric Wang (:fredw) from comment #13)
> It removes children
> of the inner HTML element and informs the doc about node removals, but for
> some reason the selection is not updated properly.

The FrameSelection::respondToNodeModification does not seem to take into account upcoming removal of children when updating the selection, leaving it in a bad state. I uploaded a patch to work around that.

Comment 16 Frédéric Wang (:fredw) 2021-06-02 06:47:15 PDT

Created attachment 430342 [details]
Patch for ASSERTION FAILED: refNode

Comment 17 Ryosuke Niwa 2021-06-07 15:11:55 PDT

Comment on attachment 430341 [details]
Patch for ASSERTION FAILED: startBlock->firstChild()

Looks like there are real test failures?

Comment 18 Frédéric Wang (:fredw) 2021-06-08 00:20:27 PDT

Comment on attachment 429872 [details]
Add an assert for Position's offset

Not going to take this.

Comment 19 Frédéric Wang (:fredw) 2021-06-08 00:20:47 PDT

Comment on attachment 430342 [details]
Patch for ASSERTION FAILED: refNode

This part is handled by bug 226527.

Comment 20 Ryosuke Niwa 2021-06-09 01:08:49 PDT

Looks like a bunch of tests are failing?

Comment 21 Frédéric Wang (:fredw) 2021-06-09 01:21:57 PDT

Comment on attachment 430341 [details]
Patch for ASSERTION FAILED: startBlock->firstChild()

Yes, sorry I wanted to check this again later. removing review for now.

Comment 22 Frédéric Wang (:fredw) 2021-06-09 05:47:46 PDT

Created attachment 430956 [details]
Patch for ASSERTION FAILED: startBlock->firstChild()

Comment 23 Frédéric Wang (:fredw) 2021-06-15 02:41:47 PDT

Created attachment 431417 [details]
Patch (fix offset)

This fixes the offset, but that actually does not help the bad determination of startBlock/canonicalPos in InsertParagraphSeparatorCommand::doApply()

Comment 24 Frédéric Wang (:fredw) 2021-06-15 06:12:43 PDT

Created attachment 431430 [details]
Tentative patch

Comment 25 Ryosuke Niwa 2021-06-15 14:59:27 PDT

Comment on attachment 431430 [details]
Tentative patch

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

> Source/WebCore/editing/InsertParagraphSeparatorCommand.cpp:192
> +    if (!startBlock->contains(visiblePos.deepEquivalent().containerNode()))
> +        return;
> +

Why do we need to do this?

Comment 26 Frédéric Wang (:fredw) 2021-06-15 22:56:55 PDT

Comment on attachment 431430 [details]
Tentative patch

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

>> Source/WebCore/editing/InsertParagraphSeparatorCommand.cpp:192
>> +
> 
> Why do we need to do this?

Yeah, so let me summarize what's happening since it's a bit complicate. First, below are the values when startBlock/canonicalPos are initialized in InsertParagraphSeparatorCommand::doApply:

(rr) p showTree(insertionPosition)
#document	0x61f00001dc80 (renderer 0x6160003cf080) 
	html	0x60c0002a5d40 (renderer (nil))  (needs style recalc)
	HTML	0x60c0002a5e00 (renderer 0x61200007dbc0) 
*		HTML	0x60c0002aa900 (renderer 0x61200006c640) 
		#text	0x60b0000e8f30 "0"
		DIV	0x60c0002aa000 (renderer 0x61200006c7c0) 
		HTML	0x60c0002a9e80 (renderer 0x61200006c940) 
		HTML	0x60c0002b1080 (renderer 0x6120000916c0) 
offset, offset:2
$1 = void
(rr) p showTree(startBlock)
Cannot resolve function showTree to any overloaded instance
(rr) p showTree((Node*) startBlock)
#document	0x61f00001dc80 (renderer 0x6160003cf080) 
	html	0x60c0002a5d40 (renderer (nil))  (needs style recalc)
	HTML	0x60c0002a5e00 (renderer 0x61200007dbc0) 
*		HTML	0x60c0002aa900 (renderer 0x61200006c640) 
		#text	0x60b0000e8f30 "0"
		DIV	0x60c0002aa000 (renderer 0x61200006c7c0) 
		HTML	0x60c0002a9e80 (renderer 0x61200006c940) 
		HTML	0x60c0002b1080 (renderer 0x6120000916c0) 
$2 = void
(rr) p showTree(canonicalPos)
#document	0x61f00001dc80 (renderer 0x6160003cf080) 
	html	0x60c0002a5d40 (renderer (nil))  (needs style recalc)
	HTML	0x60c0002a5e00 (renderer 0x61200007dbc0) 
		HTML	0x60c0002aa900 (renderer 0x61200006c640) 
*		#text	0x60b0000e8f30 "0"
		DIV	0x60c0002aa000 (renderer 0x61200006c7c0) 
		HTML	0x60c0002a9e80 (renderer 0x61200006c940) 
		HTML	0x60c0002b1080 (renderer 0x6120000916c0) 
legacy, offset, offset:0
$3 = void

Note that insertionPosition is in a bad state (empty <html> anchor node but offset equal to 2). This is explained and fixed by attachment 431417 [details] and I believe we can just rebase the expectation of the failing test if we want to take it, but that's really a separate issue here anyway...

For this testcase, we have the same output for insertionPosition/visiblePos when visiblePos is initialized a few lines below. But the bad state of insertionPosition gets fixed by the upstream/downstream move.

The interesting thing here is that startBlock is on the <html> element but the visible position is on the #text next sibling. The latter happens because Position::isCandidate() -- and a fortiori VisiblePosition::canonicalPosition() and VisiblePosition::VisiblePosition() -- skips <html> elements. This finally leads to the ASSERT failure, as explained in the preliminary debugging of comment 10.

My understanding of the function (reading the apparent assumption of the comments e.g. the one before the ASSERT hit here) is that it is wrong to have the visible position outside the startBlock. So attachment 431430 [details] just performs an early return if that happens and that indeed fixes the issue without breaking any existing test on EWS.

I also tried to fallback to applyCommandToComposite when the startBlock is <html> (since that's leading to special treatment for the calculation of the visible pos and booleans). However, that's causing another ASSERT in that case, IIRC that the position is not editable.

Comment 27 Frédéric Wang (:fredw) 2021-06-16 06:53:21 PDT

Created attachment 431541 [details]
Patch

Here is a real patch with test changelog & test.

Reducing the test was a bit tricky because event_handler_538_DOMNodeRemoved was called several times. It still not perfect, but I was not able to reduce it more.

Comment 28 Frédéric Wang (:fredw) 2021-07-03 06:12:24 PDT

Created attachment 432849 [details]
Patch (attempt to rebuild the dom tree and selection before crash)

For the record, this is the follow-up work I tried to do on the test. I get the following assert messages:

CONSOLE MESSAGE: bad anchorNode: [object Text]
CONSOLE MESSAGE: bad anchorOffset: 0
CONSOLE MESSAGE: bad focusNode: [object Text]
CONSOLE MESSAGE: bad focusOffset: 0

I can build the same DOM tree. As expected, I cannot set the invalid offset 2 but as previously explained that shouldn't matter here. However, I fail to set the anchor/focus on the inner <html> node, as that's the case with complex path with event handlers. Instead a text node is selected. 

So I'm not sure I can reduce this testcase further.

Comment 29 EWS 2021-07-26 13:47:38 PDT

Committed r280312 (239962@main): <https://commits.webkit.org/239962@main>

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