Closed Bug 1405771 Opened 2 years ago Closed 2 years ago

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

Categories

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

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla58
Tracking Status
firefox-esr52 --- unaffected
firefox56 --- unaffected
firefox57 --- unaffected
firefox58 --- fixed

People

(Reporter: tsmith, Assigned: Ehsan)

References

(Blocks 1 open bug)

Details

(Keywords: assertion, regression, testcase)

Attachments

(2 files)

Attached file 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)
Flags: in-testsuite?
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?
Blocks: 1405027
Has Regression Range: --- → yes
Component: Selection → Editor
Flags: needinfo?(ehsan)
Certainly!
Assignee: nobody → ehsan
Flags: needinfo?(ehsan)
Keywords: regression
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.
Component: Editor → DOM
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...
Attachment #8916064 - Flags: review?(bugs) → review+
Pushed by eakhgari@mozilla.com:
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
https://hg.mozilla.org/mozilla-central/rev/afebcd6c6424
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.