2011-06-24 Abhishek Arya <inferno@chromium.org>
authorinferno@chromium.org <inferno@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 24 Jun 2011 21:45:16 +0000 (21:45 +0000)
committerinferno@chromium.org <inferno@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 24 Jun 2011 21:45:16 +0000 (21:45 +0000)
        Reviewed by Darin Adler.

        Match other clampTo* functions in style with clampToInteger(float)
        function.
        https://bugs.webkit.org/show_bug.cgi?id=53449

        * wtf/MathExtras.h:
        (clampToInteger):
        (clampToFloat):
        (clampToPositiveInteger):
2011-06-24  Abhishek Arya  <inferno@chromium.org>

        Reviewed by Darin Adler.

        Add clamping for CSSPrimitiveValues and SVGInlineText font size.
        https://bugs.webkit.org/show_bug.cgi?id=53449

        Test: svg/text/svg-zoom-large-value.xhtml

        * css/CSSPrimitiveValue.cpp:
        (WebCore::CSSPrimitiveValue::CSSPrimitiveValue): add asserts to detect if the
        number created is valid.
        * css/CSSPrimitiveValue.h: add clamping checks to prevent overflows.
        (WebCore::CSSPrimitiveValue::getFloatValue):
        (WebCore::CSSPrimitiveValue::getIntValue):
        * css/CSSStyleSelector.cpp:
        (WebCore::CSSStyleSelector::getComputedSizeFromSpecifiedSize): split into two
        static functions, one specific to CSSStyleSelector and other generic to help
        in clamping font size for other callers like svg text, etc.
        * css/CSSStyleSelector.h:
        * platform/graphics/FontDescription.h: add asserts to detect if the new font
        size is valid.
        (WebCore::FontDescription::setComputedSize):
        (WebCore::FontDescription::setSpecifiedSize):
        * rendering/svg/RenderSVGInlineText.cpp:
        (WebCore::RenderSVGInlineText::computeNewScaledFontForStyle): use the new helper
        from CSSStyleSelector to help in clamping new scaled font size. do not use
        "smart minimum" since svg allows really small unreadable fonts (tested by existing
        layout tests). Document's minimum font size clamp (0 in my case) and harmless epsilon
        check in CSSStyleSelector function should still hold for svg.
2011-06-24  Abhishek Arya  <inferno@chromium.org>

        Reviewed by Darin Adler.

        Tests that font size for svg text zoom is clamped and we do not
        crash on ASSERT(isfinite(s)) in FontDescription.h
        https://bugs.webkit.org/show_bug.cgi?id=53449

        * svg/text/svg-zoom-large-value-expected.txt: Added.
        * svg/text/svg-zoom-large-value.xhtml: Added.

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

12 files changed:
LayoutTests/ChangeLog
LayoutTests/svg/text/svg-zoom-large-value-expected.txt [new file with mode: 0644]
LayoutTests/svg/text/svg-zoom-large-value.xhtml [new file with mode: 0644]
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/wtf/MathExtras.h
Source/WebCore/ChangeLog
Source/WebCore/css/CSSPrimitiveValue.cpp
Source/WebCore/css/CSSPrimitiveValue.h
Source/WebCore/css/CSSStyleSelector.cpp
Source/WebCore/css/CSSStyleSelector.h
Source/WebCore/platform/graphics/FontDescription.h
Source/WebCore/rendering/svg/RenderSVGInlineText.cpp

index 560d636..79b9121 100644 (file)
@@ -1,3 +1,14 @@
+2011-06-24  Abhishek Arya  <inferno@chromium.org>
+
+        Reviewed by Darin Adler.
+
+        Tests that font size for svg text zoom is clamped and we do not
+        crash on ASSERT(isfinite(s)) in FontDescription.h
+        https://bugs.webkit.org/show_bug.cgi?id=53449
+
+        * svg/text/svg-zoom-large-value-expected.txt: Added.
+        * svg/text/svg-zoom-large-value.xhtml: Added.
+
 2011-06-24  Dominic Cooney  <dominicc@chromium.org>
 
         Unreviewed: Skipping failing progress-clone.html on win.
