Bug 1325995 - Rewrite shader comment truncation. - r=daoshengmu draft
authorJeff Gilbert <jgilbert@mozilla.com>
Tue, 27 Dec 2016 17:47:19 -0800
changeset 454990 7a1550fdcb7d4f4c763068a0a39b615be285bf73
parent 454949 e120594f18fb68bf0cc9fe119100210f8b4a354c
child 454991 ccd59d231f6a11198b43627f3b24d3253a9c11c2
push id40101
push userbmo:jgilbert@mozilla.com
push dateSat, 31 Dec 2016 01:55:51 +0000
reviewersdaoshengmu
bugs1325995
milestone53.0a1
Bug 1325995 - Rewrite shader comment truncation. - r=daoshengmu MozReview-Commit-ID: KvgQhxAnDQl
dom/canvas/WebGLShader.cpp
dom/canvas/WebGLValidateStrings.cpp
dom/canvas/WebGLValidateStrings.h
--- a/dom/canvas/WebGLShader.cpp
+++ b/dom/canvas/WebGLShader.cpp
@@ -153,57 +153,70 @@ WebGLShader::WebGLShader(WebGLContext* w
 WebGLShader::~WebGLShader()
 {
     DeleteOnce();
 }
 
 void
 WebGLShader::ShaderSource(const nsAString& source)
 {
-    StripComments stripComments(source);
-    const nsAString& cleanSource = Substring(stripComments.result().Elements(),
-                                             stripComments.length());
-    if (!ValidateGLSLString(cleanSource, mContext, "shaderSource"))
+    const char funcName[] = "shaderSource";
+    nsString sourceWithoutComments;
+    if (!TruncateComments(source, &sourceWithoutComments)) {
+        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);
             return;
         }
     }
 
     if (PR_GetEnv("MOZ_WEBGL_DUMP_SHADERS")) {
         printf_stderr("////////////////////////////////////////\n");
         printf_stderr("// MOZ_WEBGL_DUMP_SHADERS:\n");
 
         // Wow - Roll Your Own Foreach-Lines because printf_stderr has a hard-coded
         // internal size, so long strings are truncated.
 
-        int32_t start = 0;
-        int32_t end = sourceCString.Find("\n", false, start, -1);
-        while (end > -1) {
-            const nsCString line(sourceCString.BeginReading() + start, end - start);
-            printf_stderr("%s\n", line.BeginReading());
-            start = end + 1;
-            end = sourceCString.Find("\n", false, start, -1);
+        const size_t bufSize = 1024;
+        const UniqueBuffer buf(moz_xmalloc(bufSize));
+
+        size_t chunkStart = 0;
+        while (chunkStart != cleanSource.Length()) {
+            const auto chunkEnd = std::min(chunkStart + bufSize - 1,
+                                           size_t(cleanSource.Length()));
+            const auto chunkSize = chunkEnd - chunkStart;
+
+            memcpy(buf.get(), cleanSource.BeginReading() + chunkStart, chunkSize);
+            *( (char*)buf.get() + chunkSize ) = 0;
+
+            printf_stderr("%s", buf.get());
+
+            chunkStart += chunkSize;
         }
 
         printf_stderr("////////////////////////////////////////\n");
     }
 
     mSource = source;
-    mCleanSource = sourceCString;
+    mCleanSource = cleanSource;
 }
 
 void
 WebGLShader::CompileShader()
 {
     mValidator = nullptr;
     mTranslationSuccessful = false;
     mCompilationSuccessful = false;
--- a/dom/canvas/WebGLValidateStrings.cpp
+++ b/dom/canvas/WebGLValidateStrings.cpp
@@ -26,106 +26,110 @@ bool IsValidGLSLCharacter(char16_t c)
     // Horizontal tab, line feed, vertical tab, form feed, carriage return
     // are also valid.
     if (c >= 9 && c <= 13) {
          return true;
     }
 
     return false;
 }
+/****** END CODE TAKEN FROM WEBKIT ******/
 
-void StripComments::process(char16_t c)
+bool
+TruncateComments(const nsAString& src, nsAString* const out)
 {
-    if (isNewline(c)) {
-        // No matter what state we are in, pass through newlines
-        // so we preserve line numbers.
-        emit(c);
+    const size_t dstByteCount = src.Length() * sizeof(src[0]);
+    const UniqueBuffer dst(malloc(dstByteCount));
+    if (!dst)
+        return false;
 
-        if (m_parseState != InMultiLineComment)
-            m_parseState = BeginningOfLine;
+    auto srcItr = src.BeginReading();
+    const auto srcEnd = src.EndReading();
+    const auto dstBegin = (decltype(src[0])*)dst.get();
+    auto dstItr = dstBegin;
 
-        return;
-    }
+    const auto fnEmitUntil = [&](const decltype(srcItr)& nextSrcItr) {
+        while (srcItr != nextSrcItr) {
+            *dstItr = *srcItr;
+            ++srcItr;
+            ++dstItr;
+        }
+    };
 
-    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;
+    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;
+    };
+
+    ////
 
-        if (c == '#') {
-            m_parseState = InPreprocessorDirective;
-            emit(c);
-            break;
-        }
+    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() };
 
-        // Transition to normal state and re-handle character.
-        m_parseState = MiddleOfLine;
-        process(c);
-        break;
+    while (srcItr != srcEnd) {
+        size_t foundId;
+        fnEmitUntil( fnFindSoonestOf(commentBeginnings, 2, &foundId) );
+        fnEmitUntil(srcItr + commentBeginnings[foundId].Length());
 
-    case MiddleOfLine:
-        if (c == '/' && peek(temp)) {
-            if (temp == '/') {
-                m_parseState = InSingleLineComment;
-                emit(' ');
-                advance();
+        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;
 
-            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();
+        case 1: // block comment
+            while (true) {
+                size_t endId;
+                srcItr = fnFindSoonestOf(blockCommentEndings, 2, &endId);
+                fnEmitUntil(srcItr + blockCommentEndings[endId].Length());
+                if (endId == 0)
+                    continue;
                 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;
+            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();
+        default: // not found
             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;
-    }
+    MOZ_ASSERT((dstBegin+1) - dstBegin == 1);
+    const uint32_t dstCharLen = dstItr - dstBegin;
+    if (!out->Assign(dstBegin, dstCharLen, mozilla::fallible))
+        return false;
+
+    return true;
 }
 
-/****** END CODE TAKEN FROM WEBKIT ******/
-
 bool
 ValidateGLSLString(const nsAString& string, WebGLContext* webgl, const char* funcName)
 {
     for (size_t i = 0; i < string.Length(); ++i) {
         if (!IsValidGLSLCharacter(string.CharAt(i))) {
            webgl->ErrorInvalidValue("%s: String contains the illegal character '%d'",
                                     funcName, string.CharAt(i));
            return false;
--- a/dom/canvas/WebGLValidateStrings.h
+++ b/dom/canvas/WebGLValidateStrings.h
@@ -29,126 +29,18 @@
 
 #include "nsString.h"
 #include "nsTArray.h"
 
 namespace mozilla {
 
 class WebGLContext;
 
-// The following code was taken from the WebKit WebGL implementation,
-// which can be found here:
-// 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
+TruncateComments(const nsAString& src, nsAString* const out);
 
 bool ValidateGLSLString(const nsAString& string, WebGLContext* webgl,
                         const char* funcName);
 
 bool ValidateGLSLVariableName(const nsAString& name, WebGLContext* webgl,
                               const char* funcName);
 
 } // namespace mozilla