Added: trunk/LayoutTests/svg/custom/tref-nested-events-crash.svg (0 => 124681)
--- trunk/LayoutTests/svg/custom/tref-nested-events-crash.svg (rev 0)
+++ trunk/LayoutTests/svg/custom/tref-nested-events-crash.svg 2012-08-04 01:23:58 UTC (rev 124681)
@@ -0,0 +1,36 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<svg _onload_="CFcrash()" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
+ <!-- Test for https://bugs.webkit.org/show_bug.cgi?id=93004 -->
+ <g id="test-body-content">
+ <defs>
+ <text id="hello">
+ <tspan id="tspan">##</tspan>
+ </text>
+ </defs>
+
+ <text>
+ <tref id="tref" xlink:href=""
+ </text>
+ </g>
+
+ <text>PASS: did not crash.</text>
+ <script>
+ if (window.testRunner) {
+ testRunner.waitUntilDone();
+ testRunner.dumpAsText();
+ }
+
+ var test_body_content = document.getElementById("test-body-content");
+ var tspan = document.getElementById("tspan");
+
+ function CFcrash() {
+ test_body_content.addEventListener("DOMCharacterDataModified", function () {
+ try { test_body_content.parentNode.removeChild(test_body_content); } catch (e) {}
+
+ if (window.testRunner)
+ setTimeout('testRunner.notifyDone()', 0);
+ }, false);
+ document.adoptNode(tspan);
+ }
+ </script>
+</svg>
Modified: trunk/Source/WebCore/ChangeLog (124680 => 124681)
--- trunk/Source/WebCore/ChangeLog 2012-08-04 01:13:22 UTC (rev 124680)
+++ trunk/Source/WebCore/ChangeLog 2012-08-04 01:23:58 UTC (rev 124681)
@@ -1,3 +1,51 @@
+2012-08-03 Florin Malita <[email protected]>
+
+ [SVG] Tref target event listener cleanup
+ https://bugs.webkit.org/show_bug.cgi?id=93004
+
+ Reviewed by Abhishek Arya.
+
+ Currently SVGTRefElement allocates event listeners dynamically as it attaches to its
+ targets. Synchronizing the lifetime of the target listener vs. the tref element is
+ error prone, as various events can stack and trigger nested handlers.
+
+ In order to reduce complexity and address a couple of outstanding issues, this patch
+ changes the way event listeners are allocated: only one target listener is created
+ for the lifetime of the SVGTRefElement, and gets reused if the target element changes.
+
+ Test: svg/custom/tref-nested-events-crash.svg
+
+ * dom/EventListener.h:
+ Added new <tref> target event listener type.
+ * svg/SVGTRefElement.cpp:
+ (WebCore):
+ (WebCore::SVGTRefTargetEventListener::create):
+ (WebCore::SVGTRefTargetEventListener::cast):
+ (SVGTRefTargetEventListener):
+ (WebCore::SVGTRefTargetEventListener::isAttached):
+ (WebCore::SVGTRefTargetEventListener::SVGTRefTargetEventListener):
+ (WebCore::SVGTRefTargetEventListener::attach):
+ (WebCore::SVGTRefTargetEventListener::detach):
+ (WebCore::SVGTRefTargetEventListener::operator==):
+ (WebCore::SVGTRefTargetEventListener::handleEvent):
+ No need to check m_trefElement anymore - the listener is allocated for the whole element
+ lifetime, detached when the element is removed and deallocated when the element is
+ destroyed.
+ (WebCore::SVGTRefElement::SVGTRefElement):
+ Allocate one target listener per element, at construction time.
+ (WebCore::SVGTRefElement::~SVGTRefElement):
+ Detach the listener if necessary.
+ (WebCore::SVGTRefElement::detachTarget):
+ Check whether the element is still in document after updating the text (may have been
+ removed by event handlers).
+ (WebCore::SVGTRefElement::buildPendingResource):
+ Attach the event listener before updating the text content to avoid racing with event
+ handlers (which can remove the element).
+ (WebCore::SVGTRefElement::removedFrom):
+ * svg/SVGTRefElement.h:
+ (WebCore):
+ (SVGTRefElement):
+
2012-08-03 Michael Saboff <[email protected]>
Convert HTML parser to handle 8-bit resources without converting to UChar*
Modified: trunk/Source/WebCore/dom/EventListener.h (124680 => 124681)
--- trunk/Source/WebCore/dom/EventListener.h 2012-08-04 01:13:22 UTC (rev 124680)
+++ trunk/Source/WebCore/dom/EventListener.h 2012-08-04 01:23:58 UTC (rev 124681)
@@ -42,7 +42,8 @@
CPPEventListenerType,
ConditionEventListenerType,
GObjectEventListenerType,
- NativeEventListenerType
+ NativeEventListenerType,
+ SVGTRefTargetEventListenerType
};
virtual ~EventListener() { }
Modified: trunk/Source/WebCore/svg/SVGTRefElement.cpp (124680 => 124681)
--- trunk/Source/WebCore/svg/SVGTRefElement.cpp 2012-08-04 01:13:22 UTC (rev 124680)
+++ trunk/Source/WebCore/svg/SVGTRefElement.cpp 2012-08-04 01:23:58 UTC (rev 124681)
@@ -49,13 +49,6 @@
REGISTER_PARENT_ANIMATED_PROPERTIES(SVGTextPositioningElement)
END_REGISTER_ANIMATED_PROPERTIES
-inline SVGTRefElement::SVGTRefElement(const QualifiedName& tagName, Document* document)
- : SVGTextPositioningElement(tagName, document)
-{
- ASSERT(hasTagName(SVGNames::trefTag));
- registerAnimatedPropertiesForSVGTRefElement();
-}
-
PassRefPtr<SVGTRefElement> SVGTRefElement::create(const QualifiedName& tagName, Document* document)
{
RefPtr<SVGTRefElement> element = adoptRef(new SVGTRefElement(tagName, document));
@@ -63,59 +56,83 @@
return element.release();
}
-class TargetListener : public EventListener {
+class SVGTRefTargetEventListener : public EventListener {
public:
- static PassRefPtr<TargetListener> create(SVGTRefElement* trefElement, String targetId)
+ static PassRefPtr<SVGTRefTargetEventListener> create(SVGTRefElement* trefElement)
{
- return adoptRef(new TargetListener(trefElement, targetId));
+ return adoptRef(new SVGTRefTargetEventListener(trefElement));
}
- static const TargetListener* cast(const EventListener* listener)
+ static const SVGTRefTargetEventListener* cast(const EventListener* listener)
{
- return listener->type() == CPPEventListenerType ? static_cast<const TargetListener*>(listener) : 0;
+ return listener->type() == SVGTRefTargetEventListenerType
+ ? static_cast<const SVGTRefTargetEventListener*>(listener) : 0;
}
- virtual bool operator==(const EventListener&);
+ void attach(Element* target, String& targetId);
+ void detach();
+ bool isAttached() const { return m_attached; }
- void clear()
- {
- Element* target = m_trefElement->treeScope()->getElementById(m_targetId);
- if (target) {
- target->removeEventListener(eventNames().DOMSubtreeModifiedEvent, this, false);
- target->removeEventListener(eventNames().DOMNodeRemovedFromDocumentEvent, this, false);
- }
-
- m_trefElement = 0;
- m_targetId = String();
- }
-
private:
- TargetListener(SVGTRefElement* trefElement, String targetId)
- : EventListener(CPPEventListenerType)
- , m_trefElement(trefElement)
- , m_targetId(targetId)
- {
- }
+ SVGTRefTargetEventListener(SVGTRefElement* trefElement);
- virtual void handleEvent(ScriptExecutionContext*, Event*);
+ virtual void handleEvent(ScriptExecutionContext*, Event*) OVERRIDE;
+ virtual bool operator==(const EventListener&) OVERRIDE;
SVGTRefElement* m_trefElement;
String m_targetId;
+ bool m_attached;
};
-bool TargetListener::operator==(const EventListener& listener)
+SVGTRefTargetEventListener::SVGTRefTargetEventListener(SVGTRefElement* trefElement)
+ : EventListener(SVGTRefTargetEventListenerType)
+ , m_trefElement(trefElement)
+ , m_attached(false)
{
- if (const TargetListener* subtreeModificationEventListener = TargetListener::cast(&listener))
- return m_trefElement == subtreeModificationEventListener->m_trefElement;
+ ASSERT(m_trefElement);
+}
+
+void SVGTRefTargetEventListener::attach(Element* target, String& targetId)
+{
+ ASSERT(!isAttached());
+ ASSERT(target);
+ ASSERT(target->inDocument());
+ ASSERT(!targetId.isEmpty());
+
+ target->addEventListener(eventNames().DOMSubtreeModifiedEvent, this, false);
+ target->addEventListener(eventNames().DOMNodeRemovedFromDocumentEvent, this, false);
+ m_targetId = targetId;
+ m_attached = true;
+}
+
+void SVGTRefTargetEventListener::detach()
+{
+ if (!isAttached())
+ return;
+
+ if (Element* target = m_trefElement->treeScope()->getElementById(m_targetId)) {
+ target->removeEventListener(eventNames().DOMSubtreeModifiedEvent, this, false);
+ target->removeEventListener(eventNames().DOMNodeRemovedFromDocumentEvent, this, false);
+ }
+
+ m_targetId = emptyString();
+ m_attached = false;
+}
+
+bool SVGTRefTargetEventListener::operator==(const EventListener& listener)
+{
+ if (const SVGTRefTargetEventListener* targetListener = SVGTRefTargetEventListener::cast(&listener))
+ return m_trefElement == targetListener->m_trefElement;
return false;
}
-void TargetListener::handleEvent(ScriptExecutionContext*, Event* event)
+void SVGTRefTargetEventListener::handleEvent(ScriptExecutionContext*, Event* event)
{
- if (m_trefElement && event->type() == eventNames().DOMSubtreeModifiedEvent && m_trefElement != event->target())
+ ASSERT(isAttached());
+
+ if (event->type() == eventNames().DOMSubtreeModifiedEvent && m_trefElement != event->target())
m_trefElement->updateReferencedText();
-
- if (m_trefElement && event->type() == eventNames().DOMNodeRemovedFromDocumentEvent)
+ else if (event->type() == eventNames().DOMNodeRemovedFromDocumentEvent)
m_trefElement->detachTarget();
}
@@ -148,9 +165,17 @@
}
}
+inline SVGTRefElement::SVGTRefElement(const QualifiedName& tagName, Document* document)
+ : SVGTextPositioningElement(tagName, document)
+ , m_targetListener(SVGTRefTargetEventListener::create(this))
+{
+ ASSERT(hasTagName(SVGNames::trefTag));
+ registerAnimatedPropertiesForSVGTRefElement();
+}
+
SVGTRefElement::~SVGTRefElement()
{
- clearEventListener();
+ m_targetListener->detach();
}
void SVGTRefElement::createShadowSubtree()
@@ -175,7 +200,7 @@
void SVGTRefElement::detachTarget()
{
// Remove active listeners and clear the text content.
- clearEventListener();
+ m_targetListener->detach();
String emptyContent;
ExceptionCode ignore = 0;
@@ -185,7 +210,8 @@
if (container)
container->setTextContent(emptyContent, ignore);
- ASSERT(inDocument());
+ if (!inDocument())
+ return;
// Mark the referenced ID as pending.
String id;
@@ -263,7 +289,7 @@
void SVGTRefElement::buildPendingResource()
{
// Remove any existing event listener.
- clearEventListener();
+ m_targetListener->detach();
// If we're not yet in a document, this function will be called again from insertedInto().
if (!inDocument())
@@ -280,17 +306,14 @@
return;
}
- updateReferencedText();
-
// Don't set up event listeners if this is a shadow tree node.
// SVGUseElement::transferEventListenersToShadowTree() handles this task, and addEventListener()
// expects every element instance to have an associated shadow tree element - which is not the
// case when we land here from SVGUseElement::buildShadowTree().
- if (!isInShadowTree()) {
- m_eventListener = TargetListener::create(this, id);
- target->addEventListener(eventNames().DOMSubtreeModifiedEvent, m_eventListener.get(), false);
- target->addEventListener(eventNames().DOMNodeRemovedFromDocumentEvent, m_eventListener.get(), false);
- }
+ if (!isInShadowTree())
+ m_targetListener->attach(target, id);
+
+ updateReferencedText();
}
Node::InsertionNotificationRequest SVGTRefElement::insertedInto(ContainerNode* rootParent)
@@ -305,17 +328,9 @@
{
SVGStyledElement::removedFrom(rootParent);
if (rootParent->inDocument())
- clearEventListener();
+ m_targetListener->detach();
}
-void SVGTRefElement::clearEventListener()
-{
- if (m_eventListener) {
- m_eventListener->clear();
- m_eventListener = 0;
- }
}
-}
-
#endif // ENABLE(SVG)
Modified: trunk/Source/WebCore/svg/SVGTRefElement.h (124680 => 124681)
--- trunk/Source/WebCore/svg/SVGTRefElement.h 2012-08-04 01:13:22 UTC (rev 124680)
+++ trunk/Source/WebCore/svg/SVGTRefElement.h 2012-08-04 01:23:58 UTC (rev 124681)
@@ -27,7 +27,7 @@
namespace WebCore {
-class TargetListener;
+class SVGTRefTargetEventListener;
class SVGTRefElement : public SVGTextPositioningElement,
public SVGURIReference {
@@ -35,7 +35,7 @@
static PassRefPtr<SVGTRefElement> create(const QualifiedName&, Document*);
private:
- friend class TargetListener;
+ friend class SVGTRefTargetEventListener;
SVGTRefElement(const QualifiedName&, Document*);
virtual ~SVGTRefElement();
@@ -53,8 +53,6 @@
virtual InsertionNotificationRequest insertedInto(ContainerNode*) OVERRIDE;
virtual void removedFrom(ContainerNode*) OVERRIDE;
- void clearEventListener();
-
void updateReferencedText();
void detachTarget();
@@ -65,7 +63,7 @@
DECLARE_ANIMATED_STRING(Href, href)
END_DECLARE_ANIMATED_PROPERTIES
- RefPtr<TargetListener> m_eventListener;
+ RefPtr<SVGTRefTargetEventListener> m_targetListener;
};
} // namespace WebCore
