1601416 - Assertion failure: index >= 0, at /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/RangeBoundary.h:154

Status: NEW

(Core :: DOM: Selection, defect, P2)

Description

[Jason Kratzer [:jkratzer]]

Attachment: testcase.html

Testcase found while fuzzing mozilla-central rev 6989fcd6bab3.

Assertion failure: index >= 0, at /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/RangeBoundary.h:154

rax = 0x0000562a48b0d280   rdx = 0x00007f32c87be85f
rcx = 0x0000000000000b40   rbx = 0x00007f32b9420210
rsi = 0x00007f32d44598b0   rdi = 0x00007f32d4458680
rbp = 0x00007ffee0d6fe00   rsp = 0x00007ffee0d6fde0
r8 = 0x00007f32d44598b0    r9 = 0x00007f32d55c1780
r10 = 0x0000000000000000   r11 = 0x0000000000000000
r12 = 0x00007f32b99c3600   r13 = 0x00007ffee0d6fe74
r14 = 0x0000000000000000   r15 = 0x00007ffee0d6fec0
rip = 0x00007f32c36ea6c6
OS|Linux|0.0.0 Linux 5.0.0-36-generic #39~18.04.1-Ubuntu SMP Tue Nov 12 11:09:50 UTC 2019 x86_64
CPU|amd64|family 6 model 94 stepping 3|8
GPU|||
Crash|SIGSEGV|0x0|0
0|0|libxul.so|mozilla::RangeBoundaryBase<nsCOMPtr<nsINode>, nsCOMPtr<nsIContent> >::Offset() const|hg:hg.mozilla.org/mozilla-central:dom/base/RangeBoundary.h:6989fcd6bab30c909411fbec04a4f29a78024ddd|151|0x3b
0|1|libxul.so|nsCaret::GetFrameAndOffset(mozilla::dom::Selection*, nsINode*, int, int*, nsIFrame**)|hg:hg.mozilla.org/mozilla-central:layout/base/nsCaret.cpp:6989fcd6bab30c909411fbec04a4f29a78024ddd|374|0x22
0|2|libxul.so|nsCaret::SchedulePaint(mozilla::dom::Selection*)|hg:hg.mozilla.org/mozilla-central:layout/base/nsCaret.cpp:6989fcd6bab30c909411fbec04a4f29a78024ddd|416|0x16
0|3|libxul.so|mozilla::EditorBase::FinalizeSelection()|hg:hg.mozilla.org/mozilla-central:editor/libeditor/EditorBase.cpp:6989fcd6bab30c909411fbec04a4f29a78024ddd|4908|0x15
0|4|libxul.so|nsFocusManager::ContentRemoved(mozilla::dom::Document*, nsIContent*)|hg:hg.mozilla.org/mozilla-central:dom/base/nsFocusManager.cpp:6989fcd6bab30c909411fbec04a4f29a78024ddd|815|0x10
0|5|libxul.so|mozilla::EventStateManager::ContentRemoved(mozilla::dom::Document*, nsIContent*)|hg:hg.mozilla.org/mozilla-central:dom/events/EventStateManager.cpp:6989fcd6bab30c909411fbec04a4f29a78024ddd|5600|0x10
0|6|libxul.so|mozilla::PresShell::ContentRemoved(nsIContent*, nsIContent*)|hg:hg.mozilla.org/mozilla-central:layout/base/PresShell.cpp:6989fcd6bab30c909411fbec04a4f29a78024ddd|4291|0x18
0|7|libxul.so|mozilla::dom::MutationObservers::NotifyContentRemoved(nsINode*, nsIContent*, nsIContent*)|hg:hg.mozilla.org/mozilla-central:dom/base/MutationObservers.cpp:6989fcd6bab30c909411fbec04a4f29a78024ddd|216|0x80
0|8|libxul.so|nsINode::RemoveChildNode(nsIContent*, bool)|hg:hg.mozilla.org/mozilla-central:dom/base/nsINode.cpp:6989fcd6bab30c909411fbec04a4f29a78024ddd|1879|0xe
0|9|libxul.so|nsGenericHTMLElement::SetInnerText(nsTSubstring<char16_t> const&)|hg:hg.mozilla.org/mozilla-central:dom/html/nsGenericHTMLElement.cpp:6989fcd6bab30c909411fbec04a4f29a78024ddd|2762|0x12
0|10|libxul.so|mozilla::dom::HTMLElement_Binding::set_innerText|s3:gecko-generated-sources:e7caa97f5ea43184bcd295c4c4dd2adc0833fbe2dde683fd04eaa430808bab438f6849f2ab9ce094e48dc80c9cdbb274626f8863bc4593d2b3f2c1388b6b18df/dom/bindings/HTMLElementBinding.cpp:|323|0xb
0|11|libxul.so|bool mozilla::dom::binding_detail::GenericSetter<mozilla::dom::binding_detail::NormalThisPolicy>(JSContext*, unsigned int, JS::Value*)|hg:hg.mozilla.org/mozilla-central:dom/bindings/BindingUtils.cpp:6989fcd6bab30c909411fbec04a4f29a78024ddd|3101|0x1d
0|12|libxul.so|CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), js::CallReason, JS::CallArgs const&)|hg:hg.mozilla.org/mozilla-central:js/src/vm/Interpreter.cpp:6989fcd6bab30c909411fbec04a4f29a78024ddd|457|0x15
0|13|libxul.so|js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason)|hg:hg.mozilla.org/mozilla-central:js/src/vm/Interpreter.cpp:6989fcd6bab30c909411fbec04a4f29a78024ddd|549|0x12
0|14|libxul.so|InternalCall|hg:hg.mozilla.org/mozilla-central:js/src/vm/Interpreter.cpp:6989fcd6bab30c909411fbec04a4f29a78024ddd|618|0x10
0|15|libxul.so|js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>, js::CallReason)|hg:hg.mozilla.org/mozilla-central:js/src/vm/Interpreter.cpp:6989fcd6bab30c909411fbec04a4f29a78024ddd|635|0x8
0|16|libxul.so|js::CallSetter(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::Handle<JS::Value>)|hg:hg.mozilla.org/mozilla-central:js/src/vm/Interpreter.cpp:6989fcd6bab30c909411fbec04a4f29a78024ddd|773|0x1f
0|17|libxul.so|SetExistingProperty|hg:hg.mozilla.org/mozilla-central:js/src/vm/NativeObject.cpp:6989fcd6bab30c909411fbec04a4f29a78024ddd|2956|0x1a
0|18|libxul.so|bool js::NativeSetProperty<(js::QualifiedBool)1>(JSContext*, JS::Handle<js::NativeObject*>, JS::Handle<JS::PropertyKey>, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::ObjectOpResult&)|hg:hg.mozilla.org/mozilla-central:js/src/vm/NativeObject.cpp:6989fcd6bab30c909411fbec04a4f29a78024ddd|2985|0x2d
0|19|libxul.so|Interpret|hg:hg.mozilla.org/mozilla-central:js/src/vm/Interpreter.cpp:6989fcd6bab30c909411fbec04a4f29a78024ddd|2881|0xb7
0|20|libxul.so|js::RunScript(JSContext*, js::RunState&)|hg:hg.mozilla.org/mozilla-central:js/src/vm/Interpreter.cpp:6989fcd6bab30c909411fbec04a4f29a78024ddd|424|0xb
0|21|libxul.so|js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason)|hg:hg.mozilla.org/mozilla-central:js/src/vm/Interpreter.cpp:6989fcd6bab30c909411fbec04a4f29a78024ddd|590|0xf
0|22|libxul.so|InternalCall|hg:hg.mozilla.org/mozilla-central:js/src/vm/Interpreter.cpp:6989fcd6bab30c909411fbec04a4f29a78024ddd|618|0x10
0|23|libxul.so|js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>, js::CallReason)|hg:hg.mozilla.org/mozilla-central:js/src/vm/Interpreter.cpp:6989fcd6bab30c909411fbec04a4f29a78024ddd|635|0x8
0|24|libxul.so|JS::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>)|hg:hg.mozilla.org/mozilla-central:js/src/jsapi.cpp:6989fcd6bab30c909411fbec04a4f29a78024ddd|2752|0x1f
0|25|libxul.so|mozilla::dom::EventListener::HandleEvent(JSContext*, JS::Handle<JS::Value>, mozilla::dom::Event&, mozilla::ErrorResult&)|s3:gecko-generated-sources:9ca8646d8042e9b4b76d2e1b358b984be17743b71b832c0897d61bb500e0fecbe38fa54273dc522878c87fcb2c9bfd274a8190c7bc56fbbb58cb3ca68462e527/dom/bindings/EventListenerBinding.cpp:|52|0x5
0|26|libxul.so|mozilla::EventListenerManager::HandleEventSubType(mozilla::EventListenerManager::Listener*, mozilla::dom::Event*, mozilla::dom::EventTarget*)|s3:gecko-generated-sources:f3d9c01258576daaac3afc4fb3b283652e7f1168abb5287eff6775451ebd0ab6a0e4c8d88d3a67f7147042501bc091c6dfed25b4b8ccf4e4f420897b8d0ba906/dist/include/mozilla/dom/EventListenerBinding.h:|66|0x1c
0|27|libxul.so|mozilla::EventListenerManager::HandleEventInternal(nsPresContext*, mozilla::WidgetEvent*, mozilla::dom::Event**, mozilla::dom::EventTarget*, nsEventStatus*, bool)|hg:hg.mozilla.org/mozilla-central:dom/events/EventListenerManager.cpp:6989fcd6bab30c909411fbec04a4f29a78024ddd|1270|0x15
0|28|libxul.so|mozilla::EventTargetChainItem::HandleEvent(mozilla::EventChainPostVisitor&, mozilla::ELMCreationDetector&)|hg:hg.mozilla.org/mozilla-central:dom/events/EventDispatcher.cpp:6989fcd6bab30c909411fbec04a4f29a78024ddd|355|0x21
0|29|libxul.so|mozilla::EventTargetChainItem::HandleEventTargetChain(nsTArray<mozilla::EventTargetChainItem>&, mozilla::EventChainPostVisitor&, mozilla::EventDispatchingCallback*, mozilla::ELMCreationDetector&)|hg:hg.mozilla.org/mozilla-central:dom/events/EventDispatcher.cpp:6989fcd6bab30c909411fbec04a4f29a78024ddd|557|0x12
0|30|libxul.so|mozilla::EventDispatcher::Dispatch(nsISupports*, nsPresContext*, mozilla::WidgetEvent*, mozilla::dom::Event*, nsEventStatus*, mozilla::EventDispatchingCallback*, nsTArray<mozilla::dom::EventTarget*>*)|hg:hg.mozilla.org/mozilla-central:dom/events/EventDispatcher.cpp:6989fcd6bab30c909411fbec04a4f29a78024ddd|1055|0x1a
0|31|libxul.so|mozilla::EventDispatcher::DispatchDOMEvent(nsISupports*, mozilla::WidgetEvent*, mozilla::dom::Event*, nsPresContext*, nsEventStatus*)|hg:hg.mozilla.org/mozilla-central:dom/events/EventDispatcher.cpp:6989fcd6bab30c909411fbec04a4f29a78024ddd|1161|0x19
0|32|libxul.so|nsINode::DispatchEvent(mozilla::dom::Event&, mozilla::dom::CallerType, mozilla::ErrorResult&)|hg:hg.mozilla.org/mozilla-central:dom/base/nsINode.cpp:6989fcd6bab30c909411fbec04a4f29a78024ddd|1129|0x5
0|33|libxul.so|nsContentUtils::DispatchEvent(mozilla::dom::Document*, nsISupports*, nsTSubstring<char16_t> const&, mozilla::CanBubble, mozilla::Cancelable, mozilla::Composed, mozilla::Trusted, bool*, mozilla::ChromeOnlyDispatch)|hg:hg.mozilla.org/mozilla-central:dom/base/nsContentUtils.cpp:6989fcd6bab30c909411fbec04a4f29a78024ddd|4013|0x30
0|34|libxul.so|nsContentUtils::DispatchTrustedEvent(mozilla::dom::Document*, nsISupports*, nsTSubstring<char16_t> const&, mozilla::CanBubble, mozilla::Cancelable, mozilla::Composed, bool*)|hg:hg.mozilla.org/mozilla-central:dom/base/nsContentUtils.cpp:6989fcd6bab30c909411fbec04a4f29a78024ddd|3984|0x19
0|35|libxul.so|mozilla::dom::Document::DispatchContentLoadedEvents()|hg:hg.mozilla.org/mozilla-central:dom/base/Document.cpp:6989fcd6bab30c909411fbec04a4f29a78024ddd|7006|0x5
0|36|libxul.so|mozilla::detail::RunnableMethodImpl<mozilla::dom::Document*, void (mozilla::dom::Document::*)(), true, (mozilla::RunnableKind)0>::Run()|hg:hg.mozilla.org/mozilla-central:xpcom/threads/nsThreadUtils.h:6989fcd6bab30c909411fbec04a4f29a78024ddd|1176|0x5
0|37|libxul.so|mozilla::SchedulerGroup::Runnable::Run()|hg:hg.mozilla.org/mozilla-central:xpcom/threads/SchedulerGroup.cpp:6989fcd6bab30c909411fbec04a4f29a78024ddd|295|0x15
0|38|libxul.so|nsThread::ProcessNextEvent(bool, bool*)|hg:hg.mozilla.org/mozilla-central:xpcom/threads/nsThread.cpp:6989fcd6bab30c909411fbec04a4f29a78024ddd|1250|0x15
0|39|libxul.so|NS_ProcessNextEvent(nsIThread*, bool)|hg:hg.mozilla.org/mozilla-central:xpcom/threads/nsThreadUtils.cpp:6989fcd6bab30c909411fbec04a4f29a78024ddd|486|0x11
0|40|libxul.so|mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*)|hg:hg.mozilla.org/mozilla-central:ipc/glue/MessagePump.cpp:6989fcd6bab30c909411fbec04a4f29a78024ddd|88|0xa
0|41|libxul.so|MessageLoop::RunInternal()|hg:hg.mozilla.org/mozilla-central:ipc/chromium/src/base/message_loop.cc:6989fcd6bab30c909411fbec04a4f29a78024ddd|315|0x17
0|42|libxul.so|MessageLoop::Run()|hg:hg.mozilla.org/mozilla-central:ipc/chromium/src/base/message_loop.cc:6989fcd6bab30c909411fbec04a4f29a78024ddd|290|0x8
0|43|libxul.so|nsBaseAppShell::Run()|hg:hg.mozilla.org/mozilla-central:widget/nsBaseAppShell.cpp:6989fcd6bab30c909411fbec04a4f29a78024ddd|137|0xd
0|44|libxul.so|XRE_RunAppShell()|hg:hg.mozilla.org/mozilla-central:toolkit/xre/nsEmbedFunctions.cpp:6989fcd6bab30c909411fbec04a4f29a78024ddd|932|0x11
0|45|libxul.so|mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate*)|hg:hg.mozilla.org/mozilla-central:ipc/glue/MessagePump.cpp:6989fcd6bab30c909411fbec04a4f29a78024ddd|238|0x5
0|46|libxul.so|MessageLoop::RunInternal()|hg:hg.mozilla.org/mozilla-central:ipc/chromium/src/base/message_loop.cc:6989fcd6bab30c909411fbec04a4f29a78024ddd|315|0x17
0|47|libxul.so|MessageLoop::Run()|hg:hg.mozilla.org/mozilla-central:ipc/chromium/src/base/message_loop.cc:6989fcd6bab30c909411fbec04a4f29a78024ddd|290|0x8
0|48|libxul.so|XRE_InitChildProcess(int, char**, XREChildData const*)|hg:hg.mozilla.org/mozilla-central:toolkit/xre/nsEmbedFunctions.cpp:6989fcd6bab30c909411fbec04a4f29a78024ddd|767|0x8
0|49|firefox-bin|content_process_main(mozilla::Bootstrap*, int, char**)|hg:hg.mozilla.org/mozilla-central:ipc/contentproc/plugin-container.cpp:6989fcd6bab30c909411fbec04a4f29a78024ddd|56|0x14
0|50|firefox-bin|main|hg:hg.mozilla.org/mozilla-central:browser/app/nsBrowserApp.cpp:6989fcd6bab30c909411fbec04a4f29a78024ddd|272|0x12
0|51|libc-2.27.so||||0x21b97
0|52|firefox-bin|MOZ_ReportCrash|hg:hg.mozilla.org/mozilla-central:mfbt/Assertions.h:6989fcd6bab30c909411fbec04a4f29a78024ddd|203|0x5

