Bug 223361

Summary:

Nullopt in DOMSelection::getRangeAt

Product:

WebKit

Reporter:

Ryosuke Niwa <rniwa>

Component:

HTML Editing

Assignee:

Frédéric Wang (:fredw) <fred.wang>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

achristensen, bfulgham, cgarcia, ews-feeder, fred.wang, gpoo, product-security, rbuis, svillar, webkit-bug-importer, wenson_hsieh

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

See Also:

https://bugs.webkit.org/show_bug.cgi?id=221719

Attachments:

Description	Flags
Test	none
Reduced testcase	none
Patch	rniwa: review-
Reduced testcase for SHOULD NEVER BE REACHED	none
Patch	rniwa: review+
Patch for landing	none

Ryosuke Niwa

Reported 2021-03-17 00:54:29 PDT

Created attachment 423446 [details] Test e.g. Thread 0 Crashed:: Dispatch queue: com.apple.main-thread 0 com.apple.WebCore 0x000000028119a3fe WTF::Optional<WebCore::SimpleRange>::operator*() && + 46 (Optional.h:540) 1 com.apple.WebCore 0x0000000284c3cf8a WebCore::DOMSelection::getRangeAt(unsigned int) + 1482 (DOMSelection.cpp:370) 2 com.apple.WebCore 0x0000000280afc6e2 WebCore::jsDOMSelectionPrototypeFunction_getRangeAtBody(JSC::JSGlobalObject*, JSC::CallFrame*, WebCore::JSDOMSelection*) + 546 (JSDOMSelection.cpp:405) 3 com.apple.WebCore 0x0000000280afc41c long long WebCore::IDLOperation<WebCore::JSDOMSelection>::call<&(WebCore::jsDOMSelectionPrototypeFunction_getRangeAtBody(JSC::JSGlobalObject*, JSC::CallFrame*, WebCore::JSDOMSelection*)), (WebCore::CastedThisErrorBehavior)0>(JSC::JSGlobalObject&, JSC::CallFrame&, char const*) + 252 (JSDOMOperation.h:53) 4 com.apple.WebCore 0x0000000280afc229 WebCore::jsDOMSelectionPrototypeFunction_getRangeAt(JSC::JSGlobalObject*, JSC::CallFrame*) + 9 (JSDOMSelection.cpp:410) 5 ??? 0x00003313eb4011d8 0 + 56160644239832 6 com.apple.JavaScriptCore 0x000000029fb48f5a llint_entry + 110482 (LowLevelInterpreter.asm:1093) 7 com.apple.JavaScriptCore 0x000000029fb48db1 llint_entry + 110057 (LowLevelInterpreter.asm:1093) 8 com.apple.JavaScriptCore 0x000000029fb2ddc9 vmEntryToJavaScript + 216 (LowLevelInterpreter64.asm:316) 9 com.apple.JavaScriptCore 0x00000002a13767d2 JSC::JITCode::execute(JSC::VM*, JSC::ProtoCallFrame*) + 258 (JITCodeInlines.h:42) [inlined] 10 com.apple.JavaScriptCore 0x00000002a13767d2 JSC::Interpreter::executeCall(JSC::JSGlobalObject*, JSC::JSObject*, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&) + 1554 (Interpreter.cpp:907) 11 com.apple.JavaScriptCore 0x00000002a1a5fd15 JSC::call(JSC::JSGlobalObject*, JSC::JSValue, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&) + 101 (CallData.cpp:57) 12 com.apple.JavaScriptCore 0x00000002a1a5fe10 JSC::call(JSC::JSGlobalObject*, JSC::JSValue, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&, WTF::NakedPtr<JSC::Exception>&) + 224 (CallData.cpp:64) 13 com.apple.JavaScriptCore 0x00000002a1a601cc JSC::profiledCall(JSC::JSGlobalObject*, JSC::ProfilingReason, JSC::JSValue, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&, WTF::NakedPtr<JSC::Exception>&) + 268 (CallData.cpp:85) 14 com.apple.WebCore 0x000000028332e8a9 WebCore::JSExecState::profiledCall(JSC::JSGlobalObject*, JSC::ProfilingReason, JSC::JSValue, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&, WTF::NakedPtr<JSC::Exception>&) + 233 (JSExecState.h:73) 15 com.apple.WebCore 0x000000028335abab WebCore::JSEventListener::handleEvent(WebCore::ScriptExecutionContext&, WebCore::Event&) + 2731 (JSEventListener.cpp:186) 16 com.apple.WebCore 0x0000000283c68263 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) + 1315 (EventTarget.cpp:344) 17 com.apple.WebCore 0x0000000283c67b03 WebCore::EventTarget::fireEventListeners(WebCore::Event&, WebCore::EventTarget::EventInvokePhase) + 435 (EventTarget.cpp:276) 18 com.apple.WebCore 0x0000000283c36428 WebCore::EventContext::handleLocalEvents(WebCore::Event&, WebCore::EventTarget::EventInvokePhase) const + 856 19 com.apple.WebCore 0x0000000283c37b7d WebCore::dispatchEventInDOM(WebCore::Event&, WebCore::EventPath const&) + 285 (EventDispatcher.cpp:107) 20 com.apple.WebCore 0x0000000283c36fae WebCore::EventDispatcher::dispatchEvent(WebCore::Node&, WebCore::Event&) + 1342 (EventDispatcher.cpp:188) 21 com.apple.WebCore 0x0000000283cf0c49 WebCore::Node::dispatchEvent(WebCore::Event&) + 9 (Node.cpp:2374) 22 com.apple.WebCore 0x00000002840f5334 WebCore::HTMLDetailsElement::dispatchPendingEvent(WebCore::EventSender<WebCore::HTMLDetailsElement>*) + 212 (HTMLDetailsElement.cpp:145) 23 com.apple.WebCore 0x0000000284104fe8 WebCore::EventSender<WebCore::HTMLDetailsElement>::dispatchPendingEvents(WebCore::Page*) + 376 (EventSender.h:106) 24 com.apple.WebCore 0x0000000284104e4b WebCore::EventSender<WebCore::HTMLDetailsElement>::timerFired() + 11 (EventSender.h:54) 25 com.apple.WebCore 0x0000000284106825 decltype(*(std::__1::forward<WebCore::EventSender<WebCore::HTMLDetailsElement>*&>(fp0)).*fp()) std::__1::__invoke<void (WebCore::EventSender<WebCore::HTMLDetailsElement>::*&)(), WebCore::EventSender<WebCore::HTMLDetailsElement>*&, void>(void (WebCore::EventSender<WebCore::HTMLDetailsElement>::*&)(), WebCore::EventSender<WebCore::HTMLDetailsElement>*&) + 133 (type_traits:3486) 26 com.apple.WebCore 0x000000028410679b std::__1::__bind_return<void (WebCore::EventSender<WebCore::HTMLDetailsElement>::*)(), std::__1::tuple<WebCore::EventSender<WebCore::HTMLDetailsElement>*>, std::__1::tuple<>, __is_valid_bind_return<void (WebCore::EventSender<WebCore::HTMLDetailsElement>::*)(), std::__1::tuple<WebCore::EventSender<WebCore::HTMLDetailsElement>*>, std::__1::tuple<> >::value>::type std::__1::__apply_functor<void (WebCore::EventSender<WebCore::HTMLDetailsElement>::*)(), std::__1::tuple<WebCore::EventSender<WebCore::HTMLDetailsElement>*>, 0ul, std::__1::tuple<> >(void (WebCore::EventSender<WebCore::HTMLDetailsElement>::*&)(), std::__1::tuple<WebCore::EventSender<WebCore::HTMLDetailsElement>*>&, std::__1::__tuple_indices<0ul>, std::__1::tuple<>&&) + 43 (functional:2846) 27 com.apple.WebCore 0x0000000284106714 std::__1::__bind_return<void (WebCore::EventSender<WebCore::HTMLDetailsElement>::*)(), std::__1::tuple<WebCore::EventSender<WebCore::HTMLDetailsElement>*>, std::__1::tuple<>, __is_valid_bind_return<void (WebCore::EventSender<WebCore::HTMLDetailsElement>::*)(), std::__1::tuple<WebCore::EventSender<WebCore::HTMLDetailsElement>*>, std::__1::tuple<> >::value>::type std::__1::__bind<void (WebCore::EventSender<WebCore::HTMLDetailsElement>::*&)(), WebCore::EventSender<WebCore::HTMLDetailsElement>*>::operator()<>() + 180 (functional:2879) 28 com.apple.WebCore 0x000000028410662d WTF::Detail::CallableWrapper<std::__1::__bind<void (WebCore::EventSender<WebCore::HTMLDetailsElement>::*&)(), WebCore::EventSender<WebCore::HTMLDetailsElement>*>, void>::call() + 13 (Function.h:52) 29 com.apple.WebCore 0x000000028028f5af WTF::Function<void ()>::operator()() const + 63 (Function.h:83) 30 com.apple.WebCore 0x00000002802d85cd WebCore::Timer::fired() + 13 (Timer.h:136) 31 com.apple.WebCore 0x0000000284fe6465 WebCore::ThreadTimers::sharedTimerFiredInternal() + 933 (ThreadTimers.cpp:127) 32 com.apple.WebCore 0x0000000284fef229 WebCore::ThreadTimers::setSharedTimer(WebCore::SharedTimer*)::$_0::operator()() const + 25 (ThreadTimers.cpp:67) 33 com.apple.WebCore 0x0000000284fef1f9 WTF::Detail::CallableWrapper<WebCore::ThreadTimers::setSharedTimer(WebCore::SharedTimer*)::$_0, void>::call() + 9 (Function.h:52) 34 com.apple.WebCore 0x000000028028f5af WTF::Function<void ()>::operator()() const + 63 (Function.h:83) 35 com.apple.WebCore 0x0000000284f8c7ad WebCore::MainThreadSharedTimer::fired() + 13 (MainThreadSharedTimer.cpp:83) <rdar://75433552>

