Title: [140049] trunk
Revision
140049
Author
schen...@chromium.org
Date
2013-01-17 14:51:03 -0800 (Thu, 17 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 atch fixes the prooblem, throwing JS exceptions where
possible and returning null where necessary.

Test: svg/dom/SVGViewSpec-invalid-ref-crash.html

* svg/SVGViewSpec.cpp:
(WebCore):
(WebCore::SVGViewSpec::viewTarget): Check for null target and throw an exception.
(WebCore::SVGViewSpec::transform): Check for null target and return
null. It is not possible to throw an exception here because it leads
to an invalid cast in the code generated from IDLs.
(WebCore::SVGViewSpec::viewBoxAnimated): Check for null target and throw an exception.
(WebCore::SVGViewSpec::preserveAspectRatioAnimated): Check for null target and throw an exception.
(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): Add Exception arguments to getter methods.
* svg/SVGViewSpec.idl: Mark attributes as throwing exceptions.

LayoutTests:

Test for the situation in which the target of an SVGViewSpec is
removed while the view spec lives on in JS.

* 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 (140048 => 140049)


--- trunk/LayoutTests/ChangeLog	2013-01-17 22:46:46 UTC (rev 140048)
+++ trunk/LayoutTests/ChangeLog	2013-01-17 22:51:03 UTC (rev 140049)
@@ -1,3 +1,16 @@
+2013-01-17  Stephen Chenney  <schen...@chromium.org>
+
+        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.
+
+        * svg/dom/SVGViewSpec-invalid-ref-crash-expected.txt: Added.
+        * svg/dom/SVGViewSpec-invalid-ref-crash.html: Added.
+
 2013-01-17  Julien Chaffraix  <jchaffr...@webkit.org>
 
         [CSS Grid Layout] Updating -webkit-grid-rows or -webkit-grid-columns doesn't work as expected

Added: trunk/LayoutTests/svg/dom/SVGViewSpec-invalid-ref-crash-expected.txt (0 => 140049)


--- trunk/LayoutTests/svg/dom/SVGViewSpec-invalid-ref-crash-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/svg/dom/SVGViewSpec-invalid-ref-crash-expected.txt	2013-01-17 22:51:03 UTC (rev 140049)
@@ -0,0 +1,13 @@
+Confirm that exceptions are thrown, or empty string 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; threw exception Error: InvalidStateError: DOM Exception 11.
+PASS viewTransform = svgView.transform; is null
+PASS viewViewTarget = svgView.viewTarget; threw exception Error: InvalidStateError: DOM Exception 11.
+PASS viewViewBox = svgView.viewBox; threw exception Error: InvalidStateError: DOM Exception 11.
+PASS successfullyParsed is true
+
+TEST COMPLETE
+

Added: trunk/LayoutTests/svg/dom/SVGViewSpec-invalid-ref-crash.html (0 => 140049)


--- trunk/LayoutTests/svg/dom/SVGViewSpec-invalid-ref-crash.html	                        (rev 0)
+++ trunk/LayoutTests/svg/dom/SVGViewSpec-invalid-ref-crash.html	2013-01-17 22:51:03 UTC (rev 140049)
@@ -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 exceptions are thrown, or empty string 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();
+
+    shouldThrow("viewPreserveAspectRatio = svgView.preserveAspectRatio;");
+    shouldBeNull("viewTransform = svgView.transform;");
+    shouldThrow("viewViewTarget = svgView.viewTarget;");
+    shouldThrow("viewViewBox = svgView.viewBox;");
+
+    successfullyParsed = true;
+
+</script>
+<script src=""
+</body>
+</html>

Modified: trunk/Source/WebCore/ChangeLog (140048 => 140049)


--- trunk/Source/WebCore/ChangeLog	2013-01-17 22:46:46 UTC (rev 140048)
+++ trunk/Source/WebCore/ChangeLog	2013-01-17 22:51:03 UTC (rev 140049)
@@ -1,3 +1,34 @@
+2013-01-17  Stephen Chenney  <schen...@chromium.org>
+
+        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 atch fixes the prooblem, throwing JS exceptions where
+        possible and returning null where necessary.
+
+        Test: svg/dom/SVGViewSpec-invalid-ref-crash.html
+
+        * svg/SVGViewSpec.cpp:
+        (WebCore):
+        (WebCore::SVGViewSpec::viewTarget): Check for null target and throw an exception.
+        (WebCore::SVGViewSpec::transform): Check for null target and return
+        null. It is not possible to throw an exception here because it leads
+        to an invalid cast in the code generated from IDLs.
+        (WebCore::SVGViewSpec::viewBoxAnimated): Check for null target and throw an exception.
+        (WebCore::SVGViewSpec::preserveAspectRatioAnimated): Check for null target and throw an exception.
+        (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): Add Exception arguments to getter methods.
+        * svg/SVGViewSpec.idl: Mark attributes as throwing exceptions.
+
 2013-01-17  Alec Flett  <alecfl...@chromium.org>
 
         IndexedDB: Remove unnecessary call to IDBDatabaseBackendInterface::metadata()

Modified: trunk/Source/WebCore/svg/SVGViewSpec.cpp (140048 => 140049)


--- trunk/Source/WebCore/svg/SVGViewSpec.cpp	2013-01-17 22:46:46 UTC (rev 140048)
+++ trunk/Source/WebCore/svg/SVGViewSpec.cpp	2013-01-17 22:51:03 UTC (rev 140049)
@@ -133,35 +133,51 @@
     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());
 }
 
