[Forms][Meter][Progress] Change function signature of parseToDoubleForNumberType
authoryosin@chromium.org <yosin@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 22 May 2012 07:53:22 +0000 (07:53 +0000)
committeryosin@chromium.org <yosin@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 22 May 2012 07:53:22 +0000 (07:53 +0000)
https://bugs.webkit.org/show_bug.cgi?id=87077

Reviewed by Hajime Morita.

This patch changes function signature of parseToDoubleForNumberType and
parseToDoubleForNumberTypeWithDecimalPlaces to return double value instead
of bool for reducing code in call sites for ease of maintenance. This patch
also allows to use functional style of using these functions.

No new tests. This patch doesn't change behavior.

* html/HTMLMeterElement.cpp:
(WebCore::HTMLMeterElement::min): Changed for using double return value.
(WebCore::HTMLMeterElement::max): Changed for using double return value.
(WebCore::HTMLMeterElement::value): Changed for using double return value.
(WebCore::HTMLMeterElement::low): Changed for using double return value.
(WebCore::HTMLMeterElement::high): Changed for using double return value.
(WebCore::HTMLMeterElement::optimum): Changed for using double return value.
* html/HTMLProgressElement.cpp:
(WebCore::HTMLProgressElement::value): Changed for using double return value.
(WebCore::HTMLProgressElement::max): Changed for using double return value.
* html/NumberInputType.cpp:
(WebCore::NumberInputType::typeMismatchFor): Changed for using double return value.
(WebCore::NumberInputType::sizeShouldIncludeDecoration): Changed for using double return value.
(WebCore::NumberInputType::parseToDouble): Changed for using double return value.
(WebCore::NumberInputType::parseToDoubleWithDecimalPlaces): Changed for using double return value.
(WebCore::NumberInputType::visibleValue): Changed for using double return value.
(WebCore::NumberInputType::sanitizeValue): Changed for using double return value.
* html/RangeInputType.cpp:
(WebCore::RangeInputType::parseToDouble): Changed for using double return value.
* html/StepRange.cpp:
(WebCore::StepRange::parseStep): Changed for using double return value.
* html/StepRange.h:
(WebCore::StepRange::defaultValue): Added "const" attribute
(WebCore::StepRange::proportionFromValue): Added "const" attribute
(WebCore::StepRange::valueFromProportion): Added "const" attribute
* html/parser/HTMLParserIdioms.cpp:
(WebCore::parseToDoubleForNumberType): Changed for using double return value. Added one parameter function.
(WebCore::parseToDoubleForNumberTypeWithDecimalPlaces): Changed for using double return value. Added function for providing default fallback value.
* html/parser/HTMLParserIdioms.h: Changed function prototype and added one parameter prototypes.
* html/shadow/SliderThumbElement.cpp:
(WebCore::sliderPosition): Changed for using double return value.

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

Source/WebCore/ChangeLog
Source/WebCore/html/HTMLMeterElement.cpp
Source/WebCore/html/HTMLProgressElement.cpp
Source/WebCore/html/NumberInputType.cpp
Source/WebCore/html/RangeInputType.cpp
Source/WebCore/html/StepRange.cpp
Source/WebCore/html/StepRange.h
Source/WebCore/html/parser/HTMLParserIdioms.cpp
Source/WebCore/html/parser/HTMLParserIdioms.h
Source/WebCore/html/shadow/SliderThumbElement.cpp

