Changeset 61658 in webkit


Ignore:
Timestamp:
Jun 22, 2010 11:18:49 PM (14 years ago)
Author:
eric@webkit.org
Message:

2010-06-22 Eric Seidel <​eric@webkit.org>

Reviewed by Adam Barth.

Split out HTML5DocumentParser yield/resume logic into a separate class
​https://bugs.webkit.org/show_bug.cgi?id=41018

The HTML5DocumentParser is just the coordinator, and shouldn't have
any real parsing logic of his own. Continuing along that path, I'm
moving the when-to-yield/resume logic out into a separate class.

I could have create a new HTMLParserSchedulerHost virtual interface
to allow the HTMLParserScheduler to talk back to the
HTML5DocumentParser, but instead I just exposed the one method it
needs (resumeParsing()) as a public method. Since no code besides
HTMLDocument (and DocumentFrament) ever should know about the
HTML5DocumentParser DocumentParser subclass, no class should ever
see the resumeParsing() method anyway.

Most of this change is just moving code from HTML5DocumentParser
to the new HTMLParserScheduler.

Some of this change is wrapping previous direct access to
m_continueNextChunkTimer.isActive() with isScheduledForResume().

  • Android.mk:
  • CMakeLists.txt:
  • GNUmakefile.am:
  • WebCore.gypi:
  • WebCore.pro:
  • WebCore.xcodeproj/project.pbxproj:
  • html/HTML5DocumentParser.cpp: (WebCore::HTML5DocumentParser::HTML5DocumentParser): (WebCore::HTML5DocumentParser::stopParsing): (WebCore::HTML5DocumentParser::processingData): (WebCore::HTML5DocumentParser::pumpLexerIfPossible): (WebCore::HTML5DocumentParser::isScheduledForResume): (WebCore::HTML5DocumentParser::resumeParsing): (WebCore::HTML5DocumentParser::pumpLexer): (WebCore::HTML5DocumentParser::end): (WebCore::HTML5DocumentParser::attemptToEnd): (WebCore::HTML5DocumentParser::endIfDelayed):
  • html/HTML5DocumentParser.h: (WebCore::HTML5DocumentParser::document):
    • Exposed for HTMLParserScheduler.
  • html/HTMLParserScheduler.cpp: Added. (WebCore::parserTimeLimit): Moved from HTML5DocumentParser. (WebCore::parserChunkSize): ditto. (WebCore::HTMLParserScheduler::HTMLParserScheduler): (WebCore::HTMLParserScheduler::~HTMLParserScheduler): (WebCore::isLayoutTimerActive): (WebCore::HTMLParserScheduler::continueNextChunkTimerFired):
    • Moved from HTML5DocumentParser.
  • html/HTMLParserScheduler.h: Added. (WebCore::HTMLParserScheduler::PumpSession::PumpSession):
    • Moved from HTML5DocumentParser.

(WebCore::HTMLParserScheduler::shouldContinueParsing):
(WebCore::HTMLParserScheduler::isScheduledForResume):

Location:
trunk/WebCore
Files:
2 added
9 edited

Legend:

