Bug 63300 – Stop listening for CachedScript events as soon as notifyFinished() is called

• Status: NEW
• Component: Page Loading

[James Simonsen]

Stop listening for CachedScript events as soon as notifyFinished() is called

[James Simonsen]

Created attachment 98426 [details]
Patch

[James Simonsen]

I'm hoping this will address: http://code.google.com/p/chromium/issues/detail?id=75604

[Adam Barth]

Comment on attachment 98426 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=98426&action=review

It's too bad we don't have a good repro for this issue, but this patch looks fine.

> Source/WebCore/dom/ScriptElement.cpp:316
>  void ScriptElement::notifyFinished(CachedResource* o)
>  {
>      ASSERT(!m_willBeParserExecuted);
>      ASSERT_UNUSED(o, o == m_cachedScript);

Can we rename "o" to something more readable, like "resource" ?

[Alexey Proskuryakov]

Comment on attachment 98426 [details]
Patch

This needs a good explanation of why it's OK to revert r44591 now.

[Alexey Proskuryakov]

I do not know anything special about that patch - I just did svn blame, and immediately got pointed to it.

[Tony Gentilcore]

> This needs a good explanation of why it's OK to revert r44591 now.

Yeah, I don't think this fix is quite right.  Once all handles are released, the CachedResource's underlying buffer may be purged. In the HTMLScriptRunner we created http://trac.webkit.org/browser/trunk/Source/WebCore/dom/PendingScript.h to get around this by keeping a CachedResourceHandle alive that just ignores notifyFinished() calls (see comment at top of file). Perhaps it would be useful here too?