Fix ASSERTION FAILED in WebCore::SVGLengthContext::determineViewport
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 18 Dec 2013 18:14:12 +0000 (18:14 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 18 Dec 2013 18:14:12 +0000 (18:14 +0000)
https://bugs.webkit.org/show_bug.cgi?id=120284

Patch by Tamas Gergely <tgergely.u-szeged@partner.samsung.com> on 2013-12-18
Reviewed by Philip Rogers.

Source/WebCore:

Added handling of root <svg> elements.
Blink merge: https://chromium.googlesource.com/chromium/blink/+/a7dedf81eb7008276bb6854f0e46465e039788f8

SVGLengthContext::determineViewport() currently asserts that we're not
resolving lengths for the topmost element, but there's nothing to
prevent such calls.

The patch updates determineViewport() to handle root elements geracefully
(using their current viewport). It also changes the signature slightly
to operate directly on a FloatSize, reducing some of the boiler-plate
client code.

Tests: svg/custom/svg-length-value-handled.svg
       svg/dom/svg-root-lengths.html

* svg/SVGLengthContext.cpp:
(WebCore::SVGLengthContext::convertValueFromUserUnitsToPercentage):
(WebCore::SVGLengthContext::convertValueFromPercentageToUserUnits):
(WebCore::SVGLengthContext::determineViewport):
* svg/SVGLengthContext.h:
* svg/graphics/filters/SVGFEImage.cpp:
(WebCore::FEImage::platformApplySoftware):

LayoutTests:

Added tests of handling root <svg> elements.
Blink merge: https://chromium.googlesource.com/chromium/blink/+/a7dedf81eb7008276bb6854f0e46465e039788f8

* svg/custom/svg-length-value-handled-expected.txt: Added.
* svg/custom/svg-length-value-handled.svg: Added.
    Tests whether root svg elements sizes are handled.
* svg/dom/svg-root-lengths-expected.txt: Added.
* svg/dom/svg-root-lengths.html: Added.
    Tests the correct handling of root svg elements sizes.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@160774 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/svg/custom/svg-length-value-handled-expected.txt [new file with mode: 0644]
LayoutTests/svg/custom/svg-length-value-handled.svg [new file with mode: 0644]
LayoutTests/svg/dom/svg-root-lengths-expected.txt [new file with mode: 0644]
LayoutTests/svg/dom/svg-root-lengths.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/svg/SVGLengthContext.cpp
Source/WebCore/svg/SVGLengthContext.h
Source/WebCore/svg/graphics/filters/SVGFEImage.cpp

index edfcec0..22077e4 100644 (file)
@@ -1,3 +1,20 @@
+2013-12-18  Tamas Gergely  <tgergely.u-szeged@partner.samsung.com>
+
+        Fix ASSERTION FAILED in WebCore::SVGLengthContext::determineViewport
+        https://bugs.webkit.org/show_bug.cgi?id=120284
+
+        Reviewed by Philip Rogers.
+
+        Added tests of handling root <svg> elements.
+        Blink merge: https://chromium.googlesource.com/chromium/blink/+/a7dedf81eb7008276bb6854f0e46465e039788f8
+
+        * svg/custom/svg-length-value-handled-expected.txt: Added.
+        * svg/custom/svg-length-value-handled.svg: Added.
+            Tests whether root svg elements sizes are handled.
+        * svg/dom/svg-root-lengths-expected.txt: Added.
+        * svg/dom/svg-root-lengths.html: Added.
+            Tests the correct handling of root svg elements sizes.
+
 2013-12-18  Darin Adler  <darin@apple.com>
 
         Additional refinement in MathMLSelectElement toggle implementation
diff --git a/LayoutTests/svg/custom/svg-length-value-handled-expected.txt b/LayoutTests/svg/custom/svg-length-value-handled-expected.txt
new file mode 100644 (file)
index 0000000..1cb4f5f
--- /dev/null
@@ -0,0 +1 @@
+PASS: root svg element size handled (0x0)
diff --git a/LayoutTests/svg/custom/svg-length-value-handled.svg b/LayoutTests/svg/custom/svg-length-value-handled.svg
new file mode 100644 (file)
index 0000000..2385bc1
--- /dev/null
@@ -0,0 +1,13 @@
+<svg xmlns="http://www.w3.org/2000/svg">
+  <rect width="100" height="100" fill="green"/>
+  <text id="label" y="200"/>
+    <script>
+      if (window.testRunner)
+        testRunner.dumpAsText();
+
+      document.getElementById("label").textContent =
+        "PASS: root svg element size handled (" +
+        document.rootElement.width.baseVal.value + "x" +
+        document.rootElement.height.baseVal.value + ")";
+    </script>
+</svg>
diff --git a/LayoutTests/svg/dom/svg-root-lengths-expected.txt b/LayoutTests/svg/dom/svg-root-lengths-expected.txt
new file mode 100644 (file)
index 0000000..85cc423
--- /dev/null
@@ -0,0 +1,24 @@
+This tests the behavior of root SVG length value resolution
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+ PASS successfullyParsed is true
+
+TEST COMPLETE
+Initial/default values:
+PASS svg.width.baseVal.value is 200
+PASS svg.height.baseVal.value is 200
+
+Updated relative values:
+PASS svg.width.baseVal.value is 100
+PASS svg.height.baseVal.value is 20
+
+Updated fixed values:
+PASS svg.width.baseVal.value is 150
+PASS svg.height.baseVal.value is 50
+
+viewBox has no effect on top level length resolution.
+PASS svg.width.baseVal.value is 200
+PASS svg.height.baseVal.value is 100
+
diff --git a/LayoutTests/svg/dom/svg-root-lengths.html b/LayoutTests/svg/dom/svg-root-lengths.html
new file mode 100644 (file)
index 0000000..e48cd0d
--- /dev/null
@@ -0,0 +1,58 @@
+<!DOCTYPE html>
+<html>
+  <head>
+    <script src="../../resources/js-test-pre.js"></script>
+  </head>
+  <body>
+    <p id="description"></p>
+    <div id="div" style="width: 200px; height: 200px; border: 1px solid red;">
+      <svg id="svg" xmlns="http://www.w3.org/2000/svg" style="border: 1px solid blue;">
+        <rect width="100%" height="100%" fill="green"/>
+      </svg>
+    </div>
+    <div id="console"></div>
+    <script>
+      if (window.testRunner) {
+        testRunner.waitUntilDone();
+        testRunner.dumpAsText();
+      }
+
+      setTimeout(function () {
+        var div = document.getElementById('div');
+        var svg = document.getElementById('svg');
+
+        description('This tests the behavior of root SVG length value resolution');
+
+        debug('Initial/default values:');
+        shouldBe('svg.width.baseVal.value', '200');
+        shouldBe('svg.height.baseVal.value', '200');
+
+        svg.setAttribute('width', '50%');
+        svg.setAttribute('height', '10%');
+        debug('');
+        debug('Updated relative values:');
+        shouldBe('svg.width.baseVal.value', '100');
+        shouldBe('svg.height.baseVal.value', '20');
+
+        svg.setAttribute('width', '150');
+        svg.setAttribute('height', '50');
+        debug('');
+        debug('Updated fixed values:');
+        shouldBe('svg.width.baseVal.value', '150');
+        shouldBe('svg.height.baseVal.value', '50');
+
+        svg.setAttribute('width', '100%');
+        svg.setAttribute('height', '50%');
+        svg.setAttribute('viewBox', '0 0 800 600');
+        debug('');
+        debug('viewBox has no effect on top level length resolution.');
+        shouldBe('svg.width.baseVal.value', '200');
+        shouldBe('svg.height.baseVal.value', '100');
+
+        if (window.testRunner)
+          testRunner.notifyDone();
+      }, 0);
+    </script>
+    <script src="../../resources/js-test-post.js"></script>
+  </body>
+</html>
index 19a1851..c223383 100644 (file)
@@ -1,3 +1,33 @@
+2013-12-18  Tamas Gergely  <tgergely.u-szeged@partner.samsung.com>
+
+        Fix ASSERTION FAILED in WebCore::SVGLengthContext::determineViewport
+        https://bugs.webkit.org/show_bug.cgi?id=120284
+
+        Reviewed by Philip Rogers.
+
+        Added handling of root <svg> elements.
+        Blink merge: https://chromium.googlesource.com/chromium/blink/+/a7dedf81eb7008276bb6854f0e46465e039788f8
+
+        SVGLengthContext::determineViewport() currently asserts that we're not
+        resolving lengths for the topmost element, but there's nothing to
+        prevent such calls.
+
+        The patch updates determineViewport() to handle root elements geracefully
+        (using their current viewport). It also changes the signature slightly
+        to operate directly on a FloatSize, reducing some of the boiler-plate
+        client code.
+
+        Tests: svg/custom/svg-length-value-handled.svg
+               svg/dom/svg-root-lengths.html
+
+        * svg/SVGLengthContext.cpp:
+        (WebCore::SVGLengthContext::convertValueFromUserUnitsToPercentage):
+        (WebCore::SVGLengthContext::convertValueFromPercentageToUserUnits):
+        (WebCore::SVGLengthContext::determineViewport):
+        * svg/SVGLengthContext.h:
+        * svg/graphics/filters/SVGFEImage.cpp:
+        (WebCore::FEImage::platformApplySoftware):
+
 2013-12-18  Darin Adler  <darin@apple.com>
 
         Additional refinement in MathMLSelectElement toggle implementation
index 0d084bd..dc73170 100644 (file)
@@ -161,20 +161,19 @@ float SVGLengthContext::convertValueFromUserUnits(float value, SVGLengthMode mod
 
 float SVGLengthContext::convertValueFromUserUnitsToPercentage(float value, SVGLengthMode mode, ExceptionCode& ec) const
 {
-    float width = 0;
-    float height = 0;
-    if (!determineViewport(width, height)) {
+    FloatSize viewportSize;
+    if (!determineViewport(viewportSize)) {
         ec = NOT_SUPPORTED_ERR;
         return 0;
     }
 
     switch (mode) {
     case LengthModeWidth:
-        return value / width * 100;
+        return value / viewportSize.width() * 100;
     case LengthModeHeight:
-        return value / height * 100;
+        return value / viewportSize.height() * 100;
     case LengthModeOther:
-        return value / (sqrtf((width * width + height * height) / 2)) * 100;
+        return value / (sqrtf(viewportSize.diagonalLengthSquared() / 2)) * 100;
     };
 
     ASSERT_NOT_REACHED();
@@ -183,20 +182,19 @@ float SVGLengthContext::convertValueFromUserUnitsToPercentage(float value, SVGLe
 
 float SVGLengthContext::convertValueFromPercentageToUserUnits(float value, SVGLengthMode mode, ExceptionCode& ec) const
 {
-    float width = 0;
-    float height = 0;
-    if (!determineViewport(width, height)) {
+    FloatSize viewportSize;
+    if (!determineViewport(viewportSize)) {
         ec = NOT_SUPPORTED_ERR;
         return 0;
     }
 
     switch (mode) {
     case LengthModeWidth:
-        return value * width;
+        return value * viewportSize.width();
     case LengthModeHeight:
-        return value * height;
+        return value * viewportSize.height();
     case LengthModeOther:
-        return value * sqrtf((width * width + height * height) / 2);
+        return value * sqrtf(viewportSize.diagonalLengthSquared() / 2);
     };
 
     ASSERT_NOT_REACHED();
@@ -280,34 +278,33 @@ float SVGLengthContext::convertValueFromEXSToUserUnits(float value, ExceptionCod
     return value * ceilf(style->fontMetrics().xHeight());
 }
 
-bool SVGLengthContext::determineViewport(float& width, float& height) const
+bool SVGLengthContext::determineViewport(FloatSize& viewportSize) const
 {
     if (!m_context)
         return false;
 
     // If an overriden viewport is given, it has precedence.
     if (!m_overridenViewport.isEmpty()) {
-        width = m_overridenViewport.width();
-        height = m_overridenViewport.height();
+        viewportSize = m_overridenViewport.size();
         return true;
     }
 
-    // SVGLengthContext should NEVER be used to resolve width/height values for <svg> elements,
-    // as they require special treatment, due the relationship with the CSS width/height properties.
-    ASSERT(m_context->document().documentElement() != m_context);
+    // Root <svg> element lengths are resolved against the top level viewport.
+    if (m_context->isOutermostSVGSVGElement()) {
+        viewportSize = toSVGSVGElement(m_context)->currentViewportSize();
+        return true;
+    }
 
     // Take size from nearest viewport element.
     SVGElement* viewportElement = m_context->viewportElement();
     if (!viewportElement || !isSVGSVGElement(viewportElement))
         return false;
-    
+
     const SVGSVGElement* svg = toSVGSVGElement(viewportElement);
-    FloatSize viewportSize = svg->currentViewBoxRect().size();
+    viewportSize = svg->currentViewBoxRect().size();
     if (viewportSize.isEmpty())
         viewportSize = svg->currentViewportSize();
 
-    width = viewportSize.width();
-    height = viewportSize.height();
     return true;
 }
 
index f350a7d..c7070f8 100644 (file)
@@ -68,7 +68,7 @@ public:
     float convertValueToUserUnits(float, SVGLengthMode, SVGLengthType fromUnit, ExceptionCode&) const;
     float convertValueFromUserUnits(float, SVGLengthMode, SVGLengthType toUnit, ExceptionCode&) const;
 
-    bool determineViewport(float& width, float& height) const;
+    bool determineViewport(FloatSize&) const;
 
 private:
     SVGLengthContext(const SVGElement*, const FloatRect& viewport);
index 4bbe42b..4d3a5d9 100644 (file)
@@ -128,13 +128,12 @@ void FEImage::platformApplySoftware()
         SVGElement* contextNode = toSVGElement(renderer->element());
         if (contextNode->hasRelativeLengths()) {
             SVGLengthContext lengthContext(contextNode);
-            float width = 0;
-            float height = 0;
+            FloatSize viewportSize;
 
             // If we're referencing an element with percentage units, eg. <rect with="30%"> those values were resolved against the viewport.
             // Build up a transformation that maps from the viewport space to the filter primitive subregion.
-            if (lengthContext.determineViewport(width, height))
-                resultImage->context()->concatCTM(makeMapBetweenRects(FloatRect(0, 0, width, height), destRect));
+            if (lengthContext.determineViewport(viewportSize))
+                resultImage->context()->concatCTM(makeMapBetweenRects(FloatRect(FloatPoint(), viewportSize), destRect));
         }
 
         AffineTransform contentTransformation;