Changeset 61646 in webkit


Ignore:
Timestamp:
Jun 22, 2010 6:41:39 PM (14 years ago)
Author:
eric@webkit.org
Message:

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

Unreviewed. Rolling out ​http://trac.webkit.org/changeset/61638
made a few tests crash.

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

  • 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::hasScriptsWaitingForLoad): (WebCore::HTML5ScriptRunner::watchForLoad): (WebCore::HTML5ScriptRunner::stopWatchingForLoad): (WebCore::HTML5ScriptRunner::executeScriptsWaitingForLoad): (WebCore::HTML5ScriptRunner::executeScriptsWaitingForStylesheets): (WebCore::HTML5ScriptRunner::requestScript):
  • html/HTML5ScriptRunner.h: (WebCore::HTML5ScriptRunner::PendingScript::): (WebCore::HTML5ScriptRunner::PendingScript::PendingScript): (WebCore::HTML5ScriptRunner::PendingScript::watchingForLoad):
  • html/HTML5ScriptRunnerHost.h:
Location:
trunk/WebCore
Files:
5 edited

Legend:

Unmodified
Added
Removed

  • trunk/WebCore/ChangeLog

    r61645 r61646  
     12010-06-22  Eric Seidel  <eric@webkit.org>
     2
     3        Unreviewed.  Rolling out http://trac.webkit.org/changeset/61638
     4        made a few tests crash.
     5
     6        Make PendingScript hold a CachedResourceClient open for its lifetime
     7        https://bugs.webkit.org/show_bug.cgi?id=40968
     8
     9        * html/HTML5DocumentParser.cpp:
     10        (WebCore::HTML5DocumentParser::watchForLoad):
     11        (WebCore::HTML5DocumentParser::notifyFinished):
     12        * html/HTML5ScriptRunner.cpp:
     13        (WebCore::HTML5ScriptRunner::~HTML5ScriptRunner):
     14        (WebCore::HTML5ScriptRunner::sourceFromPendingScript):
     15        (WebCore::HTML5ScriptRunner::isPendingScriptReady):
     16        (WebCore::HTML5ScriptRunner::executePendingScript):
     17        (WebCore::HTML5ScriptRunner::hasScriptsWaitingForLoad):
     18        (WebCore::HTML5ScriptRunner::watchForLoad):
     19        (WebCore::HTML5ScriptRunner::stopWatchingForLoad):
     20        (WebCore::HTML5ScriptRunner::executeScriptsWaitingForLoad):
     21        (WebCore::HTML5ScriptRunner::executeScriptsWaitingForStylesheets):
     22        (WebCore::HTML5ScriptRunner::requestScript):
     23        * html/HTML5ScriptRunner.h:
     24        (WebCore::HTML5ScriptRunner::PendingScript::):
     25        (WebCore::HTML5ScriptRunner::PendingScript::PendingScript):
     26        (WebCore::HTML5ScriptRunner::PendingScript::watchingForLoad):
     27        * html/HTML5ScriptRunnerHost.h:
     28
    1292010-06-22  Adele Peterson  <adele@apple.com>
    230

  • trunk/WebCore/html/HTML5DocumentParser.cpp

    r61640 r61646  
    407407void HTML5DocumentParser::watchForLoad(CachedResource* cachedScript)
    408408{
    409     ASSERT(!cachedScript->isLoaded());
    410     // addClient would call notifyFinished if the load were complete.
    411     // Callers do not expect to be re-entered from this call, so they should
    412     // not an already-loaded CachedResource.
    413409    cachedScript->addClient(this);
    414410}
    … …  
    429425{
    430426    ASSERT(m_scriptRunner);
     427    // Ignore calls unless we have a script blocking the parser waiting
     428    // for its own load.  Otherwise this may be a load callback from
     429    // CachedResource::addClient because the script was already in the cache.
     430    // HTML5ScriptRunner may not be ready to handle running that script yet.
     431    if (!m_scriptRunner->hasScriptsWaitingForLoad()) {
     432        ASSERT(m_scriptRunner->inScriptExecution());
     433        return;
     434    }
    431435    ASSERT(!inScriptExecution());
    432436    ASSERT(m_treeConstructor->isPaused());

  • trunk/WebCore/html/HTML5ScriptRunner.cpp

    r61638 r61646  
    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());
     102    if (script.cachedScript) {
     103        errorOccurred = script.cachedScript->errorOccurred();
     104        ASSERT(script.cachedScript->isLoaded());
     105        return ScriptSourceCode(script.cachedScript.get());
    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
     164bool 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
    164172void HTML5ScriptRunner::watchForLoad(PendingScript& pendingScript)
    165173{
    166174    ASSERT(!pendingScript.watchingForLoad());
    167     m_host->watchForLoad(pendingScript.cachedScript());
    168     pendingScript.setWatchingForLoad(true);
     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;
    169181}
    170182
    … …  
    172184{
    173185    ASSERT(pendingScript.watchingForLoad());
    174     m_host->stopWatchingForLoad(pendingScript.cachedScript());
    175     pendingScript.setWatchingForLoad(false);
     186    m_host->stopWatchingForLoad(pendingScript.cachedScript.get());
     187    pendingScript.watchingForLoadState = PendingScript::NotWatchingForLoad;
    176188}
    177189
    … …  
    213225bool HTML5ScriptRunner::executeScriptsWaitingForLoad(CachedResource* cachedScript)
    214226{
     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());
    215231    ASSERT(!m_scriptNestingLevel);
    216232    ASSERT(haveParsingBlockingScript());
    217     ASSERT_UNUSED(cachedScript, m_parsingBlockingScript.cachedScript() == cachedScript);
    218     ASSERT(m_parsingBlockingScript.cachedScript()->isLoaded());
     233    ASSERT_UNUSED(cachedScript, m_parsingBlockingScript.cachedScript == cachedScript);
     234    ASSERT(m_parsingBlockingScript.cachedScript->isLoaded());
    219235    return executeParsingBlockingScripts();
    220236}
    … …  
    224240    // Callers should check hasScriptsWaitingForStylesheets() before calling
    225241    // to prevent parser or script re-entry during </style> parsing.
    226     ASSERT(m_hasScriptsWaitingForStylesheets);
     242    ASSERT(hasScriptsWaitingForStylesheets());
    227243    ASSERT(!m_scriptNestingLevel);
    228244    ASSERT(m_document->haveStylesheetsLoaded());
    … …  
    247263        return;
    248264    }
    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);
     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.
    256275}
    257276
    … …  
    279298}
    280299
    281 HTML5ScriptRunner::PendingScript::~PendingScript()
    282 {
    283     if (m_cachedScript)
    284         m_cachedScript->removeClient(this);
    285 }
    286 
    287 void 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 
    296 CachedScript* HTML5ScriptRunner::PendingScript::cachedScript() const
    297 {
    298     return m_cachedScript.get();
    299 }
    300 
    301 }
     300}

  • trunk/WebCore/html/HTML5ScriptRunner.h

    r61640 r61646  
    4949    // Processes the passed in script and any pending scripts if possible.
    5050    bool execute(PassRefPtr<Element> scriptToProcess, int scriptStartLine);
    51     // Processes any pending scripts.
     51
     52    bool hasScriptsWaitingForLoad() const;
    5253    bool executeScriptsWaitingForLoad(CachedResource*);
     54
    5355    bool hasScriptsWaitingForStylesheets() const { return m_hasScriptsWaitingForStylesheets; }
    5456    bool executeScriptsWaitingForStylesheets();
    … …  
    5759
    5860private:
    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:
     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
    6671        PendingScript()
    67             : startingLineNumber(0)
    68             , m_watchingForLoad(false)
     72            : watchingForLoadState(NotWatchingForLoad)
     73            , startingLineNumber(0)
    6974        {
    7075        }
    7176
    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*)
     77        bool watchingForLoad()
    8178        {
     79            return watchingForLoadState != NotWatchingForLoad;
    8280        }
    8381
    8482        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;
    9288    };
    9389

  • trunk/WebCore/html/HTML5ScriptRunnerHost.h

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