Title: [136541] trunk
- Revision
- 136541
- Author
- fmal...@chromium.org
- Date
- 2012-12-04 11:35:30 -0800 (Tue, 04 Dec 2012)
Log Message
Stale SVGUseElement reference in CachedResource::checkNotify()
https://bugs.webkit.org/show_bug.cgi?id=104004
Reviewed by Eric Seidel.
Source/WebCore:
SVGUseElement tracks one CachedSVGDocument at a time (for external references), but when
the href attribute is updated it fails to unregister with the current CachedSVGDocument
and only updates its CachedSVGDocument with the new instance. This leaves an untracked
reference with the original CachedSVGDocument.
The patch adds the missing removeClient() call on href change, and encapsulates the
CachedSVGDocument manipulation in a helper method which handles the necessary cleanup.
Test: svg/custom/use-href-update-crash.svg
* svg/SVGUseElement.cpp:
(WebCore::SVGUseElement::~SVGUseElement):
(WebCore::SVGUseElement::svgAttributeChanged):
(WebCore::SVGUseElement::setCachedDocument):
(WebCore):
* svg/SVGUseElement.h:
(SVGUseElement):
LayoutTests:
* svg/custom/use-href-update-crash-expected.txt: Added.
* svg/custom/use-href-update-crash.svg: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (136540 => 136541)
--- trunk/LayoutTests/ChangeLog 2012-12-04 19:29:48 UTC (rev 136540)
+++ trunk/LayoutTests/ChangeLog 2012-12-04 19:35:30 UTC (rev 136541)
@@ -1,3 +1,13 @@
+2012-12-04 Florin Malita <fmal...@chromium.org>
+
+ Stale SVGUseElement reference in CachedResource::checkNotify()
+ https://bugs.webkit.org/show_bug.cgi?id=104004
+
+ Reviewed by Eric Seidel.
+
+ * svg/custom/use-href-update-crash-expected.txt: Added.
+ * svg/custom/use-href-update-crash.svg: Added.
+
2012-12-04 Antoine Quint <grao...@apple.com>
Missing -expected.txt files for new <track> tests
Added: trunk/LayoutTests/svg/custom/use-href-update-crash-expected.txt (0 => 136541)
--- trunk/LayoutTests/svg/custom/use-href-update-crash-expected.txt (rev 0)
+++ trunk/LayoutTests/svg/custom/use-href-update-crash-expected.txt 2012-12-04 19:35:30 UTC (rev 136541)
@@ -0,0 +1 @@
+PASS: did not crash.
Added: trunk/LayoutTests/svg/custom/use-href-update-crash.svg (0 => 136541)
--- trunk/LayoutTests/svg/custom/use-href-update-crash.svg (rev 0)
+++ trunk/LayoutTests/svg/custom/use-href-update-crash.svg 2012-12-04 19:35:30 UTC (rev 136541)
@@ -0,0 +1,17 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<svg xmlns:xlink="http://www.w3.org/1999/xlink" xmlns="http://www.w3.org/2000/svg">
+ <!-- Test for https://bugs.webkit.org/show_bug.cgi?id=104004 -->
+ <use id="use" xlink:href=""
+ <text>PASS: did not crash.</text>
+
+ <script>
+ var use = document.getElementById('use');
+ use.setAttribute('xlink:href', 'bar.svg#bar');
+ use.parentNode.removeChild(use);
+ use = null;
+ gc();
+
+ if (window.testRunner)
+ testRunner.dumpAsText();
+ </script>
+</svg>
Modified: trunk/Source/WebCore/ChangeLog (136540 => 136541)
--- trunk/Source/WebCore/ChangeLog 2012-12-04 19:29:48 UTC (rev 136540)
+++ trunk/Source/WebCore/ChangeLog 2012-12-04 19:35:30 UTC (rev 136541)
@@ -1,3 +1,28 @@
+2012-12-04 Florin Malita <fmal...@chromium.org>
+
+ Stale SVGUseElement reference in CachedResource::checkNotify()
+ https://bugs.webkit.org/show_bug.cgi?id=104004
+
+ Reviewed by Eric Seidel.
+
+ SVGUseElement tracks one CachedSVGDocument at a time (for external references), but when
+ the href attribute is updated it fails to unregister with the current CachedSVGDocument
+ and only updates its CachedSVGDocument with the new instance. This leaves an untracked
+ reference with the original CachedSVGDocument.
+
+ The patch adds the missing removeClient() call on href change, and encapsulates the
+ CachedSVGDocument manipulation in a helper method which handles the necessary cleanup.
+
+ Test: svg/custom/use-href-update-crash.svg
+
+ * svg/SVGUseElement.cpp:
+ (WebCore::SVGUseElement::~SVGUseElement):
+ (WebCore::SVGUseElement::svgAttributeChanged):
+ (WebCore::SVGUseElement::setCachedDocument):
+ (WebCore):
+ * svg/SVGUseElement.h:
+ (SVGUseElement):
+
2012-12-04 Yury Semikhatsky <yu...@chromium.org>
Web Inspector: Can't take a heap snapshot in chromium ("Uncaught ReferenceError")
Modified: trunk/Source/WebCore/svg/SVGUseElement.cpp (136540 => 136541)
--- trunk/Source/WebCore/svg/SVGUseElement.cpp 2012-12-04 19:29:48 UTC (rev 136540)
+++ trunk/Source/WebCore/svg/SVGUseElement.cpp 2012-12-04 19:35:30 UTC (rev 136541)
@@ -107,8 +107,7 @@
SVGUseElement::~SVGUseElement()
{
- if (m_cachedDocument)
- m_cachedDocument->removeClient(this);
+ setCachedDocument(0);
clearResourceReferences();
}
@@ -257,18 +256,14 @@
if (url.hasFragmentIdentifier()) {
CachedResourceRequest request(ResourceRequest(url.string()));
request.setInitiator(this);
- m_cachedDocument = document()->cachedResourceLoader()->requestSVGDocument(request);
- if (m_cachedDocument)
- m_cachedDocument->addClient(this);
+ setCachedDocument(document()->cachedResourceLoader()->requestSVGDocument(request));
}
- }
+ } else
+ setCachedDocument(0);
- if (m_cachedDocument && !isExternalReference) {
- m_cachedDocument->removeClient(this);
- m_cachedDocument = 0;
- }
if (!m_wasInsertedByParser)
buildPendingResource();
+
return;
}
@@ -992,6 +987,19 @@
}
}
+void SVGUseElement::setCachedDocument(CachedResourceHandle<CachedSVGDocument> cachedDocument)
+{
+ if (m_cachedDocument == cachedDocument)
+ return;
+
+ if (m_cachedDocument)
+ m_cachedDocument->removeClient(this);
+
+ m_cachedDocument = cachedDocument;
+ if (m_cachedDocument)
+ m_cachedDocument->addClient(this);
}
+}
+
#endif // ENABLE(SVG)
Modified: trunk/Source/WebCore/svg/SVGUseElement.h (136540 => 136541)
--- trunk/Source/WebCore/svg/SVGUseElement.h 2012-12-04 19:29:48 UTC (rev 136540)
+++ trunk/Source/WebCore/svg/SVGUseElement.h 2012-12-04 19:35:30 UTC (rev 136541)
@@ -114,6 +114,7 @@
bool instanceTreeIsLoading(SVGElementInstance*);
virtual void notifyFinished(CachedResource*);
Document* referencedDocument() const;
+ void setCachedDocument(CachedResourceHandle<CachedSVGDocument>);
// SVGTests
virtual void synchronizeRequiredFeatures() { SVGTests::synchronizeRequiredFeatures(this); }
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-changes
