Smaller CSSParser UTF16 buffers for escaped strings.

Source/core/css/CSSTokenizer-in.cpp

 /*
  * Copyright (C) 2003 Lars Knoll (knoll@kde.org)
  * Copyright (C) 2005 Allan Sandfeld Jensen (kde@carewolf.com)
  * Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011, 2012 Apple Inc. All rights reserved.
  * Copyright (C) 2007 Nicholas Shanks <webkit@nickshanks.com>
  * Copyright (C) 2008 Eric Seidel <eric@webkit.org>
  * Copyright (C) 2009 Torch Mobile Inc. All rights reserved. (http://www.torchmobile.com/)
  * Copyright (C) 2012 Adobe Systems Incorporated. All rights reserved.
  * Copyright (C) 2012 Intel Corporation. All rights reserved.
  *
@@ skipping 286 matching lines @@
 {
     return m_currentCharacter8;
 }
 
 template <>
 inline UChar*& CSSTokenizer::currentCharacter<UChar>()
 {
     return m_currentCharacter16;
 }
 
-UChar*& CSSTokenizer::currentCharacter16()
+UChar* CSSTokenizer::getStringBuffer16(size_t len)

[Julien - ping for review, 2014/03/20 20:57:39]

We usually don't put the word "get" on getters as it's redundant. Here we could
write allocateStringBuffer16 as it would better match the intent.

Maybe we could mention somehow that the string is owned by the tokenizer in the
name too but we don't want to make the name too long.

[Daniel Bratell, 2014/03/21 15:14:39]

Done.

 {
-    if (!m_currentCharacter16) {
-        m_dataStart16 = adoptArrayPtr(new UChar[m_length]);
-        m_currentCharacter16 = m_dataStart16.get();
-    }
+    OwnPtr<UChar[]> buffer = adoptArrayPtr(new UChar[len]);
 
-    return m_currentCharacter16;
+    UChar* bufferPtr = buffer.get();
+
+    m_cssStrings16.append(buffer.release());
+    return bufferPtr;
 }
 
 template <>
 inline LChar* CSSTokenizer::dataStart<LChar>()
 {
     return m_dataStart8.get();
 }
 
 template <>
 inline UChar* CSSTokenizer::dataStart<UChar>()
@@ skipping 54 matching lines @@
             currentCharacter += currentCharacter[2] == '\n' ? 3 : 2;
         } else {
             currentCharacter = checkAndSkipEscape(currentCharacter);
             if (!currentCharacter)
                 return 0;
         }
     }
 }
 
 template <typename CharacterType>
 unsigned CSSTokenizer::parseEscape(CharacterType*& src)

[Julien - ping for review, 2014/03/20 20:57:39]

This function should now have static linkage.

[Daniel Bratell, 2014/03/21 15:14:39]

Hmm, not sure I understand. A function can have internal or external linkage.
This has external linkage since it's part of a class.

I did make it a static method in the class in this patch though, mostly to
prevent it from modifying the tokenizer state by accident which it used to do
("src" and "currentCharacter()" happened to be the same pointer so it
"worked(tm)".)

I think it's doing fine as a static member function, but I can make it a file
level internal function though if you think that matches the style better.

[Julien - ping for review, 2014/03/21 17:56:26]

I was talking about file level internal function as you pointed out. All in all,
it's not really blocking and it works either way.

 {
     ASSERT(*src == '\\' && isCSSEscape(src[1]));
 
     unsigned unicode = 0;
 
     ++src;
     if (isASCIIHexDigit(*src)) {
 
         int length = 6;
 
         do {
             unicode = (unicode << 4) + toASCIIHexValue(*src++);
         } while (--length && isASCIIHexDigit(*src));
 
         // Characters above 0x10ffff are not handled.
         if (unicode > 0x10ffff)
             unicode = 0xfffd;
 
         // Optional space after the escape sequence.
         if (isHTMLSpace<CharacterType>(*src))
             ++src;
 
         return unicode;
     }
 
-    return *currentCharacter<CharacterType>()++;
+    return *src++;
 }
 
 template <>
 inline void CSSTokenizer::UnicodeToChars<LChar>(LChar*& result, unsigned unicode)
 {
     ASSERT(unicode <= 0xff);
     *result = unicode;
 
     ++result;
 }
 
 template <>
 inline void CSSTokenizer::UnicodeToChars<UChar>(UChar*& result, unsigned unicode)
 {
     // Replace unicode with a surrogate pairs when it is bigger than 0xffff
     if (U16_LENGTH(unicode) == 2) {
         *result++ = U16_LEAD(unicode);
         *result = U16_TRAIL(unicode);
     } else {
         *result = unicode;
     }
 
     ++result;
 }
 
+template <typename SrcCharacterType>
+size_t CSSTokenizer::peekMaxIdentifierLen(SrcCharacterType* src)
+{
+    // The decoded form of an identifier (after resolving escape
+    // sequences) will not contain more characters (ASCII or UTF-16
+    // codepoints) than the input. This code can therefore ignore
+    // escape sequences completely.
+    SrcCharacterType* start = src;
+    do {
+        if (LIKELY(*src != '\\'))
+            src++;
+        else
+            parseEscape<SrcCharacterType>(src);
+    } while (isCSSLetter(src[0]) || (src[0] == '\\' && isCSSEscape(src[1])));
+
+    return src - start;
+}
+
 template <typename SrcCharacterType, typename DestCharacterType>
 inline bool CSSTokenizer::parseIdentifierInternal(SrcCharacterType*& src, DestCharacterType*& result, bool& hasEscape)
 {
     hasEscape = false;
     do {
         if (LIKELY(*src != '\\')) {
             *result++ = *src++;
         } else {
             hasEscape = true;
             SrcCharacterType* savedEscapeStart = src;
@@ skipping 13 matching lines @@
 inline void CSSTokenizer::parseIdentifier(CharacterType*& result, CSSParserString& resultString, bool& hasEscape)
 {
     // If a valid identifier start is found, we can safely
     // parse the identifier until the next invalid character.
     ASSERT(isIdentifierStart<CharacterType>());
 
     CharacterType* start = currentCharacter<CharacterType>();
     if (UNLIKELY(!parseIdentifierInternal(currentCharacter<CharacterType>(), result, hasEscape))) {
         // Found an escape we couldn't handle with 8 bits, copy what has been recognized and continue
         ASSERT(is8BitSource());
-        UChar*& result16 = currentCharacter16();
+        UChar* result16 = getStringBuffer16((result - start) + peekMaxIdentifierLen(result));
         UChar* start16 = result16;
         int i = 0;
         for (; i < result - start; i++)
             result16[i] = start[i];
 
         result16 += i;
 
         parseIdentifierInternal(currentCharacter<CharacterType>(), result16, hasEscape);
 
         resultString.init(start16, result16 - start16);
 
         return;
     }
 
     resultString.init(start, result - start);
 }
 
+template <typename SrcCharacterType>
+size_t CSSTokenizer::peekMaxStringLen(SrcCharacterType* src, UChar quote)
+{
+    // The decoded form of a CSS string (after resolving escape
+    // sequences) will not contain more characters (ASCII or UTF-16
+    // codepoints) than the input. This code can therefore ignore
+    // escape sequences completely.
+    SrcCharacterType* start = src;
+    while (true) {
+        if (UNLIKELY(*src == quote)) {
+            // String parsing is done.
+            ++src;
+            break;
+        }
+        if (UNLIKELY(!*src)) {
+            // String parsing is done, but don't advance pointer if at the end of input.
+            break;
+        }
+        ASSERT(*src > '\r' || (*src < '\n' && *src) || *src == '\v');
+
+        if (LIKELY(src[0] != '\\'))
+            src++;
+        else if (src[1] == '\n' || src[1] == '\f')
+            src += 2;
+        else if (src[1] == '\r')
+            src += src[2] == '\n' ? 3 : 2;

[Julien - ping for review, 2014/03/20 20:57:39]

This looks awfully like checkAndSkipString, maybe we could share some code?

+        else
+            parseEscape<SrcCharacterType>(src);
+    }
+
+    return src - start;
+}
+
 template <typename SrcCharacterType, typename DestCharacterType>
 inline bool CSSTokenizer::parseStringInternal(SrcCharacterType*& src, DestCharacterType*& result, UChar quote)
 {
     while (true) {
         if (UNLIKELY(*src == quote)) {
             // String parsing is done.
             ++src;
             return true;
         }
         if (UNLIKELY(!*src)) {
@@ skipping 23 matching lines @@
 }
 
 template <typename CharacterType>
 inline void CSSTokenizer::parseString(CharacterType*& result, CSSParserString& resultString, UChar quote)
 {
     CharacterType* start = currentCharacter<CharacterType>();
 
     if (UNLIKELY(!parseStringInternal(currentCharacter<CharacterType>(), result, quote))) {
         // Found an escape we couldn't handle with 8 bits, copy what has been recognized and continue
         ASSERT(is8BitSource());
-        UChar*& result16 = currentCharacter16();
+        UChar* result16 = getStringBuffer16((result - start) + peekMaxStringLen(result, quote));
         UChar* start16 = result16;
         int i = 0;
         for (; i < result - start; i++)
             result16[i] = start[i];
 
         result16 += i;
 
         parseStringInternal(currentCharacter<CharacterType>(), result16, quote);
 
         resultString.init(start16, result16 - start16);
@@ skipping 27 matching lines @@
         }
     }
 
     end = skipWhiteSpace(end);
     if (*end != ')')
         return false;
 
     return true;
 }
 
+template <typename SrcCharacterType>
+inline size_t CSSTokenizer::peekMaxURILen(SrcCharacterType* src, UChar quote)
+{
+    // The decoded form of a URI (after resolving escape sequences)
+    // will not contain more characters (ASCII or UTF-16 codepoints)
+    // than the input. This code can therefore ignore escape sequences
+    // completely.
+    SrcCharacterType* start = src;
+    if (quote) {
+        ASSERT(quote == '"' || quote == '\'');
+        return peekMaxStringLen(src, quote);
+    }
+
+    while (isURILetter(*src)) {
+        if (LIKELY(*src != '\\'))
+            src++;
+        else
+            parseEscape<SrcCharacterType>(src);
+    }
+
+    return src - start;
+}
+
 template <typename SrcCharacterType, typename DestCharacterType>
 inline bool CSSTokenizer::parseURIInternal(SrcCharacterType*& src, DestCharacterType*& dest, UChar quote)
 {
     if (quote) {
         ASSERT(quote == '"' || quote == '\'');
         return parseStringInternal(src, dest, quote);
     }
 
     while (isURILetter(*src)) {
         if (LIKELY(*src != '\\')) {
             *dest++ = *src++;
         } else {
             unsigned unicode = parseEscape<SrcCharacterType>(src);
-            if (unicode > 0xff && sizeof(SrcCharacterType) == 1)
+            if (unicode > 0xff && sizeof(DestCharacterType) == 1)
                 return false;
             UnicodeToChars(dest, unicode);
         }
     }
 
     return true;
 }
 
 template <typename CharacterType>
 inline void CSSTokenizer::parseURI(CSSParserString& string)
 {
     CharacterType* uriStart;
     CharacterType* uriEnd;
     UChar quote;
     if (!findURI(uriStart, uriEnd, quote))
         return;
 
     CharacterType* dest = currentCharacter<CharacterType>() = uriStart;
     if (LIKELY(parseURIInternal(currentCharacter<CharacterType>(), dest, quote))) {
         string.init(uriStart, dest - uriStart);
     } else {
         // An escape sequence was encountered that can't be stored in 8 bits.
         // Reset the current character to the start of the URI and re-parse with
         // a 16-bit destination.
         ASSERT(is8BitSource());
-        UChar* uriStart16 = currentCharacter16();
+        UChar* result16 = getStringBuffer16(peekMaxURILen(uriStart, quote));
+        UChar* uriStart16 = result16;
         currentCharacter<CharacterType>() = uriStart;
-        bool result = parseURIInternal(currentCharacter<CharacterType>(), currentCharacter16(), quote);
+        bool result = parseURIInternal(currentCharacter<CharacterType>(), result16, quote);
         ASSERT_UNUSED(result, result);
-        string.init(uriStart16, currentCharacter16() - uriStart16);
+        string.init(uriStart16, result16 - uriStart16);
     }
 
     currentCharacter<CharacterType>() = uriEnd + 1;
     m_token = URI;
 }
 
 template <typename CharacterType>
 inline bool CSSTokenizer::parseUnicodeRange()
 {
     CharacterType* character = currentCharacter<CharacterType>() + 1;
@@ skipping 889 matching lines @@
     m_dataStart16[length - 1] = 0;
 
     m_is8BitSource = false;
     m_currentCharacter8 = 0;
     m_currentCharacter16 = m_dataStart16.get();
     setTokenStart<UChar>(m_currentCharacter16);
     m_lexFunc = &CSSTokenizer::realLex<UChar>;
 }
 
 } // namespace WebCore