- Revision
- 144714
- Author
- [email protected]
- Date
- 2013-03-04 20:57:15 -0800 (Mon, 04 Mar 2013)
Log Message
Background HTML parser can rewind the tokenizer after end-of-file
https://bugs.webkit.org/show_bug.cgi?id=111365
Reviewed by Eric Seidel.
Source/WebCore:
Prior to this patch, it was possible to call didFailSpeculation after
processing the end-of-file token because checkForSpeculationFailure
didn't zero out m_tokenizer in some control paths.
This patch renames checkForSpeculationFailure to validateSpeculations
and ensures that it always takes ownership of the main thread's
HTMLTokenizer.
This patch also adds a number of ASSERTs to make sure the parser state
machine stays in the correct configuration (e.g., that we don't have a
main thread tokenizer while we're supposed to be tokenizing on the
background thread).
Test: fast/parser/document-write-fighting-eof.html
* html/parser/BackgroundHTMLInputStream.cpp:
(WebCore::BackgroundHTMLInputStream::rewindTo):
* html/parser/BackgroundHTMLParser.cpp:
(WebCore::BackgroundHTMLParser::append):
* html/parser/HTMLDocumentParser.cpp:
(WebCore::HTMLDocumentParser::validateSpeculations):
(WebCore::HTMLDocumentParser::processParsedChunkFromBackgroundParser):
(WebCore::HTMLDocumentParser::pumpPendingSpeculations):
(WebCore::HTMLDocumentParser::resumeParsingAfterScriptExecution):
* html/parser/HTMLDocumentParser.h:
(HTMLDocumentParser):
LayoutTests:
* fast/parser/document-write-fighting-eof.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (144713 => 144714)
--- trunk/LayoutTests/ChangeLog 2013-03-05 04:37:32 UTC (rev 144713)
+++ trunk/LayoutTests/ChangeLog 2013-03-05 04:57:15 UTC (rev 144714)
@@ -1,3 +1,12 @@
+2013-03-04 Adam Barth <[email protected]>
+
+ Background HTML parser can rewind the tokenizer after end-of-file
+ https://bugs.webkit.org/show_bug.cgi?id=111365
+
+ Reviewed by Eric Seidel.
+
+ * fast/parser/document-write-fighting-eof.html: Added.
+
2013-03-04 Tim 'mithro' Ansell <[email protected]>
Make sure that clearOwnerNode also clears StyleResolver references (via didMutate).
Added: trunk/LayoutTests/fast/parser/document-write-fighting-eof-expected.txt (0 => 144714)
--- trunk/LayoutTests/fast/parser/document-write-fighting-eof-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/parser/document-write-fighting-eof-expected.txt 2013-03-05 04:57:15 UTC (rev 144714)
@@ -0,0 +1 @@
+PASS
Added: trunk/LayoutTests/fast/parser/document-write-fighting-eof.html (0 => 144714)
--- trunk/LayoutTests/fast/parser/document-write-fighting-eof.html (rev 0)
+++ trunk/LayoutTests/fast/parser/document-write-fighting-eof.html 2013-03-05 04:57:15 UTC (rev 144714)
@@ -0,0 +1,6 @@
+<script>
+if (window.testRunner)
+ testRunner.dumpAsText();
+</script>
+PA<script _onbeforeload_="document.write('<span')" src=""
+>SS
Modified: trunk/Source/WebCore/ChangeLog (144713 => 144714)
--- trunk/Source/WebCore/ChangeLog 2013-03-05 04:37:32 UTC (rev 144713)
+++ trunk/Source/WebCore/ChangeLog 2013-03-05 04:57:15 UTC (rev 144714)
@@ -1,3 +1,37 @@
+2013-03-04 Adam Barth <[email protected]>
+
+ Background HTML parser can rewind the tokenizer after end-of-file
+ https://bugs.webkit.org/show_bug.cgi?id=111365
+
+ Reviewed by Eric Seidel.
+
+ Prior to this patch, it was possible to call didFailSpeculation after
+ processing the end-of-file token because checkForSpeculationFailure
+ didn't zero out m_tokenizer in some control paths.
+
+ This patch renames checkForSpeculationFailure to validateSpeculations
+ and ensures that it always takes ownership of the main thread's
+ HTMLTokenizer.
+
+ This patch also adds a number of ASSERTs to make sure the parser state
+ machine stays in the correct configuration (e.g., that we don't have a
+ main thread tokenizer while we're supposed to be tokenizing on the
+ background thread).
+
+ Test: fast/parser/document-write-fighting-eof.html
+
+ * html/parser/BackgroundHTMLInputStream.cpp:
+ (WebCore::BackgroundHTMLInputStream::rewindTo):
+ * html/parser/BackgroundHTMLParser.cpp:
+ (WebCore::BackgroundHTMLParser::append):
+ * html/parser/HTMLDocumentParser.cpp:
+ (WebCore::HTMLDocumentParser::validateSpeculations):
+ (WebCore::HTMLDocumentParser::processParsedChunkFromBackgroundParser):
+ (WebCore::HTMLDocumentParser::pumpPendingSpeculations):
+ (WebCore::HTMLDocumentParser::resumeParsingAfterScriptExecution):
+ * html/parser/HTMLDocumentParser.h:
+ (HTMLDocumentParser):
+
2013-03-04 Tim 'mithro' Ansell <[email protected]>
Make sure that clearOwnerNode also clears StyleResolver references (via didMutate).
Modified: trunk/Source/WebCore/html/parser/BackgroundHTMLInputStream.cpp (144713 => 144714)
--- trunk/Source/WebCore/html/parser/BackgroundHTMLInputStream.cpp 2013-03-05 04:37:32 UTC (rev 144713)
+++ trunk/Source/WebCore/html/parser/BackgroundHTMLInputStream.cpp 2013-03-05 04:57:15 UTC (rev 144714)
@@ -71,6 +71,8 @@
if (isClosed && !m_current.isClosed())
m_current.close();
+ ASSERT(m_current.isClosed() == isClosed);
+
m_segments.clear();
m_checkpoints.clear();
}
Modified: trunk/Source/WebCore/html/parser/BackgroundHTMLParser.cpp (144713 => 144714)
--- trunk/Source/WebCore/html/parser/BackgroundHTMLParser.cpp 2013-03-05 04:37:32 UTC (rev 144713)
+++ trunk/Source/WebCore/html/parser/BackgroundHTMLParser.cpp 2013-03-05 04:57:15 UTC (rev 144714)
@@ -146,6 +146,7 @@
void BackgroundHTMLParser::append(const String& input)
{
+ ASSERT(!m_input.current().isClosed());
m_input.append(input);
pumpTokenizer();
}
Modified: trunk/Source/WebCore/html/parser/HTMLDocumentParser.cpp (144713 => 144714)
--- trunk/Source/WebCore/html/parser/HTMLDocumentParser.cpp 2013-03-05 04:37:32 UTC (rev 144713)
+++ trunk/Source/WebCore/html/parser/HTMLDocumentParser.cpp 2013-03-05 04:57:15 UTC (rev 144714)
@@ -328,24 +328,38 @@
InspectorInstrumentation::didWriteHTML(cookie, lineNumber().zeroBasedInt());
}
-void HTMLDocumentParser::checkForSpeculationFailure()
+void HTMLDocumentParser::validateSpeculations()
{
- if (!m_tokenizer)
+ OwnPtr<HTMLTokenizer> tokenizer = m_tokenizer.release();
+ OwnPtr<HTMLToken> token = m_token.release();
+
+ if (!tokenizer) {
+ // There must not have been any changes to the HTMLTokenizer state on
+ // the main thread, which means the speculation buffer is correct.
return;
- if (!m_currentChunk)
+ }
+
+ if (!m_currentChunk) {
+ // If there is no m_currentChunk, we must have already called didFailSpeculation
+ // for this chunk.
+ // FIXME: In this case, we're losing whatever state has been changed since
+ // we called didFailSpeculation. See https://bugs.webkit.org/show_bug.cgi?id=110546
return;
+ }
+
// Currently we're only smart enough to reuse the speculation buffer if the tokenizer
// both starts and ends in the DataState. That state is simplest because the HTMLToken
// is always in the Uninitialized state. We should consider whether we can reuse the
// speculation buffer in other states, but we'd likely need to do something more
// sophisticated with the HTMLToken.
- if (m_currentChunk->tokenizerState == HTMLTokenizer::DataState && m_tokenizer->state() == HTMLTokenizer::DataState && m_input.current().isEmpty()) {
- ASSERT(m_token->isUninitialized());
- m_tokenizer.clear();
- m_token.clear();
+ if (m_currentChunk->tokenizerState == HTMLTokenizer::DataState
+ && tokenizer->state() == HTMLTokenizer::DataState
+ && m_input.current().isEmpty()) {
+ ASSERT(token->isUninitialized());
return;
}
- didFailSpeculation(m_token.release(), m_tokenizer.release());
+
+ didFailSpeculation(token.release(), tokenizer.release());
}
void HTMLDocumentParser::didFailSpeculation(PassOwnPtr<HTMLToken> token, PassOwnPtr<HTMLTokenizer> tokenizer)
@@ -372,6 +386,8 @@
// ASSERT that this object is both attached to the Document and protected.
ASSERT(refCount() >= 2);
ASSERT(shouldUseThreading());
+ ASSERT(!m_tokenizer);
+ ASSERT(!m_token);
ActiveParserSession session(contextForParsingSession());
@@ -385,6 +401,7 @@
if (XSSInfo* xssInfo = it->xssInfo())
m_xssAuditorDelegate.didBlockScript(*xssInfo);
+
constructTreeFromCompactHTMLToken(*it);
if (isStopped())
@@ -395,25 +412,30 @@
// To match main-thread parser behavior (which never checks locationChangePending on the EOF path)
// we peek to see if this chunk has an EOF and process it anyway.
- if (tokens->last().type() == HTMLToken::EndOfFile)
+ if (tokens->last().type() == HTMLToken::EndOfFile) {
+ ASSERT(m_speculations.isEmpty());
prepareToStopParsing();
+ }
break;
}
if (isWaitingForScripts()) {
ASSERT(it + 1 == tokens->end()); // The </script> is assumed to be the last token of this bunch.
runScriptsForPausedTreeBuilder();
+ validateSpeculations();
break;
}
if (it->type() == HTMLToken::EndOfFile) {
ASSERT(it + 1 == tokens->end()); // The EOF is assumed to be the last token of this bunch.
+ ASSERT(m_speculations.isEmpty());
prepareToStopParsing();
break;
}
+
+ ASSERT(!m_tokenizer);
+ ASSERT(!m_token);
}
-
- checkForSpeculationFailure();
}
void HTMLDocumentParser::pumpPendingSpeculations()
@@ -423,6 +445,10 @@
// ASSERT that this object is both attached to the Document and protected.
ASSERT(refCount() >= 2);
+ // If this assert fails, you need to call validateSpeculations to make sure
+ // m_tokenizer and m_token don't have state that invalidates m_speculations.
+ ASSERT(!m_tokenizer);
+ ASSERT(!m_token);
// FIXME: Pass in current input length.
InspectorInstrumentationCookie cookie = InspectorInstrumentation::willWriteHTML(document(), 0, lineNumber().zeroBasedInt());
@@ -846,8 +872,7 @@
#if ENABLE(THREADED_HTML_PARSER)
if (m_haveBackgroundParser) {
- checkForSpeculationFailure();
-
+ validateSpeculations();
// processParsedChunkFromBackgroundParser can cause this parser to be detached from the Document,
// but we need to ensure it isn't deleted yet.
RefPtr<HTMLDocumentParser> protect(this);
Modified: trunk/Source/WebCore/html/parser/HTMLDocumentParser.h (144713 => 144714)
--- trunk/Source/WebCore/html/parser/HTMLDocumentParser.h 2013-03-05 04:37:32 UTC (rev 144713)
+++ trunk/Source/WebCore/html/parser/HTMLDocumentParser.h 2013-03-05 04:57:15 UTC (rev 144714)
@@ -140,7 +140,7 @@
#if ENABLE(THREADED_HTML_PARSER)
void startBackgroundParser();
void stopBackgroundParser();
- void checkForSpeculationFailure();
+ void validateSpeculations();
void didFailSpeculation(PassOwnPtr<HTMLToken>, PassOwnPtr<HTMLTokenizer>);
void processParsedChunkFromBackgroundParser(PassOwnPtr<ParsedChunk>);
void pumpPendingSpeculations();