-SVGElement* SVGViewSpec::viewTarget() const
+SVGElement* SVGViewSpec::viewTarget(ExceptionCode& ec) const
 {
-    if (!m_contextElement)
+    if (!m_contextElement) {
+        ec = INVALID_STATE_ERR;
         return 0;
+    }
     return static_cast<SVGElement*>(m_contextElement->treeScope()->getElementById(m_viewTargetString));
 }
 
 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(ExceptionCode& ec)
+{
+    if (!m_contextElement) {
+        ec = INVALID_STATE_ERR;
+        return 0;
+    }
+    return static_pointer_cast<SVGAnimatedRect>(lookupOrCreateViewBoxWrapper(this));
+}
+
+PassRefPtr<SVGAnimatedPreserveAspectRatio> SVGViewSpec::preserveAspectRatioAnimated(ExceptionCode& ec)
+{
+    if (!m_contextElement) {
+        ec = INVALID_STATE_ERR;
+        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 +185,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 +193,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 (140048 => 140049)


--- trunk/Source/WebCore/svg/SVGViewSpec.h	2013-01-17 22:46:46 UTC (rev 140048)
+++ trunk/Source/WebCore/svg/SVGViewSpec.h	2013-01-17 22:51:03 UTC (rev 140049)
@@ -49,10 +49,9 @@
     bool parseViewSpec(const String&);
     void reset();
 
-    SVGElement* viewTarget() const;
+    SVGElement* viewTarget(ExceptionCode&) 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(ExceptionCode&);
     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(ExceptionCode&);
     SVGPreserveAspectRatio& preserveAspectRatio() { return m_preserveAspectRatio; }
     SVGPreserveAspectRatio preserveAspectRatioBaseValue() const { return m_preserveAspectRatio; }
     void setPreserveAspectRatioBaseValue(const SVGPreserveAspectRatio& preserveAspectRatio) { m_preserveAspectRatio = preserveAspectRatio; }

Modified: trunk/Source/WebCore/svg/SVGViewSpec.idl (140048 => 140049)


--- trunk/Source/WebCore/svg/SVGViewSpec.idl	2013-01-17 22:46:46 UTC (rev 140048)
+++ trunk/Source/WebCore/svg/SVGViewSpec.idl	2013-01-17 22:51:03 UTC (rev 140049)
@@ -30,7 +30,8 @@
     JSGenerateToJSObject
 ] interface SVGViewSpec {
       readonly attribute SVGTransformList transform;
-      readonly attribute SVGElement viewTarget;
+      readonly attribute SVGElement viewTarget
+        getter raises(DOMException);
       readonly attribute DOMString viewBoxString;
       readonly attribute DOMString preserveAspectRatioString;
       readonly attribute DOMString transformString;
@@ -41,7 +42,9 @@
           setter raises(DOMException);
 
       // SVGFitToViewBox
-      readonly attribute SVGAnimatedRect viewBox;
-      readonly attribute SVGAnimatedPreserveAspectRatio preserveAspectRatio;
+      readonly attribute SVGAnimatedRect viewBox
+        getter raises(DOMException);
+      readonly attribute SVGAnimatedPreserveAspectRatio preserveAspectRatio
+        getter raises(DOMException);
 };
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to