Title: [120845] trunk
- Revision
- 120845
- Author
- [email protected]
- Date
- 2012-06-20 11:42:51 -0700 (Wed, 20 Jun 2012)
Log Message
Source/WebCore: Don't re-enter CachedResource::removeClient() if an XHR
is canceled and restarted multiple times.
https://bugs.webkit.org/show_bug.cgi?id=89378
Reviewed by Eric Seidel.
Test: http/tests/xmlhttprequest/reentrant-cancel.html
* loader/DocumentThreadableLoader.cpp:
(WebCore::DocumentThreadableLoader::cancel):
(WebCore::DocumentThreadableLoader::clearResource): Save off a copy of m_resource
then clear it, so we don't call clearResource() multiple times for the same resource.
LayoutTests: Test for https://bugs.webkit.org/show_bug.cgi?id=89378.
Reviewed by Eric Seidel.
* http/tests/xmlhttprequest/reentrant-cancel-expected.txt: Added.
* http/tests/xmlhttprequest/reentrant-cancel.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (120844 => 120845)
--- trunk/LayoutTests/ChangeLog 2012-06-20 18:40:09 UTC (rev 120844)
+++ trunk/LayoutTests/ChangeLog 2012-06-20 18:42:51 UTC (rev 120845)
@@ -1,3 +1,11 @@
+2012-06-20 Nate Chapin <[email protected]>
+
+ Test for https://bugs.webkit.org/show_bug.cgi?id=89378.
+ Reviewed by Eric Seidel.
+
+ * http/tests/xmlhttprequest/reentrant-cancel-expected.txt: Added.
+ * http/tests/xmlhttprequest/reentrant-cancel.html: Added.
+
2012-06-20 Robert Hogan <[email protected]>
Negative margin block doesn't properly clear a float enclosed by a previous sibling
Added: trunk/LayoutTests/http/tests/xmlhttprequest/reentrant-cancel-expected.txt (0 => 120845)
--- trunk/LayoutTests/http/tests/xmlhttprequest/reentrant-cancel-expected.txt (rev 0)
+++ trunk/LayoutTests/http/tests/xmlhttprequest/reentrant-cancel-expected.txt 2012-06-20 18:42:51 UTC (rev 120845)
@@ -0,0 +1 @@
+XThis tests that when we re-entrantly create and cancel XHRs, we don't try to disconnect the same CachedResourceClient multiple times from its CachedResource. We pass if we don't crash. XX
Added: trunk/LayoutTests/http/tests/xmlhttprequest/reentrant-cancel.html (0 => 120845)
--- trunk/LayoutTests/http/tests/xmlhttprequest/reentrant-cancel.html (rev 0)
+++ trunk/LayoutTests/http/tests/xmlhttprequest/reentrant-cancel.html 2012-06-20 18:42:51 UTC (rev 120845)
@@ -0,0 +1,21 @@
+<script>
+if (window.layoutTestController)
+ layoutTestController.dumpAsText();
+
+function addElement() {
+ document.documentElement.appendChild(document.createTextNode('X'));
+}
+document.addEventListener("DOMContentLoaded", addElement, false);
+window._onload_ = addElement;
+
+var xhr = new XMLHttpRequest;
+function sendXHR()
+{
+ xhr.open("GET", "", true);
+ xhr.send();
+}
+window.addEventListener("DOMSubtreeModified", sendXHR);
+addElement();
+</script>
+This tests that when we re-entrantly create and cancel XHRs, we don't try to disconnect the same CachedResourceClient
+multiple times from its CachedResource. We pass if we don't crash.
Modified: trunk/Source/WebCore/ChangeLog (120844 => 120845)
--- trunk/Source/WebCore/ChangeLog 2012-06-20 18:40:09 UTC (rev 120844)
+++ trunk/Source/WebCore/ChangeLog 2012-06-20 18:42:51 UTC (rev 120845)
@@ -1,3 +1,18 @@
+2012-06-20 Nate Chapin <[email protected]>
+
+ Don't re-enter CachedResource::removeClient() if an XHR
+ is canceled and restarted multiple times.
+ https://bugs.webkit.org/show_bug.cgi?id=89378
+
+ Reviewed by Eric Seidel.
+
+ Test: http/tests/xmlhttprequest/reentrant-cancel.html
+
+ * loader/DocumentThreadableLoader.cpp:
+ (WebCore::DocumentThreadableLoader::cancel):
+ (WebCore::DocumentThreadableLoader::clearResource): Save off a copy of m_resource
+ then clear it, so we don't call clearResource() multiple times for the same resource.
+
2012-06-20 Robert Hogan <[email protected]>
Negative margin block doesn't properly clear a float enclosed by a previous sibling
Modified: trunk/Source/WebCore/loader/DocumentThreadableLoader.cpp (120844 => 120845)
--- trunk/Source/WebCore/loader/DocumentThreadableLoader.cpp 2012-06-20 18:40:09 UTC (rev 120844)
+++ trunk/Source/WebCore/loader/DocumentThreadableLoader.cpp 2012-06-20 18:42:51 UTC (rev 120845)
@@ -146,7 +146,8 @@
void DocumentThreadableLoader::cancel()
{
- if (m_client) {
+ // Cacnel can re-enter and m_resource might be null here as a result.
+ if (m_client && m_resource) {
ResourceError error(errorDomainWebKitInternal, 0, m_resource->url(), "Load cancelled");
error.setIsCancellation(true);
didFail(error);
@@ -163,9 +164,13 @@
void DocumentThreadableLoader::clearResource()
{
- if (m_resource) {
- m_resource->removeClient(this);
+ // Script can cancel and restart a request reentrantly within removeClient(),
+ // which could lead to calling CachedResource::removeClient() multiple times for
+ // this DocumentThreadableLoader. Save off a copy of m_resource and clear it to
+ // prevent the reentrancy.
+ if (CachedResourceHandle<CachedRawResource> resource = m_resource) {
m_resource = 0;
+ resource->removeClient(this);
}
}
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes
