Bug 223361 – Nullopt in DOMSelection::getRangeAt

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

[Ryosuke Niwa]

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>

[Ryosuke Niwa]

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)]

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)]

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)]

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)]

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 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)]

Created attachment 423865 [details]
Patch

[Frédéric Wang (:fredw)]

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 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)]

Created attachment 424000 [details]
Patch for landing

[EWS]

Committed r274863: <https://commits.webkit.org/r274863>

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