Attachments
Test (463.97 KB, text/html) 2021-03-17 00:54 PDT, Ryosuke Niwa	no flags	Details
Reduced testcase (354 bytes, text/html) 2021-03-18 07:19 PDT, Frédéric Wang (:fredw)	no flags	Details
Patch (5.36 KB, patch) 2021-03-19 06:25 PDT, Frédéric Wang (:fredw)	rniwa: review-	Details Formatted Diff Diff
Reduced testcase for SHOULD NEVER BE REACHED (149 bytes, text/html) 2021-03-19 06:41 PDT, Frédéric Wang (:fredw)	no flags	Details
Patch (7.37 KB, patch) 2021-03-22 02:33 PDT, Frédéric Wang (:fredw)	rniwa: review+	Details Formatted Diff Diff
Patch for landing (7.36 KB, patch) 2021-03-23 02:36 PDT, Frédéric Wang (:fredw)	no flags	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Ryosuke Niwa

Comment 1 2021-03-17 00:55:00 PDT

I can reproduce with WebKitTestRunner and DumpRenderTree at r274459. This could be a duplicate of the bug 221719 given the root cause analysis there.

Frédéric Wang (:fredw)

Comment 2 2021-03-18 07:19:02 PDT

Created attachment 423597 [details] Reduced testcase Here is a reduced version of the test. > This could be a duplicate of the bug 221719 given the root cause analysis there. The current version of the patch there does not help, but let's analyze this more.

