Title: [139942] trunk/Source/WebCore
- Revision
- 139942
- Author
- morr...@google.com
- Date
- 2013-01-16 17:38:20 -0800 (Wed, 16 Jan 2013)
Log Message
WebCore::ScriptRunner::timerFired() is reported to crash.
https://bugs.webkit.org/show_bug.cgi?id=92211
Reviewed by Darin Adler.
This is a speculative fix to address some crash reports.
Here is my rough guess.
The crash report says there is a PendingScript instance whose m_element is null.
- It happens only if the instance is created through the default constructor,
- that could happen if m_pendingAsyncScripts doesn't contains matched PendingScript in ScriptRunner::notifyScriptReady(),
- that could happen when notifyScriptReady() is called more than once.
- It turns out that the call site, ScriptElement::notifyFinished(), could be called multiple times since
it doesn't remove itself from the CachedResource, which is done separately in ScriptElement::execute().
- So if notifyFinished() is called before execute(), it could happen
This change added a guard to prevent notifyFinished() from being invoked multiple times.
We should watch if these crash reports disappear.
No new tests. I couldn't create any repro for this.
* dom/ScriptElement.cpp:
(WebCore::ScriptElement::notifyFinished):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (139941 => 139942)
--- trunk/Source/WebCore/ChangeLog 2013-01-17 01:36:09 UTC (rev 139941)
+++ trunk/Source/WebCore/ChangeLog 2013-01-17 01:38:20 UTC (rev 139942)
@@ -1,3 +1,29 @@
+2013-01-16 MORITA Hajime <morr...@google.com>
+
+ WebCore::ScriptRunner::timerFired() is reported to crash.
+ https://bugs.webkit.org/show_bug.cgi?id=92211
+
+ Reviewed by Darin Adler.
+
+ This is a speculative fix to address some crash reports.
+ Here is my rough guess.
+
+ The crash report says there is a PendingScript instance whose m_element is null.
+ - It happens only if the instance is created through the default constructor,
+ - that could happen if m_pendingAsyncScripts doesn't contains matched PendingScript in ScriptRunner::notifyScriptReady(),
+ - that could happen when notifyScriptReady() is called more than once.
+ - It turns out that the call site, ScriptElement::notifyFinished(), could be called multiple times since
+ it doesn't remove itself from the CachedResource, which is done separately in ScriptElement::execute().
+ - So if notifyFinished() is called before execute(), it could happen
+
+ This change added a guard to prevent notifyFinished() from being invoked multiple times.
+ We should watch if these crash reports disappear.
+
+ No new tests. I couldn't create any repro for this.
+
+ * dom/ScriptElement.cpp:
+ (WebCore::ScriptElement::notifyFinished):
+
2013-01-16 Pablo Flouret <pab...@motorola.com>
Fix mac build system warnings introduced by r139866
Modified: trunk/Source/WebCore/dom/ScriptElement.cpp (139941 => 139942)
--- trunk/Source/WebCore/dom/ScriptElement.cpp 2013-01-17 01:36:09 UTC (rev 139941)
+++ trunk/Source/WebCore/dom/ScriptElement.cpp 2013-01-17 01:38:20 UTC (rev 139942)
@@ -331,7 +331,14 @@
void ScriptElement::notifyFinished(CachedResource* resource)
{
ASSERT(!m_willBeParserExecuted);
+
+ // CachedResource possibly invokes this notifyFinished() more than
+ // once because ScriptElement doesn't unsubscribe itself from
+ // CachedResource here and does it in execute() instead.
+ // We use m_cachedScript to check if this function is already called.
ASSERT_UNUSED(resource, resource == m_cachedScript);
+ if (!m_cachedScript)
+ return;
if (m_requestUsesAccessControl
&& !m_element->document()->securityOrigin()->canRequest(m_cachedScript->response().url())
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-changes
