Change HTMLTokenizer::Attribute::Range.start initial invalid value from 0 -> -1

third_party/WebKit/Source/core/html/parser/HTMLToken.h

Index: third_party/WebKit/Source/core/html/parser/HTMLToken.h
diff --git a/third_party/WebKit/Source/core/html/parser/HTMLToken.h b/third_party/WebKit/Source/core/html/parser/HTMLToken.h
index 679a187e1d1a14bdd36a685985f7846369b99ad0..c0d5b7f38495e6437065d024a003ac46c8b67503 100644
--- a/third_party/WebKit/Source/core/html/parser/HTMLToken.h
+++ b/third_party/WebKit/Source/core/html/parser/HTMLToken.h
@@ -81,6 +81,32 @@ public:
         class Range {
             DISALLOW_NEW();
         public:
+            static constexpr int kInvalidOffset = -1;
+
+            inline void clear()
+            {
+#if ENABLE(ASSERT)
+                start = kInvalidOffset;
+                end = kInvalidOffset;
+#endif
+            }
+
+            inline void checkValidStart() const

[dominicc (has gone to gerrit), 2016/07/05 01:08:10]

I'm not sure checkValidStart and checkValidEnd are pulling their weight and
should not just be moved into checkValid. I see places where you only
checkValidStart, but why not just check start and end at those points? When
would start be valid but end invalid?

[kouhei (in TOK), 2016/07/05 03:57:14]

We want two check methods:
- one for checking Range instances in middle of parsing (req vlaid start, but
may have invalid end)
- one for checking Range instances after parse (req. valid start+end)

+            {
+                DCHECK(start != kInvalidOffset);
+            }
+
+            inline void checkValidEnd() const
+            {
+                DCHECK(end != kInvalidOffset);

[Yoav Weiss, 2016/07/04 09:19:39]

shouldn't this check that start is not negative?

[kouhei (in TOK), 2016/07/05 03:57:14]

Done.

+            }
+
+            inline void checkValid() const

[dominicc (has gone to gerrit), 2016/07/05 01:08:10]

Is there any relationship between start and end you can check, like start <=
end? If not it is probably worth a comment explaining what values like that
mean.

+            {
+                checkValidStart();
+                checkValidEnd();
+            }
+
             int start;
             int end;
         };
@@ -122,8 +148,8 @@ public:
     void clear()
     {
         m_type = Uninitialized;
+        m_range.clear();
         m_range.start = 0;
-        m_range.end = 0;
         m_baseOffset = 0;
         // Don't call Vector::clear() as that would destroy the
         // alloced VectorBuffer. If the innerHTML'd content has
@@ -314,12 +340,8 @@ public:
         ASSERT(m_type == StartTag || m_type == EndTag);
         m_attributes.grow(m_attributes.size() + 1);
         m_currentAttribute = &m_attributes.last();
-#if ENABLE(ASSERT)
-        m_currentAttribute->mutableNameRange().start = 0;
-        m_currentAttribute->mutableNameRange().end = 0;
-        m_currentAttribute->mutableValueRange().start = 0;
-        m_currentAttribute->mutableValueRange().end = 0;
-#endif
+        m_currentAttribute->mutableNameRange().clear();
+        m_currentAttribute->mutableValueRange().clear();
     }
 
     void beginAttributeName(int offset)
@@ -337,10 +359,9 @@ public:
 
     void beginAttributeValue(int offset)
     {
+        m_currentAttribute->mutableValueRange().clear();
         m_currentAttribute->mutableValueRange().start = offset - m_baseOffset;

[Yoav Weiss, 2016/07/04 09:19:39]

Alternatively, maybe DCHECK here that offset is larger or equal to m_baseOffset?

-#if ENABLE(ASSERT)
-        m_currentAttribute->mutableValueRange().end = 0;
-#endif
+        m_currentAttribute->valueRange().checkValidStart();
     }
 
     void endAttributeValue(int offset)
@@ -352,7 +373,7 @@ public:
     {
         ASSERT(character);
         ASSERT(m_type == StartTag || m_type == EndTag);
-        ASSERT(m_currentAttribute->nameRange().start);
+        m_currentAttribute->nameRange().checkValidStart();
         m_currentAttribute->appendToName(character);
     }
 
@@ -360,7 +381,7 @@ public:
     {
         ASSERT(character);
         ASSERT(m_type == StartTag || m_type == EndTag);
-        ASSERT(m_currentAttribute->valueRange().start);
+        m_currentAttribute->valueRange().checkValidStart();
         m_currentAttribute->appendToValue(character);
     }