Modified: branches/safari-536.28-branch/LayoutTests/ChangeLog (133665 => 133666)
--- branches/safari-536.28-branch/LayoutTests/ChangeLog 2012-11-06 21:37:47 UTC (rev 133665)
+++ branches/safari-536.28-branch/LayoutTests/ChangeLog 2012-11-06 21:38:39 UTC (rev 133666)
@@ -1,5 +1,19 @@
2012-11-06 Lucas Forschler <lforsch...@apple.com>
+ Merge r126205
+
+ 2012-08-21 Florin Malita <fmal...@chromium.org>
+
+ ASSERT triggered in SVGTRefTargetEventListener::handleEvent()
+ https://bugs.webkit.org/show_bug.cgi?id=94487
+
+ Reviewed by Nikolas Zimmermann.
+
+ * svg/custom/tref-stale-listener-crash-expected.txt: Added.
+ * svg/custom/tref-stale-listener-crash.html: Added.
+
+2012-11-06 Lucas Forschler <lforsch...@apple.com>
+
Merge r126131
2012-08-20 MORITA Hajime <morr...@google.com>
@@ -11107,3 +11121,4 @@
.
.
.
+.
Copied: branches/safari-536.28-branch/LayoutTests/svg/custom/tref-stale-listener-crash-expected.txt (from rev 126205, trunk/LayoutTests/svg/custom/tref-stale-listener-crash-expected.txt) (0 => 133666)
--- branches/safari-536.28-branch/LayoutTests/svg/custom/tref-stale-listener-crash-expected.txt (rev 0)
+++ branches/safari-536.28-branch/LayoutTests/svg/custom/tref-stale-listener-crash-expected.txt 2012-11-06 21:38:39 UTC (rev 133666)
@@ -0,0 +1,4 @@
+
+
+
+PASS: did not crash.
Copied: branches/safari-536.28-branch/LayoutTests/svg/custom/tref-stale-listener-crash.html (from rev 126205, trunk/LayoutTests/svg/custom/tref-stale-listener-crash.html) (0 => 133666)
--- branches/safari-536.28-branch/LayoutTests/svg/custom/tref-stale-listener-crash.html (rev 0)
+++ branches/safari-536.28-branch/LayoutTests/svg/custom/tref-stale-listener-crash.html 2012-11-06 21:38:39 UTC (rev 133666)
@@ -0,0 +1,26 @@
+<!DOCTYPE html>
+<html>
+<body _onload_="crash()">
+ <!-- Test for https://bugs.webkit.org/show_bug.cgi?id=94487 -->
+ <input/>
+ <svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
+ <g>
+ <tref xlink:href=""
+ <g id="target"></g>
+ </g>
+ </svg>
+ <input/>
+ <div>PASS: did not crash.</div>
+
+ <script>
+ if (window.testRunner)
+ testRunner.dumpAsText();
+
+ function crash() {
+ document.designMode='on';
+ document.execCommand('selectall');
+ document.execCommand('FormatBlock', false, '<'+'pre>');
+ }
+ </script>
+</body>
+</html>
Modified: branches/safari-536.28-branch/Source/WebCore/ChangeLog (133665 => 133666)
--- branches/safari-536.28-branch/Source/WebCore/ChangeLog 2012-11-06 21:37:47 UTC (rev 133665)
+++ branches/safari-536.28-branch/Source/WebCore/ChangeLog 2012-11-06 21:38:39 UTC (rev 133666)
@@ -1,5 +1,36 @@
2012-11-06 Lucas Forschler <lforsch...@apple.com>
+ Merge r126205
+
+ 2012-08-21 Florin Malita <fmal...@chromium.org>
+
+ ASSERT triggered in SVGTRefTargetEventListener::handleEvent()
+ https://bugs.webkit.org/show_bug.cgi?id=94487
+
+ Reviewed by Nikolas Zimmermann.
+
+ The current way of tracking tref target elements by id can leave stale event listeners
+ under certain circumstances. This patch switches to storing a target RefPtr instead
+ to avoid an id lookup which may not return the original/attached element.
+
+ Test: svg/custom/tref-stale-listener-crash.html
+
+ * svg/SVGTRefElement.cpp:
+ (SVGTRefTargetEventListener):
+ (WebCore::SVGTRefTargetEventListener::isAttached): use m_target instead of an explicit bool.
+ (WebCore::SVGTRefTargetEventListener::SVGTRefTargetEventListener):
+ (WebCore::SVGTRefTargetEventListener::attach): save a target RefPtr instead of an id.
+ (WebCore::SVGTRefTargetEventListener::detach): detach the target element directly without
+ going through a lookup.
+ (WebCore::SVGTRefTargetEventListener::handleEvent):
+ (WebCore::SVGTRefElement::updateReferencedText): use an explicit target pointer instead of
+ the id-based lookup.
+ (WebCore::SVGTRefElement::buildPendingResource):
+ * svg/SVGTRefElement.h:
+ (SVGTRefElement):
+
+2012-11-06 Lucas Forschler <lforsch...@apple.com>
+
Merge r126131
2012-08-20 MORITA Hajime <morr...@google.com>
@@ -206767,3 +206798,4 @@
.
.
.
+.
Modified: branches/safari-536.28-branch/Source/WebCore/svg/SVGTRefElement.cpp (133665 => 133666)
--- branches/safari-536.28-branch/Source/WebCore/svg/SVGTRefElement.cpp 2012-11-06 21:37:47 UTC (rev 133665)
+++ branches/safari-536.28-branch/Source/WebCore/svg/SVGTRefElement.cpp 2012-11-06 21:38:39 UTC (rev 133666)
@@ -69,9 +69,9 @@
? static_cast<const SVGTRefTargetEventListener*>(listener) : 0;
}
- void attach(Element* target, String& targetId);
+ void attach(PassRefPtr<Element> target);
void detach();
- bool isAttached() const { return m_attached; }
+ bool isAttached() const { return m_target.get(); }
private:
SVGTRefTargetEventListener(SVGTRefElement* trefElement);
@@ -80,29 +80,26 @@
virtual bool operator==(const EventListener&) OVERRIDE;
SVGTRefElement* m_trefElement;
- String m_targetId;
- bool m_attached;
+ RefPtr<Element> m_target;
};
SVGTRefTargetEventListener::SVGTRefTargetEventListener(SVGTRefElement* trefElement)
: EventListener(SVGTRefTargetEventListenerType)
, m_trefElement(trefElement)
- , m_attached(false)
+ , m_target(0)
{
ASSERT(m_trefElement);
}
-void SVGTRefTargetEventListener::attach(Element* target, String& targetId)
+void SVGTRefTargetEventListener::attach(PassRefPtr<Element> target)
{
ASSERT(!isAttached());
- ASSERT(target);
+ ASSERT(target.get());
ASSERT(target->inDocument());
- ASSERT(!targetId.isEmpty());
target->addEventListener(eventNames().DOMSubtreeModifiedEvent, this, false);
target->addEventListener(eventNames().DOMNodeRemovedFromDocumentEvent, this, false);
- m_targetId = targetId;
- m_attached = true;
+ m_target = target;
}
void SVGTRefTargetEventListener::detach()
@@ -110,13 +107,9 @@
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;
+ m_target->removeEventListener(eventNames().DOMSubtreeModifiedEvent, this, false);
+ m_target->removeEventListener(eventNames().DOMNodeRemovedFromDocumentEvent, this, false);
+ m_target.clear();
}
bool SVGTRefTargetEventListener::operator==(const EventListener& listener)
@@ -131,7 +124,7 @@
ASSERT(isAttached());
if (event->type() == eventNames().DOMSubtreeModifiedEvent && m_trefElement != event->target())
- m_trefElement->updateReferencedText();
+ m_trefElement->updateReferencedText(m_target.get());
else if (event->type() == eventNames().DOMNodeRemovedFromDocumentEvent)
m_trefElement->detachTarget();
}
@@ -183,10 +176,10 @@
ShadowRoot::create(this, ShadowRoot::CreatingUserAgentShadowRoot, ASSERT_NO_EXCEPTION);
}
-void SVGTRefElement::updateReferencedText()
+void SVGTRefElement::updateReferencedText(Element* target)
{
String textContent;
- if (Element* target = SVGURIReference::targetElementFromIRIString(href(), document()))
+ if (target)
textContent = target->textContent();
ASSERT(hasShadowRoot());
@@ -296,8 +289,8 @@
return;
String id;
- Element* target = SVGURIReference::targetElementFromIRIString(href(), document(), &id);
- if (!target) {
+ RefPtr<Element> target = SVGURIReference::targetElementFromIRIString(href(), document(), &id);
+ if (!target.get()) {
if (hasPendingResources() || id.isEmpty())
return;
@@ -312,9 +305,9 @@
// 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_targetListener->attach(target, id);
+ m_targetListener->attach(target);
- updateReferencedText();
+ updateReferencedText(target.get());
}
Node::InsertionNotificationRequest SVGTRefElement::insertedInto(Node* rootParent)
Modified: branches/safari-536.28-branch/Source/WebCore/svg/SVGTRefElement.h (133665 => 133666)
--- branches/safari-536.28-branch/Source/WebCore/svg/SVGTRefElement.h 2012-11-06 21:37:47 UTC (rev 133665)
+++ branches/safari-536.28-branch/Source/WebCore/svg/SVGTRefElement.h 2012-11-06 21:38:39 UTC (rev 133666)
@@ -53,7 +53,7 @@
virtual InsertionNotificationRequest insertedInto(Node*) OVERRIDE;
virtual void removedFrom(Node*) OVERRIDE;
- void updateReferencedText();
+ void updateReferencedText(Element*);
void detachTarget();
