Bug 1325995 - Rewrite shader comment truncation. - r=daoshengmu

MozReview-Commit-ID: KvgQhxAnDQl

dom/canvas/WebGLShader.cpp

@@ -158,19 +158,24 @@ WebGLShader::~WebGLShader()
 void
 WebGLShader::ShaderSource(const nsAString& source)
 {
-    StripComments stripComments(source);
+    const char funcName[] = "shaderSource";
-    const nsAString& cleanSource = Substring(stripComments.result().Elements(),
+    nsString sourceWithoutComments;
-                                             stripComments.length());
+    if (!TruncateComments(source, &sourceWithoutComments)) {
-    if (!ValidateGLSLString(cleanSource, mContext, "shaderSource"))
+        mContext->ErrorOutOfMemory("%s: Failed to alloc for empting comment contents.",
+                                   funcName);
+        return;
+    }
+
+    if (!ValidateGLSLString(sourceWithoutComments, mContext, funcName))
         return;
 
     // We checked that the source stripped of comments is in the
     // 7-bit ASCII range, so we can skip the NS_IsAscii() check.
-    const NS_LossyConvertUTF16toASCII sourceCString(cleanSource);
+    const NS_LossyConvertUTF16toASCII cleanSource(sourceWithoutComments);
 
     if (mContext->gl->WorkAroundDriverBugs()) {
         const size_t maxSourceLength = 0x3ffff;
-        if (sourceCString.Length() > maxSourceLength) {
+        if (cleanSource.Length() > maxSourceLength) {
             mContext->ErrorInvalidValue("shaderSource: Source has more than %d"
                                         " characters. (Driver workaround)",
                                         maxSourceLength);
@@ -185,20 +190,28 @@ WebGLShader::ShaderSource(const nsAString& source)
         // Wow - Roll Your Own Foreach-Lines because printf_stderr has a hard-coded
         // internal size, so long strings are truncated.
 
-        int32_t start = 0;
+        const size_t maxChunkSize = 1024-1; // -1 for null-term.
-        int32_t end = sourceCString.Find("\n", false, start, -1);
+        const UniqueBuffer buf(moz_xmalloc(maxChunkSize+1)); // +1 for null-term
-        while (end > -1) {
+        const auto bufBegin = (char*)buf.get();
-            const nsCString line(sourceCString.BeginReading() + start, end - start);
+
-            printf_stderr("%s\n", line.BeginReading());
+        size_t chunkStart = 0;
-            start = end + 1;
+        while (chunkStart != cleanSource.Length()) {
-            end = sourceCString.Find("\n", false, start, -1);
+            const auto chunkEnd = std::min(chunkStart + maxChunkSize,
+                                           size_t(cleanSource.Length()));
+            const auto chunkSize = chunkEnd - chunkStart;
+
+            memcpy(bufBegin, cleanSource.BeginReading() + chunkStart, chunkSize);
+            bufBegin[chunkSize + 1] = '\0';
+
+            printf_stderr("%s", bufBegin);
+            chunkStart += chunkSize;
         }
 
         printf_stderr("////////////////////////////////////////\n");
     }
 
     mSource = source;
-    mCleanSource = sourceCString;
+    mCleanSource = cleanSource;
 }
 
 void

dom/canvas/WebGLValidateStrings.cpp

@@ -31,96 +31,100 @@ bool IsValidGLSLCharacter(char16_t c)
 
     return false;
 }
-
-void StripComments::process(char16_t c)
-{
-    if (isNewline(c)) {
-        // No matter what state we are in, pass through newlines
-        // so we preserve line numbers.
-        emit(c);
-
-        if (m_parseState != InMultiLineComment)
-            m_parseState = BeginningOfLine;
-
-        return;
-    }
-
-    char16_t temp = 0;
-    switch (m_parseState) {
-    case BeginningOfLine:
-        // If it's an ASCII space.
-        if (c <= ' ' && (c == ' ' || (c <= 0xD && c >= 0x9))) {
-            emit(c);
-            break;
-        }
-
-        if (c == '#') {
-            m_parseState = InPreprocessorDirective;
-            emit(c);
-            break;
-        }
-
-        // Transition to normal state and re-handle character.
-        m_parseState = MiddleOfLine;
-        process(c);
-        break;
-
-    case MiddleOfLine:
-        if (c == '/' && peek(temp)) {
-            if (temp == '/') {
-                m_parseState = InSingleLineComment;
-                emit(' ');
-                advance();
-                break;
-            }
-
-            if (temp == '*') {
-                m_parseState = InMultiLineComment;
-                // Emit the comment start in case the user has
-                // an unclosed comment and we want to later
-                // signal an error.
-                emit('/');
-                emit('*');
-                advance();
-                break;
-            }
-        }
-
-        emit(c);
-        break;
-
-    case InPreprocessorDirective:
-        // No matter what the character is, just pass it
-        // through. Do not parse comments in this state. This
-        // might not be the right thing to do long term, but it
-        // should handle the #error preprocessor directive.
-        emit(c);
-        break;
-
-    case InSingleLineComment:
-        // The newline code at the top of this function takes care
-        // of resetting our state when we get out of the
-        // single-line comment. Swallow all other characters.
-        break;
-
-    case InMultiLineComment:
-        if (c == '*' && peek(temp) && temp == '/') {
-            emit('*');
-            emit('/');
-            m_parseState = MiddleOfLine;
-            advance();
-            break;
-        }
-
-        // Swallow all other characters. Unclear whether we may
-        // want or need to just emit a space per character to try
-        // to preserve column numbers for debugging purposes.
-        break;
-    }
-}
-
 /****** END CODE TAKEN FROM WEBKIT ******/
 
+bool
+TruncateComments(const nsAString& src, nsAString* const out)
+{
+    const size_t dstByteCount = src.Length() * sizeof(src[0]);
+    const UniqueBuffer dst(malloc(dstByteCount));
+    if (!dst)
+        return false;
+
+    auto srcItr = src.BeginReading();
+    const auto srcEnd = src.EndReading();
+    const auto dstBegin = (decltype(src[0])*)dst.get();
+    auto dstItr = dstBegin;
+
+    const auto fnEmitUntil = [&](const decltype(srcItr)& nextSrcItr) {
+        while (srcItr != nextSrcItr) {
+            *dstItr = *srcItr;
+            ++srcItr;
+            ++dstItr;
+        }
+    };
+
+    const auto fnFindSoonestOf = [&](const nsString* needles, size_t needleCount,
+                                     size_t* const out_foundId)
+    {
+        auto foundItr = srcItr;
+        while (foundItr != srcEnd) {
+            const auto haystack = Substring(foundItr, srcEnd);
+            for (size_t i = 0; i < needleCount; i++) {
+                if (StringBeginsWith(haystack, needles[i])) {
+                    *out_foundId = i;
+                    return foundItr;
+                }
+            }
+            ++foundItr;
+        }
+        *out_foundId = needleCount;
+        return foundItr;
+    };
+
+    ////
+
+    const nsString commentBeginnings[] = { NS_LITERAL_STRING("//"),
+                                           NS_LITERAL_STRING("/*"),
+                                           nsString() };
+    const nsString lineCommentEndings[] = { NS_LITERAL_STRING("\\\n"),
+                                            NS_LITERAL_STRING("\n"),
+                                            nsString() };
+    const nsString blockCommentEndings[] = { NS_LITERAL_STRING("\n"),
+                                             NS_LITERAL_STRING("*/"),
+                                             nsString() };
+
+    while (srcItr != srcEnd) {
+        size_t foundId;
+        fnEmitUntil( fnFindSoonestOf(commentBeginnings, 2, &foundId) );
+        fnEmitUntil(srcItr + commentBeginnings[foundId].Length());
+
+        switch (foundId) {
+        case 0: // line comment
+            while (true) {
+                size_t endId;
+                srcItr = fnFindSoonestOf(lineCommentEndings, 2, &endId);
+                fnEmitUntil(srcItr + lineCommentEndings[endId].Length());
+                if (endId == 0)
+                    continue;
+                break;
+            }
+            break;
+
+        case 1: // block comment
+            while (true) {
+                size_t endId;
+                srcItr = fnFindSoonestOf(blockCommentEndings, 2, &endId);
+                fnEmitUntil(srcItr + blockCommentEndings[endId].Length());
+                if (endId == 0)
+                    continue;
+                break;
+            }
+            break;
+
+        default: // not found
+            break;
+        }
+    }
+
+    MOZ_ASSERT((dstBegin+1) - dstBegin == 1);
+    const uint32_t dstCharLen = dstItr - dstBegin;
+    if (!out->Assign(dstBegin, dstCharLen, mozilla::fallible))
+        return false;
+
+    return true;
+}
+
 bool
 ValidateGLSLString(const nsAString& string, WebGLContext* webgl, const char* funcName)
 {

dom/canvas/WebGLValidateStrings.h

@@ -34,116 +34,8 @@ namespace mozilla {
 
 class WebGLContext;
 
-// The following code was taken from the WebKit WebGL implementation,
+bool
-// which can be found here:
+TruncateComments(const nsAString& src, nsAString* const out);
-// http://trac.webkit.org/browser/trunk/Source/WebCore/html/canvas/WebGLRenderingContext.cpp?rev=93625#L121
-// Note that some modifications were done to adapt it to Mozilla.
-/****** BEGIN CODE TAKEN FROM WEBKIT ******/
-// Strips comments from shader text. This allows non-ASCII characters
-// to be used in comments without potentially breaking OpenGL
-// implementations not expecting characters outside the GLSL ES set.
-class StripComments {
-public:
-    explicit StripComments(const nsAString& str)
-        : m_parseState(BeginningOfLine)
-        , m_end(str.EndReading())
-        , m_current(str.BeginReading())
-        , m_position(0)
-    {
-        m_result.SetLength(str.Length());
-        parse();
-    }
-
-    const nsTArray<char16_t>& result()
-    {
-        return m_result;
-    }
-
-    size_t length()
-    {
-        return m_position;
-    }
-
-private:
-    bool hasMoreCharacters()
-    {
-        return (m_current < m_end);
-    }
-
-    void parse()
-    {
-        while (hasMoreCharacters()) {
-            process(current());
-            // process() might advance the position.
-            if (hasMoreCharacters())
-                advance();
-        }
-    }
-
-    void process(char16_t);
-
-    bool peek(char16_t& character)
-    {
-        if (m_current + 1 >= m_end)
-            return false;
-        character = *(m_current + 1);
-        return true;
-    }
-
-    char16_t current()
-    {
-        //ASSERT(m_position < m_length);
-        return *m_current;
-    }
-
-    void advance()
-    {
-        ++m_current;
-    }
-
-    bool isNewline(char16_t character)
-    {
-        // Don't attempt to canonicalize newline related characters.
-        return (character == '\n' || character == '\r');
-    }
-
-    void emit(char16_t character)
-    {
-        m_result[m_position++] = character;
-    }
-
-    enum ParseState {
-        // Have not seen an ASCII non-whitespace character yet on
-        // this line. Possible that we might see a preprocessor
-        // directive.
-        BeginningOfLine,
-
-        // Have seen at least one ASCII non-whitespace character
-        // on this line.
-        MiddleOfLine,
-
-        // Handling a preprocessor directive. Passes through all
-        // characters up to the end of the line. Disables comment
-        // processing.
-        InPreprocessorDirective,
-
-        // Handling a single-line comment. The comment text is
-        // replaced with a single space.
-        InSingleLineComment,
-
-        // Handling a multi-line comment. Newlines are passed
-        // through to preserve line numbers.
-        InMultiLineComment
-    };
-
-    ParseState m_parseState;
-    const char16_t* m_end;
-    const char16_t* m_current;
-    size_t m_position;
-    nsTArray<char16_t> m_result;
-};
-
-/****** END CODE TAKEN FROM WEBKIT ******/
 
 bool ValidateGLSLString(const nsAString& string, WebGLContext* webgl,
                         const char* funcName);