Comment 1

[Emilio Cobos Álvarez (:emilio)]

I think this is related to the assertions Mirko has recently added, but please ni? back if not :)

Comment 2

[Kagami Rosylight [:saschanaz] (they/them) (OOO until Dec 11)]

A broken tree structure during .innerText = ''. A div element has its parent element when the parent element has no child, thus returns -1.

Edit: Somehow the child is alive in focus ref while already removed from the parent.

Comment 3

[Kagami Rosylight [:saschanaz] (they/them) (OOO until Dec 11)]

Attachment: minimal testcase

In this testcase,

1. .replaceChildren() first removes the text node foo and thus invalidating the offset of the second selection range.
2. It then removes the span element, which removes the selection limiter and thus call EditorBase::FinalizeSelection inside nsFocusManager::ContentRemoved
3. The function again calls nsCaret::SetIgnoreUserModify which somehow triggers painting via nsCaret::SchedulePaint()
4. It calls Selection::FocusOffset while the range is not recomputed yet.
5. Thus it still have an invalidated offset and the span element as an outdated reference. It tries getting a new offset against an invalid reference, thus the assertion failure.

I guess the mutation notification should occur first for recomputing selection range: https://phabricator.services.mozilla.com/D95355#inline-541877

Comment 4

[Kagami Rosylight [:saschanaz] (they/them) (OOO until Dec 11)]

