Title: [140055] trunk/Source/WebCore
- Revision
- 140055
- Author
- e...@webkit.org
- Date
- 2013-01-17 15:06:00 -0800 (Thu, 17 Jan 2013)
Log Message
Threaded parser hangs when encountering an unmatched </script> tag
https://bugs.webkit.org/show_bug.cgi?id=107170
Reviewed by Adam Barth.
The bug was that the BackgroundHTMLParser naively yields to the
main thread every time it encounters a </script>
(as we may have to run script on the main thread). However, not every
</script> results in script execution, so the main thread needs to know
how to tell the BackgroundHTMLParser to continue in cases where no
script execution is needed.
This whole infrastructure will be replaced when we let the BackgroundHTMLParser
continue speculatively tokenizing after yielding.
* html/parser/BackgroundHTMLParser.cpp:
(WebCore::TokenDelivery::TokenDelivery):
(TokenDelivery):
(WebCore::TokenDelivery::execute):
(WebCore::BackgroundHTMLParser::sendTokensToMainThread):
* html/parser/HTMLDocumentParser.cpp:
(WebCore::HTMLDocumentParser::didReceiveTokensFromBackgroundParser):
* html/parser/HTMLDocumentParser.h:
(HTMLDocumentParser):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (140054 => 140055)
--- trunk/Source/WebCore/ChangeLog 2013-01-17 23:02:50 UTC (rev 140054)
+++ trunk/Source/WebCore/ChangeLog 2013-01-17 23:06:00 UTC (rev 140055)
@@ -1,5 +1,32 @@
2013-01-17 Eric Seidel <e...@webkit.org>
+ Threaded parser hangs when encountering an unmatched </script> tag
+ https://bugs.webkit.org/show_bug.cgi?id=107170
+
+ Reviewed by Adam Barth.
+
+ The bug was that the BackgroundHTMLParser naively yields to the
+ main thread every time it encounters a </script>
+ (as we may have to run script on the main thread). However, not every
+ </script> results in script execution, so the main thread needs to know
+ how to tell the BackgroundHTMLParser to continue in cases where no
+ script execution is needed.
+
+ This whole infrastructure will be replaced when we let the BackgroundHTMLParser
+ continue speculatively tokenizing after yielding.
+
+ * html/parser/BackgroundHTMLParser.cpp:
+ (WebCore::TokenDelivery::TokenDelivery):
+ (TokenDelivery):
+ (WebCore::TokenDelivery::execute):
+ (WebCore::BackgroundHTMLParser::sendTokensToMainThread):
+ * html/parser/HTMLDocumentParser.cpp:
+ (WebCore::HTMLDocumentParser::didReceiveTokensFromBackgroundParser):
+ * html/parser/HTMLDocumentParser.h:
+ (HTMLDocumentParser):
+
+2013-01-17 Eric Seidel <e...@webkit.org>
+
Stop the background parser when canceling parsing to avoid crashing on many layout tests
https://bugs.webkit.org/show_bug.cgi?id=107159
Modified: trunk/Source/WebCore/html/parser/BackgroundHTMLParser.cpp (140054 => 140055)
--- trunk/Source/WebCore/html/parser/BackgroundHTMLParser.cpp 2013-01-17 23:02:50 UTC (rev 140054)
+++ trunk/Source/WebCore/html/parser/BackgroundHTMLParser.cpp 2013-01-17 23:06:00 UTC (rev 140055)
@@ -169,17 +169,27 @@
class TokenDelivery {
WTF_MAKE_NONCOPYABLE(TokenDelivery);
public:
- TokenDelivery() { }
+ 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;
static void execute(void* context)
{
TokenDelivery* delivery = static_cast<TokenDelivery*>(context);
HTMLDocumentParser* parser = parserMap().mainThreadParsers().get(delivery->identifier);
if (parser)
- parser->didReceiveTokensFromBackgroundParser(delivery->tokens);
+ parser->didReceiveTokensFromBackgroundParser(delivery->tokens, delivery->isPausedWaitingForScripts);
// 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.
@@ -192,6 +202,7 @@
TokenDelivery* delivery = new TokenDelivery;
delivery->identifier = m_parserIdentifer;
delivery->tokens.swap(m_pendingTokens);
+ delivery->isPausedWaitingForScripts = m_isPausedWaitingForScripts;
callOnMainThread(TokenDelivery::execute, delivery);
}
Modified: trunk/Source/WebCore/html/parser/HTMLDocumentParser.cpp (140054 => 140055)
--- trunk/Source/WebCore/html/parser/HTMLDocumentParser.cpp 2013-01-17 23:02:50 UTC (rev 140054)
+++ trunk/Source/WebCore/html/parser/HTMLDocumentParser.cpp 2013-01-17 23:06:00 UTC (rev 140055)
@@ -259,11 +259,11 @@
#if ENABLE(THREADED_HTML_PARSER)
-void HTMLDocumentParser::didReceiveTokensFromBackgroundParser(const Vector<CompactHTMLToken>& tokens)
+void HTMLDocumentParser::didReceiveTokensFromBackgroundParser(const Vector<CompactHTMLToken>& tokens, bool threadIsWaitingForScripts)
{
ASSERT(shouldUseThreading());
- // feedTokens can cause this parser to be detached from the Document,
+ // didReceiveTokensFromBackgroundParser can cause this parser to be detached from the Document,
// but we need to ensure it isn't deleted yet.
RefPtr<HTMLDocumentParser> protect(this);
@@ -282,11 +282,14 @@
// as we do in canTakeNextToken;
if (isWaitingForScripts()) {
+ ASSERT(threadIsWaitingForScripts); // We expect that the thread is waiting for us.
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;
}
// FIXME: This is too abrupt a way to end parsing because we might
@@ -294,11 +297,20 @@
// 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.
DocumentParser::prepareToStopParsing();
document()->setReadyState(Document::Interactive);
end();
+ 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)
Modified: trunk/Source/WebCore/html/parser/HTMLDocumentParser.h (140054 => 140055)
--- trunk/Source/WebCore/html/parser/HTMLDocumentParser.h 2013-01-17 23:02:50 UTC (rev 140054)
+++ trunk/Source/WebCore/html/parser/HTMLDocumentParser.h 2013-01-17 23:06:00 UTC (rev 140055)
@@ -79,7 +79,7 @@
virtual void resumeScheduledTasks();
#if ENABLE(THREADED_HTML_PARSER)
- void didReceiveTokensFromBackgroundParser(const Vector<CompactHTMLToken>&);
+ void didReceiveTokensFromBackgroundParser(const Vector<CompactHTMLToken>&, bool threadIsWaitingForScripts);
#endif
protected:
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-changes