diff --git a/LayoutTests/svg/text/svg-zoom-large-value-expected.txt b/LayoutTests/svg/text/svg-zoom-large-value-expected.txt
new file mode 100644 (file)
index 0000000..69cfc5a
--- /dev/null
@@ -0,0 +1,2 @@
+PASS
+
diff --git a/LayoutTests/svg/text/svg-zoom-large-value.xhtml b/LayoutTests/svg/text/svg-zoom-large-value.xhtml
new file mode 100644 (file)
index 0000000..b42f576
--- /dev/null
@@ -0,0 +1,16 @@
+<html xmlns="http://www.w3.org/1999/xhtml">\r
+<head>\r
+<style>\r
+#svg1 { zoom: 10000000044444535353444433333333333333333333333333333333333333333333333333333330000000 }\r
+</style>\r
+</head>\r
+<body>\r
+<svg id="svg1" xmlns="http://www.w3.org/2000/svg">\r
+<text x="50" y="50">PASS</text>\r
+</svg>\r
+<script>\r
+if (window.layoutTestController)\r
+    layoutTestController.dumpAsText();\r
+</script>\r
+</body>\r
+</html>\r
index ca716eb..c99d7b9 100644 (file)
@@ -1,3 +1,16 @@
+2011-06-24  Abhishek Arya  <inferno@chromium.org>
+
+        Reviewed by Darin Adler.
+
+        Match other clampTo* functions in style with clampToInteger(float)
+        function.
+        https://bugs.webkit.org/show_bug.cgi?id=53449
+
+        * wtf/MathExtras.h:
+        (clampToInteger):
+        (clampToFloat):
+        (clampToPositiveInteger):
+
 2011-06-24  Sheriff Bot  <webkit.review.bot@gmail.com>
 
         Unreviewed, rolling out r89594.
index f1b13a5..5498316 100644 (file)
@@ -207,45 +207,71 @@ inline float deg2turn(float d) { return d / 360.0f; }
 inline float rad2grad(float r) { return r * 200.0f / piFloat; }
 inline float grad2rad(float g) { return g * piFloat / 200.0f; }
 
-inline int clampToInteger(double d)
+inline int clampToInteger(double x)
 {
-    const double minIntAsDouble = std::numeric_limits<int>::min();
-    const double maxIntAsDouble = std::numeric_limits<int>::max();
-    return static_cast<int>(std::max(std::min(d, maxIntAsDouble), minIntAsDouble));
+    const double intMax = static_cast<double>(std::numeric_limits<int>::max());
+    const double intMin = static_cast<double>(std::numeric_limits<int>::min());
+    
+    if (x >= intMax)
+        return std::numeric_limits<int>::max();
+    if (x <= intMin)
+        return std::numeric_limits<int>::min();
+    return static_cast<int>(x);
 }
 
-inline int clampToPositiveInteger(double d)
+inline float clampToFloat(double x)
 {
-    const double maxIntAsDouble = std::numeric_limits<int>::max();
-    return static_cast<int>(std::max<double>(std::min(d, maxIntAsDouble), 0));
+    const double floatMax = static_cast<double>(std::numeric_limits<float>::max());
+    const double floatMin = -static_cast<double>(std::numeric_limits<float>::max());
+    
+    if (x >= floatMax)
+        return std::numeric_limits<float>::max();
+    if (x <= floatMin)
+        return -std::numeric_limits<float>::max();
+    return static_cast<float>(x);
+}
+
+inline int clampToPositiveInteger(double x)
+{
+    const double intMax = static_cast<double>(std::numeric_limits<int>::max());
+    
+    if (x >= intMax)
+        return std::numeric_limits<int>::max();
+    if (x <= 0)
+        return 0;
+    return static_cast<int>(x);
 }
 
 inline int clampToInteger(float x)
 {
-    static const int s_intMax = std::numeric_limits<int>::max();
-    static const int s_intMin = std::numeric_limits<int>::min();
+    const float intMax = static_cast<float>(std::numeric_limits<int>::max());
+    const float intMin = static_cast<float>(std::numeric_limits<int>::min());
     
-    if (x >= static_cast<float>(s_intMax))
-        return s_intMax;
-    if (x < static_cast<float>(s_intMin))
-        return s_intMin;
+    if (x >= intMax)
+        return std::numeric_limits<int>::max();
+    if (x <= intMin)
+        return std::numeric_limits<int>::min();
     return static_cast<int>(x);
 }
 
 inline int clampToPositiveInteger(float x)
 {
-    static const int s_intMax = std::numeric_limits<int>::max();
+    const float intMax = static_cast<float>(std::numeric_limits<int>::max());
     
-    if (x >= static_cast<float>(s_intMax))
-        return s_intMax;
-    if (x < 0)
+    if (x >= intMax)
+        return std::numeric_limits<int>::max();
+    if (x <= 0)
         return 0;
     return static_cast<int>(x);
 }
 