Bug 1420547 intentionally made the PresShell notification occur earlier than mutation observer notification. But reversing the order doesn't break anything. Does the previous assumption still apply?

Comment 5

[Emilio Cobos Álvarez (:emilio)]

Yes, it does. In fact your patch does break stuff like here. That try run also doesn't run all reftests/crashtests, I hope/suspect some other tests would break.

We basically rely on the flat tree having the "old" shape before removing from the pres shell, so that we can find all the frames we need to remove. So yes, we fundamentally need to notify the pres shell first on removal.

Comment 6

[Emilio Cobos Álvarez (:emilio)]

(In reply to Kagami :saschanaz from comment #3)
How is the PresShell notification involved here? EventStateManager::ContentRemoved? If so, that also needs the old flat tree shape.

Comment 7

[Emilio Cobos Álvarez (:emilio)]

Ah, I know why this is not so problematic as when bug 1420547 was filed, which is bug 1554498.

Comment 8

[Emilio Cobos Álvarez (:emilio)]

So, basically, bug 1554498 fixed the biggest issue around this code. But as long as there are still ContentRemoved notifications that mutate the DOM, PresShell needs to go first.

Comment 9

[Kagami Rosylight [:saschanaz] (they/them) (OOO until Dec 11)]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #6)

(In reply to Kagami :saschanaz from comment #3)
How is the PresShell notification involved here? EventStateManager::ContentRemoved? If so, that also needs the old flat tree shape.

Yup, that one. BTW, is nsCaret::SchedulePaint supposed to run on the old tree shape? I guess not?

Comment 10

[Emilio Cobos Álvarez (:emilio)]

Well, it generally should be able to deal with it, nsCSSFrameConstructor::ContentRemoved will update the frame tree afterwards. I don't see why SchedulePaint running with the old tree would cause trouble.

Comment 11

[Kagami Rosylight [:saschanaz] (they/them) (OOO until Dec 11)]

It internally calls Selection::FocusOffset which tries recomputing the offset on the DOM tree that is already mutated, while the range information is still assuming the old structure.

Comment 12

[Bugmon [:jkratzer for issues]]

Bugmon Analysis:
Verified bug as reproducible on mozilla-central 20210224215151-69be3221f49a.

Comment 13

[Jens Stutte [:jstutte]]

Hi Kagami, are you still active here?

Comment 14

[Kagami Rosylight [:saschanaz] (they/them) (OOO until Dec 11)]

Unfortunately not, I'm not quite sure what should be done here. I think accessing selection information before processing mutations is invalid, but it seems we can't change the notification order. I wonder we could disable caret painting when notifying PresShell in this case?

Comment 15

[Kagami Rosylight [:saschanaz] (they/them) (OOO until Dec 11)]

Unassigning me for now, I may try again later.