Frédéric Wang (:fredw)

Comment 3 2021-03-19 05:53:06 PDT

It seems the issue is that we are extending the selection toward a pseudo element. Testcase is hitting the following assert in debug: ASSERTION FAILED: !m_anchorNode || !m_anchorNode->isPseudoElement() #0 0x00007f012e2329f6 in WebCore::Position::Position(WebCore::Node*, WebCore::Position::AnchorType) (this=0x7ffea5d8e9e0, anchorNode=0x7f01209d8100, anchorType=WebCore::Position::PositionIsAfterAnchor) at ../../Source/WebCore/dom/Position.cpp:123 #1 0x00007f012daec30f in WebCore::positionAfterNode(WebCore::Node*) (anchorNode=0x7f01209d8100) at ../../Source/WebCore/dom/Position.h:315 #2 0x00007f012e3c6ccc in WebCore::endPositionForLine(WebCore::VisiblePosition const&, WebCore::LineEndpointComputationMode) (c=..., mode=WebCore::UseLogicalOrdering) at ../../Source/WebCore/editing/VisibleUnits.cpp:852 #3 0x00007f012e3c6e9f in WebCore::endOfLine(WebCore::VisiblePosition const&, WebCore::LineEndpointComputationMode, bool*) (c=..., mode=WebCore::UseLogicalOrdering, reachedBoundary=0x0) at ../../Source/WebCore/editing/VisibleUnits.cpp:868 #4 0x00007f012e3c71e6 in WebCore::logicalEndOfLine(WebCore::VisiblePosition const&, bool*) (currentPosition=..., reachedBoundary=0x0) at ../../Source/WebCore/editing/VisibleUnits.cpp:914 #5 0x00007f012e341865 in WebCore::FrameSelection::modifyExtendingForward(WebCore::TextGranularity)

Frédéric Wang (:fredw)

Comment 4 2021-03-19 06:25:36 PDT

Created attachment 423723 [details] Patch This fixes the release crash of attachment 423446 [details] and debug assertion of 423597 ; but there is a separate debug assert reached with the original test.

Frédéric Wang (:fredw)

Comment 5 2021-03-19 06:41:59 PDT

