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

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

 /*
  * Copyright (C) 2013 Google, Inc. All Rights Reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 63 matching lines @@
         Character,
         EndOfFile,
     };
 
     class Attribute {
         DISALLOW_NEW_EXCEPT_PLACEMENT_NEW();
     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;
         };
 
         AtomicString name() const { return AtomicString(m_name); }
         String nameAttemptStaticStringCreation() const { return attemptStaticStringCreation(m_name, Likely8Bit); }
         const Vector<UChar, 32>& nameAsVector() const { return m_name; }
 
         void appendToName(UChar c) { m_name.append(c); }
 
@@ skipping 21 matching lines @@
     // By using an inline capacity of 256, we avoid spilling over into an malloced buffer
     // approximately 99% of the time based on a non-scientific browse around a number of
     // popular web sites on 23 May 2013.
     typedef Vector<UChar, 256> DataVector;
 
     HTMLToken() { clear(); }
 
     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
         // two 257 character text nodes in a row, we'll needlessly
         // thrash malloc. When we finally finish the parse the
         // HTMLToken will be destroyed and the VectorBuffer released.
         m_data.shrink(0);
         m_orAllData = 0;
     }
 
@@ skipping 170 matching lines @@
         m_attributes.clear();
 
         m_data.appendVector(characters);
     }
 
     void addNewAttribute()
     {
         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)
     {
         m_currentAttribute->mutableNameRange().start = offset - m_baseOffset;
     }
 
     void endAttributeName(int offset)
     {
         int index = offset - m_baseOffset;
         m_currentAttribute->mutableNameRange().end = index;
         m_currentAttribute->mutableValueRange().start = index;
         m_currentAttribute->mutableValueRange().end = index;
     }
 
     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)
     {
         m_currentAttribute->mutableValueRange().end = offset - m_baseOffset;
     }
 
     void appendToAttributeName(UChar character)
     {
         ASSERT(character);
         ASSERT(m_type == StartTag || m_type == EndTag);
-        ASSERT(m_currentAttribute->nameRange().start);
+        m_currentAttribute->nameRange().checkValidStart();
         m_currentAttribute->appendToName(character);
     }
 
     void appendToAttributeValue(UChar character)
     {
         ASSERT(character);
         ASSERT(m_type == StartTag || m_type == EndTag);
-        ASSERT(m_currentAttribute->valueRange().start);
+        m_currentAttribute->valueRange().checkValidStart();
         m_currentAttribute->appendToValue(character);
     }
 
     void appendToAttributeValue(size_t i, const String& value)
     {
         ASSERT(!value.isEmpty());
         ASSERT(m_type == StartTag || m_type == EndTag);
         m_attributes[i].appendToValue(value);
     }
 
@@ skipping 98 matching lines @@
     // A pointer into m_attributes used during lexing.
     Attribute* m_currentAttribute;
 
     // For DOCTYPE
     std::unique_ptr<DoctypeData> m_doctypeData;
 };
 
 } // namespace blink
 
 #endif