- Revision
- 140975
- Author
- [email protected]
- Date
- 2013-01-28 09:45:45 -0800 (Mon, 28 Jan 2013)
Log Message
SVGViewSpec fails when corresponding element has been removed
https://bugs.webkit.org/show_bug.cgi?id=106957
Reviewed by Dirk Schulze.
Source/WebCore:
When JS holds an SVGViewSpec object while deleting the object that
defines the spec (an SVGSVGElement, or one of a few others) the
pointer to the target is cleared in the SVGViewSpec but the methods
that serve JS queries do not check and try to access the now null
target. This patch fixes the prooblem, returning null when the
corresponding object has been deleted.
Also removing SVGViewSpec::setPreserveAspectRatioString, which is no
longer used by any callers.
Test: svg/dom/SVGViewSpec-invalid-ref-crash.html
* svg/SVGViewSpec.cpp:
(WebCore):
(WebCore::SVGViewSpec::viewTarget): Check for null target and return null.
(WebCore::SVGViewSpec::transform): Check for null target and return null..
(WebCore::SVGViewSpec::viewBoxAnimated): Check for null target and return null.
(WebCore::SVGViewSpec::preserveAspectRatioAnimated): Check for null target and return null.
(WebCore::SVGViewSpec::lookupOrCreateViewBoxWrapper): ASSERT non-null target.
(WebCore::SVGViewSpec::lookupOrCreatePreserveAspectRatioWrapper): ASSERT non-null target.
(WebCore::SVGViewSpec::lookupOrCreateTransformWrapper): ASSERT non-null target.
* svg/SVGViewSpec.h:
(SVGViewSpec): Move some methods out of the header and into the implementation file.
* svg/SVGViewSpec.cpp:
(WebCore):
(WebCore::SVGViewSpec::transform):
(WebCore::SVGViewSpec::viewBoxAnimated):
(WebCore::SVGViewSpec::preserveAspectRatioAnimated):
(WebCore::SVGViewSpec::lookupOrCreateViewBoxWrapper):
(WebCore::SVGViewSpec::lookupOrCreatePreserveAspectRatioWrapper):
(WebCore::SVGViewSpec::lookupOrCreateTransformWrapper):
* svg/SVGViewSpec.h:
(SVGViewSpec):
LayoutTests:
Test for the situation in which the target of an SVGViewSpec is
removed while the view spec lives on in JS. The test is expected to
fail on all JSC based platforms because the element that must be
deleted to trigger the results is not deleted upon GC.
* svg/dom/SVGViewSpec-invalid-ref-crash-expected.txt: Added.
* svg/dom/SVGViewSpec-invalid-ref-crash.html: Added.
* platform/efl/TestExpectations:
* platform/gtk/TestExpectations:
* platform/mac/TestExpectations:
* platform/qt/TestExpectations:
* platform/win/TestExpectations:
* svg/dom/SVGViewSpec-invalid-ref-crash-expected.txt: Added.
* svg/dom/SVGViewSpec-invalid-ref-crash.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (140974 => 140975)
--- trunk/LayoutTests/ChangeLog 2013-01-28 17:05:59 UTC (rev 140974)
+++ trunk/LayoutTests/ChangeLog 2013-01-28 17:45:45 UTC (rev 140975)
@@ -1,3 +1,26 @@
+2013-01-28 Stephen Chenney <[email protected]>
+
+ SVGViewSpec fails when corresponding element has been removed
+ https://bugs.webkit.org/show_bug.cgi?id=106957
+
+ Reviewed by Dirk Schulze.
+
+ Test for the situation in which the target of an SVGViewSpec is
+ removed while the view spec lives on in JS. The test is expected to
+ fail on all JSC based platforms because the element that must be
+ deleted to trigger the results is not deleted upon GC.
+
+ * svg/dom/SVGViewSpec-invalid-ref-crash-expected.txt: Added.
+ * svg/dom/SVGViewSpec-invalid-ref-crash.html: Added.
+
+ * platform/efl/TestExpectations:
+ * platform/gtk/TestExpectations:
+ * platform/mac/TestExpectations:
+ * platform/qt/TestExpectations:
+ * platform/win/TestExpectations:
+ * svg/dom/SVGViewSpec-invalid-ref-crash-expected.txt: Added.
+ * svg/dom/SVGViewSpec-invalid-ref-crash.html: Added.
+
2013-01-28 James Craig <[email protected]>
HTML5 promotes DL from specific 'definition list' to superset 'description list'; accessibility strings and accessors should be updated to match.
Modified: trunk/LayoutTests/platform/efl/TestExpectations (140974 => 140975)
--- trunk/LayoutTests/platform/efl/TestExpectations 2013-01-28 17:05:59 UTC (rev 140974)
+++ trunk/LayoutTests/platform/efl/TestExpectations 2013-01-28 17:45:45 UTC (rev 140975)
@@ -1798,6 +1798,9 @@
# https://bugs.webkit.org/show_bug.cgi?id=106883
inspector/editor/text-editor-formatter.html
+# Test fails on JSC platforms due to GC timing problems
+webkit.org/b/106957 svg/dom/SVGViewSpec-invalid-ref-crash.html [ Failure ]
+
# Fail to generate proper pixel results - let's wait with rebaselining.
webkit.org/b/106992 scrollingcoordinator/non-fast-scrollable-region-scaled-iframe.html [ Failure ]
webkit.org/b/106992 scrollingcoordinator/non-fast-scrollable-region-transformed-iframe.html [ Failure ]
Modified: trunk/LayoutTests/platform/gtk/TestExpectations (140974 => 140975)
--- trunk/LayoutTests/platform/gtk/TestExpectations 2013-01-28 17:05:59 UTC (rev 140974)
+++ trunk/LayoutTests/platform/gtk/TestExpectations 2013-01-28 17:45:45 UTC (rev 140975)
@@ -1392,6 +1392,9 @@
webkit.org/b/107257 [ Debug ] fast/dom/Window/window-postmessage-clone-deep-array.html [ Failure ]
webkit.org/b/107257 [ Debug ] fast/js/large-expressions.html [ Failure ]
+# Test fails on JSC platforms due to GC timing issues
+webkit.org/b/106957 svg/dom/SVGViewSpec-invalid-ref-crash.html [ Failure ]
+
webkit.org/b/107194 storage/indexeddb/dont-commit-on-blocked.html [ Failure ]
webkit.org/b/107194 storage/indexeddb/objectstore-basics-workers.html [ Failure ]
webkit.org/b/107194 storage/indexeddb/objectstore-basics.html [ Failure ]
Modified: trunk/LayoutTests/platform/mac/TestExpectations (140974 => 140975)
--- trunk/LayoutTests/platform/mac/TestExpectations 2013-01-28 17:05:59 UTC (rev 140974)
+++ trunk/LayoutTests/platform/mac/TestExpectations 2013-01-28 17:45:45 UTC (rev 140975)
@@ -1267,6 +1267,8 @@
webkit.org/b/106415 fast/workers/worker-document-leak.html [ Pass Failure ]
webkit.org/b/106415 fast/workers/worker-lifecycle.html [ Pass Failure ]
+webkit.org/b/106957 svg/dom/SVGViewSpec-invalid-ref-crash.html [ Failure ]
+
# Enable when support drag-and-drop autoscrolling
webkit.org/b/39725 fast/events/drag-and-drop-autoscroll.html [ Skip ]
Modified: trunk/LayoutTests/platform/qt/TestExpectations (140974 => 140975)
--- trunk/LayoutTests/platform/qt/TestExpectations 2013-01-28 17:05:59 UTC (rev 140974)
+++ trunk/LayoutTests/platform/qt/TestExpectations 2013-01-28 17:45:45 UTC (rev 140975)
@@ -1370,7 +1370,9 @@
webkit.org/b/105437 svg/filters/feImage-target-remove-from-document.svg [ ImageOnlyFailure Pass ]
webkit.org/b/105437 svg/filters/feImage-target-style-change.svg [ ImageOnlyFailure Pass ]
+webkit.org/b/106957 svg/dom/SVGViewSpec-invalid-ref-crash.html [ Failure ]
+
# ============================================================================= #
# Failing CSS Tests
# ============================================================================= #
Modified: trunk/LayoutTests/platform/win/TestExpectations (140974 => 140975)
--- trunk/LayoutTests/platform/win/TestExpectations 2013-01-28 17:05:59 UTC (rev 140974)
+++ trunk/LayoutTests/platform/win/TestExpectations 2013-01-28 17:45:45 UTC (rev 140975)
@@ -2515,6 +2515,10 @@
media/video-controls-captions-trackmenu.html
media/video-controls-visible-exiting-fullscreen.html
+# Fails on JSC platforms due to gc timing issues
+# https://bugs.webkit.org/show_bug.cgi?id=106957
+svg/dom/SVGViewSpec-invalid-ref-crash.html
+
# Enable when support drag-and-drop autoscrolling
webkit.org/b/39725 fast/events/drag-and-drop-autoscroll.html [ Skip ]
Added: trunk/LayoutTests/svg/dom/SVGViewSpec-invalid-ref-crash-expected.txt (0 => 140975)
--- trunk/LayoutTests/svg/dom/SVGViewSpec-invalid-ref-crash-expected.txt (rev 0)
+++ trunk/LayoutTests/svg/dom/SVGViewSpec-invalid-ref-crash-expected.txt 2013-01-28 17:45:45 UTC (rev 140975)
@@ -0,0 +1,13 @@
+Confirm that null is returned when an SVGViewSpec is used after its corresponding element has been removed.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS viewPreserveAspectRatio = svgView.preserveAspectRatio; is null
+PASS viewTransform = svgView.transform; is null
+PASS viewViewTarget = svgView.viewTarget; is null
+PASS viewViewBox = svgView.viewBox; is null
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/svg/dom/SVGViewSpec-invalid-ref-crash.html (0 => 140975)
--- trunk/LayoutTests/svg/dom/SVGViewSpec-invalid-ref-crash.html (rev 0)
+++ trunk/LayoutTests/svg/dom/SVGViewSpec-invalid-ref-crash.html 2013-01-28 17:45:45 UTC (rev 140975)
@@ -0,0 +1,30 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+<script src=""
+</head>
+<body>
+<p id="description"></p>
+<div id="console"></div>
+<div id="container"><svg xmlns="http://www.w3.org/2000/svg" width="200" height="200"/></div>
+<script>
+ description("Confirm that null is returned when an SVGViewSpec is used after its corresponding element has been removed.");
+
+ successfullyParsed = false;
+
+ var svgView = document.getElementById("container").childNodes[0].currentView;
+ document.getElementById("container").removeChild(document.getElementById("container").childNodes[0]);
+
+ gc();
+
+ shouldBeNull("viewPreserveAspectRatio = svgView.preserveAspectRatio;");
+ shouldBeNull("viewTransform = svgView.transform;");
+ shouldBeNull("viewViewTarget = svgView.viewTarget;");
+ shouldBeNull("viewViewBox = svgView.viewBox;");
+
+ successfullyParsed = true;
+
+</script>
+<script src=""
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (140974 => 140975)
--- trunk/Source/WebCore/ChangeLog 2013-01-28 17:05:59 UTC (rev 140974)
+++ trunk/Source/WebCore/ChangeLog 2013-01-28 17:45:45 UTC (rev 140975)
@@ -1,3 +1,45 @@
+2013-01-28 Stephen Chenney <[email protected]>
+
+ SVGViewSpec fails when corresponding element has been removed
+ https://bugs.webkit.org/show_bug.cgi?id=106957
+
+ Reviewed by Dirk Schulze.
+
+ When JS holds an SVGViewSpec object while deleting the object that
+ defines the spec (an SVGSVGElement, or one of a few others) the
+ pointer to the target is cleared in the SVGViewSpec but the methods
+ that serve JS queries do not check and try to access the now null
+ target. This patch fixes the prooblem, returning null when the
+ corresponding object has been deleted.
+
+ Also removing SVGViewSpec::setPreserveAspectRatioString, which is no
+ longer used by any callers.
+
+ Test: svg/dom/SVGViewSpec-invalid-ref-crash.html
+
+ * svg/SVGViewSpec.cpp:
+ (WebCore):
+ (WebCore::SVGViewSpec::viewTarget): Check for null target and return null.
+ (WebCore::SVGViewSpec::transform): Check for null target and return null..
+ (WebCore::SVGViewSpec::viewBoxAnimated): Check for null target and return null.
+ (WebCore::SVGViewSpec::preserveAspectRatioAnimated): Check for null target and return null.
+ (WebCore::SVGViewSpec::lookupOrCreateViewBoxWrapper): ASSERT non-null target.
+ (WebCore::SVGViewSpec::lookupOrCreatePreserveAspectRatioWrapper): ASSERT non-null target.
+ (WebCore::SVGViewSpec::lookupOrCreateTransformWrapper): ASSERT non-null target.
+ * svg/SVGViewSpec.h:
+ (SVGViewSpec): Move some methods out of the header and into the implementation file.
+
+ * svg/SVGViewSpec.cpp:
+ (WebCore):
+ (WebCore::SVGViewSpec::transform):
+ (WebCore::SVGViewSpec::viewBoxAnimated):
+ (WebCore::SVGViewSpec::preserveAspectRatioAnimated):
+ (WebCore::SVGViewSpec::lookupOrCreateViewBoxWrapper):
+ (WebCore::SVGViewSpec::lookupOrCreatePreserveAspectRatioWrapper):
+ (WebCore::SVGViewSpec::lookupOrCreateTransformWrapper):
+ * svg/SVGViewSpec.h:
+ (SVGViewSpec):
+
2013-01-28 James Craig <[email protected]>
HTML5 promotes DL from specific 'definition list' to superset 'description list'; accessibility strings and accessors should be updated to match.
Modified: trunk/Source/WebCore/svg/SVGViewSpec.cpp (140974 => 140975)
--- trunk/Source/WebCore/svg/SVGViewSpec.cpp 2013-01-28 17:05:59 UTC (rev 140974)
+++ trunk/Source/WebCore/svg/SVGViewSpec.cpp 2013-01-28 17:45:45 UTC (rev 140975)
@@ -133,13 +133,6 @@
return SVGPropertyTraits<FloatRect>::toString(viewBoxBaseValue());
}
-void SVGViewSpec::setPreserveAspectRatioString(const String& preserve)
-{
- SVGPreserveAspectRatio preserveAspectRatio;
- preserveAspectRatio.parse(preserve);
- setPreserveAspectRatioBaseValue(preserveAspectRatio);
-}
-
String SVGViewSpec::preserveAspectRatioString() const
{
return SVGPropertyTraits<SVGPreserveAspectRatio>::toString(preserveAspectRatioBaseValue());
@@ -154,14 +147,31 @@
SVGTransformListPropertyTearOff* SVGViewSpec::transform()
{
+ if (!m_contextElement)
+ return 0;
// Return the animVal here, as its readonly by default - which is exactly what we want here.
return static_cast<SVGTransformListPropertyTearOff*>(static_pointer_cast<SVGAnimatedTransformList>(lookupOrCreateTransformWrapper(this))->animVal());
}
+PassRefPtr<SVGAnimatedRect> SVGViewSpec::viewBoxAnimated()
+{
+ if (!m_contextElement)
+ return 0;
+ return static_pointer_cast<SVGAnimatedRect>(lookupOrCreateViewBoxWrapper(this));
+}
+
+PassRefPtr<SVGAnimatedPreserveAspectRatio> SVGViewSpec::preserveAspectRatioAnimated()
+{
+ if (!m_contextElement)
+ return 0;
+ return static_pointer_cast<SVGAnimatedPreserveAspectRatio>(lookupOrCreatePreserveAspectRatioWrapper(this));
+}
+
PassRefPtr<SVGAnimatedProperty> SVGViewSpec::lookupOrCreateViewBoxWrapper(void* maskedOwnerType)
{
ASSERT(maskedOwnerType);
SVGViewSpec* ownerType = static_cast<SVGViewSpec*>(maskedOwnerType);
+ ASSERT(ownerType->contextElement());
return SVGAnimatedProperty::lookupOrCreateWrapper<SVGElement, SVGAnimatedRect, FloatRect>(ownerType->contextElement(), viewBoxPropertyInfo(), ownerType->m_viewBox);
}
@@ -169,6 +179,7 @@
{
ASSERT(maskedOwnerType);
SVGViewSpec* ownerType = static_cast<SVGViewSpec*>(maskedOwnerType);
+ ASSERT(ownerType->contextElement());
return SVGAnimatedProperty::lookupOrCreateWrapper<SVGElement, SVGAnimatedPreserveAspectRatio, SVGPreserveAspectRatio>(ownerType->contextElement(), preserveAspectRatioPropertyInfo(), ownerType->m_preserveAspectRatio);
}
@@ -176,6 +187,7 @@
{
ASSERT(maskedOwnerType);
SVGViewSpec* ownerType = static_cast<SVGViewSpec*>(maskedOwnerType);
+ ASSERT(ownerType->contextElement());
return SVGAnimatedProperty::lookupOrCreateWrapper<SVGElement, SVGAnimatedTransformList, SVGTransformList>(ownerType->contextElement(), transformPropertyInfo(), ownerType->m_transform);
}
Modified: trunk/Source/WebCore/svg/SVGViewSpec.h (140974 => 140975)
--- trunk/Source/WebCore/svg/SVGViewSpec.h 2013-01-28 17:05:59 UTC (rev 140974)
+++ trunk/Source/WebCore/svg/SVGViewSpec.h 2013-01-28 17:45:45 UTC (rev 140975)
@@ -52,7 +52,6 @@
SVGElement* viewTarget() const;
String viewBoxString() const;
- void setPreserveAspectRatioString(const String&);
String preserveAspectRatioString() const;
void setTransformString(const String&);
@@ -74,21 +73,13 @@
SVGTransformList transformBaseValue() const { return m_transform; }
// Custom animated 'viewBox' property.
- PassRefPtr<SVGAnimatedRect> viewBoxAnimated()
- {
- return static_pointer_cast<SVGAnimatedRect>(lookupOrCreateViewBoxWrapper(this));
- }
-
+ PassRefPtr<SVGAnimatedRect> viewBoxAnimated();
FloatRect& viewBox() { return m_viewBox; }
FloatRect viewBoxBaseValue() const { return m_viewBox; }
void setViewBoxBaseValue(const FloatRect& viewBox) { m_viewBox = viewBox; }
// Custom animated 'preserveAspectRatio' property.
- PassRefPtr<SVGAnimatedPreserveAspectRatio> preserveAspectRatioAnimated()
- {
- return static_pointer_cast<SVGAnimatedPreserveAspectRatio>(lookupOrCreatePreserveAspectRatioWrapper(this));
- }
-
+ PassRefPtr<SVGAnimatedPreserveAspectRatio> preserveAspectRatioAnimated();
SVGPreserveAspectRatio& preserveAspectRatio() { return m_preserveAspectRatio; }
SVGPreserveAspectRatio preserveAspectRatioBaseValue() const { return m_preserveAspectRatio; }
void setPreserveAspectRatioBaseValue(const SVGPreserveAspectRatio& preserveAspectRatio) { m_preserveAspectRatio = preserveAspectRatio; }