-inline int clampToInteger(unsigned value)
+inline int clampToInteger(unsigned x)
 {
-    return static_cast<int>(std::min(value, static_cast<unsigned>(std::numeric_limits<int>::max())));
+    const unsigned intMax = static_cast<unsigned>(std::numeric_limits<int>::max());
+    
+    if (x >= intMax)
+        return std::numeric_limits<int>::max();
+    return static_cast<int>(x);
 }
 
 #if !COMPILER(MSVC) && !(COMPILER(RVCT) && PLATFORM(BREWMP)) && !OS(SOLARIS) && !OS(SYMBIAN)
index 1995e85..1886419 100644 (file)
@@ -1,3 +1,34 @@
+2011-06-24  Abhishek Arya  <inferno@chromium.org>
+
+        Reviewed by Darin Adler.
+
+        Add clamping for CSSPrimitiveValues and SVGInlineText font size.
+        https://bugs.webkit.org/show_bug.cgi?id=53449        
+
+        Test: svg/text/svg-zoom-large-value.xhtml
+
+        * css/CSSPrimitiveValue.cpp:
+        (WebCore::CSSPrimitiveValue::CSSPrimitiveValue): add asserts to detect if the
+        number created is valid.
+        * css/CSSPrimitiveValue.h: add clamping checks to prevent overflows.
+        (WebCore::CSSPrimitiveValue::getFloatValue):
+        (WebCore::CSSPrimitiveValue::getIntValue):
+        * css/CSSStyleSelector.cpp:
+        (WebCore::CSSStyleSelector::getComputedSizeFromSpecifiedSize): split into two
+        static functions, one specific to CSSStyleSelector and other generic to help
+        in clamping font size for other callers like svg text, etc.
+        * css/CSSStyleSelector.h:
+        * platform/graphics/FontDescription.h: add asserts to detect if the new font
+        size is valid.
+        (WebCore::FontDescription::setComputedSize):
+        (WebCore::FontDescription::setSpecifiedSize):
+        * rendering/svg/RenderSVGInlineText.cpp:
+        (WebCore::RenderSVGInlineText::computeNewScaledFontForStyle): use the new helper
+        from CSSStyleSelector to help in clamping new scaled font size. do not use
+        "smart minimum" since svg allows really small unreadable fonts (tested by existing
+        layout tests). Document's minimum font size clamp (0 in my case) and harmless epsilon
+        check in CSSStyleSelector function should still hold for svg.
+
 2011-06-24  Julien Chaffraix  <jchaffraix@webkit.org>
 
         Reviewed by Darin Adler.
index 14f3f5a..be889ab 100644 (file)
@@ -36,7 +36,6 @@
 #include "RenderStyle.h"
 #include <wtf/ASCIICType.h>
 #include <wtf/DecimalNumber.h>
-#include <wtf/MathExtras.h>
 #include <wtf/StdLibExtras.h>
 #include <wtf/text/StringBuffer.h>
 
@@ -130,6 +129,7 @@ CSSPrimitiveValue::CSSPrimitiveValue(double num, UnitTypes type)
     : m_type(type)
     , m_hasCachedCSSText(false)
 {
+    ASSERT(isfinite(num));
     m_value.num = num;
 }
 
@@ -158,6 +158,7 @@ CSSPrimitiveValue::CSSPrimitiveValue(const Length& length)
             break;
         case WebCore::Fixed:
             m_type = CSS_PX;
