0 WebKit 0x00000001bd8c5304 WebKit::WebPage::positionInformation(WebKit::InteractionInformationRequest const&) + 368 (Optional.h:371) 1 WebKit 0x00000001bd8c5300 WebKit::WebPage::positionInformation(WebKit::InteractionInformationRequest const&) + 364 (WebPageIOS.mm:2517) 2 WebKit 0x00000001bd8c510c WebKit::WebPage::getPositionInformation(WebKit::InteractionInformationRequest const&, WTF::CompletionHandler<void (WebKit::InteractionInformationAtPosition&&)>&&) + 76 (WebPageIOS.mm:2492) 3 WebKit 0x00000001bd9fca78 void IPC::callMemberFunctionImpl<WebKit::WebPage, void (WebKit::WebPage::*)(WebKit::InteractionInformationRequest const&, WTF::CompletionHandler<void (WebKit::InteractionInformationAtPosition&&)>&&), void (WebKit::InteractionInformationAtPosition const&), std::__1::tuple<WebKit::InteractionInformationRequest>, 0ul>(WebKit::WebPage*, void (WebKit::WebPage::*)(WebKit::InteractionInformationRequest const&, WTF::CompletionHandler<void (WebKit::InteractionInformationAtPosition&&)>&&), WTF::CompletionHandler<void (WebKit::InteractionInformationAtPosition const&)>&&, std::__1::tuple<WebKit::InteractionInformationRequest>&&, std::__1::integer_sequence<unsigned long, 0ul>) + 100 (HandleMessage.h:55) 4 WebKit 0x00000001bd9e8654 void IPC::handleMessageSynchronous<Messages::WebPage::GetPositionInformation, WebKit::WebPage, void (WebKit::WebPage::*)(WebKit::InteractionInformationRequest const&, WTF::CompletionHandler<void (WebKit::InteractionInformationAtPosition&&)>&&)>(IPC::Connection&, IPC::Decoder&, std::__1::unique_ptr<IPC::Encoder, std::__1::default_delete<IPC::Encoder> >&, WebKit::WebPage*, void (WebKit::WebPage::*)(WebKit::InteractionInformationRequest const&, WTF::CompletionHandler<void (WebKit::InteractionInformationAtPosition&&)>&&)) + 164 (HandleMessage.h:61) 5 WebKit 0x00000001bd5b9924 IPC::MessageReceiverMap::dispatchSyncMessage(IPC::Connection&, IPC::Decoder&, std::__1::unique_ptr<IPC::Encoder, std::__1::default_delete<IPC::Encoder> >&) + 112 (MessageReceiverMap.cpp:0) 6 WebKit 0x00000001bd904224 WebKit::WebProcess::didReceiveSyncMessage(IPC::Connection&, IPC::Decoder&, std::__1::unique_ptr<IPC::Encoder, std::__1::default_delete<IPC::Encoder> >&) + 40 (WebProcess.cpp:732) 7 WebKit 0x00000001bd5a73f0 IPC::Connection::dispatchSyncMessage(IPC::Decoder&) + 196 (Connection.cpp:905) 8 WebKit 0x00000001bd5a4128 IPC::Connection::dispatchMessage(std::__1::unique_ptr<IPC::Decoder, std::__1::default_delete<IPC::Decoder> >) + 136 (Connection.cpp:1008) 9 WebKit 0x00000001bd5a7944 IPC::Connection::dispatchOneIncomingMessage() + 232 (Connection.cpp:1079) 10 JavaScriptCore 0x00000001c8227534 WTF::RunLoop::performWork() + 276 (Function.h:79) 11 JavaScriptCore 0x00000001c82277f4 WTF::RunLoop::performWork(void*) + 36 (RunLoopCF.cpp:38) <rdar://problem/51597945>
We’re crashing in startPosition of the composition range: 2517 RefPtr<Range> compositionRange = frame.editor().compositionRange(); 2518 if (position < compositionRange->startPosition()) 2519 position = compositionRange->startPosition(); 2520 else if (position > compositionRange->endPosition()) 2521 position = compositionRange->endPosition(); Range’s start Position: const Position startPosition() const { return m_start.toPosition(); } inline const Position RangeBoundaryPoint::toPosition() const { ensureOffsetIsValid(); return createLegacyEditingPosition(m_containerNode.get(), m_offsetInContainer.value()); } inline void RangeBoundaryPoint::ensureOffsetIsValid() const { if (m_offsetInContainer) return; ASSERT(m_childBeforeBoundary); m_offsetInContainer = m_childBeforeBoundary->computeNodeIndex() + 1; } 0x00000001bd8c52f4: ldr x0, [x26, #0xc8] 0x00000001bd8c52f8: sub x8, x29, #0xb8 ; =0xb8 0x00000001bd8c52fc: bl 0x53037c ; symbol stub for: WebCore::Editor::compositionRange() const 0x00000001bd8c5300: ldur x26, [x29, #-0xb8] -> 0x00000001bd8c5304: ldrb w8, [x26, #0x18] 0x00000001bd8c5308: cbz w8, 0x34f314 ; <+384> [inlined] WTF::DumbPtrTraits<WebCore::Node>::unwrap(WebCore::Node* const&) at RefPtr.h:79 0x00000001bd8c530c: ldr w2, [x26, #0x1c] 0x00000001bd8c5310: b 0x34f334 ; <+416> [inlined] WTF::DumbPtrTraits<WebCore::Node>::unwrap(WebCore::Node* const&) at RefPtr.h:71 0x00000001bd8c5314: ldr x0, [x26, #0x20] 0x18 is the location of m_offsetInContainer in Range (24 = 3 pointers down from the address in x26). cbz w8 is std::nullopt check for Optional<unsigned> m_offsetInContainer in RangeBoundaryPoint. This would mean that what’s null is the range object itself. compositionRange() is only called when frame.editor().hasComposition() is true in WebPage::positionInformation. However, Editor::compositionRange() could return nullptr when start >= end: RefPtr<Range> Editor::compositionRange() const { if (!m_compositionNode) return nullptr; unsigned length = m_compositionNode->length(); unsigned start = std::min(m_compositionStart, length); unsigned end = std::min(std::max(start, m_compositionEnd), length); if (start >= end) return nullptr; return Range::create(m_compositionNode->document(), m_compositionNode.get(), start, m_compositionNode.get(), end); }
start >= end condition was introduced when Editor::compositionRange() was initially introduced 12 years ago: https://trac.webkit.org/changeset/25547/webkit
I don't quite understand why compositionRange() needs to return nullptr when start == end. Given we're clamping end to be start, it seems natural that we may want to return a collapsed Range. If this behavior is expected of compositionRange(), then alternatively, we should probably update
(In reply to Ryosuke Niwa from comment #3) > I don't quite understand why compositionRange() needs to return nullptr when > start == end. Given we're clamping end to be start, it seems natural that we > may want to return a collapsed Range. > > If this behavior is expected of compositionRange(), then alternatively, we > should probably update focusedElementPositionInformation to exit early (like when frame.editor().hasComposition() returns false) or create a collapsed range at the start when compositionRange() when compositionRange() is nullptr.
Created attachment 373474 [details] Fixes the bug
Comment on attachment 373474 [details] Fixes the bug View in context: https://bugs.webkit.org/attachment.cgi?id=373474&action=review LGTM, with a build fix for macOS. > Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivate.h:551 > +- (void)_ensurePositionInformationAt:(CGPoint)location completion:(void (^)(void))updateBlock WK_API_AVAILABLE(macos(WK_MAC_TBA), ios(WK_IOS_TBA)); This seems identical to -[WKWebView _requestActivatedElementAtPosition:completionBlock:], except it’s sync instead of async. Do we need to make a sync position info request for this test, or can we have UIScriptController piggy-back off of the existing _requestActivatedElementAtPosition:completionBlock: SPI in WKWebViewPrivate.h?
(In reply to Wenson Hsieh from comment #6) > Comment on attachment 373474 [details] > Fixes the bug > > View in context: > https://bugs.webkit.org/attachment.cgi?id=373474&action=review > > LGTM, with a build fix for macOS. > > > Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivate.h:551 > > +- (void)_ensurePositionInformationAt:(CGPoint)location completion:(void (^)(void))updateBlock WK_API_AVAILABLE(macos(WK_MAC_TBA), ios(WK_IOS_TBA)); > > This seems identical to -[WKWebView > _requestActivatedElementAtPosition:completionBlock:], except it’s sync > instead of async. > > Do we need to make a sync position info request for this test, or can we > have UIScriptController piggy-back off of the existing > _requestActivatedElementAtPosition:completionBlock: SPI in > WKWebViewPrivate.h? Sure, we can do that. It does obtain the activated element information, and I thought it might be better to have an SPI that only updates the position information but it works for this test case so we can do that for now.
Created attachment 373531 [details] Patch
Comment on attachment 373531 [details] Patch Clearing flags on attachment: 373531 Committed r247180: <https://trac.webkit.org/changeset/247180>
All reviewed patches have been landed. Closing bug.