Changeset 62410 in webkit

Timestamp:

Jul 2, 2010, 3:31:40 PM (15 years ago)

Author:

[email protected]

Message:

2010-07-02 Oliver Hunt <​[email protected]>

Reviewed by Geoffrey Garen.

Move BOM handling out of the lexer and parser
​https://bugs.webkit.org/show_bug.cgi?id=41539

Doing the BOM stripping in the lexer meant that we could
end up having to strip the BOMs from a source multiple times.
To deal with this we now require all strings provided by
a SourceProvider to already have had the BOMs stripped.
This also simplifies some of the lexer logic.

• parser/Lexer.cpp: (JSC::Lexer::setCode): (JSC::Lexer::sourceCode):
• parser/SourceProvider.h: (JSC::SourceProvider::SourceProvider): (JSC::UStringSourceProvider::create): (JSC::UStringSourceProvider::getRange): (JSC::UStringSourceProvider::UStringSourceProvider):
• wtf/text/StringImpl.h: (WebCore::StringImpl::copyStringWithoutBOMs):

2010-07-02 Oliver Hunt <​[email protected]>

Reviewed by Geoffrey Garen.

Move BOM handling out of the lexer and parser
​https://bugs.webkit.org/show_bug.cgi?id=41539

Update WebCore to ensure that SourceProviders don't
produce strings with BOMs in them.

• bindings/js/ScriptSourceProvider.h: (WebCore::ScriptSourceProvider::ScriptSourceProvider):
• bindings/js/StringSourceProvider.h: (WebCore::StringSourceProvider::StringSourceProvider):
• loader/CachedScript.cpp: (WebCore::CachedScript::CachedScript): (WebCore::CachedScript::script):
• loader/CachedScript.h: (WebCore::CachedScript::): CachedScript now stores decoded data with the BOMs stripped, and caches the presence of BOMs across memory purges.

Location:

trunk

Files:

• JavaScriptCore/ChangeLog (modified) (1 diff)
• JavaScriptCore/parser/Lexer.cpp (modified) (4 diffs)
• JavaScriptCore/parser/SourceProvider.h (modified) (2 diffs)
• JavaScriptCore/wtf/text/StringImpl.h (modified) (1 diff)
• WebCore/ChangeLog (modified) (1 diff)
• WebCore/bindings/js/ScriptSourceProvider.h (modified) (1 diff)
• WebCore/bindings/js/StringSourceProvider.h (modified) (1 diff)
• WebCore/loader/CachedScript.cpp (modified) (2 diffs)
• WebCore/loader/CachedScript.h (modified) (1 diff)

trunk/JavaScriptCore/ChangeLog

@@ +1 @@
+2010-07-02  Oliver Hunt  <[email protected]>
+
+        Reviewed by Geoffrey Garen.
+
+        Move BOM handling out of the lexer and parser
+        https://bugs.webkit.org/show_bug.cgi?id=41539
+
+        Doing the BOM stripping in the lexer meant that we could
+        end up having to strip the BOMs from a source multiple times.
+        To deal with this we now require all strings provided by
+        a SourceProvider to already have had the BOMs stripped.
+        This also simplifies some of the lexer logic.
+
+        * parser/Lexer.cpp:
+        (JSC::Lexer::setCode):
+        (JSC::Lexer::sourceCode):
+        * parser/SourceProvider.h:
+        (JSC::SourceProvider::SourceProvider):
+        (JSC::UStringSourceProvider::create):
+        (JSC::UStringSourceProvider::getRange):
+        (JSC::UStringSourceProvider::UStringSourceProvider):
+        * wtf/text/StringImpl.h:
+        (WebCore::StringImpl::copyStringWithoutBOMs):
+
 2010-07-02  Renata Hodovan  <[email protected]>
 

trunk/JavaScriptCore/parser/Lexer.cpp