+            ASSERT(isfinite(length.value()));
             m_value.num = length.value();
             break;
         case Intrinsic:
@@ -170,6 +171,7 @@ CSSPrimitiveValue::CSSPrimitiveValue(const Length& length)
             break;
         case Percent:
             m_type = CSS_PERCENTAGE;
+            ASSERT(isfinite(length.percent()));
             m_value.num = length.percent();
             break;
         case Relative:
index 3dede56..907d0b9 100644 (file)
@@ -25,6 +25,7 @@
 #include "CSSValue.h"
 #include "Color.h"
 #include <wtf/Forward.h>
+#include <wtf/MathExtras.h>
 #include <wtf/PassRefPtr.h>
 
 namespace WebCore {
@@ -147,13 +148,13 @@ public:
     double getDoubleValue() const { return m_value.num; }
 
     void setFloatValue(unsigned short unitType, double floatValue, ExceptionCode&);
-    float getFloatValue(unsigned short unitType, ExceptionCode& ec) const { return static_cast<float>(getDoubleValue(unitType, ec)); }
-    float getFloatValue(unsigned short unitType) const { return static_cast<float>(getDoubleValue(unitType)); }
-    float getFloatValue() const { return static_cast<float>(m_value.num); }
+    float getFloatValue(unsigned short unitType, ExceptionCode& ec) const { return clampToFloat(getDoubleValue(unitType, ec)); }
+    float getFloatValue(unsigned short unitType) const { return clampToFloat(getDoubleValue(unitType)); }
+    float getFloatValue() const { return clampToFloat(m_value.num); }
 
-    int getIntValue(unsigned short unitType, ExceptionCode& ec) const { return static_cast<int>(getDoubleValue(unitType, ec)); }
-    int getIntValue(unsigned short unitType) const { return static_cast<int>(getDoubleValue(unitType)); }
-    int getIntValue() const { return static_cast<int>(m_value.num); }
+    int getIntValue(unsigned short unitType, ExceptionCode& ec) const { return clampToInteger(getDoubleValue(unitType, ec)); }
+    int getIntValue(unsigned short unitType) const { return clampToInteger(getDoubleValue(unitType)); }
+    int getIntValue() const { return clampToInteger(m_value.num); }
 
     void setStringValue(unsigned short stringType, const String& stringValue, ExceptionCode&);
     String getStringValue(ExceptionCode&) const;
index 8aac7de..5fc6e29 100644 (file)
@@ -6130,13 +6130,6 @@ void CSSStyleSelector::setFontSize(FontDescription& fontDescription, float size)
 
 float CSSStyleSelector::getComputedSizeFromSpecifiedSize(Document* document, RenderStyle* style, bool isAbsoluteSize, float specifiedSize, bool useSVGZoomRules)
 {
-    // Text with a 0px font size should not be visible and therefore needs to be
-    // exempt from minimum font size rules. Acid3 relies on this for pixel-perfect
-    // rendering. This is also compatible with other browsers that have minimum
-    // font size settings (e.g. Firefox).
-    if (fabsf(specifiedSize) < std::numeric_limits<float>::epsilon())
-        return 0.0f;
-
     float zoomFactor = 1.0f;
     if (!useSVGZoomRules) {
         zoomFactor = style->effectiveZoom();
@@ -6144,6 +6137,18 @@ float CSSStyleSelector::getComputedSizeFromSpecifiedSize(Document* document, Ren
             zoomFactor *= frame->textZoomFactor();
     }
 
+    return CSSStyleSelector::getComputedSizeFromSpecifiedSize(document, zoomFactor, isAbsoluteSize, specifiedSize);
+}
+
+float CSSStyleSelector::getComputedSizeFromSpecifiedSize(Document* document, float zoomFactor, bool isAbsoluteSize, float specifiedSize, ESmartMinimumForFontSize useSmartMinimumForFontSize)
+{
+    // Text with a 0px font size should not be visible and therefore needs to be
+    // exempt from minimum font size rules. Acid3 relies on this for pixel-perfect
+    // rendering. This is also compatible with other browsers that have minimum
+    // font size settings (e.g. Firefox).
+    if (fabsf(specifiedSize) < std::numeric_limits<float>::epsilon())
+        return 0.0f;
+
     // We support two types of minimum font size.  The first is a hard override that applies to
     // all fonts.  This is "minSize."  The second type of minimum font size is a "smart minimum"
     // that is applied only when the Web page can't know what size it really asked for, e.g.,
@@ -6170,7 +6175,7 @@ float CSSStyleSelector::getComputedSizeFromSpecifiedSize(Document* document, Ren
     // after zooming.  The font size must either be relative to the user default or the original size
     // must have been acceptable.  In other words, we only apply the smart minimum whenever we're positive
     // doing so won't disrupt the layout.
-    if (zoomedSize < minLogicalSize && (specifiedSize >= minLogicalSize || !isAbsoluteSize))
+    if (useSmartMinimumForFontSize && zoomedSize < minLogicalSize && (specifiedSize >= minLogicalSize || !isAbsoluteSize))
         zoomedSize = minLogicalSize;
     
     // Also clamp to a reasonable maximum to prevent insane font sizes from causing crashes on various
index 71c7c5a..101a189 100644 (file)
@@ -35,6 +35,8 @@
 
 namespace WebCore {
 
+enum ESmartMinimumForFontSize { DoNotUseSmartMinimumForFontSize, UseSmartMinimumForFontFize };
+
 class CSSFontSelector;
 class CSSMutableStyleDeclaration;
 class CSSPageRule;
@@ -172,9 +174,12 @@ public:
         void setStyle(PassRefPtr<RenderStyle> s) { m_style = s; } // Used by the document when setting up its root style.
 
         void applyPropertyToStyle(int id, CSSValue*, RenderStyle*);
+        
+        static float getComputedSizeFromSpecifiedSize(Document*, float zoomFactor, bool isAbsoluteSize, float specifiedSize, ESmartMinimumForFontSize = UseSmartMinimumForFontFize);
 
     private:
         void setFontSize(FontDescription&, float size);
+
         static float getComputedSizeFromSpecifiedSize(Document*, RenderStyle*, bool isAbsoluteSize, float specifiedSize, bool useSVGZoomRules);
     public:
         Color getColorFromPrimitiveValue(CSSPrimitiveValue*) const;
index 421b82e..908eba2 100644 (file)
@@ -33,6 +33,7 @@
 #include "FontWidthVariant.h"
 #include "TextOrientation.h"
 #include "TextRenderingMode.h"
+#include <wtf/MathExtras.h>
 
 namespace WebCore {
 
@@ -115,8 +116,8 @@ public:
     FontWidthVariant widthVariant() const { return m_widthVariant; }
 
     void setFamily(const FontFamily& family) { m_familyList = family; }
-    void setComputedSize(float s) { m_computedSize = s; }
-    void setSpecifiedSize(float s) { m_specifiedSize = s; }
+    void setComputedSize(float s) { ASSERT(isfinite(s)); m_computedSize = s; }
+    void setSpecifiedSize(float s) { ASSERT(isfinite(s)); m_specifiedSize = s; }
     void setItalic(FontItalic i) { m_italic = i; }
     void setItalic(bool i) { setItalic(i ? FontItalicOn : FontItalicOff); }
     void setSmallCaps(FontSmallCaps c) { m_smallCaps = c; }
index a7be133..d7c2cbf 100644 (file)
@@ -250,7 +250,7 @@ void RenderSVGInlineText::computeNewScaledFontForStyle(RenderObject* renderer, c
     }
 
     FontDescription fontDescription(style->fontDescription());
-    fontDescription.setComputedSize(fontDescription.computedSize() * scalingFactor);
+    fontDescription.setComputedSize(CSSStyleSelector::getComputedSizeFromSpecifiedSize(document, scalingFactor, fontDescription.isAbsoluteSize(), fontDescription.computedSize(), DoNotUseSmartMinimumForFontSize));
 
     scaledFont = Font(fontDescription, 0, 0);
     scaledFont.update(styleSelector->fontSelector());