- Revision
- 140478
- Author
- aba...@webkit.org
- Date
- 2013-01-22 15:14:56 -0800 (Tue, 22 Jan 2013)
Log Message
The BackgroundHTMLParser shouldn't pause when waiting for scripts
https://bugs.webkit.org/show_bug.cgi?id=107584
Reviewed by Eric Seidel.
Previously, the BackgroundHTMLParser would pause itself when it
encountered a scrip tag and wait for a signal from the main thread to
continue. After this patch, the BackgroundHTMLParser continues ahead
and the main thread keeps a queue of pending tokens.
This patch brings us closer to speculative parsing because when the
BackgroundHTMLParser is continuing ahead, it is speculating that it is
in the correct state. A future patch will let us abort incorret
speculations and resume from an eariler point in the input stream.
* html/parser/BackgroundHTMLParser.cpp:
(WebCore::checkThatTokensAreSafeToSendToAnotherThread):
(WebCore::BackgroundHTMLParser::BackgroundHTMLParser):
(WebCore::BackgroundHTMLParser::simulateTreeBuilder):
(WebCore::BackgroundHTMLParser::pumpTokenizer):
(WebCore::TokenDelivery::TokenDelivery):
(TokenDelivery):
(WebCore::TokenDelivery::execute):
(WebCore::BackgroundHTMLParser::sendTokensToMainThread):
* html/parser/BackgroundHTMLParser.h:
(BackgroundHTMLParser):
* html/parser/CompactHTMLToken.h:
(WebCore):
* html/parser/HTMLDocumentParser.cpp:
(WebCore::HTMLDocumentParser::didReceiveTokensFromBackgroundParser):
(WebCore):
(WebCore::HTMLDocumentParser::processTokensFromBackgroundParser):
(WebCore::HTMLDocumentParser::resumeParsingAfterScriptExecution):
* html/parser/HTMLDocumentParser.h:
(HTMLDocumentParser):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (140477 => 140478)
--- trunk/Source/WebCore/ChangeLog 2013-01-22 23:13:58 UTC (rev 140477)
+++ trunk/Source/WebCore/ChangeLog 2013-01-22 23:14:56 UTC (rev 140478)
@@ -1,3 +1,41 @@
+2013-01-22 Adam Barth <aba...@webkit.org>
+
+ The BackgroundHTMLParser shouldn't pause when waiting for scripts
+ https://bugs.webkit.org/show_bug.cgi?id=107584
+
+ Reviewed by Eric Seidel.
+
+ Previously, the BackgroundHTMLParser would pause itself when it
+ encountered a scrip tag and wait for a signal from the main thread to
+ continue. After this patch, the BackgroundHTMLParser continues ahead
+ and the main thread keeps a queue of pending tokens.
+
+ This patch brings us closer to speculative parsing because when the
+ BackgroundHTMLParser is continuing ahead, it is speculating that it is
+ in the correct state. A future patch will let us abort incorret
+ speculations and resume from an eariler point in the input stream.
+
+ * html/parser/BackgroundHTMLParser.cpp:
+ (WebCore::checkThatTokensAreSafeToSendToAnotherThread):
+ (WebCore::BackgroundHTMLParser::BackgroundHTMLParser):
+ (WebCore::BackgroundHTMLParser::simulateTreeBuilder):
+ (WebCore::BackgroundHTMLParser::pumpTokenizer):
+ (WebCore::TokenDelivery::TokenDelivery):
+ (TokenDelivery):
+ (WebCore::TokenDelivery::execute):
+ (WebCore::BackgroundHTMLParser::sendTokensToMainThread):
+ * html/parser/BackgroundHTMLParser.h:
+ (BackgroundHTMLParser):
+ * html/parser/CompactHTMLToken.h:
+ (WebCore):
+ * html/parser/HTMLDocumentParser.cpp:
+ (WebCore::HTMLDocumentParser::didReceiveTokensFromBackgroundParser):
+ (WebCore):
+ (WebCore::HTMLDocumentParser::processTokensFromBackgroundParser):
+ (WebCore::HTMLDocumentParser::resumeParsingAfterScriptExecution):
+ * html/parser/HTMLDocumentParser.h:
+ (HTMLDocumentParser):
+
2013-01-22 Simon Fraser <simon.fra...@apple.com>
Fix scrollperf logging
Modified: trunk/Source/WebCore/html/parser/BackgroundHTMLParser.cpp (140477 => 140478)
--- trunk/Source/WebCore/html/parser/BackgroundHTMLParser.cpp 2013-01-22 23:13:58 UTC (rev 140477)
+++ trunk/Source/WebCore/html/parser/BackgroundHTMLParser.cpp 2013-01-22 23:14:56 UTC (rev 140478)
@@ -45,10 +45,10 @@
#ifndef NDEBUG
-static void checkThatTokensAreSafeToSendToAnotherThread(const Vector<CompactHTMLToken>& tokens)
+static void checkThatTokensAreSafeToSendToAnotherThread(const CompactHTMLTokenStream* tokens)
{
- for (size_t i = 0; i < tokens.size(); ++i)
- ASSERT(tokens[i].isSafeToSendToAnotherThread());
+ for (size_t i = 0; i < tokens->size(); ++i)
+ ASSERT(tokens->at(i).isSafeToSendToAnotherThread());
}
#endif
@@ -92,11 +92,11 @@
}
BackgroundHTMLParser::BackgroundHTMLParser(const HTMLParserOptions& options, ParserIdentifier identifier)
- : m_isPausedWaitingForScripts(false)
- , m_inForeignContent(false)
+ : m_inForeignContent(false)
, m_tokenizer(HTMLTokenizer::create(options))
, m_options(options)
, m_parserIdentifer(identifier)
+ , m_pendingTokens(adoptPtr(new CompactHTMLTokenStream))
{
}
@@ -106,12 +106,6 @@
pumpTokenizer();
}
-void BackgroundHTMLParser::continueParsing()
-{
- m_isPausedWaitingForScripts = false;
- pumpTokenizer();
-}
-
void BackgroundHTMLParser::finish()
{
markEndOfFile();
@@ -129,7 +123,7 @@
m_input.close();
}
-void BackgroundHTMLParser::simulateTreeBuilder(const CompactHTMLToken& token)
+bool BackgroundHTMLParser::simulateTreeBuilder(const CompactHTMLToken& token)
{
if (token.type() == HTMLTokenTypes::StartTag) {
const String& tagName = token.data();
@@ -158,28 +152,21 @@
if (threadSafeMatch(tagName, SVGNames::svgTag) || threadSafeMatch(tagName, MathMLNames::mathTag))
m_inForeignContent = false;
if (threadSafeMatch(tagName, scriptTag))
- m_isPausedWaitingForScripts = true;
+ return false;
}
// FIXME: Need to set setForceNullCharacterReplacement based on m_inForeignContent as well.
m_tokenizer->setShouldAllowCDATA(m_inForeignContent);
+ return true;
}
void BackgroundHTMLParser::pumpTokenizer()
{
- if (m_isPausedWaitingForScripts)
- return;
-
while (m_tokenizer->nextToken(m_input, m_token)) {
- m_pendingTokens.append(CompactHTMLToken(m_token, TextPosition(m_input.currentLine(), m_input.currentColumn())));
+ m_pendingTokens->append(CompactHTMLToken(m_token, TextPosition(m_input.currentLine(), m_input.currentColumn())));
m_token.clear();
- simulateTreeBuilder(m_pendingTokens.last());
-
- if (m_isPausedWaitingForScripts)
- break;
-
- if (m_pendingTokens.size() >= pendingTokenLimit)
+ if (!simulateTreeBuilder(m_pendingTokens->last()) || m_pendingTokens->size() >= pendingTokenLimit)
sendTokensToMainThread();
}
@@ -191,25 +178,17 @@
public:
TokenDelivery()
: identifier(0)
- , isPausedWaitingForScripts(false)
{
}
ParserIdentifier identifier;
- Vector<CompactHTMLToken> tokens;
- // FIXME: This bool will be replaced by a CheckPoint object once
- // we implement speculative parsing. Then the main thread will decide
- // to either accept the speculative tokens we've already given it
- // (or ask for them, depending on who ends up owning them), or send
- // us a "reset to checkpoint message".
- bool isPausedWaitingForScripts;
-
+ OwnPtr<CompactHTMLTokenStream> tokens;
static void execute(void* context)
{
TokenDelivery* delivery = static_cast<TokenDelivery*>(context);
HTMLDocumentParser* parser = parserMap().mainThreadParsers().get(delivery->identifier);
if (parser)
- parser->didReceiveTokensFromBackgroundParser(delivery->tokens, delivery->isPausedWaitingForScripts);
+ parser->didReceiveTokensFromBackgroundParser(delivery->tokens.release());
// FIXME: Ideally we wouldn't need to call delete manually. Instead
// we would like an API where the message queue owns the tasks and
// takes care of deleting them.
@@ -219,20 +198,19 @@
void BackgroundHTMLParser::sendTokensToMainThread()
{
- if (m_pendingTokens.isEmpty()) {
- ASSERT(!m_isPausedWaitingForScripts);
+ if (m_pendingTokens->isEmpty())
return;
- }
#ifndef NDEBUG
- checkThatTokensAreSafeToSendToAnotherThread(m_pendingTokens);
+ checkThatTokensAreSafeToSendToAnotherThread(m_pendingTokens.get());
#endif
TokenDelivery* delivery = new TokenDelivery;
delivery->identifier = m_parserIdentifer;
- delivery->tokens.swap(m_pendingTokens);
- delivery->isPausedWaitingForScripts = m_isPausedWaitingForScripts;
+ delivery->tokens = m_pendingTokens.release();
callOnMainThread(TokenDelivery::execute, delivery);
+
+ m_pendingTokens = adoptPtr(new CompactHTMLTokenStream);
}
void BackgroundHTMLParser::createPartial(ParserIdentifier identifier, HTMLParserOptions options)
@@ -253,12 +231,6 @@
parser->append(input);
}
-void BackgroundHTMLParser::continuePartial(ParserIdentifier identifier)
-{
- if (BackgroundHTMLParser* parser = parserMap().backgroundParsers().get(identifier))
- parser->continueParsing();
-}
-
void BackgroundHTMLParser::finishPartial(ParserIdentifier identifier)
{
if (BackgroundHTMLParser* parser = parserMap().backgroundParsers().get(identifier))
Modified: trunk/Source/WebCore/html/parser/BackgroundHTMLParser.h (140477 => 140478)
--- trunk/Source/WebCore/html/parser/BackgroundHTMLParser.h 2013-01-22 23:13:58 UTC (rev 140477)
+++ trunk/Source/WebCore/html/parser/BackgroundHTMLParser.h 2013-01-22 23:14:56 UTC (rev 140478)
@@ -42,7 +42,6 @@
WTF_MAKE_FAST_ALLOCATED;
public:
void append(const String&);
- void continueParsing();
void finish();
static PassOwnPtr<BackgroundHTMLParser> create(const HTMLParserOptions& options, ParserIdentifier identifier)
@@ -53,7 +52,6 @@
static void createPartial(ParserIdentifier, HTMLParserOptions);
static void stopPartial(ParserIdentifier);
static void appendPartial(ParserIdentifier, const String& input);
- static void continuePartial(ParserIdentifier);
static void finishPartial(ParserIdentifier);
private:
@@ -61,18 +59,17 @@
void markEndOfFile();
void pumpTokenizer();
- void simulateTreeBuilder(const CompactHTMLToken&);
+ bool simulateTreeBuilder(const CompactHTMLToken&);
void sendTokensToMainThread();
SegmentedString m_input;
HTMLToken m_token;
- bool m_isPausedWaitingForScripts;
bool m_inForeignContent; // FIXME: We need a stack of foreign content markers.
OwnPtr<HTMLTokenizer> m_tokenizer;
HTMLParserOptions m_options;
ParserIdentifier m_parserIdentifer;
- Vector<CompactHTMLToken> m_pendingTokens;
+ OwnPtr<CompactHTMLTokenStream> m_pendingTokens;
};
class ParserMap {
Modified: trunk/Source/WebCore/html/parser/CompactHTMLToken.h (140477 => 140478)
--- trunk/Source/WebCore/html/parser/CompactHTMLToken.h 2013-01-22 23:13:58 UTC (rev 140477)
+++ trunk/Source/WebCore/html/parser/CompactHTMLToken.h 2013-01-22 23:14:56 UTC (rev 140478)
@@ -81,6 +81,8 @@
TextPosition m_textPosition;
};
+typedef Vector<CompactHTMLToken> CompactHTMLTokenStream;
+
}
#endif // ENABLE(THREADED_HTML_PARSER)
Modified: trunk/Source/WebCore/html/parser/HTMLDocumentParser.cpp (140477 => 140478)
--- trunk/Source/WebCore/html/parser/HTMLDocumentParser.cpp 2013-01-22 23:13:58 UTC (rev 140477)
+++ trunk/Source/WebCore/html/parser/HTMLDocumentParser.cpp 2013-01-22 23:14:56 UTC (rev 140478)
@@ -262,8 +262,18 @@
#if ENABLE(THREADED_HTML_PARSER)
-void HTMLDocumentParser::didReceiveTokensFromBackgroundParser(const Vector<CompactHTMLToken>& tokens, bool threadIsWaitingForScripts)
+void HTMLDocumentParser::didReceiveTokensFromBackgroundParser(PassOwnPtr<CompactHTMLTokenStream> tokens)
{
+ if (isWaitingForScripts()) {
+ m_pendingTokens.append(tokens);
+ return;
+ }
+ ASSERT(m_pendingTokens.isEmpty());
+ processTokensFromBackgroundParser(tokens);
+}
+
+void HTMLDocumentParser::processTokensFromBackgroundParser(PassOwnPtr<CompactHTMLTokenStream> tokens)
+{
ASSERT(shouldUseThreading());
// didReceiveTokensFromBackgroundParser can cause this parser to be detached from the Document,
@@ -272,7 +282,7 @@
// FIXME: Add support for InspectorInstrumentation.
- for (Vector<CompactHTMLToken>::const_iterator it = tokens.begin(); it != tokens.end(); ++it) {
+ for (Vector<CompactHTMLToken>::const_iterator it = tokens->begin(); it != tokens->end(); ++it) {
ASSERT(!isWaitingForScripts());
// FIXME: Call m_xssAuditor.filterToken(*it).
@@ -286,13 +296,8 @@
// as we do in canTakeNextToken;
if (isWaitingForScripts()) {
- ASSERT(threadIsWaitingForScripts); // We expect that the thread is waiting for us.
+ ASSERT(it + 1 == tokens->end()); // The </script> is assumed to be the last token of this bunch.
runScriptsForPausedTreeBuilder();
- if (!isWaitingForScripts()) {
- ParserIdentifier identifier = ParserMap::identifierForParser(this);
- HTMLParserThread::shared()->postTask(bind(&BackgroundHTMLParser::continuePartial, identifier));
- }
- ASSERT(it + 1 == tokens.end()); // The </script> is assumed to be the last token of this bunch.
return;
}
@@ -301,18 +306,11 @@
// attemptToRunDeferredScriptsAndEnd(), prepareToStopParsing(), or
// attemptToEnd() instead.
if (it->type() == HTMLTokenTypes::EndOfFile) {
- ASSERT(it + 1 == tokens.end()); // The EOF is assumed to be the last token of this bunch.
+ ASSERT(it + 1 == tokens->end()); // The EOF is assumed to be the last token of this bunch.
prepareToStopParsing();
return;
}
}
-
- // If we got here and the parser thread is still waiting for scripts, then it paused unnecessarily
- // (as can happen with a stray </script> tag), and we need to tell it to continue.
- if (threadIsWaitingForScripts) {
- ParserIdentifier identifier = ParserMap::identifierForParser(this);
- HTMLParserThread::shared()->postTask(bind(&BackgroundHTMLParser::continuePartial, identifier));
- }
}
#endif // ENABLE(THREADED_HTML_PARSER)
@@ -659,8 +657,11 @@
#if ENABLE(THREADED_HTML_PARSER)
if (shouldUseThreading()) {
- ParserIdentifier identifier = ParserMap::identifierForParser(this);
- HTMLParserThread::shared()->postTask(bind(&BackgroundHTMLParser::continuePartial, identifier));
+ while (!m_pendingTokens.isEmpty()) {
+ processTokensFromBackgroundParser(m_pendingTokens.takeFirst());
+ if (isWaitingForScripts())
+ return;
+ }
return;
}
#endif
Modified: trunk/Source/WebCore/html/parser/HTMLDocumentParser.h (140477 => 140478)
--- trunk/Source/WebCore/html/parser/HTMLDocumentParser.h 2013-01-22 23:13:58 UTC (rev 140477)
+++ trunk/Source/WebCore/html/parser/HTMLDocumentParser.h 2013-01-22 23:14:56 UTC (rev 140478)
@@ -37,6 +37,7 @@
#include "SegmentedString.h"
#include "Timer.h"
#include "XSSAuditor.h"
+#include <wtf/Deque.h>
#include <wtf/OwnPtr.h>
#include <wtf/text/TextPosition.h>
@@ -80,7 +81,7 @@
virtual void resumeScheduledTasks();
#if ENABLE(THREADED_HTML_PARSER)
- void didReceiveTokensFromBackgroundParser(const Vector<CompactHTMLToken>&, bool threadIsWaitingForScripts);
+ void didReceiveTokensFromBackgroundParser(PassOwnPtr<CompactHTMLTokenStream>);
#endif
protected:
@@ -122,6 +123,7 @@
#if ENABLE(THREADED_HTML_PARSER)
void startBackgroundParser();
void stopBackgroundParser();
+ void processTokensFromBackgroundParser(PassOwnPtr<CompactHTMLTokenStream>);
#endif
enum SynchronousMode {
@@ -167,6 +169,10 @@
TextPosition m_textPosition;
XSSAuditor m_xssAuditor;
+#if ENABLE(THREADED_HTML_PARSER)
+ Deque<OwnPtr<CompactHTMLTokenStream> > m_pendingTokens;
+#endif
+
bool m_endWasDelayed;
bool m_haveBackgroundParser;
unsigned m_pumpSessionNestingLevel;