Unmodified
Added
Removed

  • trunk/WebCore/Android.mk

    r61584 r61658  
    267267        html/LegacyHTMLTreeConstructor.cpp \
    268268        html/HTMLParserErrorCodes.cpp \
     269        html/HTMLParserScheduler.cpp \
    269270        html/HTMLTableRowsCollection.cpp \
    270271        html/HTMLDocumentParser.cpp \

  • trunk/WebCore/CMakeLists.txt

    r61650 r61658  
    975975    html/LegacyHTMLTreeConstructor.cpp
    976976    html/HTMLParserErrorCodes.cpp
     977    html/HTMLParserScheduler.cpp
    977978    html/HTMLPlugInElement.cpp
    978979    html/HTMLPlugInImageElement.cpp

  • trunk/WebCore/ChangeLog

    r61655 r61658  
     12010-06-22  Eric Seidel  <eric@webkit.org>
     2
     3        Reviewed by Adam Barth.
     4
     5        Split out HTML5DocumentParser yield/resume logic into a separate class
     6        https://bugs.webkit.org/show_bug.cgi?id=41018
     7
     8        The HTML5DocumentParser is just the coordinator, and shouldn't have
     9        any real parsing logic of his own.  Continuing along that path, I'm
     10        moving the when-to-yield/resume logic out into a separate class.
     11
     12        I could have create a new HTMLParserSchedulerHost virtual interface
     13        to allow the HTMLParserScheduler to talk back to the
     14        HTML5DocumentParser, but instead I just exposed the one method it
     15        needs (resumeParsing()) as a public method.  Since no code besides
     16        HTMLDocument (and DocumentFrament) ever should know about the
     17        HTML5DocumentParser DocumentParser subclass, no class should ever
     18        see the resumeParsing() method anyway.
     19
     20        Most of this change is just moving code from HTML5DocumentParser
     21        to the new HTMLParserScheduler.
     22
     23        Some of this change is wrapping previous direct access to
     24        m_continueNextChunkTimer.isActive() with isScheduledForResume().
     25
     26        * Android.mk:
     27        * CMakeLists.txt:
     28        * GNUmakefile.am:
     29        * WebCore.gypi:
     30        * WebCore.pro:
     31        * WebCore.xcodeproj/project.pbxproj:
     32        * html/HTML5DocumentParser.cpp:
     33        (WebCore::HTML5DocumentParser::HTML5DocumentParser):
     34        (WebCore::HTML5DocumentParser::stopParsing):
     35        (WebCore::HTML5DocumentParser::processingData):
     36        (WebCore::HTML5DocumentParser::pumpLexerIfPossible):
     37        (WebCore::HTML5DocumentParser::isScheduledForResume):
     38        (WebCore::HTML5DocumentParser::resumeParsing):
     39        (WebCore::HTML5DocumentParser::pumpLexer):
     40        (WebCore::HTML5DocumentParser::end):
     41        (WebCore::HTML5DocumentParser::attemptToEnd):
     42        (WebCore::HTML5DocumentParser::endIfDelayed):
     43        * html/HTML5DocumentParser.h:
     44        (WebCore::HTML5DocumentParser::document):
     45         - Exposed for HTMLParserScheduler.
     46        * html/HTMLParserScheduler.cpp: Added.
     47        (WebCore::parserTimeLimit): Moved from HTML5DocumentParser.
     48        (WebCore::parserChunkSize): ditto.
     49        (WebCore::HTMLParserScheduler::HTMLParserScheduler):
     50        (WebCore::HTMLParserScheduler::~HTMLParserScheduler):
     51        (WebCore::isLayoutTimerActive):
     52        (WebCore::HTMLParserScheduler::continueNextChunkTimerFired):
     53         - Moved from HTML5DocumentParser.
     54        * html/HTMLParserScheduler.h: Added.
     55        (WebCore::HTMLParserScheduler::PumpSession::PumpSession):
     56         - Moved from HTML5DocumentParser.
     57        (WebCore::HTMLParserScheduler::shouldContinueParsing):
     58        (WebCore::HTMLParserScheduler::isScheduledForResume):
     59
    1602010-06-22  Pavel Feldman  <pfeldman@chromium.org>
    261

  • trunk/WebCore/GNUmakefile.am

    r61650 r61658  
    12271227        WebCore/html/HTMLParserErrorCodes.h \
    12281228        WebCore/html/HTMLParserQuirks.h \
     1229        WebCore/html/HTMLParserScheduler.cpp \
     1230        WebCore/html/HTMLParserScheduler.h \
    12291231        WebCore/html/HTMLPlugInElement.cpp \
    12301232        WebCore/html/HTMLPlugInElement.h \

  • trunk/WebCore/WebCore.gypi

    r61650 r61658  
    16131613            'html/HTMLParserErrorCodes.cpp',
    16141614            'html/HTMLParserErrorCodes.h',
     1615            'html/HTMLParserScheduler.cpp',
     1616            'html/HTMLParserScheduler.h',
    16151617            'html/HTMLPlugInElement.cpp',
    16161618            'html/HTMLPlugInElement.h',

  • trunk/WebCore/WebCore.pro

    r61637 r61658  
    680680    html/LegacyHTMLTreeConstructor.cpp \
    681681    html/HTMLParserErrorCodes.cpp \
     682    html/HTMLParserScheduler.cpp \
    682683    html/HTMLPlugInElement.cpp \
    683684    html/HTMLPlugInImageElement.cpp \

  • trunk/WebCore/WebCore.xcodeproj/project.pbxproj

    r61650 r61658  
    32803280                A8E545BF0CA9D1C20097D09B /* DOMSVGTextElement.h in Headers */ = {isa = PBXBuildFile; fileRef = A8E544C10CA9D1C20097D09B /* DOMSVGTextElement.h */; };
    32813281                A8E545C10CA9D1C20097D09B /* DOMSVGDescElementInternal.h in Headers */ = {isa = PBXBuildFile; fileRef = A8E544C30CA9D1C20097D09B /* DOMSVGDescElementInternal.h */; };
     3282                A8E6A78111D1661B00311F4A /* HTMLParserScheduler.h in Headers */ = {isa = PBXBuildFile; fileRef = A8E6A77F11D1661B00311F4A /* HTMLParserScheduler.h */; };
     3283                A8E6A78211D1661B00311F4A /* HTMLParserScheduler.cpp in Sources */ = {isa = PBXBuildFile; fileRef = A8E6A78011D1661B00311F4A /* HTMLParserScheduler.cpp */; };
    32823284                A8EA73C30A1900E300A8EF5F /* RenderFieldset.cpp in Sources */ = {isa = PBXBuildFile; fileRef = A8EA73AF0A1900E300A8EF5F /* RenderFieldset.cpp */; };
    32833285                A8EA73C40A1900E300A8EF5F /* RenderFieldset.h in Headers */ = {isa = PBXBuildFile; fileRef = A8EA73B00A1900E300A8EF5F /* RenderFieldset.h */; };
    … …  
    87488750                A8E544C10CA9D1C20097D09B /* DOMSVGTextElement.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = DOMSVGTextElement.h; sourceTree = "<group>"; };
    87498751                A8E544C30CA9D1C20097D09B /* DOMSVGDescElementInternal.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = DOMSVGDescElementInternal.h; sourceTree = "<group>"; };
     8752                A8E6A77F11D1661B00311F4A /* HTMLParserScheduler.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = HTMLParserScheduler.h; sourceTree = "<group>"; };
     8753                A8E6A78011D1661B00311F4A /* HTMLParserScheduler.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = HTMLParserScheduler.cpp; sourceTree = "<group>"; };
    87508754                A8EA73AF0A1900E300A8EF5F /* RenderFieldset.cpp */ = {isa = PBXFileReference; fileEncoding = 30; lastKnownFileType = sourcecode.cpp.cpp; path = RenderFieldset.cpp; sourceTree = "<group>"; };
    87518755                A8EA73B00A1900E300A8EF5F /* RenderFieldset.h */ = {isa = PBXFileReference; fileEncoding = 30; lastKnownFileType = sourcecode.c.h; path = RenderFieldset.h; sourceTree = "<group>"; };
    … …  
    1342513429                        isa = PBXGroup;
    1342613430                        children = (
    13427                                 9719AEFF11D09F2C00D45831 /* HTMLInputStream.h */,
    13428                                 49484FAE102CF01E00187DD3 /* canvas */,
    1342913431                                B0149E7911A4B21500196A7B /* AsyncImageResizer.cpp */,
    1343013432                                B0149E7A11A4B21500196A7B /* AsyncImageResizer.h */,
    … …  
    1343513437                                89BED5EA11BE11CE00448492 /* BlobBuilder.h */,
    1343613438                                89CD027911C859A80070B791 /* BlobBuilder.idl */,
     13439                                49484FAE102CF01E00187DD3 /* canvas */,
    1343713440                                93C441ED0F813A1A00C1A634 /* CollectionCache.cpp */,
    1343813441                                93C441EE0F813A1A00C1A634 /* CollectionCache.h */,
    … …  
    1361013613                                A81369B0097374F500D74463 /* HTMLInputElement.h */,
    1361113614                                A80E7E170A1A7CCB007FB8C5 /* HTMLInputElement.idl */,
     13615                                9719AEFF11D09F2C00D45831 /* HTMLInputStream.h */,
    1361213616                                A81369AF097374F500D74463 /* HTMLIsIndexElement.cpp */,
    1361313617                                A81369AE097374F500D74463 /* HTMLIsIndexElement.h */,
    … …  
    1367613680                                BC588AEF0BFA6CF900EE679E /* HTMLParserErrorCodes.h */,
    1367713681                                449B19F30FA72ECE0015CA4A /* HTMLParserQuirks.h */,
     13682                                A8E6A78011D1661B00311F4A /* HTMLParserScheduler.cpp */,
     13683                                A8E6A77F11D1661B00311F4A /* HTMLParserScheduler.h */,
    1367813684                                A871D44D0A127CBC00B12A68 /* HTMLPlugInElement.cpp */,
    1367913685                                A871D44C0A127CBC00B12A68 /* HTMLPlugInElement.h */,
    … …  
    1951319519                                9719AF0011D09F2C00D45831 /* HTMLInputStream.h in Headers */,
    1951419520                                A853123D11D0471B00D4D077 /* FragmentScriptingPermission.h in Headers */,
     19521                                A8E6A78111D1661B00311F4A /* HTMLParserScheduler.h in Headers */,
    1951519522                        );
    1951619523                        runOnlyForDeploymentPostprocessing = 0;
    … …  
    2183021837                                1AD8F81C11CAB9E900E93E54 /* PlatformStrategies.cpp in Sources */,
    2183121838                                B525A96611CA2340003A23A8 /* JSSQLException.cpp in Sources */,
     21839                                A8E6A78211D1661B00311F4A /* HTMLParserScheduler.cpp in Sources */,
    2183221840                        );
    2183321841                        runOnlyForDeploymentPostprocessing = 0;

  • trunk/WebCore/html/HTML5DocumentParser.cpp

    r61646 r61658  
    3030#include "Element.h"
    3131#include "Frame.h"
    32 #include "FrameView.h" // Only for isLayoutTimerActive
     32#include "HTMLParserScheduler.h"
    3333#include "HTML5Lexer.h"
    3434#include "HTML5PreloadScanner.h"
    … …  
    3636#include "HTML5TreeBuilder.h"
    3737#include "HTMLDocument.h"
    38 #include "Page.h"
    3938#include "XSSAuditor.h"
    4039#include <wtf/CurrentTime.h>
    … …  
    4342#include "InspectorTimelineAgent.h"
    4443#endif
    45 
    46 // defaultParserChunkSize is used to define how many tokens the parser will
    47 // process before checking against parserTimeLimit and possibly yielding.
    48 // This is a performance optimization to prevent checking after every token.
    49 static const int defaultParserChunkSize = 4096;
    50 
    51 // defaultParserTimeLimit is the seconds the parser will run in one write() call
    52 // before yielding.  Inline <script> execution can cause it to excede the limit.
    53 // FIXME: We would like this value to be 0.2.
    54 static const double defaultParserTimeLimit = 0.500;
    5544
    5645namespace WebCore {
    … …  
    7766} // namespace
    7867
    79 static double parserTimeLimit(Page* page)
    80 {
    81     // We're using the poorly named customHTMLTokenizerTimeDelay setting.
    82     if (page && page->hasCustomHTMLTokenizerTimeDelay())
    83         return page->customHTMLTokenizerTimeDelay();
    84     return defaultParserTimeLimit;
    85 }
    86 
    87 static int parserChunkSize(Page* page)
    88 {
    89     // FIXME: We may need to divide the value from customHTMLTokenizerChunkSize
    90     // by some constant to translate from the "character" based behavior of the
    91     // old HTMLDocumentParser to the token-based behavior of this parser.
    92     if (page && page->hasCustomHTMLTokenizerChunkSize())
    93         return page->customHTMLTokenizerChunkSize();
    94     return defaultParserChunkSize;
    95 }
    96 
    9768HTML5DocumentParser::HTML5DocumentParser(HTMLDocument* document, bool reportErrors)
    9869    : m_document(document)
    … …  
    10071    , m_scriptRunner(new HTML5ScriptRunner(document, this))
    10172    , m_treeConstructor(new HTML5TreeBuilder(m_lexer.get(), document, reportErrors))
     73    , m_parserScheduler(new HTMLParserScheduler(this))
    10274    , m_endWasDelayed(false)
    10375    , m_writeNestingLevel(0)
    104     , m_parserTimeLimit(parserTimeLimit(document->page()))
    105     , m_parserChunkSize(parserChunkSize(document->page()))
    106     , m_continueNextChunkTimer(this, &HTML5DocumentParser::continueNextChunkTimerFired)
    10776{
    10877    begin();
    … …  
    11786    , m_endWasDelayed(false)
    11887    , m_writeNestingLevel(0)
    119     // FIXME: Parser yielding should be a separate class.
    120     , m_parserTimeLimit(0)
    121     , m_parserChunkSize(0)
    122     , m_continueNextChunkTimer(this, &HTML5DocumentParser::continueNextChunkTimerFired)
    12388{
    12489    begin();
    … …  
    141106{
    142107    DocumentParser::stopParsing();
    143     m_continueNextChunkTimer.stop();
     108    m_parserScheduler.clear(); // Deleting the scheduler will clear any timers.
    144109}
    145110
    146111bool HTML5DocumentParser::processingData() const
    147112{
    148     return m_continueNextChunkTimer.isActive() || inWrite();
     113    return isScheduledForResume() || inWrite();
    149114}
    150115
    … …  
    154119        return;
    155120
    156     // Once a timer is set, it has control of when the parser continues.
    157     if (m_continueNextChunkTimer.isActive()) {
     121    // Once a resume is scheduled, HTMLParserScheduler controls when we next pump.
     122    if (isScheduledForResume()) {
    158123        ASSERT(mode == AllowYield);
    159124        return;
    … …  
    163128}
    164129
    165 struct HTML5DocumentParser::PumpSession {
    166     PumpSession()
    167         : processedTokens(0)
    168         , startTime(currentTime())
    169     {
    170     }
    171 
    172     int processedTokens;
    173     double startTime;
    174 };
    175 
    176 inline bool HTML5DocumentParser::shouldContinueParsing(PumpSession& session)
    177 {
    178     if (m_parserStopped)
    179         return false;
    180 
    181     if (session.processedTokens > m_parserChunkSize) {
    182         session.processedTokens = 0;
    183         double elapsedTime = currentTime() - session.startTime;
    184         if (elapsedTime > m_parserTimeLimit) {
    185             // Schedule an immediate timer and yield from the parser.
    186             m_continueNextChunkTimer.startOneShot(0);
    187             return false;
    188         }
    189     }
    190 
    191     ++session.processedTokens;
    192     return true;
     130bool HTML5DocumentParser::isScheduledForResume() const
     131{
     132    return m_parserScheduler && m_parserScheduler->isScheduledForResume();
     133}
     134
     135// Used by HTMLParserScheduler
     136void HTML5DocumentParser::resumeParsingAfterYield()
     137{
     138    // We should never be here unless we can pump immediately.  Call pumpLexer()
     139    // directly so that ASSERTS will fire if we're wrong.
     140    pumpLexer(AllowYield);
    193141}
    194142
    … …  
    209157    ASSERT(!m_parserStopped);
    210158    ASSERT(!m_treeConstructor->isPaused());
    211     ASSERT(!m_continueNextChunkTimer.isActive());
     159    ASSERT(!isScheduledForResume());
    212160
    213161    // We tell the InspectorTimelineAgent about every pump, even if we
    … …  
    215163    willPumpLexer();
    216164
    217     PumpSession session;
     165    HTMLParserScheduler::PumpSession session;
    218166    // FIXME: This loop body has is now too long and needs cleanup.
    219     while (mode == ForceSynchronous || shouldContinueParsing(session)) {
     167    while (mode == ForceSynchronous || (!m_parserStopped && m_parserScheduler->shouldContinueParsing(session))) {
    220168        if (!m_lexer->nextToken(m_input.current(), m_token))
    221169            break;
    … …  
    245193
    246194    didPumpLexer();
    247 }
    248 
    249 // FIXME: This belongs on Document.
    250 static bool isLayoutTimerActive(Document* doc)
    251 {
    252     ASSERT(doc);
    253     return doc->view() && doc->view()->layoutPending() && !doc->minimumLayoutDelay();
    254 }
    255 
    256 void HTML5DocumentParser::continueNextChunkTimerFired(Timer<HTML5DocumentParser>* timer)
    257 {
    258     ASSERT_UNUSED(timer, timer == &m_continueNextChunkTimer);
    259     // FIXME: The timer class should handle timer priorities instead of this code.
    260     // If a layout is scheduled, wait again to let the layout timer run first.
    261     if (isLayoutTimerActive(m_document)) {
    262         m_continueNextChunkTimer.startOneShot(0);
    263         return;
    264     }
    265 
    266     // We should never be here unless we can pump immediately.  Call pumpLexer()
    267     // directly so that ASSERTS will fire if we're wrong.
    268     pumpLexer(AllowYield);
    269195}
    270196
    … …  
    315241void HTML5DocumentParser::end()
    316242{
    317     ASSERT(!m_continueNextChunkTimer.isActive());
     243    ASSERT(!isScheduledForResume());
    318244    // NOTE: This pump should only ever emit buffered character tokens,
    319245    // so ForceSynchronous vs. AllowYield should be meaningless.
    … …  
    329255    // an external script to load, we can't finish parsing quite yet.
    330256
    331     if (inWrite() || isWaitingForScripts() || executingScript() || m_continueNextChunkTimer.isActive()) {
     257    if (inWrite() || isWaitingForScripts() || executingScript() || isScheduledForResume()) {
    332258        m_endWasDelayed = true;
    333259        return;
    … …  
    340266    // We don't check inWrite() here since inWrite() will be true if this was
    341267    // called from write().
    342     if (!m_endWasDelayed || isWaitingForScripts() || executingScript() || m_continueNextChunkTimer.isActive())
     268    if (!m_endWasDelayed || isWaitingForScripts() || executingScript() || isScheduledForResume())
    343269        return;
    344270

  • trunk/WebCore/html/HTML5DocumentParser.h

    r61637 r61658  
    4242class DocumentFragment;
    4343class HTMLDocument;
     44class HTMLParserScheduler;
    4445class HTML5Lexer;
    4546class HTML5ScriptRunner;
    … …  
    8384    virtual void notifyFinished(CachedResource*);
    8485
     86    // Exposed for HTMLParserScheduler
     87    Document* document() const { return m_document; }
     88    void resumeParsingAfterYield();
     89
    8590private:
    8691    void willPumpLexer();
    8792    void didPumpLexer();
    88 
    89     struct PumpSession;
    90     inline bool shouldContinueParsing(PumpSession&);
    91     bool runScriptsForPausedTreeConstructor();
    9293
    9394    enum SynchronousMode {
    … …  
    9798    void pumpLexer(SynchronousMode);
    9899    void pumpLexerIfPossible(SynchronousMode);
    99     void continueNextChunkTimerFired(Timer<HTML5DocumentParser>*);
     100
     101    bool runScriptsForPausedTreeConstructor();
    100102    void resumeParsingAfterScriptExecution();
    101103
    … …  
    103105    void endIfDelayed();
    104106
     107    bool isScheduledForResume() const;
    105108    bool inScriptExecution() const;
    106109    bool inWrite() const { return m_writeNestingLevel > 0; }
    … …  
    120123    OwnPtr<HTML5TreeBuilder> m_treeConstructor;
    121124    OwnPtr<HTML5PreloadScanner> m_preloadScanner;
     125    OwnPtr<HTMLParserScheduler> m_parserScheduler;
    122126
    123127    bool m_endWasDelayed;
    124128    int m_writeNestingLevel;
    125 
    126     // Used for yielding the parser to make WebKit applications more responsive.
    127     // FIXME: This should a separate class, used by HTML5DocumentParser.
    128     double m_parserTimeLimit;
    129     int m_parserChunkSize;
    130     Timer<HTML5DocumentParser> m_continueNextChunkTimer;
    131129};
    132130
Note: See TracChangeset for help on using the changeset viewer.