Title: [89719] trunk/Source/WebCore
- Revision
- 89719
- Author
- [email protected]
- Date
- 2011-06-24 17:39:33 -0700 (Fri, 24 Jun 2011)
Log Message
2011-06-24 Gavin Peters <[email protected]>
Reviewed by Darin Adler.
fix possible race in LinkLoader
https://bugs.webkit.org/show_bug.cgi?id=63360
In chromium bug 80729
http://code.google.com/p/chromium/issues/detail?id=80729 I am
seeing some kind of double triggering of the timer; I am concerned
that it is possible that a Link element errors out or succeeds,
sets a timer, and shortly before the timer is triggered it is
editted, launches another request. After that, the first timer
triggers, zeroing out m_cachedResource. Then, the second load
finishes, and *crash*. If this is the case, this fix should stop
it.
No new tests; I haven't reproduced this. I hope chrome's crash
telemetry will give good feedback; this crash is occuring many times a
day so the difference should be obvious.
* loader/LinkLoader.cpp:
(WebCore::LinkLoader::LinkLoader):
(WebCore::LinkLoader::linkLoadTimerFired):
(WebCore::LinkLoader::linkLoadingErrorTimerFired):
(WebCore::LinkLoader::notifyFinished):
* loader/LinkLoader.h:
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (89718 => 89719)
--- trunk/Source/WebCore/ChangeLog 2011-06-25 00:16:36 UTC (rev 89718)
+++ trunk/Source/WebCore/ChangeLog 2011-06-25 00:39:33 UTC (rev 89719)
@@ -1,3 +1,31 @@
+2011-06-24 Gavin Peters <[email protected]>
+
+ Reviewed by Darin Adler.
+
+ fix possible race in LinkLoader
+ https://bugs.webkit.org/show_bug.cgi?id=63360
+
+ In chromium bug 80729
+ http://code.google.com/p/chromium/issues/detail?id=80729 I am
+ seeing some kind of double triggering of the timer; I am concerned
+ that it is possible that a Link element errors out or succeeds,
+ sets a timer, and shortly before the timer is triggered it is
+ editted, launches another request. After that, the first timer
+ triggers, zeroing out m_cachedResource. Then, the second load
+ finishes, and *crash*. If this is the case, this fix should stop
+ it.
+
+ No new tests; I haven't reproduced this. I hope chrome's crash
+ telemetry will give good feedback; this crash is occuring many times a
+ day so the difference should be obvious.
+
+ * loader/LinkLoader.cpp:
+ (WebCore::LinkLoader::LinkLoader):
+ (WebCore::LinkLoader::linkLoadTimerFired):
+ (WebCore::LinkLoader::linkLoadingErrorTimerFired):
+ (WebCore::LinkLoader::notifyFinished):
+ * loader/LinkLoader.h:
+
2011-06-24 Jer Noble <[email protected]>
Reviewed by Eric Carlson.
Modified: trunk/Source/WebCore/loader/LinkLoader.cpp (89718 => 89719)
--- trunk/Source/WebCore/loader/LinkLoader.cpp 2011-06-25 00:16:36 UTC (rev 89718)
+++ trunk/Source/WebCore/loader/LinkLoader.cpp 2011-06-25 00:39:33 UTC (rev 89719)
@@ -48,7 +48,8 @@
LinkLoader::LinkLoader(LinkLoaderClient* client)
: m_client(client)
- , m_linkLoadedTimer(this, &LinkLoader::linkLoadedTimerFired)
+ , m_linkLoadTimer(this, &LinkLoader::linkLoadTimerFired)
+ , m_linkLoadingErrorTimer(this, &LinkLoader::linkLoadingErrorTimerFired)
{
}
@@ -58,21 +59,29 @@
m_cachedLinkResource->removeClient(this);
}
-void LinkLoader::linkLoadedTimerFired(Timer<LinkLoader>* timer)
+void LinkLoader::linkLoadTimerFired(Timer<LinkLoader>* timer)
{
- ASSERT_UNUSED(timer, timer == &m_linkLoadedTimer);
- if (m_cachedLinkResource->errorOccurred())
- m_client->linkLoadingErrored();
- else if (!m_cachedLinkResource->wasCanceled())
- m_client->linkLoaded();
- m_cachedLinkResource->removeClient(this);
- m_cachedLinkResource = 0;
+ ASSERT_UNUSED(timer, timer == &m_linkLoadTimer);
+ m_client->linkLoaded();
}
+void LinkLoader::linkLoadingErrorTimerFired(Timer<LinkLoader>* timer)
+{
+ ASSERT_UNUSED(timer, timer == &m_linkLoadingErrorTimer);
+ m_client->linkLoadingErrored();
+}
+
void LinkLoader::notifyFinished(CachedResource* resource)
{
ASSERT_UNUSED(resource, m_cachedLinkResource.get() == resource);
- m_linkLoadedTimer.startOneShot(0);
+
+ if (m_cachedLinkResource->errorOccurred())
+ m_linkLoadingErrorTimer.startOneShot(0);
+ else
+ m_linkLoadTimer.startOneShot(0);
+
+ m_cachedLinkResource->removeClient(this);
+ m_cachedLinkResource = 0;
}
bool LinkLoader::loadLink(const LinkRelAttribute& relAttribute, const String& type,
Modified: trunk/Source/WebCore/loader/LinkLoader.h (89718 => 89719)
--- trunk/Source/WebCore/loader/LinkLoader.h 2011-06-25 00:16:36 UTC (rev 89718)
+++ trunk/Source/WebCore/loader/LinkLoader.h 2011-06-25 00:39:33 UTC (rev 89719)
@@ -53,12 +53,14 @@
bool loadLink(const LinkRelAttribute&, const String& type, const KURL&, Document*);
private:
- void linkLoadedTimerFired(Timer<LinkLoader>*);
+ void linkLoadTimerFired(Timer<LinkLoader>*);
+ void linkLoadingErrorTimerFired(Timer<LinkLoader>*);
LinkLoaderClient* m_client;
CachedResourceHandle<CachedResource> m_cachedLinkResource;
- Timer<LinkLoader> m_linkLoadedTimer;
+ Timer<LinkLoader> m_linkLoadTimer;
+ Timer<LinkLoader> m_linkLoadingErrorTimer;
};
}
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes
