1405771 - Assertion failure: mParent->GetChildAt(Offset()) == mRef->GetNextSibling() in [@ mozilla::HTMLEditor::GetElementOrParentByTagName]

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Tyson Smith [:tsmith]]

Attachment: test_case.html

Assertion failure: mParent->GetChildAt(Offset()) == mRef->GetNextSibling(), at /src/obj-firefox/dist/include/mozilla/RangeBoundary.h:123

#0 mozilla::RangeBoundaryBase<nsCOMPtr<nsINode>, nsCOMPtr<nsIContent> >::GetChildAtOffset() const /src/obj-firefox/dist/include/mozilla/RangeBoundary.h:120:7
#1 mozilla::HTMLEditor::GetElementOrParentByTagName(nsTSubstring<char16_t> const&, nsINode*) /src/editor/libeditor/HTMLEditor.cpp:2239:25
#2 mozilla::HTMLEditor::GetElementOrParentByTagName(nsTSubstring<char16_t> const&, nsIDOMNode*, nsIDOMElement**) /src/editor/libeditor/HTMLEditor.cpp:2312:5
#3 mozilla::HTMLEditor::CheckSelectionStateForAnonymousButtons(nsISelection*) /src/editor/libeditor/HTMLAnonymousNodeEditor.cpp:377:10
#4 mozilla::ResizerSelectionListener::NotifySelectionChanged(nsIDOMDocument*, nsISelection*, short) /src/editor/libeditor/HTMLEditorObjectResizer.cpp:93:19
#5 mozilla::dom::Selection::NotifySelectionListeners() /src/dom/base/Selection.cpp:3894:15
#6 mozilla::dom::Selection::NotifySelectionListeners(bool) /src/dom/base/Selection.cpp:3820:10
#7 nsRange::DoSetRange(mozilla::RangeBoundaryBase<nsINode*, nsIContent*> const&, mozilla::RangeBoundaryBase<nsINode*, nsIContent*> const&, nsINode*, bool) /src/dom/base/nsRange.cpp:1046:16
#8 nsRange::ContentRemoved(nsIDocument*, nsIContent*, nsIContent*, int, nsIContent*) /src/dom/base/nsRange.cpp:781:5
#9 nsNodeUtils::ContentRemoved(nsINode*, nsIContent*, int, nsIContent*) /src/dom/base/nsNodeUtils.cpp:227:3
#10 nsINode::doRemoveChildAt(unsigned int, bool, nsIContent*, nsAttrAndChildArray&) /src/dom/base/nsINode.cpp:1942:5
#11 mozilla::dom::FragmentOrElement::RemoveChildAt(unsigned int, bool) /src/dom/base/FragmentOrElement.cpp:1363:5
#12 nsINode::ReplaceOrInsertBefore(bool, nsINode*, nsINode*, mozilla::ErrorResult&) /src/dom/base/nsINode.cpp:2432:5
#13 mozilla::dom::NodeBinding::replaceChild(JSContext*, JS::Handle<JSObject*>, nsINode*, JSJitMethodCallArgs const&) /src/obj-firefox/dom/bindings/NodeBinding.cpp:955:45
#14 mozilla::dom::GenericBindingMethod(JSContext*, unsigned int, JS::Value*) /src/dom/bindings/BindingUtils.cpp:3053:13
#15 js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) /src/js/src/jscntxtinlines.h:293:15
#16 js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /src/js/src/vm/Interpreter.cpp:495:16
#17 InternalCall(JSContext*, js::AnyInvokeArgs const&) /src/js/src/vm/Interpreter.cpp:540:12
#18 Interpret(JSContext*, js::RunState&) /src/js/src/vm/Interpreter.cpp:3085:18
#19 js::RunScript(JSContext*, js::RunState&) /src/js/src/vm/Interpreter.cpp:435:12
#20 js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /src/js/src/vm/Interpreter.cpp:513:15
#21 InternalCall(JSContext*, js::AnyInvokeArgs const&) /src/js/src/vm/Interpreter.cpp:540:12
#22 js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) /src/js/src/vm/Interpreter.cpp:559:10
#23 JS::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) /src/js/src/jsapi.cpp:2975:12
#24 mozilla::dom::EventHandlerNonNull::Call(JSContext*, JS::Handle<JS::Value>, mozilla::dom::Event&, JS::MutableHandle<JS::Value>, mozilla::ErrorResult&) /src/obj-firefox/dom/bindings/EventHandlerBinding.cpp:260:37
#25 void mozilla::dom::EventHandlerNonNull::Call<nsISupports*>(nsISupports* const&, mozilla::dom::Event&, JS::MutableHandle<JS::Value>, mozilla::ErrorResult&, char const*, mozilla::dom::CallbackObject::ExceptionHandling, JSCompartment*) /src/obj-firefox/dist/include/mozilla/dom/EventHandlerBinding.h:362:12
#26 mozilla::JSEventHandler::HandleEvent(nsIDOMEvent*) /src/dom/events/JSEventHandler.cpp:215:12
#27 mozilla::EventListenerManager::HandleEventSubType(mozilla::EventListenerManager::Listener*, nsIDOMEvent*, mozilla::dom::EventTarget*) /src/dom/events/EventListenerManager.cpp:1112:51
#28 mozilla::EventListenerManager::HandleEventInternal(nsPresContext*, mozilla::WidgetEvent*, nsIDOMEvent**, mozilla::dom::EventTarget*, nsEventStatus*) /src/dom/events/EventListenerManager.cpp:1283:20
#29 mozilla::EventTargetChainItem::HandleEvent(mozilla::EventChainPostVisitor&, mozilla::ELMCreationDetector&) /src/dom/events/EventDispatcher.cpp:313:17
#30 mozilla::EventTargetChainItem::HandleEventTargetChain(nsTArray<mozilla::EventTargetChainItem>&, mozilla::EventChainPostVisitor&, mozilla::EventDispatchingCallback*, mozilla::ELMCreationDetector&) /src/dom/events/EventDispatcher.cpp:462:16
#31 mozilla::EventDispatcher::Dispatch(nsISupports*, nsPresContext*, mozilla::WidgetEvent*, nsIDOMEvent*, nsEventStatus*, mozilla::EventDispatchingCallback*, nsTArray<mozilla::dom::EventTarget*>*) /src/dom/events/EventDispatcher.cpp:822:9
#32 nsDocumentViewer::LoadComplete(nsresult) /src/layout/base/nsDocumentViewer.cpp:1081:7
#33 nsDocShell::EndPageLoad(nsIWebProgress*, nsIChannel*, nsresult) /src/docshell/base/nsDocShell.cpp:7760:21
#34 nsDocShell::OnStateChange(nsIWebProgress*, nsIRequest*, unsigned int, nsresult) /src/docshell/base/nsDocShell.cpp:7558:7
#35 non-virtual thunk to nsDocShell::OnStateChange(nsIWebProgress*, nsIRequest*, unsigned int, nsresult) /src/docshell/base/nsDocShell.cpp:7455:13
#36 nsDocLoader::DoFireOnStateChange(nsIWebProgress*, nsIRequest*, int&, nsresult) /src/uriloader/base/nsDocLoader.cpp:1320:3
#37 nsDocLoader::doStopDocumentLoad(nsIRequest*, nsresult) /src/uriloader/base/nsDocLoader.cpp:861:14
#38 nsDocLoader::DocLoaderIsEmpty(bool) /src/uriloader/base/nsDocLoader.cpp:750:9
#39 nsDocLoader::OnStopRequest(nsIRequest*, nsISupports*, nsresult) /src/uriloader/base/nsDocLoader.cpp:632:5
#40 non-virtual thunk to nsDocLoader::OnStopRequest(nsIRequest*, nsISupports*, nsresult) /src/uriloader/base/nsDocLoader.cpp:488:14
#41 mozilla::net::nsLoadGroup::RemoveRequest(nsIRequest*, nsISupports*, nsresult) /src/netwerk/base/nsLoadGroup.cpp:629:28
#42 nsDocument::DoUnblockOnload() /src/dom/base/nsDocument.cpp:9328:18
#43 nsDocument::UnblockOnload(bool) /src/dom/base/nsDocument.cpp:9250:9
#44 nsDocument::DispatchContentLoadedEvents() /src/dom/base/nsDocument.cpp:5601:3
#45 mozilla::detail::RunnableMethodImpl<nsDocument*, void (nsDocument::*)(), true, (mozilla::RunnableKind)0>::Run() /src/obj-firefox/dist/include/nsThreadUtils.h:1192:13
#46 nsThread::ProcessNextEvent(bool, bool*) /src/xpcom/threads/nsThread.cpp:1039:14
#47 NS_ProcessNextEvent(nsIThread*, bool) /src/xpcom/threads/nsThreadUtils.cpp:524:10
#48 mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /src/ipc/glue/MessagePump.cpp:97:21
#49 MessageLoop::RunInternal() /src/ipc/chromium/src/base/message_loop.cc:326:10
#50 MessageLoop::Run() /src/ipc/chromium/src/base/message_loop.cc:299:3
#51 nsBaseAppShell::Run() /src/widget/nsBaseAppShell.cpp:158:27
#52 nsAppStartup::Run() /src/toolkit/components/startup/nsAppStartup.cpp:288:30
#53 XREMain::XRE_mainRun() /src/toolkit/xre/nsAppRunner.cpp:4701:22
#54 XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) /src/toolkit/xre/nsAppRunner.cpp:4865:8
#55 XRE_main(int, char**, mozilla::BootstrapConfig const&) /src/toolkit/xre/nsAppRunner.cpp:4960:21
#56 do_main(int, char**, char**) /src/browser/app/nsBrowserApp.cpp:231:22
#57 main /src/browser/app/nsBrowserApp.cpp:304:16
#58 __libc_start_main /build/glibc-bfm8X4/glibc-2.23/csu/../csu/libc-start.c:291
#59 _start (/home/user/workspace/browsers/m-c-1507023849-asan-debug/firefox+0x41eaf4)

Comment 1

[Ryan VanderMeulen [:RyanVM]]

INFO: Last good revision: abf8e429f50d041ad747820df96f4e9a671d75c0
INFO: First bad revision: dd7266c2e716480ac13ecf3fd89a9133cbe4ebc7
INFO: Pushlog:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=abf8e429f50d041ad747820df96f4e9a671d75c0&tochange=dd7266c2e716480ac13ecf3fd89a9133cbe4ebc7

Ehsan, can you please take a look?

Comment 2

[(no longer active)]

Certainly!

Comment 3

[(no longer active)]

This bug is quite tricky.  It's more of a DOM bug actually.  Here is what happens.  The assertion occurs under this call stack:

#0  0x00007f1dd579fbd4 in mozilla::RangeBoundaryBase<nsCOMPtr<nsINode>, nsCOMPtr<nsIContent> >::GetChildAtOffset() const (this=0x7f1db7376540)
    at /home/ehsan/moz/src.1347035/obj-ff-dbg/dist/include/mozilla/RangeBoundary.h:123
#1  0x00007f1dd5795e8c in nsRange::GetChildAtStartOffset() const (this=0x7f1db73764c0) at /home/ehsan/moz/src.1347035/dom/base/nsRange.h:123
#2  0x00007f1dd5772427 in mozilla::dom::Selection::GetChildAtAnchorOffset() (this=0x7f1db84fcf40)
    at /home/ehsan/moz/src.1347035/dom/base/Selection.cpp:934
