Remove SegmentedString::m_pushedChar{1,2} optimization

Before r201294, SegmentedString::push() had an exotic behavior where two
consecutive push() results are swapped.  SegmentedString stored those two
chars into m_pushedChar{1,2} to avoid messing up SegmentedSubString.

After r201294, SegmentedString::push() is guaranteed to be used for
ungetc()-like usecase, where it reverts the SegmentedString::advance()
which was issued just before the call.

This CL removes m_pushedChar{1,2} optimization, and replace it with
SegmentedSubstring::push() which simply decrement m_data.string{8,16}Ptr.
In pessimistic case, this may end up prepending wtf::String inside
SegmentedSubstring, but this should be very rare.

BUG=None
TESTS=SegmentedStringTest

Review URL: https://codereview.chromium.org/1319913002

git-svn-id: svn://svn.chromium.org/blink/trunk@201517 bbb929c8-8fbe-4397-9dbb-9b2b20218538

third_party/WebKit/Source/platform/text/SegmentedString.cpp

@@ -25,11 +25,6 @@ namespace blink {
 unsigned SegmentedString::length() const
 {
     unsigned length = m_currentString.length();
-    if (m_pushedChar1) {
-        ++length;
-        if (m_pushedChar2)
-            ++length;
-    }
     if (isComposite()) {
         Deque<SegmentedSubstring>::const_iterator it = m_substrings.begin();
         Deque<SegmentedSubstring>::const_iterator e = m_substrings.end();
@@ -52,8 +47,6 @@ void SegmentedString::setExcludeLineNumbers()
 
 void SegmentedString::clear()
 {
-    m_pushedChar1 = 0;
-    m_pushedChar2 = 0;
     m_currentChar = 0;
     m_currentString.clear();
     m_numberOfCharactersConsumedPriorToCurrentString = 0;
@@ -83,9 +76,19 @@ void SegmentedString::append(const SegmentedSubstring& s)
     m_empty = false;
 }
 
+void SegmentedString::push(UChar c)
+{
+    ASSERT(c);
+    if (m_currentString.pushIfPossible(c)) {
+        m_currentChar = c;
+        return;
+    }
+
+    prepend(SegmentedString(String(&c, 1)));
+}
+
 void SegmentedString::prepend(const SegmentedSubstring& s)
 {
-    ASSERT(!escaped());
     ASSERT(!s.numberOfCharactersConsumed());
     if (!s.length())
         return;
@@ -119,13 +122,6 @@ void SegmentedString::close()
 void SegmentedString::append(const SegmentedString& s)
 {
     ASSERT(!m_closed);
-    if (s.m_pushedChar1) {
-        Vector<UChar, 2> unconsumedData;
-        unconsumedData.append(s.m_pushedChar1);
-        if (s.m_pushedChar2)
-            unconsumedData.append(s.m_pushedChar2);
-        append(SegmentedSubstring(String(unconsumedData)));
-    }
 
     append(s.m_currentString);
     if (s.isComposite()) {
@@ -134,13 +130,11 @@ void SegmentedString::append(const SegmentedString& s)
         for (; it != e; ++it)
             append(*it);
     }
-    m_currentChar = m_pushedChar1 ? m_pushedChar1 : (m_currentString.length() ? m_currentString.getCurrentChar() : 0);
+    m_currentChar = m_currentString.length() ? m_currentString.getCurrentChar() : 0;
 }
 
 void SegmentedString::prepend(const SegmentedString& s)
 {
-    ASSERT(!escaped());
-    ASSERT(!s.escaped());
     if (s.isComposite()) {
         Deque<SegmentedSubstring>::const_reverse_iterator it = s.m_substrings.rbegin();
         Deque<SegmentedSubstring>::const_reverse_iterator e = s.m_substrings.rend();
@@ -173,11 +167,6 @@ void SegmentedString::advanceSubstring()
 String SegmentedString::toString() const
 {
     StringBuilder result;
-    if (m_pushedChar1) {
-        result.append(m_pushedChar1);
-        if (m_pushedChar2)
-            result.append(m_pushedChar2);
-    }
     m_currentString.appendTo(result);
     if (isComposite()) {
         Deque<SegmentedSubstring>::const_iterator it = m_substrings.begin();
@@ -199,21 +188,18 @@ void SegmentedString::advance(unsigned count, UChar* consumedCharacters)
 
 void SegmentedString::advance8()
 {
-    ASSERT(!m_pushedChar1);
     decrementAndCheckLength();
     m_currentChar = m_currentString.incrementAndGetCurrentChar8();
 }
 
 void SegmentedString::advance16()
 {
-    ASSERT(!m_pushedChar1);
     decrementAndCheckLength();
     m_currentChar = m_currentString.incrementAndGetCurrentChar16();
 }
 
 void SegmentedString::advanceAndUpdateLineNumber8()
 {
-    ASSERT(!m_pushedChar1);
     ASSERT(m_currentString.getCurrentChar() == m_currentChar);
     if (m_currentChar == '\n') {
         ++m_currentLine;
@@ -225,7 +211,6 @@ void SegmentedString::advanceAndUpdateLineNumber8()
 
 void SegmentedString::advanceAndUpdateLineNumber16()
 {
-    ASSERT(!m_pushedChar1);
     ASSERT(m_currentString.getCurrentChar() == m_currentChar);
     if (m_currentChar == '\n') {
         ++m_currentLine;
@@ -237,17 +222,7 @@ void SegmentedString::advanceAndUpdateLineNumber16()
 
 void SegmentedString::advanceSlowCase()
 {
-    if (m_pushedChar1) {
-        m_pushedChar1 = m_pushedChar2;
-        m_pushedChar2 = 0;
-
-        if (m_pushedChar1) {
-            m_currentChar = m_pushedChar1;
-            return;
-        }
-
-        updateAdvanceFunctionPointers();
-    } else if (m_currentString.length()) {
+    if (m_currentString.length()) {
         m_currentString.decrementLength();
         if (!m_currentString.length())
             advanceSubstring();
@@ -263,17 +238,7 @@ void SegmentedString::advanceSlowCase()
 
 void SegmentedString::advanceAndUpdateLineNumberSlowCase()
 {
-    if (m_pushedChar1) {
-        m_pushedChar1 = m_pushedChar2;
-        m_pushedChar2 = 0;
-
-        if (m_pushedChar1) {
-            m_currentChar = m_pushedChar1;
-            return;
-        }
-
-        updateAdvanceFunctionPointers();
-    } else if (m_currentString.length()) {
+    if (m_currentString.length()) {
         if (m_currentString.getCurrentChar() == '\n' && m_currentString.doNotExcludeLineNumbers()) {
             ++m_currentLine;
             // Plus 1 because numberOfCharactersConsumed value hasn't incremented yet; it does with length() decrement below.

third_party/WebKit/Source/platform/text/SegmentedString.h

@@ -55,10 +55,11 @@ public:
             }
         } else {
             m_is8Bit = false;
+            m_data.string8Ptr = nullptr;
         }
     }
 
-    void clear() { m_length = 0; m_data.string16Ptr = 0; m_is8Bit = false;}
+    void clear() { m_length = 0; m_data.string16Ptr = nullptr; m_is8Bit = false;}
 
     bool is8Bit() { return m_is8Bit; }
 
@@ -81,6 +82,30 @@ public:
         }
     }
 
+    bool pushIfPossible(UChar c)
+    {
+        if (!m_length)
+            return false;
+
+        if (m_is8Bit) {
+            if (m_data.string8Ptr == m_string.characters8())
+                return false;
+
+            --m_data.string8Ptr;
+            ASSERT(*m_data.string8Ptr == c);
+            ++m_length;
+            return true;
+        }
+
+        if (m_data.string16Ptr == m_string.characters16())
+            return false;
+
+        --m_data.string16Ptr;
+        ASSERT(*m_data.string16Ptr == static_cast<LChar>(c));
+        ++m_length;
+        return true;
+    }
+
     UChar getCurrentChar8()
     {
         return *m_data.string8Ptr;
@@ -148,9 +173,7 @@ private:
 class PLATFORM_EXPORT SegmentedString {
 public:
     SegmentedString()
-        : m_pushedChar1(0)
-        , m_pushedChar2(0)
-        , m_currentChar(0)
+        : m_currentChar(0)
         , m_numberOfCharactersConsumedPriorToCurrentString(0)
         , m_numberOfCharactersConsumedPriorToCurrentLine(0)
         , m_currentLine(0)
@@ -163,9 +186,7 @@ public:
     }
 
     SegmentedString(const String& str)
-        : m_pushedChar1(0)
-        , m_pushedChar2(0)
-        , m_currentString(str)
+        : m_currentString(str)
         , m_currentChar(0)
         , m_numberOfCharactersConsumedPriorToCurrentString(0)
         , m_numberOfCharactersConsumedPriorToCurrentLine(0)
@@ -188,19 +209,7 @@ public:
     bool excludeLineNumbers() const { return m_currentString.excludeLineNumbers(); }
     void setExcludeLineNumbers();
 
-    void push(UChar c)
-    {
-        ASSERT(c);
-
-        if (!m_pushedChar1) {
-            m_currentChar = m_pushedChar1 = c;
-            updateSlowCaseFunctionPointers();
-        } else {
-            ASSERT(!m_pushedChar2);
-            m_pushedChar2 = m_pushedChar1;
-            m_currentChar = m_pushedChar1 = c;
-        }
-    }
+    void push(UChar);
 
     bool isEmpty() const { return m_empty; }
     unsigned length() const;
@@ -219,7 +228,6 @@ public:
     void advance()
     {
         if (m_fastPathFlags & Use8BitAdvance) {
-            ASSERT(!m_pushedChar1);
             m_currentChar = m_currentString.incrementAndGetCurrentChar8();
             decrementAndCheckLength();
             return;
@@ -231,8 +239,6 @@ public:
     inline void advanceAndUpdateLineNumber()
     {
         if (m_fastPathFlags & Use8BitAdvance) {
-            ASSERT(!m_pushedChar1);
-
             bool haveNewLine = (m_currentChar == '\n') & !!(m_fastPathFlags & Use8BitAdvanceAndUpdateLineNumbers);
             m_currentChar = m_currentString.incrementAndGetCurrentChar8();
             decrementAndCheckLength();
@@ -269,7 +275,7 @@ public:
     void advancePastNewlineAndUpdateLineNumber()
     {
         ASSERT(currentChar() == '\n');
-        if (!m_pushedChar1 && m_currentString.length() > 1) {
+        if (m_currentString.length() > 1) {
             int newLineFlag = m_currentString.doNotExcludeLineNumbers();
             m_currentLine += newLineFlag;
             if (newLineFlag)
@@ -285,16 +291,9 @@ public:
     // have space for at least |count| characters.
     void advance(unsigned count, UChar* consumedCharacters);
 
-    bool escaped() const { return m_pushedChar1; }
-
     int numberOfCharactersConsumed() const
     {
         int numberOfPushedCharacters = 0;
-        if (m_pushedChar1) {
-            ++numberOfPushedCharacters;
-            if (m_pushedChar2)
-                ++numberOfPushedCharacters;
-        }
         return m_numberOfCharactersConsumedPriorToCurrentString + m_currentString.numberOfCharactersConsumed() - numberOfPushedCharacters;
     }
 
@@ -340,7 +339,7 @@ private:
 
     void updateAdvanceFunctionPointers()
     {
-        if ((m_currentString.length() > 1) && !m_pushedChar1) {
+        if (m_currentString.length() > 1) {
             if (m_currentString.is8Bit()) {
                 m_advanceFunc = &SegmentedString::advance8;
                 m_fastPathFlags = Use8BitAdvance;
@@ -373,7 +372,7 @@ private:
 
     inline LookAheadResult lookAheadInline(const String& string, TextCaseSensitivity caseSensitivity)
     {
-        if (!m_pushedChar1 && string.length() <= static_cast<unsigned>(m_currentString.length())) {
+        if (string.length() <= static_cast<unsigned>(m_currentString.length())) {
             String currentSubstring = m_currentString.currentSubString(string.length());
             if (currentSubstring.startsWith(string, caseSensitivity))
                 return DidMatch;
@@ -399,8 +398,6 @@ private:
 
     bool isComposite() const { return !m_substrings.isEmpty(); }
 
-    UChar m_pushedChar1;
-    UChar m_pushedChar2;
     SegmentedSubstring m_currentString;
     UChar m_currentChar;
     int m_numberOfCharactersConsumedPriorToCurrentString;