Changeset 142673 in webkit

Timestamp:

Feb 12, 2013 2:44:25 PM (11 years ago)

Author:

abarth@webkit.org

Message:

Threaded HTML parser should pass the remaining fast/tokenizer tests
​https://bugs.webkit.org/show_bug.cgi?id=109607

Reviewed by Eric Seidel.

This patch fixes some edge cases involving document.write. Previously,
we would drop input characters on the floor if the tokenizer wasn't
able to consume them synchronously. In this patch, we send the unparsed
characters to the background thread for consumption after rewinding the
input stream.

• html/parser/BackgroundHTMLInputStream.cpp:

(WebCore::BackgroundHTMLInputStream::rewindTo):

• html/parser/BackgroundHTMLInputStream.h:

(BackgroundHTMLInputStream):

• html/parser/BackgroundHTMLParser.cpp:

(WebCore::BackgroundHTMLParser::resumeFrom):

• html/parser/BackgroundHTMLParser.h:

(Checkpoint):

• html/parser/HTMLDocumentParser.cpp:

(WebCore::HTMLDocumentParser::canTakeNextToken):
(WebCore::HTMLDocumentParser::didFailSpeculation):
(WebCore::HTMLDocumentParser::pumpTokenizer):
(WebCore::HTMLDocumentParser::finish):

• html/parser/HTMLInputStream.h:

(WebCore::HTMLInputStream::closeWithoutMarkingEndOfFile):
(HTMLInputStream):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• html/parser/BackgroundHTMLInputStream.cpp (modified) (2 diffs)
• html/parser/BackgroundHTMLInputStream.h (modified) (1 diff)
• html/parser/BackgroundHTMLParser.cpp (modified) (1 diff)
• html/parser/BackgroundHTMLParser.h (modified) (1 diff)
• html/parser/HTMLDocumentParser.cpp (modified) (6 diffs)
• html/parser/HTMLInputStream.h (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2013-02-12  Adam Barth  <abarth@webkit.org>
+
+        Threaded HTML parser should pass the remaining fast/tokenizer tests
+        https://bugs.webkit.org/show_bug.cgi?id=109607
+
+        Reviewed by Eric Seidel.
+
+        This patch fixes some edge cases involving document.write. Previously,
+        we would drop input characters on the floor if the tokenizer wasn't
+        able to consume them synchronously. In this patch, we send the unparsed
+        characters to the background thread for consumption after rewinding the
+        input stream.
+
+        * html/parser/BackgroundHTMLInputStream.cpp:
+        (WebCore::BackgroundHTMLInputStream::rewindTo):
+        * html/parser/BackgroundHTMLInputStream.h:
+        (BackgroundHTMLInputStream):
+        * html/parser/BackgroundHTMLParser.cpp:
+        (WebCore::BackgroundHTMLParser::resumeFrom):
+        * html/parser/BackgroundHTMLParser.h:
+        (Checkpoint):
+        * html/parser/HTMLDocumentParser.cpp:
+        (WebCore::HTMLDocumentParser::canTakeNextToken):
+        (WebCore::HTMLDocumentParser::didFailSpeculation):
+        (WebCore::HTMLDocumentParser::pumpTokenizer):
+        (WebCore::HTMLDocumentParser::finish):
+        * html/parser/HTMLInputStream.h:
+        (WebCore::HTMLInputStream::closeWithoutMarkingEndOfFile):
+        (HTMLInputStream):
+
 2013-02-12  Csaba Osztrogonác  <ossy@webkit.org>
 

trunk/Source/WebCore/html/parser/BackgroundHTMLInputStream.cpp

@@ -54 +54 @@
 }
 
-void BackgroundHTMLInputStream::rewindTo(HTMLInputCheckpoint checkpointIndex)
+void BackgroundHTMLInputStream::rewindTo(HTMLInputCheckpoint checkpointIndex, const String& unparsedInput)
 {
+    ASSERT(checkpointIndex < m_checkpoints.size()); // If this ASSERT fires, checkpointIndex is invalid.
     const Checkpoint& checkpoint = m_checkpoints[checkpointIndex];
 
@@ -61 +62 @@
 
     m_current = checkpoint.input;
+
     for (size_t i = checkpoint.numberOfSegmentsAlreadyAppended; i < m_segments.size(); ++i)
         m_current.append(SegmentedString(m_segments[i]));
 
+    if (!unparsedInput.isEmpty())
+        m_current.prepend(SegmentedString(unparsedInput));
+
     if (isClosed && !m_current.isClosed())
         m_current.close();
+
+    // FIXME: We should be able to actively invalidate all the outstanding checkpoints
+    // by clearing m_segments and m_checkpoints, but that causes
+    // fast/tokenizer/write-before-load.html to hit the ASSERT at the beginning of
+    // this function.
 }
 

