1878191 - Assertion failure: mFrameSelection->GetPresShell()->GetDocument() == content->GetComposedDoc(), at /builds/worker/checkouts/gecko/dom/base/Selection.cpp:1654

Status: VERIFIED FIXED

(Core :: DOM: Selection, defect)

Description

[Tyson Smith [:tsmith]]

Attachment: testcase.html

Found while fuzzing m-c 20240119-1ccd39433268 (--enable-debug --enable-fuzzing)

To reproduce via Grizzly Replay:

$ pip install fuzzfetch grizzly-framework
$ python -m fuzzfetch -d --fuzzing -n firefox
$ python -m grizzly.replay.bugzilla ./firefox/firefox <bugid>

Assertion failure: mFrameSelection->GetPresShell()->GetDocument() == content->GetComposedDoc(), at /builds/worker/checkouts/gecko/dom/base/Selection.cpp:1654

#0 0x7f8aefa4c90c in mozilla::dom::Selection::GetPrimaryFrameForCaretAtFocusNode(bool) const /builds/worker/checkouts/gecko/dom/base/Selection.cpp:1653:3
#1 0x7f8aefa59962 in mozilla::dom::Selection::Modify(nsTSubstring<char16_t> const&, nsTSubstring<char16_t> const&, nsTSubstring<char16_t> const&, mozilla::ErrorResult&) /builds/worker/checkouts/gecko/dom/base/Selection.cpp:3798:7
#2 0x7f8af03f3be1 in mozilla::dom::Selection_Binding::modify(JSContext*, JS::Handle<JSObject*>, void*, JSJitMethodCallArgs const&) /builds/worker/workspace/obj-build/dom/bindings/./SelectionBinding.cpp:1073:24
#3 0x7f8af0dc9a4e in bool mozilla::dom::binding_detail::GenericMethod<mozilla::dom::binding_detail::NormalThisPolicy, mozilla::dom::binding_detail::ThrowExceptions>(JSContext*, unsigned int, JS::Value*) /builds/worker/checkouts/gecko/dom/bindings/BindingUtils.cpp:3258:13
#4 0x7f8af52bd094 in CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), js::CallReason, JS::CallArgs const&) /builds/worker/checkouts/gecko/js/src/vm/Interpreter.cpp:480:13
#5 0x7f8af52bc9eb in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) /builds/worker/checkouts/gecko/js/src/vm/Interpreter.cpp:574:12
#6 0x7f8af52cc308 in CallFromStack /builds/worker/checkouts/gecko/js/src/vm/Interpreter.cpp:646:10
#7 0x7f8af52cc308 in js::Interpret(JSContext*, js::RunState&) /builds/worker/checkouts/gecko/js/src/vm/Interpreter.cpp:3061:16
#8 0x7f8af52bbf72 in js::RunScript(JSContext*, js::RunState&) /builds/worker/checkouts/gecko/js/src/vm/Interpreter.cpp:452:13
#9 0x7f8af52bca08 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) /builds/worker/checkouts/gecko/js/src/vm/Interpreter.cpp:606:13
#10 0x7f8af52bdcbd in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>, js::CallReason) /builds/worker/checkouts/gecko/js/src/vm/Interpreter.cpp:673:8
#11 0x7f8af53b6594 in JS::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) /builds/worker/checkouts/gecko/js/src/vm/CallAndConstruct.cpp:119:10
#12 0x7f8af0b0aa0b in mozilla::dom::EventHandlerNonNull::Call(mozilla::dom::BindingCallContext&, JS::Handle<JS::Value>, mozilla::dom::Event&, JS::MutableHandle<JS::Value>, mozilla::ErrorResult&) /builds/worker/workspace/obj-build/dom/bindings/./EventHandlerBinding.cpp:65:37
#13 0x7f8af145c7d9 in void mozilla::dom::EventHandlerNonNull::Call<nsCOMPtr<mozilla::dom::EventTarget>>(nsCOMPtr<mozilla::dom::EventTarget> const&, mozilla::dom::Event&, JS::MutableHandle<JS::Value>, mozilla::ErrorResult&, char const*, mozilla::dom::CallbackObject::ExceptionHandling, JS::Realm*) /builds/worker/workspace/obj-build/dist/include/mozilla/dom/EventHandlerBinding.h:82:12
#14 0x7f8af145b8a7 in mozilla::JSEventHandler::HandleEvent(mozilla::dom::Event*) /builds/worker/checkouts/gecko/dom/events/JSEventHandler.cpp:199:12
#15 0x7f8af1437df5 in mozilla::EventListenerManager::HandleEventSingleListener(mozilla::EventListenerManager::Listener*, nsAtom*, mozilla::WidgetEvent*, mozilla::dom::Event*, mozilla::dom::EventTarget*, bool) /builds/worker/checkouts/gecko/dom/events/EventListenerManager.cpp:1349:22
#16 0x7f8af1438ef4 in mozilla::EventListenerManager::HandleEventWithListenerArray(mozilla::EventListenerManager::ListenerArray*, nsAtom*, mozilla::EventMessage, nsPresContext*, mozilla::WidgetEvent*, mozilla::dom::Event**, mozilla::dom::EventTarget*, bool) /builds/worker/checkouts/gecko/dom/events/EventListenerManager.cpp:1664:12
#17 0x7f8af1438769 in mozilla::EventListenerManager::HandleEventInternal(nsPresContext*, mozilla::WidgetEvent*, mozilla::dom::Event**, mozilla::dom::EventTarget*, nsEventStatus*, bool) /builds/worker/checkouts/gecko/dom/events/EventListenerManager.cpp:1561:35
#18 0x7f8af142be8f in HandleEvent /builds/worker/workspace/obj-build/dist/include/mozilla/EventListenerManager.h:465:5
#19 0x7f8af142be8f in mozilla::EventTargetChainItem::HandleEvent(mozilla::EventChainPostVisitor&, mozilla::ELMCreationDetector&) /builds/worker/checkouts/gecko/dom/events/EventDispatcher.cpp:364:17
#20 0x7f8af142b55b in mozilla::EventTargetChainItem::HandleEventTargetChain(nsTArray<mozilla::EventTargetChainItem>&, mozilla::EventChainPostVisitor&, mozilla::EventDispatchingCallback*, mozilla::ELMCreationDetector&) /builds/worker/checkouts/gecko/dom/events/EventDispatcher.cpp:642:14
#21 0x7f8af142de46 in mozilla::EventDispatcher::Dispatch(mozilla::dom::EventTarget*, nsPresContext*, mozilla::WidgetEvent*, mozilla::dom::Event*, nsEventStatus*, mozilla::EventDispatchingCallback*, nsTArray<mozilla::dom::EventTarget*>*) /builds/worker/checkouts/gecko/dom/events/EventDispatcher.cpp:1222:11
#22 0x7f8af3580f9e in mozilla::AnimationEventInfo::Dispatch(nsPresContext*) /builds/worker/workspace/obj-build/dist/include/mozilla/AnimationEventDispatcher.h:267:5
#23 0x7f8af357779a in mozilla::AnimationEventDispatcher::DispatchEvents() /builds/worker/workspace/obj-build/dist/include/mozilla/AnimationEventDispatcher.h:298:12
#24 0x7f8af35775c4 in nsRefreshDriver::DispatchAnimationEvents() /builds/worker/checkouts/gecko/layout/base/nsRefreshDriver.cpp:2295:17
#25 0x7f8af3578c03 in nsRefreshDriver::TickObserverArray(unsigned int, mozilla::TimeStamp) /builds/worker/checkouts/gecko/layout/base/nsRefreshDriver.cpp:2481:5
#26 0x7f8af3575a18 in nsRefreshDriver::Tick(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp, nsRefreshDriver::IsExtraTick) /builds/worker/checkouts/gecko/layout/base/nsRefreshDriver.cpp:2736:28
#27 0x7f8af357f3b1 in TickDriver /builds/worker/checkouts/gecko/layout/base/nsRefreshDriver.cpp:367:13
#28 0x7f8af357f3b1 in mozilla::RefreshDriverTimer::TickRefreshDrivers(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp, nsTArray<RefPtr<nsRefreshDriver>>&) /builds/worker/checkouts/gecko/layout/base/nsRefreshDriver.cpp:345:7
#29 0x7f8af357f2b0 in mozilla::RefreshDriverTimer::Tick(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp) /builds/worker/checkouts/gecko/layout/base/nsRefreshDriver.cpp:361:5
#30 0x7f8af357f14d in mozilla::VsyncRefreshDriverTimer::RunRefreshDrivers(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp) /builds/worker/checkouts/gecko/layout/base/nsRefreshDriver.cpp:951:5
#31 0x7f8af357e3ec in mozilla::VsyncRefreshDriverTimer::TickRefreshDriver(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp) /builds/worker/checkouts/gecko/layout/base/nsRefreshDriver.cpp:861:5
#32 0x7f8af357d659 in mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::NotifyVsyncTimerOnMainThread() /builds/worker/checkouts/gecko/layout/base/nsRefreshDriver.cpp:592:14
#33 0x7f8af289bdab in mozilla::dom::VsyncMainChild::RecvNotify(mozilla::VsyncEvent const&, float const&) /builds/worker/checkouts/gecko/dom/ipc/VsyncMainChild.cpp:66:15
#34 0x7f8af2b8bd1d in mozilla::dom::PVsyncChild::OnMessageReceived(IPC::Message const&) /builds/worker/workspace/obj-build/ipc/ipdl/PVsyncChild.cpp:237:78
#35 0x7f8aee9c3ed1 in mozilla::ipc::PBackgroundChild::OnMessageReceived(IPC::Message const&) /builds/worker/workspace/obj-build/ipc/ipdl/PBackgroundChild.cpp:5555:32
#36 0x7f8aee9577cf in mozilla::ipc::MessageChannel::DispatchAsyncMessage(mozilla::ipc::ActorLifecycleProxy*, IPC::Message const&) /builds/worker/checkouts/gecko/ipc/glue/MessageChannel.cpp:1813:25
#37 0x7f8aee954522 in mozilla::ipc::MessageChannel::DispatchMessage(mozilla::ipc::ActorLifecycleProxy*, mozilla::UniquePtr<IPC::Message, mozilla::DefaultDelete<IPC::Message>>) /builds/worker/checkouts/gecko/ipc/glue/MessageChannel.cpp:1732:9
#38 0x7f8aee9551a2 in mozilla::ipc::MessageChannel::RunMessage(mozilla::ipc::ActorLifecycleProxy*, mozilla::ipc::MessageChannel::MessageTask&) /builds/worker/checkouts/gecko/ipc/glue/MessageChannel.cpp:1525:3
#39 0x7f8aee9562ef in mozilla::ipc::MessageChannel::MessageTask::Run() /builds/worker/checkouts/gecko/ipc/glue/MessageChannel.cpp:1623:14
#40 0x7f8aedc65a77 in mozilla::RunnableTask::Run() /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:578:16
#41 0x7f8aedc5b1e6 in mozilla::TaskController::DoExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:905:26
#42 0x7f8aedc599c7 in mozilla::TaskController::ExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:728:15
#43 0x7f8aedc59e45 in mozilla::TaskController::ProcessPendingMTTask(bool) /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:514:36
#44 0x7f8aedc69a89 in operator() /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:235:37
#45 0x7f8aedc69a89 in mozilla::detail::RunnableFunction<mozilla::TaskController::TaskController()::$_1>::Run() /builds/worker/checkouts/gecko/xpcom/threads/nsThreadUtils.h:548:5
#46 0x7f8aedc7ed82 in nsThread::ProcessNextEvent(bool, bool*) /builds/worker/checkouts/gecko/xpcom/threads/nsThread.cpp:1199:16
#47 0x7f8aedc85ecd in NS_ProcessNextEvent(nsIThread*, bool) /builds/worker/checkouts/gecko/xpcom/threads/nsThreadUtils.cpp:480:10
#48 0x7f8aee95d6e3 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /builds/worker/checkouts/gecko/ipc/glue/MessagePump.cpp:107:5
#49 0x7f8aee8779d1 in RunHandler /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:363:3
#50 0x7f8aee8779d1 in MessageLoop::Run() /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:345:3
#51 0x7f8af31afda8 in nsBaseAppShell::Run() /builds/worker/checkouts/gecko/widget/nsBaseAppShell.cpp:148:27
#52 0x7f8af326d3e8 in nsAppShell::Run() /builds/worker/checkouts/gecko/widget/gtk/nsAppShell.cpp:470:33
#53 0x7f8af508009b in XRE_RunAppShell() /builds/worker/checkouts/gecko/toolkit/xre/nsEmbedFunctions.cpp:721:20
#54 0x7f8aee95e616 in mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate*) /builds/worker/checkouts/gecko/ipc/glue/MessagePump.cpp:235:9
#55 0x7f8aee8779d1 in RunHandler /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:363:3
#56 0x7f8aee8779d1 in MessageLoop::Run() /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:345:3
#57 0x7f8af507f902 in XRE_InitChildProcess(int, char**, XREChildData const*) /builds/worker/checkouts/gecko/toolkit/xre/nsEmbedFunctions.cpp:656:34
#58 0x55ace6a073b6 in content_process_main /builds/worker/checkouts/gecko/browser/app/../../ipc/contentproc/plugin-container.cpp:57:28
#59 0x55ace6a073b6 in main /builds/worker/checkouts/gecko/browser/app/nsBrowserApp.cpp:375:18
#60 0x7f8b02429d8f in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
#61 0x7f8b02429e3f in __libc_start_main csu/../csu/libc-start.c:392:3
#62 0x55ace69dd0e8 in _start (/home/user/workspace/browsers/m-c-20240201173838-fuzzing-debug/firefox-bin+0x590e8) (BuildId: 3158970e7ac1bc3dd3d8206f2d8c2427846e3ccf)