Created attachment 423725 [details] Reduced testcase for SHOULD NEVER BE REACHED #0 WTFCrash() () at ../../Source/WTF/wtf/Assertions.cpp:295 #1 0x00007fa184a0c6e1 in CRASH_WITH_INFO(...) () at DerivedSources/ForwardingHeaders/wtf/Assertions.h:713 #2 0x00007fa188af0b6b in WebCore::RenderElement::visibleInViewportStateChanged() (this=0x7fa17a1fc610) at ../../Source/WebCore/rendering/RenderElement.cpp:1394 #3 0x00007fa188af0b33 in WebCore::RenderElement::setVisibleInViewportState(WebCore::VisibleInViewportState) (this=0x7fa17a1fc610, state=WebCore::VisibleInViewportState::No) at ../../Source/WebCore/rendering/RenderElement.cpp:1389 #4 0x00007fa188cb74dd in WebCore::RenderView::updateVisibleViewportRect(WebCore::IntRect const&) (this=0x7fa17a219270, visibleRect=...) at ../../Source/WebCore/rendering/RenderView.cpp:758 #5 0x00007fa1883dfea0 in operator()(WebCore::FrameView&, WebCore::IntRect const&) const (__closure=0x7fa1132a13d8, frameView=..., visibleRect=...) at ../../Source/WebCore/page/FrameView.cpp:2029 #6 0x00007fa1883f3ba4 in WTF::Detail::CallableWrapper<WebCore::FrameView::viewportContentsChanged()::<lambda(WebCore::FrameView&, const WebCore::IntRect&)>, void, WebCore::FrameView&, const WebCore::IntRect&>::call(WebCore::FrameView &, const WebCore::IntRect &) (this=0x7fa1132a13d0, in#0=..., in#1=...) at DerivedSources/ForwardingHeaders/wtf/Function.h:52 #7 0x00007fa1883fb41b in WTF::Function<void (WebCore::FrameView&, WebCore::IntRect const&)>::operator()(WebCore::FrameView&, WebCore::IntRect const&) const (this=0x7fff30554748, in#0=..., in#1=...) at DerivedSources/ForwardingHeaders/wtf/Function.h:83 #8 0x00007fa1883e26ab in WebCore::FrameView::applyRecursivelyWithVisibleRect(WTF::Function<void (WebCore::FrameView&, WebCore::IntRect const&)> const&) (this=0x7fa17a218010, apply=...) at ../../Source/WebCore/page/FrameView.cpp:2539 #9 0x00007fa1883dff2e in WebCore::FrameView::viewportContentsChanged() (this=0x7fa17a218010) at ../../Source/WebCore/page/FrameView.cpp:2024 #10 0x00007fa1883e50ee in WebCore::FrameView::performPostLayoutTasks() (this=0x7fa17a218010) at ../../Source/WebCore/page/FrameView.cpp:3363 #11 0x00007fa1883f0304 in WebCore::FrameViewLayoutContext::runAsynchronousTasks() (this=0x7fa17a218158) at ../../Source/WebCore/page/FrameViewLayoutContext.cpp:301 #12 0x00007fa1883f0251 in WebCore::FrameViewLayoutContext::runOrScheduleAsynchronousTasks() (this=0x7fa17a218158) at ../../Source/WebCore/page/FrameViewLayoutContext.cpp:287 #13 0x00007fa1883f009d in WebCore::FrameViewLayoutContext::layout() (this=0x7fa17a218158) at ../../Source/WebCore/page/FrameViewLayoutContext.cpp:260 #14 0x00007fa1883f0dc1 in WebCore::FrameViewLayoutContext::layoutTimerFired() (this=0x7fa17a218158) at ../../Source/WebCore/page/FrameViewLayoutContext.cpp:468

Ryosuke Niwa

Comment 6 2021-03-19 12:33:48 PDT

Comment on attachment 423723 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=423723&action=review > Source/WebCore/editing/VisibleUnits.cpp:751 > + if (!startNode) This isn't right. We should be skipping pseudo elements and getting to the real element instead. See the loop right below this code.

Frédéric Wang (:fredw)

Comment 7 2021-03-22 02:33:33 PDT

Created attachment 423865 [details] Patch

Frédéric Wang (:fredw)

Comment 8 2021-03-22 02:33:45 PDT

Comment on attachment 423723 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=423723&action=review >> Source/WebCore/editing/VisibleUnits.cpp:751 >> + if (!startNode) > > This isn't right. We should be skipping pseudo elements and getting to the real element instead. See the loop right below this code. Done.

Ryosuke Niwa

Comment 9 2021-03-22 22:59:30 PDT

Comment on attachment 423865 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=423865&action=review > Source/WebCore/ChangeLog:10 > + release mode. This patches fixes start/endPositionForLine to avoid that issue. Nit: This patch?

Frédéric Wang (:fredw)

Comment 10 2021-03-23 02:36:55 PDT

Created attachment 424000 [details] Patch for landing

EWS

Comment 11 2021-03-23 06:37:49 PDT

Committed r274863: <https://commits.webkit.org/r274863> All reviewed patches have been landed. Closing bug and clearing flags on attachment 424000 [details].

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