Changeset 140478 in webkit


Ignore:
Timestamp:
Jan 22, 2013, 3:14:56 PM (12 years ago)
Author:
abarth@webkit.org
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):

Location:
trunk/Source/WebCore
Files:
6 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r140475 r140478  
     12013-01-22  Adam Barth  <abarth@webkit.org>
     2
     3        The BackgroundHTMLParser shouldn't pause when waiting for scripts
     4        https://bugs.webkit.org/show_bug.cgi?id=107584
     5
     6        Reviewed by Eric Seidel.
     7
     8        Previously, the BackgroundHTMLParser would pause itself when it
     9        encountered a scrip tag and wait for a signal from the main thread to
     10        continue. After this patch, the BackgroundHTMLParser continues ahead
     11        and the main thread keeps a queue of pending tokens.
     12
     13        This patch brings us closer to speculative parsing because when the
     14        BackgroundHTMLParser is continuing ahead, it is speculating that it is
     15        in the correct state. A future patch will let us abort incorret
     16        speculations and resume from an eariler point in the input stream.
     17
     18        * html/parser/BackgroundHTMLParser.cpp:
     19        (WebCore::checkThatTokensAreSafeToSendToAnotherThread):
     20        (WebCore::BackgroundHTMLParser::BackgroundHTMLParser):
     21        (WebCore::BackgroundHTMLParser::simulateTreeBuilder):
     22        (WebCore::BackgroundHTMLParser::pumpTokenizer):
     23        (WebCore::TokenDelivery::TokenDelivery):
     24        (TokenDelivery):
     25        (WebCore::TokenDelivery::execute):
     26        (WebCore::BackgroundHTMLParser::sendTokensToMainThread):
     27        * html/parser/BackgroundHTMLParser.h:
     28        (BackgroundHTMLParser):
     29        * html/parser/CompactHTMLToken.h:
     30        (WebCore):
     31        * html/parser/HTMLDocumentParser.cpp:
     32        (WebCore::HTMLDocumentParser::didReceiveTokensFromBackgroundParser):
     33        (WebCore):
     34        (WebCore::HTMLDocumentParser::processTokensFromBackgroundParser):
     35        (WebCore::HTMLDocumentParser::resumeParsingAfterScriptExecution):
     36        * html/parser/HTMLDocumentParser.h:
     37        (HTMLDocumentParser):
     38
    1392013-01-22  Simon Fraser  <simon.fraser@apple.com>
    240

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

    r140467 r140478  
    4646#ifndef NDEBUG
    4747
    48 static void checkThatTokensAreSafeToSendToAnotherThread(const Vector<CompactHTMLToken>& tokens)
    49 {
    50     for (size_t i = 0; i < tokens.size(); ++i)
    51         ASSERT(tokens[i].isSafeToSendToAnotherThread());
     48static void checkThatTokensAreSafeToSendToAnotherThread(const CompactHTMLTokenStream* tokens)
     49{
     50    for (size_t i = 0; i < tokens->size(); ++i)
     51        ASSERT(tokens->at(i).isSafeToSendToAnotherThread());
    5252}
    5353
    … …  
    9393
    9494BackgroundHTMLParser::BackgroundHTMLParser(const HTMLParserOptions& options, ParserIdentifier identifier)
    95     : m_isPausedWaitingForScripts(false)
    96     , m_inForeignContent(false)
     95    : m_inForeignContent(false)
    9796    , m_tokenizer(HTMLTokenizer::create(options))
    9897    , m_options(options)
    9998    , m_parserIdentifer(identifier)
     99    , m_pendingTokens(adoptPtr(new CompactHTMLTokenStream))
    100100{
    101101}
    … …  
    104104{
    105105    m_input.append(SegmentedString(input));
    106     pumpTokenizer();
    107 }
    108 
    109 void BackgroundHTMLParser::continueParsing()
    110 {
    111     m_isPausedWaitingForScripts = false;
    112106    pumpTokenizer();
    113107}
    … …  
    130124}
    131125
    132 void BackgroundHTMLParser::simulateTreeBuilder(const CompactHTMLToken& token)
     126bool BackgroundHTMLParser::simulateTreeBuilder(const CompactHTMLToken& token)
    133127{
    134128    if (token.type() == HTMLTokenTypes::StartTag) {
    … …  
    159153            m_inForeignContent = false;
    160154        if (threadSafeMatch(tagName, scriptTag))
    161             m_isPausedWaitingForScripts = true;
     155            return false;
    162156    }
    163157
    164158    // FIXME: Need to set setForceNullCharacterReplacement based on m_inForeignContent as well.
    165159    m_tokenizer->setShouldAllowCDATA(m_inForeignContent);
     160    return true;
    166161}
    167162
    168163void BackgroundHTMLParser::pumpTokenizer()
    169164{
    170     if (m_isPausedWaitingForScripts)
    171         return;
    172 
    173165    while (m_tokenizer->nextToken(m_input, m_token)) {
    174         m_pendingTokens.append(CompactHTMLToken(m_token, TextPosition(m_input.currentLine(), m_input.currentColumn())));
     166        m_pendingTokens->append(CompactHTMLToken(m_token, TextPosition(m_input.currentLine(), m_input.currentColumn())));
    175167        m_token.clear();
    176168
    177         simulateTreeBuilder(m_pendingTokens.last());
    178 
    179         if (m_isPausedWaitingForScripts)
    180             break;
    181 
    182         if (m_pendingTokens.size() >= pendingTokenLimit)
     169        if (!simulateTreeBuilder(m_pendingTokens->last()) || m_pendingTokens->size() >= pendingTokenLimit)
    183170            sendTokensToMainThread();
    184171    }
    … …  
    192179    TokenDelivery()
    193180        : identifier(0)
    194         , isPausedWaitingForScripts(false)
    195181    {
    196182    }
    197183
    198184    ParserIdentifier identifier;
    199     Vector<CompactHTMLToken> tokens;
    200     // FIXME: This bool will be replaced by a CheckPoint object once
    201     // we implement speculative parsing. Then the main thread will decide
    202     // to either accept the speculative tokens we've already given it
    203     // (or ask for them, depending on who ends up owning them), or send
    204     // us a "reset to checkpoint message".
    205     bool isPausedWaitingForScripts;
    206 
     185    OwnPtr<CompactHTMLTokenStream> tokens;
    207186    static void execute(void* context)
    208187    {
    … …  
    210189        HTMLDocumentParser* parser = parserMap().mainThreadParsers().get(delivery->identifier);
    211190        if (parser)
    212             parser->didReceiveTokensFromBackgroundParser(delivery->tokens, delivery->isPausedWaitingForScripts);
     191            parser->didReceiveTokensFromBackgroundParser(delivery->tokens.release());
    213192        // FIXME: Ideally we wouldn't need to call delete manually. Instead
    214193        // we would like an API where the message queue owns the tasks and
    … …  
    220199void BackgroundHTMLParser::sendTokensToMainThread()
    221200{
    222     if (m_pendingTokens.isEmpty()) {
    223         ASSERT(!m_isPausedWaitingForScripts);
     201    if (m_pendingTokens->isEmpty())
    224202        return;
    225     }
    226203
    227204#ifndef NDEBUG
    228     checkThatTokensAreSafeToSendToAnotherThread(m_pendingTokens);
     205    checkThatTokensAreSafeToSendToAnotherThread(m_pendingTokens.get());
    229206#endif
    230207
    231208    TokenDelivery* delivery = new TokenDelivery;
    232209    delivery->identifier = m_parserIdentifer;
    233     delivery->tokens.swap(m_pendingTokens);
    234     delivery->isPausedWaitingForScripts = m_isPausedWaitingForScripts;
     210    delivery->tokens = m_pendingTokens.release();
    235211    callOnMainThread(TokenDelivery::execute, delivery);
     212
     213    m_pendingTokens = adoptPtr(new CompactHTMLTokenStream);
    236214}
    237215
    … …  
    254232}
    255233
    256 void BackgroundHTMLParser::continuePartial(ParserIdentifier identifier)
    257 {
    258     if (BackgroundHTMLParser* parser = parserMap().backgroundParsers().get(identifier))
    259         parser->continueParsing();
    260 }
    261 
    262234void BackgroundHTMLParser::finishPartial(ParserIdentifier identifier)
    263235{

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

    r140446 r140478  
    4343public:
    4444    void append(const String&);
    45     void continueParsing();
    4645    void finish();
    4746
    … …  
    5453    static void stopPartial(ParserIdentifier);
    5554    static void appendPartial(ParserIdentifier, const String& input);
    56     static void continuePartial(ParserIdentifier);
    5755    static void finishPartial(ParserIdentifier);
    5856
    … …  
    6260    void markEndOfFile();
    6361    void pumpTokenizer();
    64     void simulateTreeBuilder(const CompactHTMLToken&);
     62    bool simulateTreeBuilder(const CompactHTMLToken&);
    6563
    6664    void sendTokensToMainThread();
    … …  
    6866    SegmentedString m_input;
    6967    HTMLToken m_token;
    70     bool m_isPausedWaitingForScripts;
    7168    bool m_inForeignContent; // FIXME: We need a stack of foreign content markers.
    7269    OwnPtr<HTMLTokenizer> m_tokenizer;
    7370    HTMLParserOptions m_options;
    7471    ParserIdentifier m_parserIdentifer;
    75     Vector<CompactHTMLToken> m_pendingTokens;
     72    OwnPtr<CompactHTMLTokenStream> m_pendingTokens;
    7673};
    7774

  • trunk/Source/WebCore/html/parser/CompactHTMLToken.h

    r140473 r140478  
    8282};
    8383
     84typedef Vector<CompactHTMLToken> CompactHTMLTokenStream;
     85
    8486}
    8587

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

    r140467 r140478  
    263263#if ENABLE(THREADED_HTML_PARSER)
    264264
    265 void HTMLDocumentParser::didReceiveTokensFromBackgroundParser(const Vector<CompactHTMLToken>& tokens, bool threadIsWaitingForScripts)
     265void HTMLDocumentParser::didReceiveTokensFromBackgroundParser(PassOwnPtr<CompactHTMLTokenStream> tokens)
     266{
     267    if (isWaitingForScripts()) {
     268        m_pendingTokens.append(tokens);
     269        return;
     270    }
     271    ASSERT(m_pendingTokens.isEmpty());
     272    processTokensFromBackgroundParser(tokens);
     273}
     274
     275void HTMLDocumentParser::processTokensFromBackgroundParser(PassOwnPtr<CompactHTMLTokenStream> tokens)
    266276{
    267277    ASSERT(shouldUseThreading());
    … …  
    273283    // FIXME: Add support for InspectorInstrumentation.
    274284
    275     for (Vector<CompactHTMLToken>::const_iterator it = tokens.begin(); it != tokens.end(); ++it) {
     285    for (Vector<CompactHTMLToken>::const_iterator it = tokens->begin(); it != tokens->end(); ++it) {
    276286        ASSERT(!isWaitingForScripts());
    277287
    … …  
    287297
    288298        if (isWaitingForScripts()) {
    289             ASSERT(threadIsWaitingForScripts); // We expect that the thread is waiting for us.
     299            ASSERT(it + 1 == tokens->end()); // The </script> is assumed to be the last token of this bunch.
    290300            runScriptsForPausedTreeBuilder();
    291             if (!isWaitingForScripts()) {
    292                 ParserIdentifier identifier = ParserMap::identifierForParser(this);
    293                 HTMLParserThread::shared()->postTask(bind(&BackgroundHTMLParser::continuePartial, identifier));
    294             }
    295             ASSERT(it + 1 == tokens.end()); // The </script> is assumed to be the last token of this bunch.
    296301            return;
    297302        }
    … …  
    302307        // attemptToEnd() instead.
    303308        if (it->type() == HTMLTokenTypes::EndOfFile) {
    304             ASSERT(it + 1 == tokens.end()); // The EOF is assumed to be the last token of this bunch.
     309            ASSERT(it + 1 == tokens->end()); // The EOF is assumed to be the last token of this bunch.
    305310            prepareToStopParsing();
    306311            return;
    307312        }
    308     }
    309 
    310     // If we got here and the parser thread is still waiting for scripts, then it paused unnecessarily
    311     // (as can happen with a stray </script> tag), and we need to tell it to continue.
    312     if (threadIsWaitingForScripts) {
    313         ParserIdentifier identifier = ParserMap::identifierForParser(this);
    314         HTMLParserThread::shared()->postTask(bind(&BackgroundHTMLParser::continuePartial, identifier));
    315313    }
    316314}
    … …  
    660658#if ENABLE(THREADED_HTML_PARSER)
    661659    if (shouldUseThreading()) {
    662         ParserIdentifier identifier = ParserMap::identifierForParser(this);
    663         HTMLParserThread::shared()->postTask(bind(&BackgroundHTMLParser::continuePartial, identifier));
     660        while (!m_pendingTokens.isEmpty()) {
     661            processTokensFromBackgroundParser(m_pendingTokens.takeFirst());
     662            if (isWaitingForScripts())
     663                return;
     664        }
    664665        return;
    665666    }

  • trunk/Source/WebCore/html/parser/HTMLDocumentParser.h

    r140467 r140478  
    3838#include "Timer.h"
    3939#include "XSSAuditor.h"
     40#include <wtf/Deque.h>
    4041#include <wtf/OwnPtr.h>
    4142#include <wtf/text/TextPosition.h>
    … …  
    8182
    8283#if ENABLE(THREADED_HTML_PARSER)
    83     void didReceiveTokensFromBackgroundParser(const Vector<CompactHTMLToken>&, bool threadIsWaitingForScripts);
     84    void didReceiveTokensFromBackgroundParser(PassOwnPtr<CompactHTMLTokenStream>);
    8485#endif
    8586
    … …  
    123124    void startBackgroundParser();
    124125    void stopBackgroundParser();
     126    void processTokensFromBackgroundParser(PassOwnPtr<CompactHTMLTokenStream>);
    125127#endif
    126128
    … …  
    168170    XSSAuditor m_xssAuditor;
    169171
     172#if ENABLE(THREADED_HTML_PARSER)
     173    Deque<OwnPtr<CompactHTMLTokenStream> > m_pendingTokens;
     174#endif
     175
    170176    bool m_endWasDelayed;
    171177    bool m_haveBackgroundParser;
Note: See TracChangeset for help on using the changeset viewer.