1029710 - crash in mozilla::dom::ShadowRoot::IsPooledNode(nsIContent*, nsIContent*, nsIContent*)

Status: VERIFIED FIXED

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

Description

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

This bug was filed from the Socorro interface and is 
report bp-4a2d2202-3c02-44ca-a154-4f9102140613.
=============================================================
Crashing Thread
Frame 	Module 	Signature 	Source
0 	libxul.so 	mozilla::dom::ShadowRoot::IsPooledNode(nsIContent*, nsIContent*, nsIContent*) 	/builds/slave/b2g_m-aurora_flame_ntly-000000/build/objdir-gecko/content/base/src/../../../dist/include/nsCOMPtr.h:857
1 	libxul.so 	mozilla::dom::ShadowRoot::AttributeChanged(nsIDocument*, mozilla::dom::Element*, int, nsIAtom*, int) 	content/base/src/ShadowRoot.cpp
2 	libxul.so 	nsNodeUtils::AttributeChanged(mozilla::dom::Element*, int, nsIAtom*, int) 	content/base/src/nsNodeUtils.cpp
3 	libxul.so 	mozilla::dom::Element::SetAttrAndNotify(int, nsIAtom*, nsIAtom*, nsAttrValue const&, nsAttrValue&, unsigned char, bool, bool, bool) 	content/base/src/Element.cpp
4 	libxul.so 	mozilla::dom::Element::SetAttr(int, nsIAtom*, nsIAtom*, nsAString_internal const&, bool) 	content/base/src/Element.cpp
5 	libxul.so 	nsGenericHTMLElement::SetAttr(int, nsIAtom*, nsIAtom*, nsAString_internal const&, bool) 	content/html/content/src/nsGenericHTMLElement.cpp
6 	libxul.so 	mozilla::dom::Element::SetAttr(int, nsIAtom*, nsAString_internal const&, bool) 	content/html/content/src/nsGenericHTMLElement.h
7 	libxul.so 	mozilla::dom::Element::SetAttribute(nsAString_internal const&, nsAString_internal const&, mozilla::ErrorResult&) 	content/base/src/Element.cpp
8 	libxul.so 	mozilla::dom::ElementBinding::setAttribute 	/builds/slave/b2g_m-aurora_flame_ntly-000000/build/objdir-gecko/dom/bindings/ElementBinding.cpp:332
9 	libxul.so 	mozilla::dom::GenericBindingMethod(JSContext*, unsigned int, JS::Value*) 	dom/bindings/BindingUtils.cpp
10 	libxul.so 	js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) 	js/src/jscntxtinlines.h
11 	libxul.so 	Interpret 	js/src/vm/Interpreter.cpp
12 	libxul.so 	js::RunScript 	js/src/vm/Interpreter.cpp
13 	libxul.so 	js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) 	js/src/vm/Interpreter.cpp
14 	libxul.so 	js::Invoke(JSContext*, JS::Value const&, JS::Value const&, unsigned int, JS::Value const*, JS::MutableHandle<JS::Value>) 	js/src/vm/Interpreter.cpp
15 	libxul.so 	JS::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) 	js/src/jsapi.cpp
16 	libxul.so 	mozilla::dom::EventListener::HandleEvent(JSContext*, JS::Handle<JS::Value>, mozilla::dom::Event&, mozilla::ErrorResult&) 	/builds/slave/b2g_m-aurora_flame_ntly-000000/build/objdir-gecko/dom/bindings/EventListenerBinding.cpp:47
17 	libxul.so 	void mozilla::dom::EventListener::HandleEvent<mozilla::dom::EventTarget*>(mozilla::dom::EventTarget* const&, mozilla::dom::Event&, mozilla::ErrorResult&, mozilla::dom::CallbackObject::ExceptionHandling) 	/builds/slave/b2g_m-aurora_flame_ntly-000000/build/objdir-gecko/dom/events/../../dist/include/mozilla/dom/EventListenerBinding.h:54
18 	libxul.so 	mozilla::EventListenerManager::HandleEventSubType(mozilla::EventListenerManager::Listener*, nsIDOMEvent*, mozilla::dom::EventTarget*) 	dom/events/EventListenerManager.cpp
19 	libxul.so 	mozilla::EventListenerManager::HandleEventInternal(nsPresContext*, mozilla::WidgetEvent*, nsIDOMEvent**, mozilla::dom::EventTarget*, nsEventStatus*) 	dom/events/EventListenerManager.cpp
20 	libxul.so 	mozilla::EventTargetChainItem::HandleEvent(mozilla::EventChainPostVisitor&, mozilla::ELMCreationDetector&) 	/builds/slave/b2g_m-aurora_flame_ntly-000000/build/objdir-gecko/dom/events/../../dist/include/mozilla/EventListenerManager.h:328
21 	libxul.so 	mozilla::EventTargetChainItem::HandleEventTargetChain(nsTArray<mozilla::EventTargetChainItem>&, mozilla::EventChainPostVisitor&, mozilla::EventDispatchingCallback*, mozilla::ELMCreationDetector&) 	dom/events/EventDispatcher.cpp
22 	libxul.so 	mozilla::EventDispatcher::Dispatch(nsISupports*, nsPresContext*, mozilla::WidgetEvent*, nsIDOMEvent*, nsEventStatus*, mozilla::EventDispatchingCallback*, nsCOMArray<mozilla::dom::EventTarget>*) 	dom/events/EventDispatcher.cpp
23 	libxul.so 	PresShell::HandleDOMEventWithTarget(nsIContent*, mozilla::WidgetEvent*, nsEventStatus*) 	layout/base/nsPresShell.cpp
24 	libxul.so 	mozilla::dom::HTMLButtonElement::PostHandleEvent(mozilla::EventChainPostVisitor&) 	content/html/content/src/HTMLButtonElement.cpp
25 	libxul.so 	mozilla::EventTargetChainItem::PostHandleEvent(mozilla::EventChainPostVisitor&) 	dom/events/EventDispatcher.cpp
26 	libxul.so 	mozilla::EventTargetChainItem::HandleEventTargetChain(nsTArray<mozilla::EventTargetChainItem>&, mozilla::EventChainPostVisitor&, mozilla::EventDispatchingCallback*, mozilla::ELMCreationDetector&) 	dom/events/EventDispatcher.cpp
27 	libxul.so 	mozilla::EventTargetChainItem::HandleEventTargetChain(nsTArray<mozilla::EventTargetChainItem>&, mozilla::EventChainPostVisitor&, mozilla::EventDispatchingCallback*, mozilla::ELMCreationDetector&) 	dom/events/EventDispatcher.cpp
28 	libxul.so 	mozilla::EventDispatcher::Dispatch(nsISupports*, nsPresContext*, mozilla::WidgetEvent*, nsIDOMEvent*, nsEventStatus*, mozilla::EventDispatchingCallback*, nsCOMArray<mozilla::dom::EventTarget>*) 	dom/events/EventDispatcher.cpp
29 	libxul.so 	PresShell::HandleDOMEventWithTarget(nsIContent*, mozilla::WidgetEvent*, nsEventStatus*) 	layout/base/nsPresShell.cpp
30 	libxul.so 	mozilla::dom::HTMLInputElement::MaybeSubmitForm(nsPresContext*) 	content/html/content/src/HTMLInputElement.cpp
31 	libxul.so 	mozilla::dom::HTMLInputElement::PostHandleEvent(mozilla::EventChainPostVisitor&) 	content/html/content/src/HTMLInputElement.cpp
32 	libxul.so 	mozilla::EventTargetChainItem::PostHandleEvent(mozilla::EventChainPostVisitor&) 	dom/events/EventDispatcher.cpp
33 	libxul.so 	mozilla::EventTargetChainItem::HandleEventTargetChain(nsTArray<mozilla::EventTargetChainItem>&, mozilla::EventChainPostVisitor&, mozilla::EventDispatchingCallback*, mozilla::ELMCreationDetector&) 	dom/events/EventDispatcher.cpp
34 	libxul.so 	mozilla::EventTargetChainItem::HandleEventTargetChain(nsTArray<mozilla::EventTargetChainItem>&, mozilla::EventChainPostVisitor&, mozilla::EventDispatchingCallback*, mozilla::ELMCreationDetector&) 	dom/events/EventDispatcher.cpp
35 	libxul.so 	mozilla::EventDispatcher::Dispatch(nsISupports*, nsPresContext*, mozilla::WidgetEvent*, nsIDOMEvent*, nsEventStatus*, mozilla::EventDispatchingCallback*, nsCOMArray<mozilla::dom::EventTarget>*) 	dom/events/EventDispatcher.cpp
36 	libxul.so 	PresShell::HandleEventInternal(mozilla::WidgetEvent*, nsEventStatus*) 	layout/base/nsPresShell.cpp
37 	libxul.so 	PresShell::HandleEvent(nsIFrame*, mozilla::WidgetGUIEvent*, bool, nsEventStatus*) 	layout/base/nsPresShell.cpp
38 	libxul.so 	nsViewManager::DispatchEvent(mozilla::WidgetGUIEvent*, nsView*, nsEventStatus*) 	view/src/nsViewManager.cpp
39 	libxul.so 	nsView::HandleEvent(mozilla::WidgetGUIEvent*, bool) 	view/src/nsView.cpp
40 	libxul.so 	mozilla::widget::PuppetWidget::DispatchEvent(mozilla::WidgetGUIEvent*, nsEventStatus&) 	widget/xpwidgets/PuppetWidget.cpp
41 	libxul.so 	nsDOMWindowUtils::SendKeyEvent(nsAString_internal const&, int, int, int, unsigned int, bool*) 	dom/base/nsDOMWindowUtils.cpp
42 	libxul.so 	NS_InvokeByIndex 	xpcom/reflect/xptcall/src/md/unix/xptcinvoke_arm.cpp
43 	libxul.so 	XPCWrappedNative::CallMethod(XPCCallContext&, XPCWrappedNative::CallMode) 	js/xpconnect/src/XPCWrappedNative.cpp
44 	libxul.so 	XPC_WN_CallMethod(JSContext*, unsigned int, JS::Value*) 	js/xpconnect/src/XPCWrappedNativeJSOps.cpp
45 	libxul.so 	js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) 	js/src/jscntxtinlines.h
46 	libxul.so 	Interpret 	js/src/vm/Interpreter.cpp
47 	libxul.so 	js::RunScript 	js/src/vm/Interpreter.cpp
48 	libxul.so 	js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) 	js/src/vm/Interpreter.cpp
49 	libxul.so 	js::Invoke(JSContext*, JS::Value const&, JS::Value const&, unsigned int, JS::Value const*, JS::MutableHandle<JS::Value>) 	js/src/vm/Interpreter.cpp
50 	libxul.so 	JS_CallFunctionValue(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) 	js/src/jsapi.cpp
51 	libxul.so 	nsFrameMessageManager::ReceiveMessage(nsISupports*, nsAString_internal const&, bool, mozilla::dom::StructuredCloneData const*, CpowHolder*, nsIPrincipal*, nsTArray<nsString>*) 	content/base/src/nsFrameMessageManager.cpp
52 	libxul.so 	mozilla::dom::TabChild::RecvAsyncMessage(nsString const&, mozilla::dom::ClonedMessageData const&, nsTArray<mozilla::jsipc::CpowEntry> const&, IPC::Principal const&) 	dom/ipc/TabChild.cpp
53 	libxul.so 	mozilla::dom::PBrowserChild::OnMessageReceived(IPC::Message const&) 	/builds/slave/b2g_m-aurora_flame_ntly-000000/build/objdir-gecko/ipc/ipdl/PBrowserChild.cpp:1826
54 	libxul.so 	mozilla::dom::PContentChild::OnMessageReceived(IPC::Message const&) 	/builds/slave/b2g_m-aurora_flame_ntly-000000/build/objdir-gecko/ipc/ipdl/PContentChild.cpp:3533
55 	libxul.so 	mozilla::ipc::MessageChannel::DispatchAsyncMessage(IPC::Message const&) 	ipc/glue/MessageChannel.cpp
56 	libxul.so 	mozilla::ipc::MessageChannel::OnMaybeDequeueOne() 	ipc/glue/MessageChannel.cpp
57 	libxul.so 	RunnableMethod<FdWatcher, void (FdWatcher::*)(), Tuple0>::Run() 	ipc/chromium/src/base/tuple.h
58 	libxul.so 	mozilla::ipc::MessageChannel::DequeueTask::Run() 	/builds/slave/b2g_m-aurora_flame_ntly-000000/build/objdir-gecko/ipc/glue/../../dist/include/mozilla/ipc/MessageChannel.h:385
59 	libxul.so 	MessageLoop::RunTask(Task*) 	ipc/chromium/src/base/message_loop.cc
60 	libxul.so 	MessageLoop::DeferOrRunPendingTask(MessageLoop::PendingTask const&) 	ipc/chromium/src/base/message_loop.cc
61 	libxul.so 	MessageLoop::DoWork() 	ipc/chromium/src/base/message_loop.cc
62 	libxul.so 	mozilla::ipc::DoWorkRunnable::Run() 	ipc/glue/MessagePump.cpp
63 	libxul.so 	nsThread::ProcessNextEvent(bool, bool*) 	xpcom/threads/nsThread.cpp
64 	libxul.so 	NS_ProcessNextEvent(nsIThread*, bool) 	xpcom/glue/nsThreadUtils.cpp
65 	libxul.so 	mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) 	ipc/glue/MessagePump.cpp
66 	libxul.so 	MessageLoop::RunInternal() 	ipc/chromium/src/base/message_loop.cc
67 	libxul.so 	MessageLoop::Run() 	ipc/chromium/src/base/message_loop.cc
68 	libxul.so 	nsBaseAppShell::Run() 	widget/xpwidgets/nsBaseAppShell.cpp
69 	libxul.so 	XRE_RunAppShell 	toolkit/xre/nsEmbedFunctions.cpp
70 	libxul.so 	MessageLoop::RunInternal() 	ipc/chromium/src/base/message_loop.cc
71 	libxul.so 	MessageLoop::Run() 	ipc/chromium/src/base/message_loop.cc
72 	libxul.so 	XRE_InitChildProcess 	toolkit/xre/nsEmbedFunctions.cpp
73 	plugin-container 	main 	ipc/app/MozillaRuntimeMain.cpp
74 	libc.so 	__libc_init 	/local/build/soul_3.5_ff-release/v123/bionic/libc/bionic/libc_init_dynamic.cpp:112
75 	plugin-container 	plugin-container@0x5ee 	
76 	linker 	set_soinfo_pool_protection 	/builds/slave/b2g_m-cen_flame_ntly-000000000/build/bionic/linker/linker.cpp:291
77 		@0xbee39ada

More reports : 
https://crash-stats.mozilla.com/report/list?product=B2G&signature=mozilla%3A%3Adom%3A%3AShadowRoot%3A%3AIsPooledNode%28nsIContent%2A%2C+nsIContent%2A%2C+nsIContent%2A%29

occurred on one build on flame : 20140611160205 ; 2.1, 2.0

Comment 1

[Martijn Wargers (dead)]

This can also be reproduced on desktop when the dom.webcomponents.enabled pref is set to true.

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

Please make ShadowRoot/Shadow DOM bug block bug 811542 so that we can keep some track of them.
And yes, currently there are several ways to crash Shadow DOM.

Comment 3

[Martijn Wargers (dead)]

Are all those crashes different, even though the top stack is the same?
Like this one for instance? https://crash-stats.mozilla.com/report/index/f2ade5e4-1445-4c80-a8e9-10d9b2140805
You're right, it's very easy to crash with the dom.webcomponents.enabled pref enabled.

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

Is there are testcase for this?
(I tried loading https://mobile.twitter.com/ but didn't see a crash.)

Comment 5

[Martijn Wargers (dead)]

Attachment: crash1.htm

Managed to get a minimal testcase, this crashes like this: https://crash-stats.mozilla.com/report/index/82720efb-d9a4-42a2-9797-442f32140805
It's basically this script:
  var x = document.createElementNS("http://www.w3.org/1999/xhtml", 'span');
  x.createShadowRoot();
  x.remove();
  x.id = 'a';

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

Thanks!

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

William, I don't quite understand what IsPooledNode is trying to recognize.
Why may it take also host and host's parent as params?

Shouldn't we filter out the case when aContent == aHost?

Comment 8

[William Chen [:wchen]]

IsPooledNode is trying to recognize nodes in the light DOM that will be distributed, i.e. light DOM nodes that end up in the pool according to the "pool population algorithm" [1].

It takes the host as a param because children of the host are inserted into the pool.
It takes the content's container as a param (not the host's parent) in order to make sure that the container is the host. And also to check for the for the special case where the node is fallback content (container is content insertion point with no distributed nodes).

[1] http://w3c.github.io/webcomponents/spec/shadow/#dfn-pool-population-algorithm

I don't think there is anything particularly special about aContent == aHost that we need to filter out. The bug here just looks like we are missing a null check on aContainer before we check to see if it is a content insertion point.

Comment 9

[William Chen [:wchen]]

Attachment: Add null check for container in ShadowRoot::IsPooledNode

Comment 10

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8470313 [details] [diff] [review]
Add null check for container in ShadowRoot::IsPooledNode

(I still think IsPooledNode should return early when aChild is the host, but this should fix the crash, sure)

Comment 11

[William Chen [:wchen]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/74aae2dec3c9

Comment 12

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/74aae2dec3c9

Comment 13

[Martijn Wargers (dead)]

Verified fixed in latest Nightly.