[XHR] Move the code to clear variables to internalAbort()

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 6ea48fd8bfa15a6bd129c4f07c4c1af5890b0188..2ea4f5ce9797932e690153a67bc7ac40d9f3353f 100644
--- a/Source/core/xml/XMLHttpRequest.cpp
+++ b/Source/core/xml/XMLHttpRequest.cpp
@@ -545,10 +545,6 @@ void XMLHttpRequest::open(const AtomicString& method, const KURL& url, bool asyn
     m_error = false;
     m_uploadComplete = false;
 
-    // clear stuff from possible previous load
-    clearResponse();
-    clearRequest();
-
     ASSERT(m_state == UNSENT);

[yhirano, 2014/08/21 11:23:09]

We don't need this assertion: it's too obvious.

[tyoshino (SeeGerritForStatus), 2014/08/21 14:14:58]

Done.

 
     if (!isValidHTTPToken(method)) {
@@ -898,11 +894,6 @@ void XMLHttpRequest::abort()
     if (!internalAbort())
         return;
 
-    clearResponse();
-
-    // Clear headers as required by the spec
-    m_requestHeaders.clear();
-
     if (!((m_state <= OPENED && !sendFlag) || m_state == DONE)) {
         ASSERT(!m_loader);
         handleRequestError(0, EventTypeNames::abort, receivedLength, expectedLength);
@@ -935,6 +926,9 @@ bool XMLHttpRequest::internalAbort()
         m_responseStream->error(DOMException::create(!m_async && m_exceptionCode ? m_exceptionCode : AbortError, "XMLHttpRequest::abort"));
     }
 
+    clearResponse();
+    clearRequest();

[yhirano, 2014/08/21 11:23:09]

Doesn't Moving clearing code beyond loader->cancel() cause a behavior change?

[tyoshino (SeeGerritForStatus), 2014/08/21 12:19:48]

If internalAbort() returned true, it means send() didn't happen, but open() may
still have happened. So, m_requestHeaders may contain after cancel(), but we
still have m_error set. So, there shouldn't be any problem.

[tyoshino (SeeGerritForStatus), 2014/08/21 14:14:58]

And, as we're clearing the variables before cancel(), in window.onload (see
below):
BEFORE THIS CL: Some properties e.g. responseURL was working
AFTER THIS CL: Everything are cleared
I think we don't have to care this.

+
     if (!m_loader)
         return true;
 
@@ -990,14 +984,6 @@ void XMLHttpRequest::clearRequest()
     m_requestHeaders.clear();
 }
 
-void XMLHttpRequest::handleDidFailGeneric()
-{
-    clearResponse();
-    clearRequest();
-
-    m_error = true;
-}
-
 void XMLHttpRequest::dispatchProgressEvent(const AtomicString& type, long long receivedLength, long long expectedLength)
 {
     bool lengthComputable = expectedLength > 0 && receivedLength <= expectedLength;
@@ -1026,7 +1012,6 @@ void XMLHttpRequest::handleNetworkError()
     if (!internalAbort())
         return;
 
-    handleDidFailGeneric();
     handleRequestError(NetworkError, EventTypeNames::error, receivedLength, expectedLength);
 }
 
@@ -1038,7 +1023,9 @@ void XMLHttpRequest::handleDidCancel()
     long long expectedLength = m_response.expectedContentLength();
     long long receivedLength = m_receivedLength;
 
-    handleDidFailGeneric();
+    if (!internalAbort())
+        return;
+
     handleRequestError(AbortError, EventTypeNames::abort, receivedLength, expectedLength);
 }
 
@@ -1458,7 +1445,6 @@ void XMLHttpRequest::handleDidTimeout()
     if (!internalAbort())
         return;
 
-    handleDidFailGeneric();
     handleRequestError(TimeoutError, EventTypeNames::timeout, receivedLength, expectedLength);
 }