[XHR] Abort method execution when m_loader->cancel() in internalAbort() caused reentry

Source/core/xml/XMLHttpRequest.cpp

Index: Source/core/xml/XMLHttpRequest.cpp
diff --git a/Source/core/xml/XMLHttpRequest.cpp b/Source/core/xml/XMLHttpRequest.cpp
index 886d51490bf07df9c14c9cf04eda81181ba9c236..051a5e026038dba4c546dba07fba96f5f96cdf37 100644
--- a/Source/core/xml/XMLHttpRequest.cpp
+++ b/Source/core/xml/XMLHttpRequest.cpp
@@ -481,7 +481,9 @@ void XMLHttpRequest::open(const String& method, const KURL& url, ExceptionState&
 
 void XMLHttpRequest::open(const String& method, const KURL& url, bool async, ExceptionState& es)
 {
-    internalAbort();
+    if (!internalAbort())
+        return;
+
     State previousState = m_state;
     m_state = UNSENT;
     m_error = false;
@@ -792,9 +794,6 @@ void XMLHttpRequest::createRequest(ExceptionState& es)
             // Neither this object nor the JavaScript wrapper should be deleted while
             // a request is in progress because we need to keep the listeners alive,
             // and they are referenced by the JavaScript wrapper.
-
-            // m_loader was null, so there should be no pending activity at this point.
-            ASSERT(!hasPendingActivity());
             setPendingActivity(this);
         }
     } else {
@@ -814,7 +813,8 @@ void XMLHttpRequest::abort()
 
     bool sendFlag = m_loader;
 
-    internalAbort();
+    if (!internalAbort())
+        return;
 
     clearResponseBuffers();
 
@@ -837,7 +837,7 @@ void XMLHttpRequest::abort()
     }
 }
 
-void XMLHttpRequest::internalAbort(DropProtection async)
+bool XMLHttpRequest::internalAbort(DropProtection async)
 {
     m_error = true;
 
@@ -851,15 +851,31 @@ void XMLHttpRequest::internalAbort(DropProtection async)
         m_responseStream->abort();
 
     if (!m_loader)
-        return;
-
-    m_loader->cancel();
-    m_loader = 0;
+        return true;
+
+    // Cancelling the ThreadableLoader m_loader may result in calling
+    // window.onload synchronously. If such an onload handler contains open()
+    // call on the same XMLHttpRequest object, reentry happens. If m_loader
+    // is left to be non 0, internalAbort() call for the inner open() makes
+    // an extra dropProtection() call (when we're back to the outer open(),
+    // we'll call dropProtection()). To avoid that, clears m_loader before
+    // calling cancel.
+    //
+    // If, window.onload contains open() and send(), m_loader will be set to
+    // non 0 value. So, we cannot continue the outer open(). In such case,
+    // just abort the outer open() by returning false.
+    RefPtr<ThreadableLoader> loader = m_loader.release();
+    loader->cancel();
+
+    // Save to a local variable since we're going to drop protection.
+    bool newLoadStarted = m_loader;
 
     if (async == DropProtectionAsync)
         dropProtectionSoon();
     else
         dropProtection();
+
+    return !newLoadStarted;
 }
 
 void XMLHttpRequest::clearResponse()
@@ -1251,7 +1267,9 @@ void XMLHttpRequest::handleDidTimeout()
 {
     // internalAbort() calls dropProtection(), which may release the last reference.
     RefPtr<XMLHttpRequest> protect(this);
-    internalAbort();
+
+    if (!internalAbort())
+        return;
 
     m_exceptionCode = TimeoutError;