1377256 - crash at null [@ mozilla::HTMLEditor::DeleteRefToAnonymousNode]

Status: RESOLVED FIXED

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

Description

[Tyson Smith [:tsmith]]

Attachment: test_case.html

Found with Changeset: f3483af8ecf997453064201c49c48a682c7f3c29, Build ID: 20170629155230

==24738==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7f4556e43450 bp 0x7fff28336490 sp 0x7fff283363a0 T0)
==24738==The signal is caused by a READ memory access.
==24738==Hint: address points to the zero page.
    #0 0x7f4556e4344f in Hdr src/obj-firefox/dist/include/nsTArray.h:525:32
    #1 0x7f4556e4344f in Elements src/obj-firefox/dist/include/nsTArray.h:1038
    #2 0x7f4556e4344f in IndexOf<nsIContent *, nsDefaultComparator<mozilla::dom::Element *, nsIContent *> > src/obj-firefox/dist/include/nsTArray.h:1173
    #3 0x7f4556e4344f in RemoveElement<nsIContent *, nsDefaultComparator<mozilla::dom::Element *, nsIContent *> > src/obj-firefox/dist/include/nsTArray.h:1756
    #4 0x7f4556e4344f in RemoveElement<nsIContent *> src/obj-firefox/dist/include/nsTArray.h:1770
    #5 0x7f4556e4344f in mozilla::HTMLEditor::DeleteRefToAnonymousNode(nsIContent*, nsIContent*, nsIPresShell*) src/editor/libeditor/HTMLAnonymousNodeEditor.cpp:321
    #6 0x7f4556e4839d in mozilla::HTMLEditor::HideInlineTableEditingUI() src/editor/libeditor/HTMLInlineTableEditor.cpp:116:3
    #7 0x7f4556ecdd42 in HideAnonymousEditingUIs src/editor/libeditor/HTMLEditor.cpp:185:5
    #8 0x7f4556ecdd42 in mozilla::HTMLEditor::PreDestroy(bool) src/editor/libeditor/HTMLEditor.cpp:341
    #9 0x7f455a420b27 in SetEditor src/docshell/base/nsDocShellEditorData.cpp:116:16
    #10 0x7f455a420b27 in nsDocShell::SetEditor(nsIEditor*) src/docshell/base/nsDocShell.cpp:13121
    #11 0x7f4556fc70d0 in nsEditingSession::TearDownEditorOnWindow(mozIDOMWindowProxy*) src/editor/composer/nsEditingSession.cpp:568:13
    #12 0x7f4555677cb2 in nsHTMLDocument::TurnEditingOff() src/dom/html/nsHTMLDocument.cpp:2688:21
    #13 0x7f45556780c1 in nsHTMLDocument::EditingStateChanged() src/dom/html/nsHTMLDocument.cpp:2735:12
    #14 0x7f455568a9a4 in nsHTMLDocument::MaybeEditingStateChanged() src/dom/html/nsHTMLDocument.cpp:2500:7
    #15 0x7f4553568ad1 in ~mozAutoDocUpdate src/dom/base/mozAutoDocUpdate.h:40:18
    #16 0x7f4553568ad1 in nsINode::doRemoveChildAt(unsigned int, bool, nsIContent*, nsAttrAndChildArray&) src/dom/base/nsINode.cpp:1936
    #17 0x7f45532abe83 in mozilla::dom::FragmentOrElement::RemoveChildAt(unsigned int, bool) src/dom/base/FragmentOrElement.cpp:1242:5
    #18 0x7f455355e1ba in nsINode::RemoveChild(nsINode&, mozilla::ErrorResult&) src/dom/base/nsINode.cpp:606:3
    #19 0x7f4553bc49e1 in mozilla::dom::NodeBinding::removeChild(JSContext*, JS::Handle<JSObject*>, nsINode*, JSJitMethodCallArgs const&) src/obj-firefox/dom/bindings/NodeBinding.cpp:969:45
    #20 0x7f4554eba20e in mozilla::dom::GenericBindingMethod(JSContext*, unsigned int, JS::Value*) src/dom/bindings/BindingUtils.cpp:2960:13
    #21 0x7f455b3b61b3 in CallJSNative src/js/src/jscntxtinlines.h:293:15
    #22 0x7f455b3b61b3 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) src/js/src/vm/Interpreter.cpp:470
    #23 0x7f455b39eeac in CallFromStack src/js/src/vm/Interpreter.cpp:521:12
    #24 0x7f455b39eeac in Interpret(JSContext*, js::RunState&) src/js/src/vm/Interpreter.cpp:3065
    #25 0x7f455b385d38 in js::RunScript(JSContext*, js::RunState&) src/js/src/vm/Interpreter.cpp:410:12
    #26 0x7f455b3b6338 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) src/js/src/vm/Interpreter.cpp:488:15
    #27 0x7f455b3b6b62 in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) src/js/src/vm/Interpreter.cpp:534:10
    #28 0x7f455bd28d5b in JS::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) src/js/src/jsapi.cpp:2948:12
    #29 0x7f4554926eb7 in mozilla::dom::EventListener::HandleEvent(JSContext*, JS::Handle<JS::Value>, mozilla::dom::Event&, mozilla::ErrorResult&) src/obj-firefox/dom/bindings/EventListenerBinding.cpp:47:8
    #30 0x7f45552731ff in HandleEvent<mozilla::dom::EventTarget *> src/obj-firefox/dist/include/mozilla/dom/EventListenerBinding.h:65:12
    #31 0x7f45552731ff in mozilla::EventListenerManager::HandleEventSubType(mozilla::EventListenerManager::Listener*, nsIDOMEvent*, mozilla::dom::EventTarget*) src/dom/events/EventListenerManager.cpp:1139
    #32 0x7f4555275112 in mozilla::EventListenerManager::HandleEventInternal(nsPresContext*, mozilla::WidgetEvent*, nsIDOMEvent**, mozilla::dom::EventTarget*, nsEventStatus*) src/dom/events/EventListenerManager.cpp:1314:20
    #33 0x7f4555255241 in mozilla::EventTargetChainItem::HandleEventTargetChain(nsTArray<mozilla::EventTargetChainItem>&, mozilla::EventChainPostVisitor&, mozilla::EventDispatchingCallback*, mozilla::ELMCreationDetector&) src/dom/events/EventDispatcher.cpp:464:16
    #34 0x7f4555258712 in mozilla::EventDispatcher::Dispatch(nsISupports*, nsPresContext*, mozilla::WidgetEvent*, nsIDOMEvent*, nsEventStatus*, mozilla::EventDispatchingCallback*, nsTArray<mozilla::dom::EventTarget*>*) src/dom/events/EventDispatcher.cpp:824:9
    #35 0x7f4555227ada in mozilla::EventDispatcher::DispatchDOMEvent(nsISupports*, mozilla::WidgetEvent*, nsIDOMEvent*, nsPresContext*, nsEventStatus*) src/dom/events/EventDispatcher.cpp:893:12
    #36 0x7f4553564431 in nsINode::DispatchEvent(nsIDOMEvent*, bool*) src/dom/base/nsINode.cpp:1343:5
    #37 0x7f45530997ca in nsContentUtils::DispatchEvent(nsIDocument*, nsISupports*, nsAString const&, bool, bool, bool, bool*, bool) src/dom/base/nsContentUtils.cpp:4448:18
    #38 0x7f455309958b in nsContentUtils::DispatchTrustedEvent(nsIDocument*, nsISupports*, nsAString const&, bool, bool, bool*) src/dom/base/nsContentUtils.cpp:4416:10
    #39 0x7f455347a870 in nsDocument::DispatchContentLoadedEvents() src/dom/base/nsDocument.cpp:5273:3
    #40 0x7f455353d952 in applyImpl<nsDocument, void (nsDocument::*)()> src/obj-firefox/dist/include/nsThreadUtils.h:1138:12
    #41 0x7f455353d952 in apply<nsDocument, void (nsDocument::*)()> src/obj-firefox/dist/include/nsThreadUtils.h:1144
    #42 0x7f455353d952 in mozilla::detail::RunnableMethodImpl<nsDocument*, void (nsDocument::*)(), true, (mozilla::RunnableKind)0>::Run() src/obj-firefox/dist/include/nsThreadUtils.h:1187
    #43 0x7f45508a9558 in nsThread::ProcessNextEvent(bool, bool*) src/xpcom/threads/nsThread.cpp:1422:14
    #44 0x7f45508af6a8 in NS_ProcessNextEvent(nsIThread*, bool) src/xpcom/threads/nsThreadUtils.cpp:474:10
    #45 0x7f4551688021 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:97:21
    #46 0x7f45515e4a10 in RunInternal src/ipc/chromium/src/base/message_loop.cc:320:10
    #47 0x7f45515e4a10 in RunHandler src/ipc/chromium/src/base/message_loop.cc:313
    #48 0x7f45515e4a10 in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:293
    #49 0x7f4556cbeb9f in nsBaseAppShell::Run() src/widget/nsBaseAppShell.cpp:156:27
    #50 0x7f455ad1a381 in nsAppStartup::Run() src/toolkit/components/startup/nsAppStartup.cpp:287:30
    #51 0x7f455aee9044 in XREMain::XRE_mainRun() src/toolkit/xre/nsAppRunner.cpp:4590:22
    #52 0x7f455aeeabb0 in XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) src/toolkit/xre/nsAppRunner.cpp:4773:8
    #53 0x7f455aeebf01 in XRE_main(int, char**, mozilla::BootstrapConfig const&) src/toolkit/xre/nsAppRunner.cpp:4868:21
    #54 0x4eb613 in do_main src/browser/app/nsBrowserApp.cpp:237:22
    #55 0x4eb613 in main src/browser/app/nsBrowserApp.cpp:310
    #56 0x7f456cfa082f in __libc_start_main /build/glibc-bfm8X4/glibc-2.23/csu/../csu/libc-start.c:291
    #57 0x41d168 in _start (central-asan08465078f3c2/firefox+0x41d168)