index 47b8617..4265c1d 100644 (file)
@@ -1,3 +1,49 @@
+2012-05-22  Yoshifumi Inoue  <yosin@chromium.org>
+
+        [Forms][Meter][Progress] Change function signature of parseToDoubleForNumberType
+        https://bugs.webkit.org/show_bug.cgi?id=87077
+
+        Reviewed by Hajime Morita.
+
+        This patch changes function signature of parseToDoubleForNumberType and
+        parseToDoubleForNumberTypeWithDecimalPlaces to return double value instead
+        of bool for reducing code in call sites for ease of maintenance. This patch
+        also allows to use functional style of using these functions.
+
+        No new tests. This patch doesn't change behavior.
+
+        * html/HTMLMeterElement.cpp:
+        (WebCore::HTMLMeterElement::min): Changed for using double return value.
+        (WebCore::HTMLMeterElement::max): Changed for using double return value.
+        (WebCore::HTMLMeterElement::value): Changed for using double return value.
+        (WebCore::HTMLMeterElement::low): Changed for using double return value.
+        (WebCore::HTMLMeterElement::high): Changed for using double return value.
+        (WebCore::HTMLMeterElement::optimum): Changed for using double return value.
+        * html/HTMLProgressElement.cpp:
+        (WebCore::HTMLProgressElement::value): Changed for using double return value.
+        (WebCore::HTMLProgressElement::max): Changed for using double return value.
+        * html/NumberInputType.cpp:
+        (WebCore::NumberInputType::typeMismatchFor): Changed for using double return value.
+        (WebCore::NumberInputType::sizeShouldIncludeDecoration): Changed for using double return value.
+        (WebCore::NumberInputType::parseToDouble): Changed for using double return value.
+        (WebCore::NumberInputType::parseToDoubleWithDecimalPlaces): Changed for using double return value.
+        (WebCore::NumberInputType::visibleValue): Changed for using double return value.
+        (WebCore::NumberInputType::sanitizeValue): Changed for using double return value.
+        * html/RangeInputType.cpp:
+        (WebCore::RangeInputType::parseToDouble): Changed for using double return value.
+        * html/StepRange.cpp:
+        (WebCore::StepRange::parseStep): Changed for using double return value.
+        * html/StepRange.h:
+        (WebCore::StepRange::defaultValue): Added "const" attribute
+        (WebCore::StepRange::proportionFromValue): Added "const" attribute
+        (WebCore::StepRange::valueFromProportion): Added "const" attribute
+        * html/parser/HTMLParserIdioms.cpp:
+        (WebCore::parseToDoubleForNumberType): Changed for using double return value. Added one parameter function.
+        (WebCore::parseToDoubleForNumberTypeWithDecimalPlaces): Changed for using double return value. Added function for providing default fallback value.
+        * html/parser/HTMLParserIdioms.h: Changed function prototype and added one parameter prototypes.
+        * html/shadow/SliderThumbElement.cpp:
+        (WebCore::sliderPosition): Changed for using double return value.
+
 2012-05-22  Kentaro Hara  <haraken@chromium.org>
 
         REGRESSION r110315: Event handler throws TypeError for an input element with name="arguments"
index 4f48229..07f4d77 100644 (file)
@@ -81,9 +81,7 @@ void HTMLMeterElement::parseAttribute(const Attribute& attribute)
 
 double HTMLMeterElement::min() const
 {
-    double min = 0;
-    parseToDoubleForNumberType(getAttribute(minAttr), &min);
-    return min;
+    return parseToDoubleForNumberType(getAttribute(minAttr), 0);
 }
 
 void HTMLMeterElement::setMin(double min, ExceptionCode& ec)
@@ -97,9 +95,7 @@ void HTMLMeterElement::setMin(double min, ExceptionCode& ec)
 
 double HTMLMeterElement::max() const
 {
-    double max = std::max(1.0, min());
-    parseToDoubleForNumberType(getAttribute(maxAttr), &max);
-    return std::max(max, min());
+    return std::max(parseToDoubleForNumberType(getAttribute(maxAttr), std::max(1.0, min())), min());
 }
 
 void HTMLMeterElement::setMax(double max, ExceptionCode& ec)
@@ -113,8 +109,7 @@ void HTMLMeterElement::setMax(double max, ExceptionCode& ec)
 
 double HTMLMeterElement::value() const
 {
-    double value = 0;
-    parseToDoubleForNumberType(getAttribute(valueAttr), &value);
+    double value = parseToDoubleForNumberType(getAttribute(valueAttr), 0);
     return std::min(std::max(value, min()), max());
 }
 
@@ -129,8 +124,7 @@ void HTMLMeterElement::setValue(double value, ExceptionCode& ec)
 
 double HTMLMeterElement::low() const
 {
-    double low = min();
-    parseToDoubleForNumberType(getAttribute(lowAttr), &low);
+    double low = parseToDoubleForNumberType(getAttribute(lowAttr), min());
     return std::min(std::max(low, min()), max());
 }
 
@@ -145,8 +139,7 @@ void HTMLMeterElement::setLow(double low, ExceptionCode& ec)
 
 double HTMLMeterElement::high() const
 {
-    double high = max();
-    parseToDoubleForNumberType(getAttribute(highAttr), &high);
+    double high = parseToDoubleForNumberType(getAttribute(highAttr), max());
     return std::min(std::max(high, low()), max());
 }
 
