Fix XMLHttpRequest leak document when send() is called multiple times.

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 733 matching lines @@
     m_error = false;
 
     if (m_async) {
         if (m_upload)
             request.setReportUploadProgress(true);
 
         // 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 {
         request.setPriority(ResourceLoadPriorityVeryHigh);
         InspectorInstrumentation::willLoadXHRSynchronously(scriptExecutionContext());
         ThreadableLoader::loadResourceSynchronously(scriptExecutionContext(), request, *this, options);
         InspectorInstrumentation::didLoadXHRSynchronously(scriptExecutionContext());
     }
 
     if (!m_exceptionCode && m_error)
@@ skipping 25 matching lines @@
     }
 
     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()
+void XMLHttpRequest::internalAbort(DropProtection async)
 {
-    bool hadLoader = m_loader;
-
     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;
-
-    if (hadLoader) {
-        m_loader->cancel();
-        m_loader = 0;
-    }
-
     m_decoder = 0;
 
     InspectorInstrumentation::didFailXHRLoading(scriptExecutionContext(), this);
 
-    if (hadLoader)
-        dropProtectionSoon();
+    if (m_loader) {
+        m_loader->cancel();
+        m_loader = 0;
+
+        if (async == DropProtectionAsync)
+            dropProtectionSoon();
+        else
+            dropProtection();
+    }
 }
 
 void XMLHttpRequest::clearResponse()
 {
     m_response = ResourceResponse();
     clearResponseBuffers();
 }
 
 void XMLHttpRequest::clearResponseBuffers()
 {
@@ skipping 239 matching lines @@
         return;
 
     if (m_state < HEADERS_RECEIVED)
         changeState(HEADERS_RECEIVED);
 
     if (m_decoder)
         m_responseText = m_responseText.concatenateWith(m_decoder->flush());
 
     InspectorInstrumentation::didFinishXHRLoading(scriptExecutionContext(), this, identifier, m_responseText, m_url, m_lastSendURL, m_lastSendLineNumber);
 
-    bool hadLoader = m_loader;
-    m_loader = 0;
+    RefPtr<XMLHttpRequest> protect;
+    if (m_loader) {
+        m_loader = 0;
+
+        // Prevent dropProtection releasing the last reference, and retain |this| until the end of this method.
+        protect = this;

[abarth-chromium, 2013/08/19 17:53:17]

I probably would have just done this unconditionally on line 1086.

[kouhei (in TOK), 2013/08/19 23:44:59]

Done.

+        dropProtection();
+    }
 
     changeState(DONE);
     m_responseEncoding = String();
     m_decoder = 0;
-
-    if (hadLoader)
-        dropProtection();
 }
 
 void XMLHttpRequest::didSendData(unsigned long long bytesSent, unsigned long long totalBytesToBeSent)
 {
     if (!m_upload)
         return;
 
     if (m_uploadEventsAllowed)
         m_upload->dispatchEvent(XMLHttpRequestProgressEvent::create(eventNames().progressEvent, true, bytesSent, totalBytesToBeSent));
 
@@ skipping 113 matching lines @@
     m_progressEventThrottle.suspend();
 }
 
 void XMLHttpRequest::resume()
 {
     m_progressEventThrottle.resume();
 }
 
 void XMLHttpRequest::stop()
 {
-    internalAbort();
+    internalAbort(DropProtectionAsync);
 }
 
 void XMLHttpRequest::contextDestroyed()
 {
     ASSERT(!m_loader);
     ActiveDOMObject::contextDestroyed();
 }
 
 const AtomicString& XMLHttpRequest::interfaceName() const
 {
     return eventNames().interfaceForXMLHttpRequest;
 }
 
 ScriptExecutionContext* XMLHttpRequest::scriptExecutionContext() const
 {
     return ActiveDOMObject::scriptExecutionContext();
 }
 
 EventTargetData* XMLHttpRequest::eventTargetData()
 {
     return &m_eventTargetData;
 }
 
 EventTargetData* XMLHttpRequest::ensureEventTargetData()
 {
     return &m_eventTargetData;
 }
 
 } // namespace WebCore