trunk/Source/WebCore/html/parser/BackgroundHTMLInputStream.h

@@ -47 +47 @@
     SegmentedString& current() { return m_current; }
 
+    // An HTMLInputCheckpoint is valid until the next call to rewindTo, at which
+    // point all outstanding checkpoints are invalidated.
     HTMLInputCheckpoint createCheckpoint();
-    void rewindTo(HTMLInputCheckpoint);
+    void rewindTo(HTMLInputCheckpoint, const String& unparsedInput);
 
 private:

trunk/Source/WebCore/html/parser/BackgroundHTMLParser.cpp

@@ -82 +82 @@
     m_token = checkpoint->token.release();
     m_tokenizer = checkpoint->tokenizer.release();
-    m_input.rewindTo(checkpoint->inputCheckpoint);
+    m_input.rewindTo(checkpoint->inputCheckpoint, checkpoint->unparsedInput);
     pumpTokenizer();
 }

trunk/Source/WebCore/html/parser/BackgroundHTMLParser.h

@@ -59 +59 @@
         OwnPtr<HTMLTokenizer> tokenizer;
         HTMLInputCheckpoint inputCheckpoint;
+        String unparsedInput;
     };
 

trunk/Source/WebCore/html/parser/HTMLDocumentParser.cpp

@@ -250 +250 @@
         return false;
 
-    ASSERT(!shouldUseThreading() || mode == ForceSynchronous);
+    ASSERT(!m_haveBackgroundParser || mode == ForceSynchronous);
 
     if (isWaitingForScripts()) {
@@ -313 +313 @@
     checkpoint->tokenizer = tokenizer;
     checkpoint->inputCheckpoint = m_currentChunk->checkpoint;
-
+    checkpoint->unparsedInput = m_input.current().toString().isolatedCopy();
+    m_input.current().clear();
+
+    ASSERT(checkpoint->unparsedInput.isSafeToSendToAnotherThread());
     HTMLParserThread::shared()->postTask(bind(&BackgroundHTMLParser::resumeFrom, m_backgroundParser, checkpoint.release()));
 }
@@ -401 +404 @@
     ASSERT(m_tokenizer);
     ASSERT(m_token);
-    ASSERT(!shouldUseThreading() || mode == ForceSynchronous);
+    ASSERT(!m_haveBackgroundParser || mode == ForceSynchronous);
 
     PumpSession session(m_pumpSessionNestingLevel, contextForParsingSession());
@@ -672 +675 @@
     // However, FrameLoader::stop calls DocumentParser::finish unconditionally.
 
-    // We're not going to get any more data off the network, so we tell the
-    // input stream we've reached the end of file. finish() can be called more
-    // than once, if the first time does not call end().
-    if (!m_input.haveSeenEndOfFile())
-        m_input.markEndOfFile();
-
 #if ENABLE(THREADED_HTML_PARSER)
     // Empty documents never got an append() call, and thus have never started
@@ -683 +680 @@
     // and fall through to the non-threading case.
     if (m_haveBackgroundParser) {
+        if (!m_input.haveSeenEndOfFile())
+            m_input.closeWithoutMarkingEndOfFile();
         HTMLParserThread::shared()->postTask(bind(&BackgroundHTMLParser::finish, m_backgroundParser));
         return;
     }
+
     if (shouldUseThreading() && !wasCreatedByScript()) {
         ASSERT(!m_tokenizer && !m_token);
@@ -695 +695 @@
     }
 #endif
+
+    // We're not going to get any more data off the network, so we tell the
+    // input stream we've reached the end of file. finish() can be called more
+    // than once, if the first time does not call end().
+    if (!m_input.haveSeenEndOfFile())
+        m_input.markEndOfFile();
 
     attemptToEnd();

trunk/Source/WebCore/html/parser/HTMLInputStream.h

@@ -78 +78 @@
     }
 
+    void closeWithoutMarkingEndOfFile()
+    {
+        m_last->close();
+    }
+
     bool haveSeenEndOfFile() const
     {