@@ -161,8 +154,7 @@ void HTMLMeterElement::setHigh(double high, ExceptionCode& ec)
 
 double HTMLMeterElement::optimum() const
 {
-    double optimum = (max() + min()) / 2;
-    parseToDoubleForNumberType(getAttribute(optimumAttr), &optimum);
+    double optimum = parseToDoubleForNumberType(getAttribute(optimumAttr), (max() + min()) / 2);
     return std::min(std::max(optimum, min()), max());
 }
 
index 2d47d82..c571a74 100644 (file)
@@ -91,11 +91,8 @@ void HTMLProgressElement::attach()
 
 double HTMLProgressElement::value() const
 {
-    double value;
-    bool ok = parseToDoubleForNumberType(fastGetAttribute(valueAttr), &value);
-    if (!ok || value < 0)
-        return 0;
-    return (value > max()) ? max() : value;
+    double value = parseToDoubleForNumberType(fastGetAttribute(valueAttr));
+    return !isfinite(value) || value < 0 ? 0 : std::min(value, max());
 }
 
 void HTMLProgressElement::setValue(double value, ExceptionCode& ec)
@@ -109,11 +106,8 @@ void HTMLProgressElement::setValue(double value, ExceptionCode& ec)
 
 double HTMLProgressElement::max() const
 {
-    double max;
-    bool ok = parseToDoubleForNumberType(getAttribute(maxAttr), &max);
-    if (!ok || max <= 0)
-        return 1;
-    return max;
+    double max = parseToDoubleForNumberType(getAttribute(maxAttr));
+    return !isfinite(max) || max <= 0 ? 1 : max;
 }
 
 void HTMLProgressElement::setMax(double max, ExceptionCode& ec)
