Fix TextDocumentParser to play nice with threading

https://bugs.webkit.org/show_bug.cgi?id=109240

Reviewed by Adam Barth.

Before the HTML5 parser re-write the text document parser
was completely custom.  With the HTML5 parser, we just made
the TextDocumentParser use the HTMLDocumentParser with an
artificial script tag.

However, our solution was slightly over-engineered to avoid
lying about the column numbers of the first line of the text document
during parsing. :)

This change makes us use a simpler (and threading-compatible)
solution by just inserting a real "<pre>" tag into the
input stream instead of hacking one together with the treebuilder
and manually setting the Tokenizer state.

fast/parser/empty-text-resource.html covers this case.

* html/parser/TextDocumentParser.cpp:
(WebCore::TextDocumentParser::TextDocumentParser):
(WebCore::TextDocumentParser::insertFakePreElement):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@142363 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index a293088f99fad8c9212fb6d63047d24f951f01e9..e46f099a58f30260282a90847f7819312ea3811e 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,30 @@
+2013-02-09  Eric Seidel  <eric@webkit.org>
+
+        Fix TextDocumentParser to play nice with threading
+        https://bugs.webkit.org/show_bug.cgi?id=109240
+
+        Reviewed by Adam Barth.
+
+        Before the HTML5 parser re-write the text document parser
+        was completely custom.  With the HTML5 parser, we just made
+        the TextDocumentParser use the HTMLDocumentParser with an
+        artificial script tag.
+
+        However, our solution was slightly over-engineered to avoid
+        lying about the column numbers of the first line of the text document
+        during parsing. :)
+
+        This change makes us use a simpler (and threading-compatible)
+        solution by just inserting a real "<pre>" tag into the
+        input stream instead of hacking one together with the treebuilder
+        and manually setting the Tokenizer state.
+
+        fast/parser/empty-text-resource.html covers this case.
+
+        * html/parser/TextDocumentParser.cpp:
+        (WebCore::TextDocumentParser::TextDocumentParser):
+        (WebCore::TextDocumentParser::insertFakePreElement):
+
 2013-02-09  Kent Tamura  <tkent@chromium.org>
 
         Add missing copyright header

diff --git a/Source/WebCore/html/parser/BackgroundHTMLParser.cpp b/Source/WebCore/html/parser/BackgroundHTMLParser.cpp
index c0676b509596b9e5e7bf9a4bbaada99a0d1c1546..daebdb87e9b442804aa6e147e0f0b3433eb20da8 100644 (file)
--- a/Source/WebCore/html/parser/BackgroundHTMLParser.cpp
+++ b/Source/WebCore/html/parser/BackgroundHTMLParser.cpp
@@ -96,6 +96,14 @@ void BackgroundHTMLParser::stop()
     delete this;
 }
 
