Changeset 61940 in webkit


Ignore:
Timestamp:
Jun 25, 2010 10:27:37 PM (14 years ago)
Author:
tonyg@chromium.org
Message:

2010-06-25 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
appraoch for preventing unexpected purges: always keep a client open.
This 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 purse itself is not
testable from a LayoutTest.

  • html/HTMLDocumentParser.cpp: (WebCore::HTMLDocumentParser::watchForLoad): (WebCore::HTMLDocumentParser::notifyFinished):
  • html/HTMLScriptRunner.cpp: (WebCore::HTMLScriptRunner::~HTMLScriptRunner): (WebCore::HTMLScriptRunner::sourceFromPendingScript): (WebCore::HTMLScriptRunner::isPendingScriptReady): (WebCore::HTMLScriptRunner::executePendingScript): (WebCore::HTMLScriptRunner::watchForLoad): (WebCore::HTMLScriptRunner::stopWatchingForLoad): (WebCore::HTMLScriptRunner::executeScriptsWaitingForLoad): (WebCore::HTMLScriptRunner::requestScript): (WebCore::HTMLScriptRunner::PendingScript::~PendingScript): (WebCore::HTMLScriptRunner::PendingScript::releaseElementAndClear): (WebCore::HTMLScriptRunner::PendingScript::setCachedScript): (WebCore::HTMLScriptRunner::PendingScript::cachedScript):
  • html/HTMLScriptRunner.h: (WebCore::HTMLScriptRunner::PendingScript::PendingScript): (WebCore::HTMLScriptRunner::PendingScript::watchingForLoad): (WebCore::HTMLScriptRunner::PendingScript::setWatchingForLoad): (WebCore::HTMLScriptRunner::PendingScript::notifyFinished):
  • html/HTMLScriptRunnerHost.h:
Location:
trunk/WebCore
Files:
5 edited

Legend:

Unmodified
Added
Removed

  • trunk/WebCore/ChangeLog

    r61939 r61940  
     12010-06-25  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        appraoch for preventing unexpected purges: always keep a client open.
     10        This 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 purse itself is not
     15        testable from a LayoutTest.
     16
     17        * html/HTMLDocumentParser.cpp:
     18        (WebCore::HTMLDocumentParser::watchForLoad):
     19        (WebCore::HTMLDocumentParser::notifyFinished):
     20        * html/HTMLScriptRunner.cpp:
     21        (WebCore::HTMLScriptRunner::~HTMLScriptRunner):
     22        (WebCore::HTMLScriptRunner::sourceFromPendingScript):
     23        (WebCore::HTMLScriptRunner::isPendingScriptReady):
     24        (WebCore::HTMLScriptRunner::executePendingScript):
     25        (WebCore::HTMLScriptRunner::watchForLoad):
     26        (WebCore::HTMLScriptRunner::stopWatchingForLoad):
     27        (WebCore::HTMLScriptRunner::executeScriptsWaitingForLoad):
     28        (WebCore::HTMLScriptRunner::requestScript):
     29        (WebCore::HTMLScriptRunner::PendingScript::~PendingScript):
     30        (WebCore::HTMLScriptRunner::PendingScript::releaseElementAndClear):
     31        (WebCore::HTMLScriptRunner::PendingScript::setCachedScript):
     32        (WebCore::HTMLScriptRunner::PendingScript::cachedScript):
     33        * html/HTMLScriptRunner.h:
     34        (WebCore::HTMLScriptRunner::PendingScript::PendingScript):
     35        (WebCore::HTMLScriptRunner::PendingScript::watchingForLoad):
     36        (WebCore::HTMLScriptRunner::PendingScript::setWatchingForLoad):
     37        (WebCore::HTMLScriptRunner::PendingScript::notifyFinished):
     38        * html/HTMLScriptRunnerHost.h:
     39
    1402010-06-25  Zhenyao Mo  <zmo@google.com>
    241

  • trunk/WebCore/html/HTMLDocumentParser.cpp

    r61904 r61940  
    341341void HTMLDocumentParser::watchForLoad(CachedResource* cachedScript)
    342342{
     343    ASSERT(!cachedScript->isLoaded());
     344    // addClient would call notifyFinished if the load were complete.
     345    // Callers do not expect to be re-entered from this call, so they should
     346    // not an already-loaded CachedResource.
    343347    cachedScript->addClient(this);
    344348}
    … …  
    359363{
    360364    ASSERT(m_scriptRunner);
    361     // Ignore calls unless we have a script blocking the parser waiting
    362     // for its own load.  Otherwise this may be a load callback from
    363     // CachedResource::addClient because the script was already in the cache.
    364     // HTMLScriptRunner may not be ready to handle running that script yet.
    365     if (!m_scriptRunner->hasScriptsWaitingForLoad()) {
    366         ASSERT(m_scriptRunner->inScriptExecution());
    367         return;
    368     }
    369365    ASSERT(!inScriptExecution());
    370366    ASSERT(m_treeBuilder->isPaused());

  • trunk/WebCore/html/HTMLScriptRunner.cpp

    r61673 r61940  
    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 HTMLScriptRunner::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
    133133    // Clear the pending script before possible rentrancy from executeScript()
    134     RefPtr<Element> scriptElement = m_parsingBlockingScript.element.release();
    135     m_parsingBlockingScript = PendingScript();
     134    RefPtr<Element> scriptElement = m_parsingBlockingScript.releaseElementAndClear();
    136135    {
    137136        NestScript nestingLevel(m_scriptNestingLevel, m_host->inputStream());
    … …  
    162161}
    163162
    164 bool HTMLScriptRunner::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 
    172163void HTMLScriptRunner::watchForLoad(PendingScript& pendingScript)
    173164{
    174165    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;
     166    m_host->watchForLoad(pendingScript.cachedScript());
     167    pendingScript.setWatchingForLoad(true);
    181168}
    182169
    … …  
    184171{
    185172    ASSERT(pendingScript.watchingForLoad());
    186     m_host->stopWatchingForLoad(pendingScript.cachedScript.get());
    187     pendingScript.watchingForLoadState = PendingScript::NotWatchingForLoad;
     173    m_host->stopWatchingForLoad(pendingScript.cachedScript());
     174    pendingScript.setWatchingForLoad(false);
    188175}
    189176
    … …  
    225212bool HTMLScriptRunner::executeScriptsWaitingForLoad(CachedResource* cachedScript)
    226213{
    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());
    231214    ASSERT(!m_scriptNestingLevel);
    232215    ASSERT(haveParsingBlockingScript());
    233     ASSERT_UNUSED(cachedScript, m_parsingBlockingScript.cachedScript == cachedScript);
    234     ASSERT(m_parsingBlockingScript.cachedScript->isLoaded());
     216    ASSERT_UNUSED(cachedScript, m_parsingBlockingScript.cachedScript() == cachedScript);
     217    ASSERT(m_parsingBlockingScript.cachedScript()->isLoaded());
    235218    return executeParsingBlockingScripts();
    236219}
    … …  
    263246        return;
    264247    }
    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.
     248
     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 }
     281HTMLScriptRunner::PendingScript::~PendingScript()
     282{
     283    if (m_cachedScript)
     284        m_cachedScript->removeClient(this);
     285}
     286
     287PassRefPtr<Element> HTMLScriptRunner::PendingScript::releaseElementAndClear()
     288{
     289    setCachedScript(0);
     290    startingLineNumber = 0;
     291    m_watchingForLoad = false;
     292    return element.release();
     293}
     294
     295void HTMLScriptRunner::PendingScript::setCachedScript(CachedScript* cachedScript)
     296{
     297    if (m_cachedScript == cachedScript)
     298        return;
     299    if (m_cachedScript)
     300        m_cachedScript->removeClient(this);
     301    m_cachedScript = cachedScript;
     302    if (m_cachedScript)
     303        m_cachedScript->addClient(this);
     304}
     305
     306CachedScript* HTMLScriptRunner::PendingScript::cachedScript() const
     307{
     308    return m_cachedScript.get();
     309}
     310
     311}

  • trunk/WebCore/html/HTMLScriptRunner.h

    r61673 r61940  
    5050    bool execute(PassRefPtr<Element> scriptToProcess, int scriptStartLine);
    5151
    52     bool hasScriptsWaitingForLoad() const;
    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    //
     65    // FIXME: Finish turning this into a proper class.
     66    class PendingScript : public CachedResourceClient, Noncopyable {
     67    public:
    7168        PendingScript()
    72             : watchingForLoadState(NotWatchingForLoad)
    73             , startingLineNumber(0)
     69            : startingLineNumber(0)
     70            , m_watchingForLoad(false)
    7471        {
    7572        }
    7673
    77         bool watchingForLoad()
     74        ~PendingScript();
     75
     76        PassRefPtr<Element> releaseElementAndClear();
     77
     78        bool watchingForLoad() const { return m_watchingForLoad; }
     79        void setWatchingForLoad(bool b) { m_watchingForLoad = b; }
     80
     81        CachedScript* cachedScript() const;
     82        void setCachedScript(CachedScript*);
     83
     84        virtual void notifyFinished(CachedResource*)
    7885        {
    79             return watchingForLoadState != NotWatchingForLoad;
    8086        }
    8187
    8288        RefPtr<Element> element;
    83         CachedResourceHandle<CachedScript> cachedScript;
    84         WatchingForLoadState watchingForLoadState;
    8589        int startingLineNumber; // Only used for inline script tags.
    8690        // HTML5 has an isReady parameter, however isReady ends up equivalent to
    8791        // m_document->haveStylesheetsLoaded() && cachedScript->isLoaded()
     92
     93    private:
     94        bool m_watchingForLoad;
     95        CachedResourceHandle<CachedScript> m_cachedScript;
    8896    };
    8997

  • trunk/WebCore/html/HTMLScriptRunnerHost.h

    r61673 r61940  
    3939    virtual ~HTMLScriptRunnerHost() { }
    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.