#3  0x00007f1dd7f302e0 in mozilla::HTMLEditor::GetElementOrParentByTagName(nsTSubstring<char16_t> const&, nsINode*) (this=0x7f1db734cc00, aTagName=..., aNode=0x0) at /home/ehsan/moz/src.1347035/editor/libeditor/HTMLEditor.cpp:2239
#4  0x00007f1dd7edf9d3 in mozilla::HTMLEditor::GetElementOrParentByTagName(nsTSubstring<char16_t> const&, nsIDOMNode*, nsIDOMElement**) (this=0x7f1db734cc00, aTagName=..., aNode=0x0, aReturn=0x7ffea63d17e8) at /home/ehsan/moz/src.1347035/editor/libeditor/HTMLEditor.cpp:2312
#5  0x00007f1dd7edec4c in mozilla::HTMLEditor::CheckSelectionStateForAnonymousButtons(nsISelection*) (this=0x7f1db734cc00, aSelection=0x7f1db84fcf40)
    at /home/ehsan/moz/src.1347035/editor/libeditor/HTMLAnonymousNodeEditor.cpp:374
#6  0x00007f1dd7f54585 in mozilla::ResizerSelectionListener::NotifySelectionChanged(nsIDOMDocument*, nsISelection*, short) (this=0x7f1dcda5ef70, aDOMDocument=0x7f1db74f5380, aSelection=0x7f1db84fcf40, aReason=8) at /home/ehsan/moz/src.1347035/editor/libeditor/HTMLEditorObjectResizer.cpp:93
#7  0x00007f1dd577c68e in mozilla::dom::Selection::NotifySelectionListeners() (this=0x7f1db84fcf40)
    at /home/ehsan/moz/src.1347035/dom/base/Selection.cpp:3894