+void BackgroundHTMLParser::forcePlaintextForTextDocument()
+{
+    // This is only used by the TextDocumentParser (a subclass of HTMLDocumentParser)
+    // to force us into the PLAINTEXT state w/o using a <plaintext> tag.
+    // The TextDocumentParser uses a <pre> tag for historical/compatibility reasons.
+    m_tokenizer->setState(HTMLTokenizerState::PLAINTEXTState);
+}
+
 void BackgroundHTMLParser::markEndOfFile()
 {
     // FIXME: This should use InputStreamPreprocessor::endOfFileMarker

diff --git a/Source/WebCore/html/parser/BackgroundHTMLParser.h b/Source/WebCore/html/parser/BackgroundHTMLParser.h
index 6a39310b40145ddca57c0f9937a91099fc009627..6c0e0a1e2c82c21c0b38721eea0929662d4e3d9e 100644 (file)
--- a/Source/WebCore/html/parser/BackgroundHTMLParser.h
+++ b/Source/WebCore/html/parser/BackgroundHTMLParser.h
@@ -58,6 +58,8 @@ public:
     void finish();
     void stop();
 
+    void forcePlaintextForTextDocument();
+
 private:
     BackgroundHTMLParser(PassRefPtr<WeakReference<BackgroundHTMLParser> >, const HTMLParserOptions&, const WeakPtr<HTMLDocumentParser>&, PassOwnPtr<XSSAuditor>);
 

diff --git a/Source/WebCore/html/parser/HTMLDocumentParser.cpp b/Source/WebCore/html/parser/HTMLDocumentParser.cpp
index 2b0caf12bd58370f14b6b96d3c2c06f1c129f3cf..da8463c585e03b4d885988923f14f1d6d6dd867f 100644 (file)
--- a/Source/WebCore/html/parser/HTMLDocumentParser.cpp
+++ b/Source/WebCore/html/parser/HTMLDocumentParser.cpp
@@ -355,6 +355,21 @@ void HTMLDocumentParser::processParsedChunkFromBackgroundParser(PassOwnPtr<Parse
 
 #endif // ENABLE(THREADED_HTML_PARSER)
 
+void HTMLDocumentParser::forcePlaintextForTextDocument()
+{
+#if ENABLE(THREADED_HTML_PARSER)
+    if (shouldUseThreading()) {
+        // This method is called before any data is appended, so we have to start
+        // the background parser ourselves.
+        if (!m_haveBackgroundParser)
+            startBackgroundParser();
+
+        HTMLParserThread::shared()->postTask(bind(&BackgroundHTMLParser::forcePlaintextForTextDocument, m_backgroundParser));
+    } else
+#endif
+        m_tokenizer->setState(HTMLTokenizerState::PLAINTEXTState);
+}
+
 void HTMLDocumentParser::pumpTokenizer(SynchronousMode mode)
 {
     ASSERT(!isStopped());

diff --git a/Source/WebCore/html/parser/HTMLDocumentParser.h b/Source/WebCore/html/parser/HTMLDocumentParser.h
index 7c4278525de92c05912ba42c448b7350f6a0f879..19c8d76e8ca5e2f7485b0b61c2388aa908a0059d 100644 (file)
--- a/Source/WebCore/html/parser/HTMLDocumentParser.h
+++ b/Source/WebCore/html/parser/HTMLDocumentParser.h
@@ -101,6 +101,8 @@ protected:
 
     HTMLTreeBuilder* treeBuilder() const { return m_treeBuilder.get(); }
 
+    void forcePlaintextForTextDocument();
+
 private:
     static PassRefPtr<HTMLDocumentParser> create(DocumentFragment* fragment, Element* contextElement, FragmentScriptingPermission permission)
     {

diff --git a/Source/WebCore/html/parser/TextDocumentParser.cpp b/Source/WebCore/html/parser/TextDocumentParser.cpp
index 796c7ec060eee4edf33003d3c0a45a923c2eddf3..fd102cba84f458aa1dc18662e7b849146f60e05e 100644 (file)
--- a/Source/WebCore/html/parser/TextDocumentParser.cpp
+++ b/Source/WebCore/html/parser/TextDocumentParser.cpp
@@ -38,9 +38,6 @@ TextDocumentParser::TextDocumentParser(HTMLDocument* document)
     : HTMLDocumentParser(document, false)
     , m_haveInsertedFakePreElement(false)
 {
-    // FIXME: If we're using threading, we need to tell the BackgroundHTMLParser to use PLAINTEXTState.
-    if (tokenizer())
-        tokenizer()->setState(HTMLTokenizerState::PLAINTEXTState);
 }
 
 TextDocumentParser::~TextDocumentParser()
@@ -61,16 +58,19 @@ void TextDocumentParser::insertFakePreElement()
     // We create a fake token and give it to the tree builder rather than
     // sending fake bytes through the front-end of the parser to avoid
     // distrubing the line/column number calculations.
-
     Vector<Attribute> attributes;
     attributes.append(Attribute(styleAttr, "word-wrap: break-word; white-space: pre-wrap;"));
     RefPtr<AtomicHTMLToken> fakePre = AtomicHTMLToken::create(HTMLTokenTypes::StartTag, preTag.localName(), attributes);
-
     treeBuilder()->constructTree(fakePre.get());
+
     // Normally we would skip the first \n after a <pre> element, but we don't
     // want to skip the first \n for text documents!
     treeBuilder()->setShouldSkipLeadingNewline(false);
 
+    // Although Text Documents expose a "pre" element in their DOM, they
+    // act like a <plaintext> tag, so we have to force plaintext mode.
+    forcePlaintextForTextDocument();
+
     m_haveInsertedFakePreElement = true;
 }