Extended error reporting for SVG attribute parsing

third_party/WebKit/Source/core/svg/SVGParsingError.h

Index: third_party/WebKit/Source/core/svg/SVGParsingError.h
diff --git a/third_party/WebKit/Source/core/svg/SVGParsingError.h b/third_party/WebKit/Source/core/svg/SVGParsingError.h
index 5a40f9f4c101fe0bb403d1eaf80b9d6030b319c8..499c91cc4bae59300d967886b45bb4fb6607e8cf 100644
--- a/third_party/WebKit/Source/core/svg/SVGParsingError.h
+++ b/third_party/WebKit/Source/core/svg/SVGParsingError.h
@@ -27,14 +27,69 @@
 #ifndef SVGParsingError_h
 #define SVGParsingError_h
 
+#include "wtf/MathExtras.h"
+#include "wtf/text/WTFString.h"
+
 namespace blink {
 
-enum SVGParsingError {
+class QualifiedName;
+
+enum class SVGStatus {

[pdr., 2016/01/15 20:56:53]

Bikeshed: SVGParseStatus? Or anything a little less generic.

[fs, 2016/01/15 21:05:13]

I guess we could go with that - I was trying for something not too lengthy (the
main downside of enum classes is their qualification...)

[fs, 2016/01/18 13:56:51]

Done.

     NoError,
-    ParsingAttributeFailedError,
-    NegativeValueForbiddenError
+
+    // Syntax errors
+    TrailingGarbage,
+    ExpectedBoolean,
+    ExpectedEnumeration,
+
+    // Semantic errors
+    NegativeValue,
+
+    // Generic error
+    ParsingFailed,
+};
+
+class SVGParsingError {
+    STACK_ALLOCATED();
+public:
+    SVGParsingError(SVGStatus status = SVGStatus::NoError, size_t locus = 0)
+        : m_status(static_cast<unsigned>(status))
+        , m_locus(narrowLocus(locus))
+    {
+        ASSERT(this->status() == status);
+    }
+
+    SVGStatus status() const { return static_cast<SVGStatus>(m_status); }
+
+    bool hasLocus() const { return m_locus != kNoLocus; }
+    unsigned locus() const { return m_locus; }
+
+    // Generates a string describing this error for |value| in the context of
+    // an <element, attribute>-name pair.
+    String format(const String& tagName, const QualifiedName&, const AtomicString& value) const;
+
+private:
+    static const int kLocusBits = 24;
+    static const int kNoLocus = (1u << kLocusBits) - 1;
+
+    static unsigned narrowLocus(size_t locus)
+    {
+        // Clamp to fit in the number of bits available. This means that very
+        // long values will be output in their entirety. That should however be

[pdr., 2016/01/15 20:56:53]

will *not* be output

[fs, 2016/01/15 21:05:12]

It's actually correct as is, but I take it that I should clarify that this is
the case when the locus requires more than 24 bits of storage.

[fs, 2016/01/18 13:56:51]

Added an extra sentence and renamed the helper to checkLocus.

+        // rather uncommon.
+        return clampTo<unsigned>(locus, 0, kNoLocus);
+    }
+
+    unsigned m_status : 8;
+    unsigned m_locus : kLocusBits;

[pdr., 2016/01/15 20:56:53]

Can you add a comment about what locus is?

[fs, 2016/01/15 21:05:13]

And here I thought this was as obvious a name you could get... =)

[fs, 2016/01/18 13:56:51]

Comment added.

 };
 
+inline bool operator==(const SVGParsingError& error, SVGStatus status)
+{
+    return error.status() == status;
+}
+inline bool operator!=(const SVGParsingError& error, SVGStatus status) { return !(error == status); }
+
 } // namespace blink
 
 #endif // SVGParsingError_h