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

Source/core/xml/XMLHttpRequest.cpp

 /*
  *  Copyright (C) 2004, 2006, 2008 Apple Inc. All rights reserved.
  *  Copyright (C) 2005-2007 Alexey Proskuryakov <ap@webkit.org>
  *  Copyright (C) 2007, 2008 Julien Chaffraix <jchaffraix@webkit.org>
  *  Copyright (C) 2008, 2011 Google Inc. All rights reserved.
  *  Copyright (C) 2012 Intel Corporation
  *
  *  This library is free software; you can redistribute it and/or
  *  modify it under the terms of the GNU Lesser General Public
  *  License as published by the Free Software Foundation; either
@@ skipping 463 matching lines @@
         && !name.startsWith(staticData->m_secHeaderPrefix, false);
 }
 
 void XMLHttpRequest::open(const String& method, const KURL& url, ExceptionState& es)
 {
     open(method, url, true, es);
 }
 
 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;
     m_uploadComplete = false;
 
     // clear stuff from possible previous load
     clearResponse();
     clearRequest();
 
     ASSERT(m_state == UNSENT);
@@ skipping 290 matching lines @@
         // ThreadableLoader::create can return null here, for example if we're no longer attached to a page.
         // This is true while running onunload handlers.
         // FIXME: Maybe we need to be able to send XMLHttpRequests from onunload, <http://bugs.webkit.org/show_bug.cgi?id=10904>.
         // FIXME: Maybe create() can return null for other reasons too?
         ASSERT(!m_loader);
         m_loader = ThreadableLoader::create(scriptExecutionContext(), this, request, options);
         if (m_loader) {
             // 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 {
         ThreadableLoader::loadResourceSynchronously(scriptExecutionContext(), request, *this, options);
     }
 
     if (!m_exceptionCode && m_error)
         m_exceptionCode = NetworkError;
     if (m_exceptionCode)
         es.throwDOMException(m_exceptionCode);
 }
 
 void XMLHttpRequest::abort()
 {
     // internalAbort() calls dropProtection(), which may release the last reference.
     RefPtr<XMLHttpRequest> protect(this);
 
     bool sendFlag = m_loader;
 
-    internalAbort();
+    if (!internalAbort())
+        return;
 
     clearResponseBuffers();
 
     // Clear headers as required by the spec
     m_requestHeaders.clear();
 
     if ((m_state <= OPENED && !sendFlag) || m_state == DONE)
         m_state = UNSENT;
     else {
         ASSERT(!m_loader);
         changeState(DONE);
         m_state = UNSENT;
     }
 
     m_progressEventThrottle.dispatchEventAndLoadEnd(XMLHttpRequestProgressEvent::create(eventNames().abortEvent));
     if (!m_uploadComplete) {
         m_uploadComplete = true;
         if (m_upload && m_uploadEventsAllowed)
             m_upload->dispatchEventAndLoadEnd(XMLHttpRequestProgressEvent::create(eventNames().abortEvent));
     }
 }
 
-void XMLHttpRequest::internalAbort(DropProtection async)
+bool XMLHttpRequest::internalAbort(DropProtection async)
 {
     m_error = true;
 
     // FIXME: when we add the support for multi-part XHR, we will have to think be careful with this initialization.
     m_receivedLength = 0;
     m_decoder = 0;
 
     InspectorInstrumentation::didFailXHRLoading(scriptExecutionContext(), this);
 
     if (m_responseStream && m_state != DONE)
         m_responseStream->abort();
 
     if (!m_loader)
-        return;
+        return true;
 
-    m_loader->cancel();
-    m_loader = 0;
+    // 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()
 {
     m_response = ResourceResponse();
     clearResponseBuffers();
 }
 
 void XMLHttpRequest::clearResponseBuffers()
 {
@@ skipping 371 matching lines @@
         else
             // Firefox calls readyStateChanged every time it receives data, 4449442
             callReadyStateChangeListener();
     }
 }
 
 void XMLHttpRequest::handleDidTimeout()
 {
     // internalAbort() calls dropProtection(), which may release the last reference.
     RefPtr<XMLHttpRequest> protect(this);
-    internalAbort();
+
+    if (!internalAbort())
+        return;
 
     m_exceptionCode = TimeoutError;
 
     handleDidFailGeneric();
 
     if (!m_async) {
         m_state = DONE;
         return;
     }
     changeState(DONE);
@@ skipping 31 matching lines @@
 {
     return eventNames().interfaceForXMLHttpRequest;
 }
 
 ScriptExecutionContext* XMLHttpRequest::scriptExecutionContext() const
 {
     return ActiveDOMObject::scriptExecutionContext();
 }
 
 } // namespace WebCore