Changeset 61638 in webkit


Ignore:
Timestamp:
Jun 22, 2010 4:46:53 PM (14 years ago)
Author:
abarth@webkit.org
Message:

2010-06-22 Tony Gentilcore <​tonyg@chromium.org>

Reviewed by Eric Seidel.

Make PendingScript hold a CachedResourceClient open for its lifetime
​https://bugs.webkit.org/show_bug.cgi?id=40968

This replaces the mechanism introduced in r61374 with a simpler
approach from preventing unexpected purges; always keep a client open.
This will approach will allow deferred scripts to add a client after
the resource may have already been loaded without having to worry about
the buffer being purged in the meantime.

No new tests because making a CachedResource purge itself is not
testable from a LayoutTest.

  • html/HTML5DocumentParser.cpp: (WebCore::HTML5DocumentParser::watchForLoad): (WebCore::HTML5DocumentParser::notifyFinished):
  • html/HTML5ScriptRunner.cpp: (WebCore::HTML5ScriptRunner::~HTML5ScriptRunner): (WebCore::HTML5ScriptRunner::sourceFromPendingScript): (WebCore::HTML5ScriptRunner::isPendingScriptReady): (WebCore::HTML5ScriptRunner::executePendingScript): (WebCore::HTML5ScriptRunner::watchForLoad): (WebCore::HTML5ScriptRunner::stopWatchingForLoad): (WebCore::HTML5ScriptRunner::executeScriptsWaitingForLoad): (WebCore::HTML5ScriptRunner::executeScriptsWaitingForStylesheets): (WebCore::HTML5ScriptRunner::requestScript):
  • html/HTML5ScriptRunner.h: (WebCore::HTML5ScriptRunner::PendingScript::PendingScript): (WebCore::HTML5ScriptRunner::PendingScript::~PendingScript): (WebCore::HTML5ScriptRunner::PendingScript::setCachedScript): (WebCore::HTML5ScriptRunner::PendingScript::cachedScript): (WebCore::HTML5ScriptRunner::PendingScript::notifyFinished):
  • html/HTML5ScriptRunnerHost.h:
Location:
trunk/WebCore
Files:
5 edited

Legend:

Unmodified
Added
Removed

  • trunk/WebCore/ChangeLog

    r61637 r61638  
     12010-06-22  Tony Gentilcore  <tonyg@chromium.org>
     2
     3        Reviewed by Eric Seidel.
     4
     5        Make PendingScript hold a CachedResourceClient open for its lifetime
     6        https://bugs.webkit.org/show_bug.cgi?id=40968
     7
     8        This replaces the mechanism introduced in r61374 with a simpler
     9        approach from preventing unexpected purges; always keep a client open.
     10        This will approach will allow deferred scripts to add a client after
     11        the resource may have already been loaded without having to worry about
     12        the buffer being purged in the meantime.
     13
     14        No new tests because making a CachedResource purge itself is not
     15        testable from a LayoutTest.
     16
     17        * html/HTML5DocumentParser.cpp:
     18        (WebCore::HTML5DocumentParser::watchForLoad):
     19        (WebCore::HTML5DocumentParser::notifyFinished):
     20        * html/HTML5ScriptRunner.cpp:
     21        (WebCore::HTML5ScriptRunner::~HTML5ScriptRunner):
     22        (WebCore::HTML5ScriptRunner::sourceFromPendingScript):
     23        (WebCore::HTML5ScriptRunner::isPendingScriptReady):
     24        (WebCore::HTML5ScriptRunner::executePendingScript):
     25        (WebCore::HTML5ScriptRunner::watchForLoad):
     26        (WebCore::HTML5ScriptRunner::stopWatchingForLoad):
     27        (WebCore::HTML5ScriptRunner::executeScriptsWaitingForLoad):
     28        (WebCore::HTML5ScriptRunner::executeScriptsWaitingForStylesheets):
     29        (WebCore::HTML5ScriptRunner::requestScript):
     30        * html/HTML5ScriptRunner.h:
     31        (WebCore::HTML5ScriptRunner::PendingScript::PendingScript):
     32        (WebCore::HTML5ScriptRunner::PendingScript::~PendingScript):
     33        (WebCore::HTML5ScriptRunner::PendingScript::setCachedScript):
     34        (WebCore::HTML5ScriptRunner::PendingScript::cachedScript):
     35        (WebCore::HTML5ScriptRunner::PendingScript::notifyFinished):
     36        * html/HTML5ScriptRunnerHost.h:
     37
    1382010-06-22  Eric Seidel  <eric@webkit.org>
    239

  • trunk/WebCore/html/HTML5DocumentParser.cpp

    r61637 r61638  
    414414void HTML5DocumentParser::watchForLoad(CachedResource* cachedScript)
    415415{
     416    ASSERT(!cachedScript->isLoaded());
     417    // addClient would call notifyFinished if the load were complete.
     418    // Callers do not expect to be re-entered from this call, so they should
     419    // not an already-loaded CachedResource.
    416420    cachedScript->addClient(this);
    417421}
    … …  
    432436{
    433437    ASSERT(m_scriptRunner);
    434     // Ignore calls unless we have a script blocking the parser waiting
    435     // for its own load.  Otherwise this may be a load callback from
    436     // CachedResource::addClient because the script was already in the cache.
    437     // HTML5ScriptRunner may not be ready to handle running that script yet.
    438     if (!m_scriptRunner->hasScriptsWaitingForLoad()) {
    439         ASSERT(m_scriptRunner->inScriptExecution());
    440         return;
    441     }
    442438    ASSERT(!inScriptExecution());
    443439    ASSERT(m_treeConstructor->isPaused());

  • trunk/WebCore/html/HTML5ScriptRunner.cpp

    r61608 r61638  
    7575{
    7676    // FIXME: Should we be passed a "done loading/parsing" callback sooner than destruction?
    77     if (m_parsingBlockingScript.cachedScript && m_parsingBlockingScript.watchingForLoad())
     77    if (m_parsingBlockingScript.cachedScript() && m_parsingBlockingScript.watchingForLoad())
    7878        stopWatchingForLoad(m_parsingBlockingScript);
    7979}
    … …  
    100100ScriptSourceCode HTML5ScriptRunner::sourceFromPendingScript(const PendingScript& script, bool& errorOccurred)
    101101{
    102     if (script.cachedScript) {
    103         errorOccurred = script.cachedScript->errorOccurred();
    104         ASSERT(script.cachedScript->isLoaded());
    105         return ScriptSourceCode(script.cachedScript.get());
     102    if (script.cachedScript()) {
     103        errorOccurred = script.cachedScript()->errorOccurred();
     104        ASSERT(script.cachedScript()->isLoaded());
     105        return ScriptSourceCode(script.cachedScript());
    106106    }
    107107    errorOccurred = false;
    … …  
    114114    if (m_hasScriptsWaitingForStylesheets)
    115115        return false;
    116     if (script.cachedScript && !script.cachedScript->isLoaded())
     116    if (script.cachedScript() && !script.cachedScript()->isLoaded())
    117117        return false;
    118118    return true;
    … …  
    128128
    129129    // Stop watching loads before executeScript to prevent recursion if the script reloads itself.
    130     if (m_parsingBlockingScript.cachedScript && m_parsingBlockingScript.watchingForLoad())
     130    if (m_parsingBlockingScript.cachedScript() && m_parsingBlockingScript.watchingForLoad())
    131131        stopWatchingForLoad(m_parsingBlockingScript);
    132132
    … …  
    162162}
    163163
    164 bool HTML5ScriptRunner::hasScriptsWaitingForLoad() const
    165 {
    166     // We're only actually waiting for a load.  This allows us to ignore load
    167     // callbacks when CachedResource::addClient calls notifyFinished because
    168     // of a cache hit (not because of a load we were set up to wait for).
    169     return m_parsingBlockingScript.watchingForLoadState == PendingScript::WatchingForLoad;
    170 }
    171 
    172164void HTML5ScriptRunner::watchForLoad(PendingScript& pendingScript)
    173165{
    174166    ASSERT(!pendingScript.watchingForLoad());
    175     // CachedResource::addClient will call notifyFinished if the load is already
    176     // complete.  We set watchingForLoadState to RegisteringForWatch so that we
    177     // know to ignore any notifyFinished call during addClient.
    178     pendingScript.watchingForLoadState = PendingScript::RegisteringForWatch;
    179     m_host->watchForLoad(pendingScript.cachedScript.get());
    180     pendingScript.watchingForLoadState = PendingScript::WatchingForLoad;
     167    m_host->watchForLoad(pendingScript.cachedScript());
     168    pendingScript.setWatchingForLoad(true);
    181169}
    182170
    … …  
    184172{
    185173    ASSERT(pendingScript.watchingForLoad());
    186     m_host->stopWatchingForLoad(pendingScript.cachedScript.get());
    187     pendingScript.watchingForLoadState = PendingScript::NotWatchingForLoad;
     174    m_host->stopWatchingForLoad(pendingScript.cachedScript());
     175    pendingScript.setWatchingForLoad(false);
    188176}
    189177
    … …  
    225213bool HTML5ScriptRunner::executeScriptsWaitingForLoad(CachedResource* cachedScript)
    226214{
    227     // Callers should check hasScriptsWaitingForLoad() before calling
    228     // to prevent parser or script re-entry during due to
    229     // CachedResource::addClient calling notifyFinished on cache-hits.
    230     ASSERT(hasScriptsWaitingForLoad());
    231215    ASSERT(!m_scriptNestingLevel);
    232216    ASSERT(haveParsingBlockingScript());
    233     ASSERT_UNUSED(cachedScript, m_parsingBlockingScript.cachedScript == cachedScript);
    234     ASSERT(m_parsingBlockingScript.cachedScript->isLoaded());
     217    ASSERT_UNUSED(cachedScript, m_parsingBlockingScript.cachedScript() == cachedScript);
     218    ASSERT(m_parsingBlockingScript.cachedScript()->isLoaded());
    235219    return executeParsingBlockingScripts();
    236220}
    … …  
    240224    // Callers should check hasScriptsWaitingForStylesheets() before calling
    241225    // to prevent parser or script re-entry during </style> parsing.
    242     ASSERT(hasScriptsWaitingForStylesheets());
     226    ASSERT(m_hasScriptsWaitingForStylesheets);
    243227    ASSERT(!m_scriptNestingLevel);
    244228    ASSERT(m_document->haveStylesheetsLoaded());
    … …  
    263247        return;
    264248    }
    265     m_parsingBlockingScript.cachedScript = cachedScript;
    266 
    267     // Always call watchForLoad, even if the script is already loaded.
    268     // CachedResource may purge its data if it has no clients, which would cause
    269     // later script execution to fail.  watchForLoad sets m_parsingBlockingScript
    270     // to the RegisteringForWatch state so we know to ignore any
    271     // executeScriptsWaitingForLoad callbacks during the watchForLoad call.
    272     watchForLoad(m_parsingBlockingScript);
    273     // Callers will attempt to run the m_parsingBlockingScript if possible
    274     // before returning control to the parser.
     249    m_parsingBlockingScript.setCachedScript(cachedScript);
     250
     251    // We only care about a load callback if cachedScript is not already
     252    // in the cache.  Callers will attempt to run the m_parsingBlockingScript
     253    // if possible before returning control to the parser.
     254    if (!m_parsingBlockingScript.cachedScript()->isLoaded())
     255        watchForLoad(m_parsingBlockingScript);
    275256}
    276257
    … …  
    298279}
    299280
    300 }
     281HTML5ScriptRunner::PendingScript::~PendingScript()
     282{
     283    if (m_cachedScript)
     284        m_cachedScript->removeClient(this);
     285}
     286
     287void HTML5ScriptRunner::PendingScript::setCachedScript(CachedScript* cachedScript)
     288{
     289    if (m_cachedScript)
     290        m_cachedScript->removeClient(this);
     291    m_cachedScript = cachedScript;
     292    if (m_cachedScript)
     293        m_cachedScript->addClient(this);
     294}
     295
     296CachedScript* HTML5ScriptRunner::PendingScript::cachedScript() const
     297{
     298    return m_cachedScript.get();
     299}
     300
     301}

  • trunk/WebCore/html/HTML5ScriptRunner.h

    r61374 r61638  
    4949    // Processes the passed in script and any pending scripts if possible.
    5050    bool execute(PassRefPtr<Element> scriptToProcess, int scriptStartLine);
    51 
    52     bool hasScriptsWaitingForLoad() const;
     51    // Processes any pending scripts.
    5352    bool executeScriptsWaitingForLoad(CachedResource*);
    54 
    5553    bool hasScriptsWaitingForStylesheets() const { return m_hasScriptsWaitingForStylesheets; }
    5654    bool executeScriptsWaitingForStylesheets();
    … …  
    5957
    6058private:
    61     struct PendingScript {
    62         // This state controls whether we need to do anything with this script
    63         // when we get a executeScriptsWaitingForLoad callback.
    64         // We ignore callbacks during RegisteringForWatch.
    65         enum WatchingForLoadState {
    66             NotWatchingForLoad,
    67             RegisteringForWatch,
    68             WatchingForLoad,
    69         };
    70 
     59    // A container for an external script which may be loaded and executed.
     60    //
     61    // A CachedResourceHandle alone does not prevent the underlying CachedResource
     62    // from purging its data buffer. This class holds a dummy client open for its
     63    // lifetime in order to guarantee that the data buffer will not be purged.
     64    class PendingScript : public CachedResourceClient {
     65    public:
    7166        PendingScript()
    72             : watchingForLoadState(NotWatchingForLoad)
     67            : m_watchingForLoad(false)
    7368            , startingLineNumber(0)
    7469        {
    7570        }
    7671
    77         bool watchingForLoad()
     72        ~PendingScript();
     73
     74        bool watchingForLoad() const { return m_watchingForLoad; }
     75        void setWatchingForLoad(bool b) { m_watchingForLoad = b; }
     76
     77        CachedScript* cachedScript() const;
     78        void setCachedScript(CachedScript* cachedScript);
     79
     80        virtual void notifyFinished(CachedResource*)
    7881        {
    79             return watchingForLoadState != NotWatchingForLoad;
    8082        }
    8183
    8284        RefPtr<Element> element;
    83         CachedResourceHandle<CachedScript> cachedScript;
    84         WatchingForLoadState watchingForLoadState;
    8585        int startingLineNumber; // Only used for inline script tags.
    8686        // HTML5 has an isReady parameter, however isReady ends up equivalent to
    8787        // m_document->haveStylesheetsLoaded() && cachedScript->isLoaded()
     88
     89    private:
     90        bool m_watchingForLoad; // Did we pass the cachedScript to the HTML5ScriptRunnerHost.
     91        CachedResourceHandle<CachedScript> m_cachedScript;
    8892    };
    8993

  • trunk/WebCore/html/HTML5ScriptRunnerHost.h

    r61607 r61638  
    3939    virtual ~HTML5ScriptRunnerHost() { }
    4040
    41     // Implementors must call cachedResource->addClient() immediately.
     41    // Implementors should call cachedResource->addClient() here or soon after.
    4242    virtual void watchForLoad(CachedResource*) = 0;
    4343    // Implementors must call cachedResource->removeClient() immediately.
Note: See TracChangeset for help on using the changeset viewer.