Shrink Document::parseQualifiedName a bit.

Source/core/dom/Document.cpp

Index: Source/core/dom/Document.cpp
diff --git a/Source/core/dom/Document.cpp b/Source/core/dom/Document.cpp
index ac3d14065363a169bfbaab9e2a1207145b4f692b..d39c88e98c6ceea0fd8f0d22e100ee140cf15cbc 100644
--- a/Source/core/dom/Document.cpp
+++ b/Source/core/dom/Document.cpp
@@ -4207,8 +4207,25 @@ bool Document::isValidName(const String& name)
     return isValidNameNonASCII(characters, length);
 }
 
+enum ParseQualifiedNameException {

[fs, 2014/09/11 09:17:48]

Lots of "Exception" here, maybe it'd be possible to drop the "Exception" part(s)
in favor of some other term (status?), and then leave
Document::parseQualifiedName to deal with exception bits. ("PQNNoException"
would then be be something like "PQNValid" or "PQNSuccess" - or maybe even using
"QName" as a prefix could work here - giving "QNameValid" etc.)

+    PQNNoException,
+    PQNMultipleColonsException,
+    PQNInvalidStartCharException,
+    PQNInvalidCharException,
+    PQNEmptyPrefix,
+    PQNEmptyLocalName
+};
+
+struct ParseQualifiedNameResult {
+    ParseQualifiedNameException result;
+    UChar32 character;
+    ParseQualifiedNameResult() { }
+    ParseQualifiedNameResult(ParseQualifiedNameException result) : result(result) { }

[fs, 2014/09/11 09:17:48]

Coding-style dictates the use of explicit here. Not using it would of course
allow for slightly "nicer" code in most of the error cases below - but also lead
to inconsistencies with the invalid-char cases...

+    ParseQualifiedNameResult(ParseQualifiedNameException result, UChar32 character) : result(result), character(character) { }
+};
+
 template<typename CharType>
-static bool parseQualifiedNameInternal(const AtomicString& qualifiedName, const CharType* characters, unsigned length, AtomicString& prefix, AtomicString& localName, ExceptionState& exceptionState)
+static ParseQualifiedNameResult parseQualifiedNameInternal(const AtomicString& qualifiedName, const CharType* characters, unsigned length, AtomicString& prefix, AtomicString& localName)
 {
     bool nameStart = true;
     bool sawColon = false;
@@ -4218,36 +4235,18 @@ static bool parseQualifiedNameInternal(const AtomicString& qualifiedName, const
         UChar32 c;
         U16_NEXT(characters, i, length, c)
         if (c == ':') {
-            if (sawColon) {
-                exceptionState.throwDOMException(NamespaceError, "The qualified name provided ('" + qualifiedName + "') contains multiple colons.");
-                return false; // multiple colons: not allowed
-            }
+            if (sawColon)
+                return ParseQualifiedNameResult(PQNMultipleColonsException);
             nameStart = true;
             sawColon = true;
             colonPos = i - 1;
         } else if (nameStart) {
-            if (!isValidNameStart(c)) {
-                StringBuilder message;
-                message.appendLiteral("The qualified name provided ('");
-                message.append(qualifiedName);
-                message.appendLiteral("') contains the invalid name-start character '");
-                message.append(c);
-                message.appendLiteral("'.");
-                exceptionState.throwDOMException(InvalidCharacterError, message.toString());
-                return false;
-            }
+            if (!isValidNameStart(c))
+                return ParseQualifiedNameResult(PQNInvalidStartCharException, c);
             nameStart = false;
         } else {
-            if (!isValidNamePart(c)) {
-                StringBuilder message;
-                message.appendLiteral("The qualified name provided ('");
-                message.append(qualifiedName);
-                message.appendLiteral("') contains the invalid character '");
-                message.append(c);
-                message.appendLiteral("'.");
-                exceptionState.throwDOMException(InvalidCharacterError, message.toString());
-                return false;
-            }
+            if (!isValidNamePart(c))
+                return ParseQualifiedNameResult(PQNInvalidCharException, c);
         }
     }
 
@@ -4256,20 +4255,16 @@ static bool parseQualifiedNameInternal(const AtomicString& qualifiedName, const
         localName = qualifiedName;
     } else {
         prefix = AtomicString(characters, colonPos);
-        if (prefix.isEmpty()) {
-            exceptionState.throwDOMException(NamespaceError, "The qualified name provided ('" + qualifiedName + "') has an empty namespace prefix.");
-            return false;
-        }
+        if (prefix.isEmpty())
+            return ParseQualifiedNameResult(PQNEmptyPrefix);
         int prefixStart = colonPos + 1;
         localName = AtomicString(characters + prefixStart, length - prefixStart);
     }
 
-    if (localName.isEmpty()) {
-        exceptionState.throwDOMException(NamespaceError, "The qualified name provided ('" + qualifiedName + "') has an empty local name.");
-        return false;
-    }
+    if (localName.isEmpty())
+        return ParseQualifiedNameResult(PQNEmptyLocalName);
 
-    return true;
+    return ParseQualifiedNameResult(PQNNoException);
 }
 
 bool Document::parseQualifiedName(const AtomicString& qualifiedName, AtomicString& prefix, AtomicString& localName, ExceptionState& exceptionState)
@@ -4281,9 +4276,41 @@ bool Document::parseQualifiedName(const AtomicString& qualifiedName, AtomicStrin
         return false;
     }
 
+    ParseQualifiedNameResult returnValue;
     if (qualifiedName.is8Bit())
-        return parseQualifiedNameInternal(qualifiedName, qualifiedName.characters8(), length, prefix, localName, exceptionState);
-    return parseQualifiedNameInternal(qualifiedName, qualifiedName.characters16(), length, prefix, localName, exceptionState);
+        returnValue = parseQualifiedNameInternal(qualifiedName, qualifiedName.characters8(), length, prefix, localName);
+    else
+        returnValue = parseQualifiedNameInternal(qualifiedName, qualifiedName.characters16(), length, prefix, localName);
+    if (returnValue.result == PQNNoException)
+        return true;
+
+    StringBuilder message;
+    message.appendLiteral("The qualified name provided ('");
+    message.append(qualifiedName);
+    message.appendLiteral("') ");
+
+    if (returnValue.result == PQNMultipleColonsException) {

[fs, 2014/09/11 09:17:48]

I suspect that using a switch here might give slightly less "busy" code
(result-values lining up for instance) - and hopefully not affect code-gen. I'll
let you decide though =)

+        message.appendLiteral("contains multiple colons.");
+    } else if (returnValue.result == PQNInvalidStartCharException) {
+        message.appendLiteral("contains the invalid name-start character '");
+        message.append(returnValue.character);
+        message.appendLiteral("'.");
+    } else if (returnValue.result == PQNInvalidCharException) {
+        message.appendLiteral("contains the invalid character '");
+        message.append(returnValue.character);
+        message.appendLiteral("'.");
+    } else if (returnValue.result == PQNEmptyPrefix) {
+        message.appendLiteral("has an empty namespace prefix.");
+    } else {
+        ASSERT(returnValue.result == PQNEmptyLocalName);
+        message.appendLiteral("has an empty local name.");
+    }
+
+    if (returnValue.result == PQNInvalidStartCharException || returnValue.result == PQNInvalidCharException)
+        exceptionState.throwDOMException(InvalidCharacterError, message.toString());
+    else
+        exceptionState.throwDOMException(NamespaceError, message.toString());
+    return false;
 }
 
 void Document::setEncodingData(const DocumentEncodingData& newData)