[Webkit-unassigned] [Bug 21329] New: REGRESSION: crash in ScriptElement::notifyFinished

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=21329

           Summary: REGRESSION: crash in ScriptElement::notifyFinished
           Product: WebKit
           Version: 528+ (Nightly build)
          Platform: PC
               URL: http://www.kino.de/kinosuche.php4
        OS/Version: Windows XP
            Status: NEW
          Severity: Critical
          Priority: P1
         Component: HTML DOM
        AssignedTo: webkit-unassigned at lists.webkit.org
        ReportedBy: darin at chromium.org
                CC: eric at webkit.org


REGRESSION: crash in ScriptElement::notifyFinished

I observed this crash on http://www.kino.de/kinosuche.php4
(you might have to load that site a couple times to repro the crash)

I found that some code changed recently that probably explains the crash.

Compare:

-void HTMLScriptElement::notifyFinished(CachedResource* o)
-{
-    CachedScript* cs = static_cast<CachedScript*>(o);
-
-    ASSERT(cs == m_cachedScript);
-
-    // Evaluating the script could lead to a garbage collection which
-    // can delete the script element so we need to protect it.
-    RefPtr<HTMLScriptElement> protect(this);
-    
-    if (cs->errorOccurred())
-        dispatchHTMLEvent(errorEvent, true, false);
-    else {
-        evaluateScript(cs->url(), cs->script());
-        dispatchHTMLEvent(loadEvent, false, false);
-    }
-
-    // script evaluation may have dereffed it already
-    if (m_cachedScript) {
-        m_cachedScript->removeClient(this);
-        m_cachedScript = 0;
-    }
-}

To:

void ScriptElementData::notifyFinished(CachedResource* o)
{
    CachedScript* cs = static_cast<CachedScript*>(o);
    ASSERT(cs == m_cachedScript);

    if (cs->errorOccurred())
        m_scriptElement->dispatchErrorEvent();
    else {
        RefPtr<Element> protector(m_element);
        evaluateScript(cs->url(), cs->script());
        m_scriptElement->dispatchLoadEvent();
    }

    stopLoadRequest();
}

The crash I get is inside stopLoadRequest() with |this| having been trashed. 
m_element owns |this|.

Here's the interesting portion of the crash stack:

        chrome.dll!WTF::HashTable<WebCore::RenderObject
*,std::pair<WebCore::RenderObject *,WebCore::RenderBlock::FloatingObject 
*>,WTF::PairFirstExtractor<std::pair<WebCore::RenderObject
*,WebCore::RenderBlock::FloatingObject *> >,WTF::PtrHash<WebCore::RenderObject 
*>,WTF::PairHashTraits<WTF::HashTraits<WebCore::RenderObject
*>,WTF::HashTraits<WebCore::RenderBlock::FloatingObject *> 
>,WTF::HashTraits<WebCore::RenderObject *> >::find<WebCore::RenderObject *,WTF::IdentityHashTranslator<WebCore::RenderObject 
*,std::pair<WebCore::RenderObject *,WebCore::RenderBlock::FloatingObject
*>,WTF::PtrHash<WebCore::RenderObject *> > >(WebCore::RenderObject * const & 
key=0x0232775c)  Line 751       C++
       
chrome.dll!WebCore::CachedResource::removeClient(WebCore::CachedResourceClient
* c=0x0232775c)  Line 131 + 0x12 bytes   C++
>	chrome.dll!WebCore::ScriptElementData::notifyFinished(WebCore::CachedResource * o=0x02330e58)  Line 191 + 0xd bytes	C++
        chrome.dll!WebCore::CachedScript::checkNotify()  Line 95 + 0xa bytes   
C++
       
chrome.dll!WebCore::CachedScript::data(WTF::PassRefPtr<WebCore::SharedBuffer>
data={...}, bool allDataReceived=true)  Line 85 + 0xe bytes       
C++
       
chrome.dll!WebCore::Loader::Host::didFinishLoading(WebCore::SubresourceLoader *
loader=0x034297f8)  Line 278    C++
        chrome.dll!WebCore::SubresourceLoader::didFinishLoading()  Line 195    
C++
       
chrome.dll!WebCore::ResourceLoader::didFinishLoading(WebCore::ResourceHandle *
__formal=0x01f31cf8)  Line 399   C++

Originally identified here:
http://code.google.com/p/chromium/issues/detail?id=3056

In the old code, the protector was also ensuring that |this| remained valid
until after stopLoadRequest() is called.  I think this is an apparent goof in
the recent code change.


-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.