#8  0x00007f1dd577c157 in mozilla::dom::Selection::NotifySelectionListeners(bool) (this=0x7f1db84fcf40, aCalledByJS=false)
    at /home/ehsan/moz/src.1347035/dom/base/Selection.cpp:3820
#9  0x00007f1dd593490f in nsRange::DoSetRange(mozilla::RangeBoundaryBase<nsINode*, nsIContent*> const&, mozilla::RangeBoundaryBase<nsINode*, nsIContent*> const&, nsINode*, bool) (this=0x7f1db7375380, aStart=..., aEnd=..., aRoot=0x7f1db74f5000, aNotInsertedYet=false)
    at /home/ehsan/moz/src.1347035/dom/base/nsRange.cpp:1043
#10 0x00007f1dd59370aa in nsRange::ContentRemoved(nsIDocument*, nsIContent*, nsIContent*, nsIContent*) (this=0x7f1db7375380, aDocument=0x7f1db74f5000, aContainer=0x7f1db75cf550, aChild=0x7f1dcdbc9b80, aPreviousSibling=0x7f1db76c7300) at /home/ehsan/moz/src.1347035/dom/base/nsRange.cpp:778
#11 0x00007f1dd5927620 in nsNodeUtils::ContentRemoved(nsINode*, nsIContent*, nsIContent*) (aContainer=0x7f1db75cf550, aChild=0x7f1dcdbc9b80, aPreviousSibling=0x7f1db76c7300) at /home/ehsan/moz/src.1347035/dom/base/nsNodeUtils.cpp:223
#12 0x00007f1dd58e3469 in nsINode::doRemoveChildAt(unsigned int, bool, nsIContent*, nsAttrAndChildArray&) (this=0x7f1db75cf550, aIndex=1, aNotify=true, aKid=0x7f1dcdbc9b80, aChildArray=...) at /home/ehsan/moz/src.1347035/dom/base/nsINode.cpp:1942
#13 0x00007f1dd5708768 in mozilla::dom::FragmentOrElement::RemoveChildAt(unsigned int, bool) (this=0x7f1db75cf550, aIndex=1, aNotify=true)
    at /home/ehsan/moz/src.1347035/dom/base/FragmentOrElement.cpp:1363
#14 0x00007f1dd58e4964 in nsINode::ReplaceOrInsertBefore(bool, nsINode*, nsINode*, mozilla::ErrorResult&) (this=0x7f1db75cf550, aReplace=true, aNewChild=0x7f1db844b1a0, aRefChild=0x7f1dcdbc9b80, aError=...) at /home/ehsan/moz/src.1347035/dom/base/nsINode.cpp:2432
#15 0x00007f1dd5714d24 in nsINode::ReplaceChild(nsINode&, nsINode&, mozilla::ErrorResult&) (this=0x7f1db75cf550, aNode=..., aChild=..., aError=...)
    at /home/ehsan/moz/src.1347035/dom/base/nsINode.h:1849

Here, we're running the replaceChild() call from the test case.  Note this code: <https://searchfox.org/mozilla-central/rev/7ba03cebf1dcf3b4c40c85a4efad42a5022cf1ec/dom/base/nsRange.cpp#1031>

Here we have a Range which is inside a selection (the one being manipulated by the test case earlier on) which is a mutation observer.  It is calling the selection listeners before some of the other mutation observers have had a chance to run, contrary to what the comment there suggests.  One of those mutation observers that hasn't run yet is the nsRange object which GetChildAtStartOffset() gets called on in the callstack above.  So, when GetChildAtStartOffset() gets called, its mRef is stale, hence the assertion gets triggered.

The right fix seems to run the selection listener off of the event loop, after all mutation observers have had a chance to run.

Comment 4

[(no longer active)]

Running the listeners off of the event loop causes Web observable test failures: https://treeherder.mozilla.org/#/jobs?repo=try&revision=c0e5e57079f23b581b40ec3055213b78c7047a94

Now I'm trying to use script blockers to run the listeners at the end of the mutation observers for Range instead...

Comment 5

[(no longer active)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=97bbc03bd0aad9a6c78f576345d51429e9aa20f9

Comment 6

[(no longer active)]

Attachment: Run the selection listeners after Range mutation observers have finished running to make sure no stale Ranges are observable from the listeners

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/afebcd6c6424
Run the selection listeners after Range mutation observers have finished running to make sure no stale Ranges are observable from the listeners; r=smaug

Comment 8

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/afebcd6c6424