Title: [122144] branches/chromium/1132
Revision
122144
Author
[email protected]
Date
2012-07-09 12:51:04 -0700 (Mon, 09 Jul 2012)

Log Message

Merge 120845
BUG=132241
Review URL: https://chromiumcodereview.appspot.com/10765006

Modified Paths

Added Paths

Diff

Copied: branches/chromium/1132/LayoutTests/http/tests/xmlhttprequest/reentrant-cancel-expected.txt (from rev 120845, trunk/LayoutTests/http/tests/xmlhttprequest/reentrant-cancel-expected.txt) (0 => 122144)


--- branches/chromium/1132/LayoutTests/http/tests/xmlhttprequest/reentrant-cancel-expected.txt	                        (rev 0)
+++ branches/chromium/1132/LayoutTests/http/tests/xmlhttprequest/reentrant-cancel-expected.txt	2012-07-09 19:51:04 UTC (rev 122144)
@@ -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

Copied: branches/chromium/1132/LayoutTests/http/tests/xmlhttprequest/reentrant-cancel.html (from rev 120845, trunk/LayoutTests/http/tests/xmlhttprequest/reentrant-cancel.html) (0 => 122144)


--- branches/chromium/1132/LayoutTests/http/tests/xmlhttprequest/reentrant-cancel.html	                        (rev 0)
+++ branches/chromium/1132/LayoutTests/http/tests/xmlhttprequest/reentrant-cancel.html	2012-07-09 19:51:04 UTC (rev 122144)
@@ -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: branches/chromium/1132/Source/WebCore/loader/DocumentThreadableLoader.cpp (122143 => 122144)


--- branches/chromium/1132/Source/WebCore/loader/DocumentThreadableLoader.cpp	2012-07-09 19:42:43 UTC (rev 122143)
+++ branches/chromium/1132/Source/WebCore/loader/DocumentThreadableLoader.cpp	2012-07-09 19:51:04 UTC (rev 122144)
@@ -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/webkit-changes

Reply via email to