@@ -46 +46 @@
 namespace JSC {
 
-static const UChar byteOrderMark = 0xFEFF;
 
 enum CharacterTypes {
@@ -257 +256 @@
     m_buffer16.reserveInitialCapacity((m_codeEnd - m_code) / 2);
 
-    // ECMA-262 calls for stripping all Cf characters, but we only strip BOM characters.
-    // See <https://bugs.webkit.org/show_bug.cgi?id=4931> for details.
-    if (source.provider()->hasBOMs()) {
-        for (const UChar* p = m_codeStart; p < m_codeEnd; ++p) {
-            if (UNLIKELY(*p == byteOrderMark)) {
-                copyCodeWithoutBOMs();
-                break;
-            }
-        }
-    }
-
     if (LIKELY(m_code < m_codeEnd))
         m_current = *m_code;
@@ -273 +261 @@
         m_current = -1;
     ASSERT(currentOffset() == source.startOffset());
-}
-
-void Lexer::copyCodeWithoutBOMs()
-{
-    // Note: In this case, the character offset data for debugging will be incorrect.
-    // If it's important to correctly debug code with extraneous BOMs, then the caller
-    // should strip the BOMs when creating the SourceProvider object and do its own
-    // mapping of offsets within the stripped text to original text offset.
-
-    m_codeWithoutBOMs.reserveCapacity(m_codeEnd - m_code);
-    for (const UChar* p = m_code; p < m_codeEnd; ++p) {
-        UChar c = *p;
-        if (c != byteOrderMark)
-            m_codeWithoutBOMs.append(c);
-    }
-    ptrdiff_t startDelta = m_codeStart - m_code;
-    m_code = m_codeWithoutBOMs.data();
-    m_codeStart = m_code + startDelta;
-    m_codeEnd = m_codeWithoutBOMs.data() + m_codeWithoutBOMs.size();
 }
 
@@ -1181 +1150 @@
 SourceCode Lexer::sourceCode(int openBrace, int closeBrace, int firstLine)
 {
-    if (m_codeWithoutBOMs.isEmpty())
-        return SourceCode(m_source->provider(), openBrace, closeBrace + 1, firstLine);
-
-    const UChar* data = m_source->provider()->data();
-    
-    ASSERT(openBrace < closeBrace);
-    int i;
-    for (i = m_source->startOffset(); i < openBrace; ++i) {
-        if (data[i] == byteOrderMark) {
-            openBrace++;
-            closeBrace++;
-        }
-    }
-    for (; i < closeBrace; ++i) {
-        if (data[i] == byteOrderMark)
-            closeBrace++;
-    }
-
-    ASSERT(openBrace < closeBrace);
-
     return SourceCode(m_source->provider(), openBrace, closeBrace + 1, firstLine);
 }

trunk/JavaScriptCore/parser/SourceProvider.h

@@ -35 +35 @@
 namespace JSC {
 
-    enum SourceBOMPresence { SourceHasNoBOMs, SourceCouldHaveBOMs };
-
     class SourceProvider : public RefCounted<SourceProvider> {
     public:
-        SourceProvider(const UString& url, SourceBOMPresence hasBOMs = SourceCouldHaveBOMs)
+        SourceProvider(const UString& url)
             : m_url(url)
-            , m_hasBOMs(hasBOMs)
         {
         }
@@ -53 +50 @@
         intptr_t asID() { return reinterpret_cast<intptr_t>(this); }
 
-        SourceBOMPresence hasBOMs() const { return m_hasBOMs; }
-
     private:
         UString m_url;
-        SourceBOMPresence m_hasBOMs;
     };
 
     class UStringSourceProvider : public SourceProvider {
     public:
-        static PassRefPtr<UStringSourceProvider> create(const UString& source, const UString& url)
+        static PassRefPtr<UStringSourceProvider> create(const UString& source, const UString& url, bool hasBOMs = true)
         {
-            return adoptRef(new UStringSourceProvider(source, url));
+            return adoptRef(new UStringSourceProvider(source, url, hasBOMs));
         }
 
-        UString getRange(int start, int end) const { return m_source.substr(start, end - start); }
+        UString getRange(int start, int end) const
+        {
+            return m_source.substr(start, end - start);
+        }
         const UChar* data() const { return m_source.data(); }
         int length() const { return m_source.size(); }
 
     private:
-        UStringSourceProvider(const UString& source, const UString& url)
+        UStringSourceProvider(const UString& source, const UString& url, bool hasBOMs)
             : SourceProvider(url)
             , m_source(source)
         {
+            if (hasBOMs && m_source.size()) {
+                bool scratch = false;
+                m_source = UString(m_source.rep()->copyStringWithoutBOMs(false, scratch));
+            }
         }
 

trunk/JavaScriptCore/wtf/text/StringImpl.h

@@ -258 +258 @@
     }
 
+    PassRefPtr<StringImpl> copyStringWithoutBOMs(bool definitelyHasBOMs, bool& hasBOMs)
+    {
+        static const UChar byteOrderMark = 0xFEFF;
+        size_t i = 0;
+        if (!definitelyHasBOMs) {
+            hasBOMs = false;
+            // ECMA-262 calls for stripping all Cf characters, but we only strip BOM characters.
+            // See <https://bugs.webkit.org/show_bug.cgi?id=4931> for details.
+            for (; i < m_length; i++) {
+                if (UNLIKELY(m_data[i] == byteOrderMark)) {
+                    hasBOMs = true;
+                    break;
+                }
+            }
+            if (!hasBOMs)
+                return this;
+        }
+        Vector<UChar> result;
+        result.reserveInitialCapacity(m_length);
+        for (; i < m_length; i++)
+            result.append(m_data[i]);
+        for (; i < m_length; i++) {
+            UChar c = m_data[i];
+            if (c != byteOrderMark)
+                result.append(c);
+        }
+        return StringImpl::adopt(result);
+    }
+
     // Returns a StringImpl suitable for use on another thread.
     PassRefPtr<StringImpl> crossThreadString();

trunk/WebCore/ChangeLog

@@ +1 @@
+2010-07-02  Oliver Hunt  <[email protected]>
+
+        Reviewed by Geoffrey Garen.
+
+        Move BOM handling out of the lexer and parser
+        https://bugs.webkit.org/show_bug.cgi?id=41539
+
+        Update WebCore to ensure that SourceProviders don't
+        produce strings with BOMs in them.
+
+        * bindings/js/ScriptSourceProvider.h:
+        (WebCore::ScriptSourceProvider::ScriptSourceProvider):
+        * bindings/js/StringSourceProvider.h:
+        (WebCore::StringSourceProvider::StringSourceProvider):
+        * loader/CachedScript.cpp:
+        (WebCore::CachedScript::CachedScript):
+        (WebCore::CachedScript::script):
+        * loader/CachedScript.h:
+        (WebCore::CachedScript::):
+          CachedScript now stores decoded data with the BOMs stripped,
+          and caches the presence of BOMs across memory purges.
+
 2010-07-02  Sam Weinig  <[email protected]>
 

trunk/WebCore/bindings/js/ScriptSourceProvider.h

@@ -36 +36 @@
     class ScriptSourceProvider : public JSC::SourceProvider {
     public:
-        ScriptSourceProvider(const JSC::UString& url, JSC::SourceBOMPresence hasBOMs = JSC::SourceCouldHaveBOMs)
-            : SourceProvider(url, hasBOMs)
+        ScriptSourceProvider(const JSC::UString& url)
+            : SourceProvider(url)
         {
         }

trunk/WebCore/bindings/js/StringSourceProvider.h

@@ -50 +50 @@
             , m_source(source)
         {
+            if (m_source.length()) {
+                bool scratch = false;
+                m_source = String(source.impl()->copyStringWithoutBOMs(false, scratch));
+            }
         }
         

trunk/WebCore/loader/CachedScript.cpp

@@ -38 +38 @@
 CachedScript::CachedScript(const String& url, const String& charset)
     : CachedResource(url, Script)
+    , m_scriptHasBOMs(SourceCouldHaveBOMs)
     , m_decoder(TextResourceDecoder::create("application/javascript", charset))
     , m_decodedDataDeletionTimer(this, &CachedScript::decodedDataDeletionTimerFired)
@@ -79 +80 @@
         m_script = m_decoder->decode(m_data->data(), encodedSize());
         m_script += m_decoder->flush();
+        if (m_scriptHasBOMs != SourceHasNoBOMs && m_script.length()) {
+            bool hasBOMs = false;
+            m_script = String(m_script.impl()->copyStringWithoutBOMs(m_scriptHasBOMs == SourceHasBOMs, hasBOMs));
+            m_scriptHasBOMs = hasBOMs ? SourceHasBOMs : SourceHasNoBOMs;
+        }
         setDecodedSize(m_script.length() * sizeof(UChar));
     }

trunk/WebCore/loader/CachedScript.h

@@ -60 +60 @@
 
         String m_script;
+        enum { SourceHasNoBOMs, SourceCouldHaveBOMs, SourceHasBOMs } m_scriptHasBOMs;
         RefPtr<TextResourceDecoder> m_decoder;
         Timer<CachedScript> m_decodedDataDeletionTimer;