Comment 1

[Bugmon [:jkratzer for issues]]

Verified bug as reproducible on mozilla-central 20240201173838-366005a91eda.
The bug appears to have been introduced in the following build range:

Start: 6d9a0abd0a3c26f4337aff723c15795fe91fe884 (20231227091935)
End: 856e86584c4c811f064f93e2b734dd374c787eaf (20231227145854)
Pushlog: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=6d9a0abd0a3c26f4337aff723c15795fe91fe884&tochange=856e86584c4c811f064f93e2b734dd374c787eaf

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1816581

:masayuki, since you are the author of the regressor, bug 1816581, could you take a look? Also, could you set the severity field?

For more information, please visit BugBot documentation.

Comment 3

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Must be able to fix this with a simple patch.

Comment 4

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Oh, looks like that this is a path for bug 1703040.

nsRange allows to update itself into an orphan fragment even if it's associated with Selection. Chrome removes the range from the Selection immediately. I think that we should follow that. However, it means that the loops like:

uint32_t rangeCount = selection->RangeCount();
for (uint32_t i = 0; i < rangeCount; i++) {
  RefPtr<nsRange> range = selection->GetRangeAt(i);
  range->DoSomething(); // May crash if `rangeCount` is larger than 1.
}

become crash or handle something with wrong ranges.