Comment 1

[Makoto Kato [:m_kato]]

This seems to be regression by bug 1374999.  Could you look this?

Comment 2

[Cameron McCormack (:heycam)]

Thanks I'll look at this.

Comment 3

[Cameron McCormack (:heycam)]

The problem is that the HTMLEditor's mRootElement, which we use as the parent of some manually created NAC, changes between the time we create the anonymous content (in HTMLEditor::ShowInlineTableEditingUI) and remove it (in HTMLEditor::HideInlineTableEditingUI), because the <body> is removed.

Actually I'm not sure why in HTMLEditor::DeleteRefToAnonymousNode we can't just look at aContent->GetParent(), instead of having aParentContent be passed in.

Comment 4

[Cameron McCormack (:heycam)]

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

Comment 5

[Cameron McCormack (:heycam)]

Attachment: Bug 1377256 - Part 1: Don't pass parent explicitly into HTMLEditor::DeleteRefToAnonymousNode.

Review commit: https://reviewboard.mozilla.org/r/154316/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/154316/

Comment 6

[Cameron McCormack (:heycam)]

Attachment: Bug 1377256 - Part 2: Crashtest.

Review commit: https://reviewboard.mozilla.org/r/154318/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/154318/

Comment 7

[Makoto Kato [:m_kato]]

Comment on attachment 8883415 [details]
Bug 1377256 - Part 1: Don't pass parent explicitly into HTMLEditor::DeleteRefToAnonymousNode.

https://reviewboard.mozilla.org/r/154316/#review159476

Comment 8

[Makoto Kato [:m_kato]]

Comment on attachment 8883416 [details]
Bug 1377256 - Part 2: Crashtest.

https://reviewboard.mozilla.org/r/154318/#review159480

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9709908ae447
Part 1: Don't pass parent explicitly into HTMLEditor::DeleteRefToAnonymousNode. r=m_kato
https://hg.mozilla.org/integration/autoland/rev/58c5df4fd06b
Part 2: Crashtest. r=m_kato

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/9709908ae447
https://hg.mozilla.org/mozilla-central/rev/58c5df4fd06b