index a1c9612..941d252 100644 (file)
@@ -99,7 +99,7 @@ void NumberInputType::setValueAsNumber(double newValue, TextFieldEventBehavior e
 
 bool NumberInputType::typeMismatchFor(const String& value) const
 {
-    return !value.isEmpty() && !parseToDoubleForNumberType(value, 0);
+    return !value.isEmpty() && !isfinite(parseToDoubleForNumberType(value));
 }
 
 bool NumberInputType::typeMismatch() const
@@ -127,15 +127,15 @@ bool NumberInputType::sizeShouldIncludeDecoration(int defaultSize, int& preferre
     preferredSize = defaultSize;
 
     unsigned minValueDecimalPlaces;
-    double minValueDouble;
     String minValue = element()->fastGetAttribute(minAttr);
-    if (!parseToDoubleForNumberTypeWithDecimalPlaces(minValue, &minValueDouble, &minValueDecimalPlaces))
+    double minValueDouble = parseToDoubleForNumberTypeWithDecimalPlaces(minValue, &minValueDecimalPlaces);
+    if (!isfinite(minValueDouble))
         return false;
 
     unsigned maxValueDecimalPlaces;
-    double maxValueDouble;
     String maxValue = element()->fastGetAttribute(maxAttr);
-    if (!parseToDoubleForNumberTypeWithDecimalPlaces(maxValue, &maxValueDouble, &maxValueDecimalPlaces))
+    double maxValueDouble = parseToDoubleForNumberTypeWithDecimalPlaces(maxValue, &maxValueDecimalPlaces);
+    if (!isfinite(maxValueDouble))
         return false;
 
     if (maxValueDouble < minValueDouble) {
@@ -143,12 +143,12 @@ bool NumberInputType::sizeShouldIncludeDecoration(int defaultSize, int& preferre
         maxValueDecimalPlaces = minValueDecimalPlaces;
     }
 
-    unsigned stepValueDecimalPlaces;
-    double stepValueDouble;
     String stepValue = element()->fastGetAttribute(stepAttr);
     if (equalIgnoringCase(stepValue, "any"))
         return false;
-    if (!parseToDoubleForNumberTypeWithDecimalPlaces(stepValue, &stepValueDouble, &stepValueDecimalPlaces)) {
+    unsigned stepValueDecimalPlaces;
+    double stepValueDouble = parseToDoubleForNumberTypeWithDecimalPlaces(stepValue, &stepValueDecimalPlaces);
+    if (!isfinite(stepValueDouble)) {
         stepValueDouble = 1;
         stepValueDecimalPlaces = 0;
     }
@@ -188,20 +188,12 @@ void NumberInputType::handleWheelEvent(WheelEvent* event)
 
 double NumberInputType::parseToDouble(const String& src, double defaultValue) const
 {
-    double numberValue;
-    if (!parseToDoubleForNumberType(src, &numberValue))
-        return defaultValue;
-    ASSERT(isfinite(numberValue));
-    return numberValue;
+    return parseToDoubleForNumberType(src, defaultValue);
 }
 
 double NumberInputType::parseToDoubleWithDecimalPlaces(const String& src, double defaultValue, unsigned *decimalPlaces) const
 {
-    double numberValue;
-    if (!parseToDoubleForNumberTypeWithDecimalPlaces(src, &numberValue, decimalPlaces))
-        return defaultValue;
-    ASSERT(isfinite(numberValue));
-    return numberValue;
+    return parseToDoubleForNumberTypeWithDecimalPlaces(src, decimalPlaces, defaultValue);
 }
 
 String NumberInputType::serialize(double value) const
@@ -236,9 +228,10 @@ String NumberInputType::visibleValue() const
         return currentValue;
     // FIXME: The following three lines should be removed when we
     // remove the second argument of convertToLocalizedNumber().
-    double doubleValue = numeric_limits<double>::quiet_NaN();
+    // Note: parseToDoubleForNumberTypeWithDecimalPlaces set zero to decimalPlaces
+    // if currentValue isn't valid floating pointer number.
     unsigned decimalPlace;
-    parseToDoubleForNumberTypeWithDecimalPlaces(currentValue, &doubleValue, &decimalPlace);
+    parseToDoubleForNumberTypeWithDecimalPlaces(currentValue, &decimalPlace);
     return convertToLocalizedNumber(currentValue, decimalPlace);
 }
 
@@ -262,7 +255,7 @@ String NumberInputType::sanitizeValue(const String& proposedValue) const
 {
     if (proposedValue.isEmpty())
         return proposedValue;
-    return parseToDoubleForNumberType(proposedValue, 0) ? proposedValue : emptyAtom.string();
+    return isfinite(parseToDoubleForNumberType(proposedValue)) ? proposedValue : emptyString();
 }
 
 bool NumberInputType::hasUnacceptableValue()
index 6f590e4..b417dc3 100644 (file)
@@ -221,11 +221,7 @@ RenderObject* RangeInputType::createRenderer(RenderArena* arena, RenderStyle*) c
 
 double RangeInputType::parseToDouble(const String& src, double defaultValue) const
 {
-    double numberValue;
-    if (!parseToDoubleForNumberType(src, &numberValue))
-        return defaultValue;
-    ASSERT(isfinite(numberValue));
-    return numberValue;
+    return parseToDoubleForNumberType(src, defaultValue);
 }
 
 String RangeInputType::serialize(double value) const
index e973afe..9b9b7dc 100644 (file)
@@ -122,7 +122,8 @@ StepRange::DoubleWithDecimalPlacesOrMissing StepRange::parseStep(AnyStepHandling
     }
 
     DoubleWithDecimalPlacesOrMissing step(0);
-    if (!parseToDoubleForNumberTypeWithDecimalPlaces(stepString, &step.value.value, &step.value.decimalPlaces) || step.value.value <= 0.0)
+    step.value.value = parseToDoubleForNumberTypeWithDecimalPlaces(stepString, &step.value.decimalPlaces);
+    if (!isfinite(step.value.value) || step.value.value <= 0.0)
         return DoubleWithDecimalPlacesOrMissing(stepDescription.defaultValue());
 
     switch (stepDescription.stepValueShouldBe) {
index 1a11019..0adbfd8 100644 (file)
@@ -106,13 +106,13 @@ public:
     bool stepMismatch(double) const;
 
     // Clamp the middle value according to the step
-    double defaultValue()
+    double defaultValue() const
     {
         return clampValue((m_minimum + m_maximum) / 2);
     }
 
     // Map value into 0-1 range
-    double proportionFromValue(double value)
+    double proportionFromValue(double value) const
     {
         if (m_minimum == m_maximum)
             return 0;
@@ -121,7 +121,7 @@ public:
     }
 
     // Map from 0-1 range to value
-    double valueFromProportion(double proportion)
+    double valueFromProportion(double proportion) const
     {
         return m_minimum + proportion * (m_maximum - m_minimum);
     }
index b860202..a99a21a 100644 (file)
@@ -66,47 +66,49 @@ String serializeForNumberType(double number)
     return String(numberToString(number, buffer));
 }
 
-bool parseToDoubleForNumberType(const String& string, double* result)
+double parseToDoubleForNumberType(const String& string, double fallbackValue)
 {
     // See HTML5 2.5.4.3 `Real numbers.'
 
     // String::toDouble() accepts leading + and whitespace characters, which are not valid here.
     UChar firstCharacter = string[0];
     if (firstCharacter != '-' && firstCharacter != '.' && !isASCIIDigit(firstCharacter))
-        return false;
+        return fallbackValue;
 
     bool valid = false;
     double value = string.toDouble(&valid);
     if (!valid)
-        return false;
+        return fallbackValue;
 
     // NaN and infinity are considered valid by String::toDouble, but not valid here.
     if (!isfinite(value))
-        return false;
+        return fallbackValue;
 
     // Numbers are considered finite IEEE 754 single-precision floating point values.
     // See HTML5 2.5.4.3 `Real numbers.'
     if (-std::numeric_limits<float>::max() > value || value > std::numeric_limits<float>::max())
-        return false;
+        return fallbackValue;
 
-    if (result) {
-        // The following expression converts -0 to +0.
-        *result = value ? value : 0;
-    }
+    // The following expression converts -0 to +0.
+    return value ? value : 0;
+}
 
-    return true;
+double parseToDoubleForNumberType(const String& string)
+{
+    return parseToDoubleForNumberType(string, std::numeric_limits<double>::quiet_NaN());
 }
 
-bool parseToDoubleForNumberTypeWithDecimalPlaces(const String& string, double *result, unsigned *decimalPlaces)
+double parseToDoubleForNumberTypeWithDecimalPlaces(const String& string, unsigned *decimalPlaces, double fallbackValue)
 {
     if (decimalPlaces)
         *decimalPlaces = 0;
 
-    if (!parseToDoubleForNumberType(string, result))
-        return false;
+    double value = parseToDoubleForNumberType(string, std::numeric_limits<double>::quiet_NaN());
+    if (!isfinite(value))
+        return fallbackValue;
 
     if (!decimalPlaces)
-        return true;
+        return value;
 
     size_t dotIndex = string.find('.');
     size_t eIndex = string.find('e');
@@ -166,7 +168,12 @@ bool parseToDoubleForNumberTypeWithDecimalPlaces(const String& string, double *r
     else
         *decimalPlaces = static_cast<unsigned>(intDecimalPlaces);
 
-    return true;
+    return value;
+}
+
+double parseToDoubleForNumberTypeWithDecimalPlaces(const String& string, unsigned *decimalPlaces)
+{
+    return parseToDoubleForNumberTypeWithDecimalPlaces(string, decimalPlaces, std::numeric_limits<double>::quiet_NaN());
 }
 
 // http://www.whatwg.org/specs/web-apps/current-work/#rules-for-parsing-integers
index 19cd276..8a774f9 100644 (file)
@@ -44,8 +44,10 @@ String serializeForNumberType(double);
 // Convert the specified string to a double. If the conversion fails, the return value is false.
 // Leading or trailing illegal characters cause failure, as does passing an empty string.
 // The double* parameter may be 0 to check if the string can be parsed without getting the result.
-bool parseToDoubleForNumberType(const String&, double*);
-bool parseToDoubleForNumberTypeWithDecimalPlaces(const String&, double*, unsigned*);
+double parseToDoubleForNumberType(const String&);
+double parseToDoubleForNumberType(const String&, double fallbackValue);
+double parseToDoubleForNumberTypeWithDecimalPlaces(const String&, unsigned*);
+double parseToDoubleForNumberTypeWithDecimalPlaces(const String&, unsigned*, double fallbackValue);
 
 // http://www.whatwg.org/specs/web-apps/current-work/#rules-for-parsing-integers
 bool parseHTMLInteger(const String&, int&);
index 1ce7e9b..8422a30 100644 (file)
@@ -53,14 +53,9 @@ namespace WebCore {
 
 inline static double sliderPosition(HTMLInputElement* element)
 {
-    StepRange stepRange(element->createStepRange(RejectAny));
-
-    double oldValue;
-    bool parseSuccess = parseToDoubleForNumberType(element->value(), &oldValue);
-    if (!parseSuccess)
-        oldValue = stepRange.defaultValue();
-    double newValue = stepRange.clampValue(oldValue);
-    return stepRange.proportionFromValue(newValue);
+    const StepRange stepRange(element->createStepRange(RejectAny));
+    const double oldValue = parseToDoubleForNumberType(element->value(), stepRange.defaultValue());
+    return stepRange.proportionFromValue(stepRange.clampValue(oldValue));
 }
 
 inline static bool hasVerticalAppearance(HTMLInputElement* input)