https://searchfox.org/mozilla-central/search?q=Selection%3A%3ARangeCount&path=&case=true&regexp=false

Comment 5

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

It's done here in Chromium:
https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/dom/range.cc;l=98;drc=31e0219771446c6adaf09cb23f34f737079764a2

Comment 6

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1878191 - Make `nsRange` delete itself from `Selection`s if it becomes a range in different root r=smaug!,jjaschke!,#dom-core

Users of Selection assume that nsRange instances are ranges in the
corresponding document. However, Selection may keep storing ranges in
the different root only when the range is updated. I think that no spec
defines this behavior, but Chromium makes RangeUpdateScope delete ranges
which are in different root after the range is updated [1]. Let's follow
this behavior for compatibility and keeping the assumption of Selection
users.

Comment 7

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1878191 - Fix 2 iterating loops of selection ranges r=smaug!,Jamie!

These loops may run script.

In HyperTextAccessible, it calls
RemoveRangeAndUnselectFramesAndNotifyListeners. So, chrome script which is not
directly related to this module may run in each call. I think that using
RemoveAllRanges is better for here. Before bug 1735446, this loop tried
to keep first range, but accidentally, I changed this loop delete all ranges.
However, SetSelectionBoundsAt will add a range if it's required and there is
no bug reports. Therefore, I think keep the new behavior is better.

In nsRange::DeleteFromDocument, the loop may run mutation events too.
Therefore, it needs to store all ranges first. Then, the preceding patch
changes the behavior here if a selection range is moved to different root.
Previously, it was deleted, but now, they are not touched.

Depends on D200606

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cfc39990144c
Make `nsRange` delete itself from `Selection`s if it becomes a range in different root r=smaug,jjaschke,dom-core
https://hg.mozilla.org/integration/autoland/rev/67ca67aed444
Fix 2 iterating loops of selection ranges r=smaug,Jamie

Comment 9

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/44431 for changes under testing/web-platform/tests

Comment 10

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 11

[pstanciu]

https://hg.mozilla.org/mozilla-central/rev/cfc39990144c
https://hg.mozilla.org/mozilla-central/rev/67ca67aed444

Comment 12

[Bugmon [:jkratzer for issues]]

Verified bug as fixed on rev mozilla-central 20240207161531-612d82d4c66a.
Removing bugmon keyword as no further action possible. Please review the bug and re-add the keyword for further analysis.

Comment 13

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

The patch changes Selection API behavior, so I think that it should ride the train unless this causes breaking some web apps.