Bug 84171 - WebCore::EventTarget::addEventListener crash
Summary: WebCore::EventTarget::addEventListener crash
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P1 Normal
Assignee: Florin Malita
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-04-17 11:21 PDT by Florin Malita
Modified: 2012-04-24 07:49 PDT (History)
3 users (show)

See Also:


Attachments
Crash repro. (155 bytes, image/svg+xml)
2012-04-17 11:21 PDT, Florin Malita
no flags Details
Patch (4.26 KB, patch)
2012-04-23 15:56 PDT, Florin Malita
no flags Details | Formatted Diff | Diff
Patch for landing (4.27 KB, patch)
2012-04-24 06:40 PDT, Florin Malita
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Florin Malita 2012-04-17 11:21:37 PDT
Created attachment 137564 [details]
Crash repro.

[2134:2134:166495129177:ERROR:process_util_posix.cc(143)] Received signal 11
	base::debug::StackTrace::StackTrace() [0x7fc4ae32ea9e]
	base::(anonymous namespace)::StackDumpSignalHandler() [0x7fc4ae38d549]
	0x7fc4a2c15af0
	WebCore::Node::addEventListener() [0x7fc4a973e06f]
	WebCore::SVGTRefElement::buildPendingResource() [0x7fc4a9ad54d0]
	WebCore::SVGTRefElement::insertedIntoDocument() [0x7fc4a9ad5587]
	WebCore::notifyChildInserted() [0x7fc4a969dbd4]
	WebCore::updateTreeAfterInsertion() [0x7fc4a969e62a]
	WebCore::ContainerNode::appendChild() [0x7fc4a969c139]
	WebCore::SVGUseElement::buildShadowTree() [0x7fc4a9aea930]
	WebCore::SVGUseElement::buildShadowAndInstanceTree() [0x7fc4a9ae9c99]
	WebCore::SVGUseElement::buildPendingResource() [0x7fc4a9ae9873]
	WebCore::SVGUseElement::finishParsingChildren() [0x7fc4a9aebefc]
	WebCore::XMLDocumentParser::endElementNs() [0x7fc4aa3dbeea]
	WebCore::endElementNsHandler() [0x7fc4aa3dce87]
	xmlParseTryOrFinish [0x7fc4aa5e136b]
	xmlParseChunk [0x7fc4aa5e387f]
	WebCore::XMLDocumentParser::doWrite() [0x7fc4aa3db105]
	WebCore::XMLDocumentParser::append() [0x7fc4aa3d7bd6]
	WebCore::DecodedDataDocumentParser::appendBytes() [0x7fc4a96a6884]
	WebCore::DocumentWriter::addData() [0x7fc4aa275c60]

Chromium bug: http://code.google.com/p/chromium/issues/detail?id=123465

Most likely introduced in http://trac.webkit.org/changeset/108082
Comment 1 Alexey Proskuryakov 2012-04-18 14:47:46 PDT
ASSERTION FAILED: (*it)->shadowTreeElement()

OpenSource/Source/WebCore/dom/Node.cpp(2482) : virtual bool WebCore::Node::addEventListener(const WTF::AtomicString &, PassRefPtr<WebCore::EventListener>, bool)
1   0x105397b58 WebCore::SVGTRefElement::buildPendingResource()
2   0x105397c70 WebCore::SVGTRefElement::insertedInto(WebCore::Node*)
3   0x103f7f009 WebCore::ChildNodeInsertionNotifier::notifyNodeInsertedIntoDocument(WebCore::Node*)
4   0x103f7cae2 WebCore::ChildNodeInsertionNotifier::notify(WebCore::Node*)
5   0x103f792c7 _ZN7WebCoreL24updateTreeAfterInsertionEPNS_13ContainerNodeEPNS_4NodeEb
6   0x103f78d6d WebCore::ContainerNode::appendChild(WTF::PassRefPtr<WebCore::Node>, int&, bool)
7   0x10539c17c WebCore::SVGUseElement::buildShadowTree(WebCore::SVGElement*, WebCore::SVGElementInstance*)
8   0x10539bb15 WebCore::SVGUseElement::buildShadowAndInstanceTree(WebCore::SVGElement*)
9   0x10539b6b9 WebCore::SVGUseElement::buildPendingResource()
10  0x10539e2a2 WebCore::SVGUseElement::finishParsingChildren()
Comment 2 Florin Malita 2012-04-23 15:11:12 PDT
I believe I got to the bottom of this. SVGUseElement::buildShadowAndInstanceTree():

  m_targetElementInstance = SVGElementInstance::create(this, this, target);
  ...
  buildShadowTree(target, m_targetElementInstance.get());
  ...
  associateInstancesWithShadowTreeElements(shadowTreeRootElement->firstChild(), m_targetElementInstance.get());
  ...
  transferEventListenersToShadowTree(m_targetElementInstance.get());
  ...



BuildShadowTree() triggers SVGTRefElement::buildPendingResource() for the shadow <tref> node, which in turn attempts to install event handlers on the target node (same <tref> in this example). The problem is that Node::addEventListener() asserts that each element instance must have an associated shadow tree entry - but buildShadowAndInstanceTree() only sets up the instance<->shadow pointers later (associateInstancesWithShadowTreeElements).

We can prevent this by skipping event handler activation for shadow tree tref's in SVGTRefElement::buildPendingResource(), as buildShadowAndInstanceTree() copies the active handlers anyway (transferEventListenersToShadowTree).
Comment 3 Florin Malita 2012-04-23 15:56:01 PDT
Created attachment 138446 [details]
Patch
Comment 4 Nikolas Zimmermann 2012-04-24 05:00:28 PDT
Comment on attachment 138446 [details]
Patch

Looks great, r=me in case cr-linux EWS turns green.
Comment 5 Florin Malita 2012-04-24 06:40:26 PDT
Created attachment 138549 [details]
Patch for landing
Comment 6 WebKit Review Bot 2012-04-24 07:49:54 PDT
Comment on attachment 138549 [details]
Patch for landing

Clearing flags on attachment: 138549

Committed r115053: <http://trac.webkit.org/changeset/115053>
Comment 7 WebKit Review Bot 2012-04-24 07:49:59 PDT
All reviewed patches have been landed.  Closing bug.