Changeset 140055 in webkit


Ignore:
Timestamp:
Jan 17, 2013 3:06:00 PM (11 years ago)
Author:
eric@webkit.org
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):

Location:
trunk/Source/WebCore
Files:
4 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/WebCore/ChangeLog

    r140050 r140055  
     12013-01-17  Eric Seidel  <eric@webkit.org>
     2
     3        Threaded parser hangs when encountering an unmatched </script> tag
     4        https://bugs.webkit.org/show_bug.cgi?id=107170
     5
     6        Reviewed by Adam Barth.
     7
     8        The bug was that the BackgroundHTMLParser naively yields to the
     9        main thread every time it encounters a </script>
     10        (as we may have to run script on the main thread).  However, not every
     11        </script> results in script execution, so the main thread needs to know
     12        how to tell the BackgroundHTMLParser to continue in cases where no
     13        script execution is needed.
     14
     15        This whole infrastructure will be replaced when we let the BackgroundHTMLParser
     16        continue speculatively tokenizing after yielding.
     17
     18        * html/parser/BackgroundHTMLParser.cpp:
     19        (WebCore::TokenDelivery::TokenDelivery):
     20        (TokenDelivery):
     21        (WebCore::TokenDelivery::execute):
     22        (WebCore::BackgroundHTMLParser::sendTokensToMainThread):
     23        * html/parser/HTMLDocumentParser.cpp:
     24        (WebCore::HTMLDocumentParser::didReceiveTokensFromBackgroundParser):
     25        * html/parser/HTMLDocumentParser.h:
     26        (HTMLDocumentParser):
     27
    1282013-01-17  Eric Seidel  <eric@webkit.org>
    229
  • trunk/Source/WebCore/html/parser/BackgroundHTMLParser.cpp

    r139954 r140055  
    170170    WTF_MAKE_NONCOPYABLE(TokenDelivery);
    171171public:
    172     TokenDelivery() { }
     172    TokenDelivery()
     173        : identifier(0)
     174        , isPausedWaitingForScripts(false)
     175    {
     176    }
    173177
    174178    ParserIdentifier identifier;
    175179    Vector<CompactHTMLToken> tokens;
     180    // FIXME: This bool will be replaced by a CheckPoint object once
     181    // we implement speculative parsing. Then the main thread will decide
     182    // to either accept the speculative tokens we've already given it
     183    // (or ask for them, depending on who ends up owning them), or send
     184    // us a "reset to checkpoint message".
     185    bool isPausedWaitingForScripts;
    176186
    177187    static void execute(void* context)
     
    180190        HTMLDocumentParser* parser = parserMap().mainThreadParsers().get(delivery->identifier);
    181191        if (parser)
    182             parser->didReceiveTokensFromBackgroundParser(delivery->tokens);
     192            parser->didReceiveTokensFromBackgroundParser(delivery->tokens, delivery->isPausedWaitingForScripts);
    183193        // FIXME: Ideally we wouldn't need to call delete manually. Instead
    184194        // we would like an API where the message queue owns the tasks and
     
    193203    delivery->identifier = m_parserIdentifer;
    194204    delivery->tokens.swap(m_pendingTokens);
     205    delivery->isPausedWaitingForScripts = m_isPausedWaitingForScripts;
    195206    callOnMainThread(TokenDelivery::execute, delivery);
    196207}
  • trunk/Source/WebCore/html/parser/HTMLDocumentParser.cpp

    r140050 r140055  
    260260#if ENABLE(THREADED_HTML_PARSER)
    261261
    262 void HTMLDocumentParser::didReceiveTokensFromBackgroundParser(const Vector<CompactHTMLToken>& tokens)
     262void HTMLDocumentParser::didReceiveTokensFromBackgroundParser(const Vector<CompactHTMLToken>& tokens, bool threadIsWaitingForScripts)
    263263{
    264264    ASSERT(shouldUseThreading());
    265265
    266     // feedTokens can cause this parser to be detached from the Document,
     266    // didReceiveTokensFromBackgroundParser can cause this parser to be detached from the Document,
    267267    // but we need to ensure it isn't deleted yet.
    268268    RefPtr<HTMLDocumentParser> protect(this);
     
    283283
    284284        if (isWaitingForScripts()) {
     285            ASSERT(threadIsWaitingForScripts); // We expect that the thread is waiting for us.
    285286            runScriptsForPausedTreeBuilder();
    286287            if (!isWaitingForScripts()) {
     
    288289                HTMLParserThread::shared()->postTask(bind(&BackgroundHTMLParser::continuePartial, identifier));
    289290            }
     291            ASSERT(it + 1 == tokens.end()); // The </script> is assumed to be the last token of this bunch.
     292            return;
    290293        }
    291294
     
    295298        // attemptToEnd() instead.
    296299        if (it->type() == HTMLTokenTypes::EndOfFile) {
     300            ASSERT(it + 1 == tokens.end()); // The EOF is assumed to be the last token of this bunch.
    297301            DocumentParser::prepareToStopParsing();
    298302            document()->setReadyState(Document::Interactive);
    299303            end();
     304            return;
    300305        }
     306    }
     307
     308    // If we got here and the parser thread is still waiting for scripts, then it paused unnecessarily
     309    // (as can happen with a stray </script> tag), and we need to tell it to continue.
     310    if (threadIsWaitingForScripts) {
     311        ParserIdentifier identifier = ParserMap::identifierForParser(this);
     312        HTMLParserThread::shared()->postTask(bind(&BackgroundHTMLParser::continuePartial, identifier));
    301313    }
    302314}
  • trunk/Source/WebCore/html/parser/HTMLDocumentParser.h

    r140036 r140055  
    8080
    8181#if ENABLE(THREADED_HTML_PARSER)
    82     void didReceiveTokensFromBackgroundParser(const Vector<CompactHTMLToken>&);
     82    void didReceiveTokensFromBackgroundParser(const Vector<CompactHTMLToken>&, bool threadIsWaitingForScripts);
    8383#endif
    8484
Note: See TracChangeset for help on using the changeset viewer.