[SVG] Fix root element length values handling.

Source/core/svg/SVGLengthContext.cpp

Index: Source/core/svg/SVGLengthContext.cpp
diff --git a/Source/core/svg/SVGLengthContext.cpp b/Source/core/svg/SVGLengthContext.cpp
index 0b75f922ea2a8ae58592f97796c27ecb32bc074f..09992adfadb46d4befaa2a2eebe15559e5cd8565 100644
--- a/Source/core/svg/SVGLengthContext.cpp
+++ b/Source/core/svg/SVGLengthContext.cpp
@@ -160,20 +160,19 @@ float SVGLengthContext::convertValueFromUserUnits(float value, SVGLengthMode mod
 
 float SVGLengthContext::convertValueFromUserUnitsToPercentage(float value, SVGLengthMode mode, ExceptionState& es) const
 {
-    float width = 0;
-    float height = 0;
-    if (!determineViewport(width, height)) {
+    FloatSize viewportSize;

[pdr., 2013/11/11 21:03:14]

Nit: I think we prefer to return the FloatSize and then check isEmpty. Up to
you, but I think it's easier to understand than out parameters.

[f(malita), 2013/11/11 21:14:29]

Doh, good point - if we only have one out-param now, why not just return it?
Possibly an extra copy, but I'm guessing RVO takes care of it.

Will do.

[f(malita), 2013/11/11 21:50:05]

On second thought, this causes a subtle change in semantics: we can no longer
differentiate between "cannot compute the viewport" (for which
SVGLengthContext::convertValueFromUserUnitsToPercentag throws an exception) and
"a valid, empty viewport" (no exception). Maybe it's OK to conflate the two, but
I'm thinking we shouldn't do it in this patch. What say you?

+    if (!determineViewport(viewportSize)) {
         es.throwUninformativeAndGenericDOMException(NotSupportedError);
         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;

[pdr., 2013/11/11 21:03:14]

square root of length squared? Please refactor for speed :)

[f(malita), 2013/11/11 21:14:29]

The name is misleading, there's no redundant squaring (what it's trying to say
is diagonalLengthBeforeSqrtf: w^2 + h^2) :)

[pdr., 2013/11/11 21:16:31]

Gah, I just spaced out here. You're right. Carry on :)

     };
 
     ASSERT_NOT_REACHED();
@@ -182,20 +181,19 @@ float SVGLengthContext::convertValueFromUserUnitsToPercentage(float value, SVGLe
 
 float SVGLengthContext::convertValueFromPercentageToUserUnits(float value, SVGLengthMode mode, ExceptionState& es) const
 {
-    float width = 0;
-    float height = 0;
-    if (!determineViewport(width, height)) {
+    FloatSize viewportSize;
+    if (!determineViewport(viewportSize)) {
         es.throwUninformativeAndGenericDOMException(NotSupportedError);
         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();
@@ -279,21 +277,22 @@ float SVGLengthContext::convertValueFromEXSToUserUnits(float value, ExceptionSta
     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();
@@ -301,12 +300,10 @@ bool SVGLengthContext::determineViewport(float& width, float& height) const
         return false;
 
     const SVGSVGElement* svg = static_cast<const SVGSVGElement*>(viewportElement);
-    FloatSize viewportSize = svg->currentViewBoxRect().size();
+    viewportSize = svg->currentViewBoxRect().size();
     if (viewportSize.isEmpty())
         viewportSize = svg->currentViewportSize();
 
-    width = viewportSize.width();
-    height = viewportSize.height();
     return true;
 }