Closed Bug 1423583 Opened 2 years ago Closed 2 years ago

Assertion failure: IsInUncomposedDoc() || IsInShadowTree() (This will end badly!), at /home/worker/workspace/build/src/dom/base/nsIContent.h:919

Categories

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

59 Branch
defect
Not set

Tracking

()

RESOLVED DUPLICATE of bug 1419799
Tracking Status
firefox59 --- affected

People

(Reporter: jkratzer, Assigned: emilio)

References

(Blocks 1 open bug)

Details

(Keywords: assertion, testcase)

Attachments

(3 files)

Attached file trigger.html
Testcase found while fuzzing esr52 rev d61516b059c1.

OS|Linux|0.0.0 Linux 4.4.0-98-generic #121-Ubuntu SMP Tue Oct 10 14:24:03 UTC 2017 x86_64
CPU|amd64|family 6 model 78 stepping 3|1
GPU|||
Crash|SIGSEGV|0x0|0
0|0|libxul.so|nsIContent::SetPrimaryFrame|hg:hg.mozilla.org/releases/mozilla-esr52:dom/base/nsIContent.h:d61516b059c1|919|0x0
0|1|libxul.so|nsCSSFrameConstructor::ConstructFrameFromItemInternal|hg:hg.mozilla.org/releases/mozilla-esr52:layout/base/nsCSSFrameConstructor.cpp:d61516b059c1|4099|0x16
0|2|libxul.so|nsCSSFrameConstructor::ConstructFramesFromItem|hg:hg.mozilla.org/releases/mozilla-esr52:layout/base/nsCSSFrameConstructor.cpp:d61516b059c1|6196|0x16
0|3|libxul.so|nsCSSFrameConstructor::ConstructFramesFromItemList|hg:hg.mozilla.org/releases/mozilla-esr52:layout/base/nsCSSFrameConstructor.cpp:d61516b059c1|10641|0x15
0|4|libxul.so|nsCSSFrameConstructor::ContentAppended|hg:hg.mozilla.org/releases/mozilla-esr52:layout/base/nsCSSFrameConstructor.cpp:d61516b059c1|7524|0x5
0|5|libxul.so|PresShell::ContentAppended|hg:hg.mozilla.org/releases/mozilla-esr52:layout/base/nsPresShell.cpp:d61516b059c1|4401|0x14
0|6|libxul.so|nsNodeUtils::ContentAppended|hg:hg.mozilla.org/releases/mozilla-esr52:dom/base/nsNodeUtils.cpp:d61516b059c1|167|0x1c
0|7|libxul.so|nsINode::doInsertChildAt|hg:hg.mozilla.org/releases/mozilla-esr52:dom/base/nsINode.cpp:d61516b059c1|1628|0x8
0|8|libxul.so|nsINode::ReplaceOrInsertBefore|hg:hg.mozilla.org/releases/mozilla-esr52:dom/base/nsINode.cpp:d61516b059c1|2514|0x1b
0|9|libxul.so|mozilla::InsertNodeTransaction::DoTransaction|hg:hg.mozilla.org/releases/mozilla-esr52:dom/base/nsINode.h:d61516b059c1|1850|0x14
0|10|libxul.so|nsTransactionManager::BeginTransaction|hg:hg.mozilla.org/releases/mozilla-esr52:editor/txmgr/nsTransactionManager.cpp:d61516b059c1|661|0x10
0|11|libxul.so|nsTransactionManager::DoTransaction|hg:hg.mozilla.org/releases/mozilla-esr52:editor/txmgr/nsTransactionManager.cpp:d61516b059c1|74|0xd
0|12|libxul.so|mozilla::EditorBase::DoTransaction|hg:hg.mozilla.org/releases/mozilla-esr52:editor/libeditor/EditorBase.cpp:d61516b059c1|735|0x13
0|13|libxul.so|mozilla::EditorBase::InsertNode|hg:hg.mozilla.org/releases/mozilla-esr52:editor/libeditor/EditorBase.cpp:d61516b059c1|1425|0xf
0|14|libxul.so|mozilla::EditorBase::InsertNode|hg:hg.mozilla.org/releases/mozilla-esr52:editor/libeditor/EditorBase.cpp:d61516b059c1|1408|0x24
0|15|libxul.so|mozilla::TextEditRules::CreateBogusNodeIfNeeded|hg:hg.mozilla.org/releases/mozilla-esr52:editor/libeditor/TextEditRules.cpp:d61516b059c1|1242|0x19
0|16|libxul.so|mozilla::TextEditRules::Init|hg:hg.mozilla.org/releases/mozilla-esr52:editor/libeditor/TextEditRules.cpp:d61516b059c1|130|0xc
0|17|libxul.so|mozilla::TextEditor::InitRules|hg:hg.mozilla.org/releases/mozilla-esr52:editor/libeditor/TextEditor.cpp:d61516b059c1|331|0x14
0|18|libxul.so|mozilla::TextEditor::EndEditorInit|hg:hg.mozilla.org/releases/mozilla-esr52:editor/libeditor/TextEditor.cpp:d61516b059c1|207|0xc
0|19|libxul.so|mozilla::AutoEditInitRulesTrigger::~AutoEditInitRulesTrigger|hg:hg.mozilla.org/releases/mozilla-esr52:editor/libeditor/TextEditUtils.cpp:d61516b059c1|109|0xc
0|20|libxul.so|mozilla::TextEditor::Init|hg:hg.mozilla.org/releases/mozilla-esr52:editor/libeditor/TextEditor.cpp:d61516b059c1|138|0x5
0|21|libxul.so|nsTextEditorState::PrepareEditor|hg:hg.mozilla.org/releases/mozilla-esr52:dom/html/nsTextEditorState.cpp:d61516b059c1|1376|0x30
0|22|libxul.so|PrepareEditorEvent::Run|hg:hg.mozilla.org/releases/mozilla-esr52:dom/html/nsTextEditorState.cpp:d61516b059c1|1153|0xb
0|23|libxul.so|nsContentUtils::RemoveScriptBlocker|hg:hg.mozilla.org/releases/mozilla-esr52:dom/base/nsContentUtils.cpp:d61516b059c1|5195|0xe
0|24|libxul.so|nsDocument::EndUpdate|hg:hg.mozilla.org/releases/mozilla-esr52:dom/base/nsDocument.cpp:d61516b059c1|4795|0x5
0|25|libxul.so|nsHTMLDocument::EndUpdate|hg:hg.mozilla.org/releases/mozilla-esr52:dom/html/nsHTMLDocument.cpp:d61516b059c1|2424|0x5
0|26|libxul.so|mozAutoDocUpdate::~mozAutoDocUpdate|hg:hg.mozilla.org/releases/mozilla-esr52:dom/base/mozAutoDocUpdate.h:d61516b059c1|40|0x14
0|27|libxul.so|nsINode::ReplaceOrInsertBefore|hg:hg.mozilla.org/releases/mozilla-esr52:dom/base/nsINode.cpp:d61516b059c1|2384|0xc
0|28|libxul.so|nsINode::Prepend|hg:hg.mozilla.org/releases/mozilla-esr52:dom/base/nsINode.cpp:d61516b059c1|1890|0x19
0|29|libxul.so|mozilla::dom::DocumentFragmentBinding::prepend|hg:hg.mozilla.org/releases/mozilla-esr52:obj-firefox/dom/bindings/DocumentFragmentBinding.cpp:d61516b059c1|316|0x12
0|30|libxul.so|mozilla::dom::GenericBindingMethod|hg:hg.mozilla.org/releases/mozilla-esr52:dom/bindings/BindingUtils.cpp:d61516b059c1|2904|0x9
0|31|libxul.so|js::CallJSNative|hg:hg.mozilla.org/releases/mozilla-esr52:js/src/jscntxtinlines.h:d61516b059c1|239|0x9
0|32|libxul.so|js::InternalCallOrConstruct|hg:hg.mozilla.org/releases/mozilla-esr52:js/src/vm/Interpreter.cpp:d61516b059c1|459|0xf
0|33|libxul.so|Interpret|hg:hg.mozilla.org/releases/mozilla-esr52:js/src/vm/Interpreter.cpp:d61516b059c1|510|0xf
0|34|libxul.so|js::RunScript|hg:hg.mozilla.org/releases/mozilla-esr52:js/src/vm/Interpreter.cpp:d61516b059c1|405|0xb
0|35|libxul.so|js::ExecuteKernel|hg:hg.mozilla.org/releases/mozilla-esr52:js/src/vm/Interpreter.cpp:d61516b059c1|686|0x5
0|36|libxul.so|js::Execute|hg:hg.mozilla.org/releases/mozilla-esr52:js/src/vm/Interpreter.cpp:d61516b059c1|719|0x28
0|37|libxul.so|Evaluate|hg:hg.mozilla.org/releases/mozilla-esr52:js/src/jsapi.cpp:d61516b059c1|4440|0xf
0|38|libxul.so|Evaluate|hg:hg.mozilla.org/releases/mozilla-esr52:js/src/jsapi.cpp:d61516b059c1|4466|0x1d
0|39|libxul.so|nsJSUtils::EvaluateString|hg:hg.mozilla.org/releases/mozilla-esr52:dom/base/nsJSUtils.cpp:d61516b059c1|207|0x1c
0|40|libxul.so|nsJSUtils::EvaluateString|hg:hg.mozilla.org/releases/mozilla-esr52:dom/base/nsJSUtils.cpp:d61516b059c1|275|0x2a
0|41|libxul.so|nsScriptLoader::EvaluateScript|hg:hg.mozilla.org/releases/mozilla-esr52:dom/base/nsScriptLoader.cpp:d61516b059c1|2194|0x21
0|42|libxul.so|nsScriptLoader::ProcessRequest|hg:hg.mozilla.org/releases/mozilla-esr52:dom/base/nsScriptLoader.cpp:d61516b059c1|1979|0xb
0|43|libxul.so|nsScriptLoader::ProcessScriptElement|hg:hg.mozilla.org/releases/mozilla-esr52:dom/base/nsScriptLoader.cpp:d61516b059c1|1712|0xf
0|44|libxul.so|nsScriptElement::MaybeProcessScript|hg:hg.mozilla.org/releases/mozilla-esr52:dom/base/nsScriptElement.cpp:d61516b059c1|149|0x13
0|45|libxul.so|nsIScriptElement::AttemptToExecute|hg:hg.mozilla.org/releases/mozilla-esr52:dom/base/nsIScriptElement.h:d61516b059c1|222|0x3
0|46|libxul.so|nsHtml5TreeOpExecutor::RunScript|hg:hg.mozilla.org/releases/mozilla-esr52:parser/html/nsHtml5TreeOpExecutor.cpp:d61516b059c1|666|0x10
0|47|libxul.so|nsHtml5TreeOpExecutor::RunFlushLoop|hg:hg.mozilla.org/releases/mozilla-esr52:parser/html/nsHtml5TreeOpExecutor.cpp:d61516b059c1|489|0x8
0|48|libxul.so|nsHtml5ExecutorFlusher::Run|hg:hg.mozilla.org/releases/mozilla-esr52:parser/html/nsHtml5StreamParser.cpp:d61516b059c1|128|0x8
0|49|libxul.so|nsThread::ProcessNextEvent|hg:hg.mozilla.org/releases/mozilla-esr52:xpcom/threads/nsThread.cpp:d61516b059c1|1216|0x11
0|50|libxul.so|NS_ProcessNextEvent|hg:hg.mozilla.org/releases/mozilla-esr52:xpcom/glue/nsThreadUtils.cpp:d61516b059c1|361|0xd
0|51|libxul.so|mozilla::ipc::MessagePump::Run|hg:hg.mozilla.org/releases/mozilla-esr52:ipc/glue/MessagePump.cpp:d61516b059c1|96|0xa
0|52|libxul.so|MessageLoop::RunInternal|hg:hg.mozilla.org/releases/mozilla-esr52:ipc/chromium/src/base/message_loop.cc:d61516b059c1|232|0x17
0|53|libxul.so|MessageLoop::Run|hg:hg.mozilla.org/releases/mozilla-esr52:ipc/chromium/src/base/message_loop.cc:d61516b059c1|225|0x8
0|54|libxul.so|nsBaseAppShell::Run|hg:hg.mozilla.org/releases/mozilla-esr52:widget/nsBaseAppShell.cpp:d61516b059c1|156|0xd
0|55|libxul.so|XRE_RunAppShell|hg:hg.mozilla.org/releases/mozilla-esr52:toolkit/xre/nsEmbedFunctions.cpp:d61516b059c1|866|0x6
0|56|libxul.so|mozilla::ipc::MessagePumpForChildProcess::Run|hg:hg.mozilla.org/releases/mozilla-esr52:ipc/glue/MessagePump.cpp:d61516b059c1|269|0x5
0|57|libxul.so|MessageLoop::RunInternal|hg:hg.mozilla.org/releases/mozilla-esr52:ipc/chromium/src/base/message_loop.cc:d61516b059c1|232|0x17
0|58|libxul.so|MessageLoop::Run|hg:hg.mozilla.org/releases/mozilla-esr52:ipc/chromium/src/base/message_loop.cc:d61516b059c1|225|0x8
0|59|libxul.so|XRE_InitChildProcess|hg:hg.mozilla.org/releases/mozilla-esr52:toolkit/xre/nsEmbedFunctions.cpp:d61516b059c1|698|0xf
0|60|plugin-container|content_process_main|hg:hg.mozilla.org/releases/mozilla-esr52:ipc/contentproc/plugin-container.cpp:d61516b059c1|197|0xe
0|61|libc-2.23.so||||0x20830
0|62|plugin-container|MOZ_ReportAssertionFailure|hg:hg.mozilla.org/releases/mozilla-esr52:mfbt/Assertions.h:d61516b059c1|170|0x5
Flags: in-testsuite?
I'm guessing dom.webcomponents.enabled needs to be set to true in about:config first
given the use of createShadowRoot() in the test.
In a local debug build on Linux, I get this fatal assertion:
Assertion failure: aFirstNewContent->GetParentNode() == container, at dom/base/nsRange.cpp:678
with this stack:
nsRange::ContentAppended(nsIDocument*, nsIContent*, nsIContent*)
nsNodeUtils::ContentAppended(nsIContent*, nsIContent*) 
nsINode::doInsertChildAt
mozilla::dom::FragmentOrElement::InsertChildAt
nsINode::ReplaceOrInsertBefore
nsINode::InsertBefore
mozilla::InsertNodeTransaction::DoTransaction()
mozilla::EditorBase::DoTransaction(mozilla::dom::Selection*, nsITransaction*)
[...]

So maybe an Editor / DOM / Selection issue?
Component: Layout → Editor
This is a Shadow DOM bug, actually... The problem is that nsContentUtils::IsInSameAnonymousTree(this, aContent) in HTMLTextareaElement returns true on HTMLTextAreaElement::ContentChanged, thus removing the node that was just inserted.

That is bogus, because the node is definitely in a different anonymous subtree...
Assignee: nobody → emilio
Component: Editor → DOM: Core & HTML
(In reply to Emilio Cobos Álvarez [:emilio] from comment #3)
> This is a Shadow DOM bug, actually... The problem is that
> nsContentUtils::IsInSameAnonymousTree(this, aContent) in HTMLTextareaElement
> returns true on HTMLTextAreaElement::ContentChanged, thus removing the node
> that was just inserted.
> 
> That is bogus, because the node is definitely in a different anonymous
> subtree...

This seems to be similar to Bug 1419799.
(In reply to Adrian Aichner [:anaran] from comment #6)
> (In reply to Emilio Cobos Álvarez [:emilio] from comment #3)
> > This is a Shadow DOM bug, actually... The problem is that
> > nsContentUtils::IsInSameAnonymousTree(this, aContent) in HTMLTextareaElement
> > returns true on HTMLTextAreaElement::ContentChanged, thus removing the node
> > that was just inserted.
> > 
> > That is bogus, because the node is definitely in a different anonymous
> > subtree...
> 
> This seems to be similar to bug 1419803.

Err, indeed it is...

Jessica, I don't mind whether your patch or mine lands. Mine is going to be needed if we move Shadow DOM away from XBL (which I'd like to do).

Otherwise it's pretty much the same. What do you think? Should I just dupe this bug to yours? Or vice versa?

In any case here's a crashtest :)
Flags: needinfo?(jjong)
See Also: → 1419799
Comment on attachment 8935478 [details]
Bug 1423583: Fix IsInSameAnonymousTree in Shadow DOM.

https://reviewboard.mozilla.org/r/206380/#review212110

As of now I think Jessica's patch should be fine. But please let me know if you disagree.

::: dom/base/nsContentUtils.cpp:5462
(Diff revision 1)
>  
> -  const nsIContent* nodeAsContent = static_cast<const nsIContent*>(aNode);
> +  const nsIContent* nodeAsContent = aNode->AsContent();
>  
> -  // For nodes in a shadow tree, it is insufficient to simply compare
> -  // the binding parent because a node may host multiple ShadowRoots,
> -  // thus nodes in different shadow tree may have the same binding parent.
> +  // NOTE(emilio): This check below is not technically required right now, since
> +  // there are no multiple shadow roots per element anymore, but will be if we
> +  // ever move Shadow DOM outside of XBL.

I don't understand the comment "will be if ..". We aren't going to support XBL and Shadow DOM on the same elements.
Attachment #8935478 - Flags: review?(bugs)
(In reply to Olli Pettay [:smaug] (ni?s and f?s processed after r?s) from comment #10)
> Comment on attachment 8935478 [details]
> Bug 1423583: Fix IsInSameAnonymousTree in Shadow DOM.
> 
> https://reviewboard.mozilla.org/r/206380/#review212110
> 
> As of now I think Jessica's patch should be fine. But please let me know if
> you disagree.

Agreed. Just sorry I didn't find it earlier :)

> ::: dom/base/nsContentUtils.cpp:5462
> (Diff revision 1)
> >  
> > -  const nsIContent* nodeAsContent = static_cast<const nsIContent*>(aNode);
> > +  const nsIContent* nodeAsContent = aNode->AsContent();
> >  
> > -  // For nodes in a shadow tree, it is insufficient to simply compare
> > -  // the binding parent because a node may host multiple ShadowRoots,
> > -  // thus nodes in different shadow tree may have the same binding parent.
> > +  // NOTE(emilio): This check below is not technically required right now, since
> > +  // there are no multiple shadow roots per element anymore, but will be if we
> > +  // ever move Shadow DOM outside of XBL.
> 
> I don't understand the comment "will be if ..". We aren't going to support
> XBL and Shadow DOM on the same elements.

Sure, but we probably don't want to return IsInSameAnonymousTree for elements within different shadow roots (and those still have same BindingParent (null) if we stop using XBL).
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1419799
Landed with crashtest.
Flags: needinfo?(jjong)
You need to log in before you can comment on or make changes to this bug.