Rework the "rules for parsing dimension values" implementation

third_party/WebKit/Source/core/html/HTMLDimension.cpp

Index: third_party/WebKit/Source/core/html/HTMLDimension.cpp
diff --git a/third_party/WebKit/Source/core/html/HTMLDimension.cpp b/third_party/WebKit/Source/core/html/HTMLDimension.cpp
index eb5086168e83742a34d87b67a4a91bcaef042f25..72e037a140f9ff16fc28e280e6d68e64bd53aaa2 100644
--- a/third_party/WebKit/Source/core/html/HTMLDimension.cpp
+++ b/third_party/WebKit/Source/core/html/HTMLDimension.cpp
@@ -30,7 +30,9 @@
 
 #include "core/html/HTMLDimension.h"
 
+#include "core/html/parser/HTMLParserIdioms.h"
 #include "wtf/MathExtras.h"
+#include "wtf/text/ParsingUtilities.h"
 #include "wtf/text/StringToNumber.h"
 #include "wtf/text/WTFString.h"
 
@@ -146,4 +148,52 @@ Vector<HTMLDimension> parseListOfDimensions(const String& input) {
   return parsedDimensions;
 }
 
+template <typename CharacterType>
+static bool parseDimensionValue(const CharacterType* current,
+                                const CharacterType* end,
+                                HTMLDimension& dimension) {
+  skipWhile<CharacterType, isHTMLSpace>(current, end);
+  // Deviation: HTML allows '+' here.
+  const CharacterType* numberStart = current;
+  if (!skipExactly<CharacterType, isASCIIDigit>(current, end))
+    return false;
+  skipWhile<CharacterType, isASCIIDigit>(current, end);
+  if (skipExactly<CharacterType>(current, end, '.')) {
+    // Deviation: HTML requires a digit after the full stop. Gecko and Edge

[Timothy Loh, 2016/11/25 00:07:47]

I think you misread the spec here? From my reading it's fine to not have a digit
after, we just return the value immediately (so 10.... -> 10, 10.% -> 10). It
looks like we currently interpret 10.% as a percentage and so does FF.

[fs, 2016/11/25 10:18:43]

My comment could be made more clear I guess. I was thinking of the case you
mention (10.% -> 10), which both we, Gecko and Edge (and WebKit) interpret as a
percentage (while we should not per spec.) I'll make sure to update the comment
to mention this.

[fs, 2016/11/30 18:27:18]

Clarified.

+    // does not.
+    skipWhile<CharacterType, isASCIIDigit>(current, end);
+  }
+  bool ok;
+  double value = charactersToDouble(numberStart, current - numberStart, &ok);
+  if (!ok)
+    return false;
+  HTMLDimension::HTMLDimensionType type = HTMLDimension::Absolute;
+  if (current < end) {
+    if (*current == '%') {
+      type = HTMLDimension::Percentage;
+    } else if (*current == '*') {
+      // Deviation: HTML does not recognize '*' in this context, and we don't
+      // treat it as a valid value. We do count it though, so this is purely
+      // for statistics. Note though that per the specced behavior, "<number>*"
+      // would be the same as "<number>" (i.e '*' would just be trailing
+      // garbage.)
+      type = HTMLDimension::Relative;
+    }
+  }
+  dimension = HTMLDimension(value, type);
+  return true;
+}
+
+// https://html.spec.whatwg.org/multipage/infrastructure.html#rules-for-parsing-dimension-values
+bool parseDimensionValue(const String& input, HTMLDimension& dimension) {
+  if (input.isEmpty())
+    return false;
+  if (input.is8Bit()) {
+    return parseDimensionValue(input.characters8(),
+                               input.characters8() + input.length(), dimension);
+  }
+  return parseDimensionValue(input.characters16(),
+                             input.characters16() + input.length(), dimension);
+}
+
 }  // namespace blink